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

Missing spec for Span ID creation for dropped/record-only spans #1060

Closed
Oberon00 opened this issue Oct 6, 2020 · 4 comments · Fixed by #1225
Closed

Missing spec for Span ID creation for dropped/record-only spans #1060

Oberon00 opened this issue Oct 6, 2020 · 4 comments · Fixed by #1225
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:trace Related to the specification/trace directory

Comments

@Oberon00
Copy link
Member

Oberon00 commented Oct 6, 2020

What are you trying to achieve?

I am trying to ensure that span ID creation for spans that are dropped or only recorded (but not sampled) is uniform across different language implementations.

Additional context.

As explained in #864 (comment) the spec is silent on what to do for IDs of spans that are not sampled. #998 suggest to reuse the parent-span-id for dropped or use zero if there is none, but use a new span ID for everything else. A simpler alternative would be to simply add the sentence "A new Span ID MUST always be generated, even for dropped spans".

This is not purely an optimization issue, it impacts the observable behavior of the monitored application when the unsampled span is injected (and then potentially used as parent on the other side).

@Oberon00 Oberon00 added area:sdk Related to the SDK area:sampling Related to trace sampling spec:trace Related to the specification/trace directory labels Oct 6, 2020
@Oberon00
Copy link
Member Author

Oberon00 commented Oct 6, 2020

CC @alolita @anuraaga

@andrewhsu andrewhsu added priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA 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 and removed priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA labels Oct 6, 2020
@marcingrzejszczak
Copy link

Is this issue also related to being able to drop a span whatsover? That means that you're starting a span and after some time you decide not to record it. You want to end it but not report it anywhere. Currently I don't see that the spec allows it nor the java implementation.

@Oberon00
Copy link
Member Author

Oberon00 commented Oct 15, 2020

Is this issue also related to being able to drop a span whatsover? That means that you're starting a span and after some time you decide not to record it.

No, not at all. This issue is only about the reaction to the existing (head-based) DROP sampling decision. If you want to discuss aborting/dropping a span later, please open a new issue. EDIT: Or look at open-telemetry/oteps#115

@andrewhsu andrewhsu added priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA and removed 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 labels Oct 27, 2020
@andrewhsu
Copy link
Member

from the spec sig mtg today, re-triaging as required-for-ga

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK priority:p1 Highest priority level release:required-for-ga Must be resolved before GA release, or nice to have before GA spec:trace Related to the specification/trace directory
Projects
None yet
3 participants