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

Behavior of Span.End is unclear. #1010

Closed
Oberon00 opened this issue Sep 25, 2020 · 4 comments · Fixed by #1011
Closed

Behavior of Span.End is unclear. #1010

Oberon00 opened this issue Sep 25, 2020 · 4 comments · Fixed by #1011
Labels
area:api Cross language API specification issue priority:p2 Medium priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:trace Related to the specification/trace directory

Comments

@Oberon00
Copy link
Member

Oberon00 commented Sep 25, 2020

The behavior of Span.End could use some clarifications. For example, I think I heard from @tsloughter that in Erlang, ending a Span inactivates it on a Context there, which seems problematic, especially after Context is the only way to specify a parent span. Other points that are unclear are when a span needs to be ended, and what the relation to IsRecording is.

I created PR #1011 to clarify these points.

@Oberon00 Oberon00 added area:api Cross language API specification issue spec:trace Related to the specification/trace directory labels Sep 25, 2020
@tsloughter
Copy link
Member

So, yes, we don't have users use Span.End directly in Erlang/Elixir, similar to why Java uses Scope instead of requiring a user to explicitly call End and then manually change the active span to the previous active Scope's span.

Instead users use ot_tracer:end_span(Tracer) which ends the active Span in the current context.

This allows for Tracer to be responsible for Span Processors, if it wasn't then we'd have to copy Span Processors from the Tracer into each started Span.

The Tracer's end_span calls the Span modules end_span, runs the OnEnd processors and then makes the Span that was previously active now active and the current active span no longer active.

So it isn't a function of Erlang's Span.End that inactivates the Span in the Context but a function "above" that, currently in the ot_tracer module.

@andrewhsu andrewhsu added release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs priority:p2 Medium priority level labels Sep 25, 2020
@andrewhsu
Copy link
Member

from the issue triage mtg today with TC, allowing changes related to this issue for GA if editorial changes only

@tsloughter
Copy link
Member

I'd be interested in seeing how other languages allow users to end a span and return to the previously active span. I don't think I've seen any that requires the user do:

PreviousSpan = Tracer.get_active_span()
Span = Tracer.start_active_span()
Span.End()
Tracer.set_active_span(PreviousSpan)

@tsloughter
Copy link
Member

I guess what I'm suggesting is adding an End in "Tracer Context Utilities" that ends the active span and updates the context. But maybe only Erlang "needs" this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Cross language API specification issue priority:p2 Medium priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:trace Related to the specification/trace directory
Projects
None yet
3 participants