Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spec: SpanProcessor.onStart MUST receive a mutable "read/write" span #1620

Closed
arminru opened this issue Oct 23, 2020 · 5 comments · Fixed by #1621
Closed

Spec: SpanProcessor.onStart MUST receive a mutable "read/write" span #1620

arminru opened this issue Oct 23, 2020 · 5 comments · Fixed by #1621

Comments

@arminru
Copy link
Member

arminru commented Oct 23, 2020

See https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/sdk.md#onstart

Currently only an immutable ReadableSpan is passed:

/**
* Called when a {@link ReadableSpan} is started, if the `span.isRecording()`
* returns true.
* @param span the Span that just started.
*/
onStart(span: ReadableSpan): void;

@dyladan
Copy link
Member

dyladan commented Oct 23, 2020

Thanks for reporting this. The concrete span sent to onStart is already a Span so this should be a very simple change.

@mayurkale22
Copy link
Member

Just curious to know the use-case where we need writeable span in SpanProcessor.onStart?

@arminru
Copy link
Member Author

arminru commented Oct 23, 2020

One use case for SpanProcessors is to add certain attributes on a span. The thread ID (of the starting span), for example, is a property that calls for being added automatically rather than bugging users with that. The code.* attributes could also be added automatically by a SpanProcessor which inspects the call stack.

@dyladan
Copy link
Member

dyladan commented Oct 23, 2020

@mayurkale22 user @GasimGasimzada has already asked for this in gitter too

@GasimGasimzada
Copy link

GasimGasimzada commented Oct 26, 2020

@mayurkale22

Just curious to know the use-case where we need writeable span in SpanProcessor.onStart?

For example, we have a tracer in the browser and for all spans we add http user agent, the current URL, and other custom attributes that are applicable to our app for all spans. In my case, I extends SimpleSpanProcessor and overrode onStart, which adds all the tags. Since the underlying JS object already had all the necessary attributes, I casted ReadableSpan to Span (it is hacky but it got the job done for now):

const span = readableSpan as unknown as Span;

pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
… if the last layer is not REQUEST_HANDLER (open-telemetry#1620)

* fix(express): make rpcMetadata.route capture the last layer even when if the last layer is mot REQUEST_HANDLER

* fix lint issue

* remove test.only

* revert code to change ignore order

* update test

* remove comment related to update span name

* Move rpcRoute.metadata calculation logic up

* Add more test

* Fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants