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

Parent-based sampler disconnected family trees #878

Closed
benlandrum opened this issue Jun 25, 2021 · 1 comment · Fixed by #879
Closed

Parent-based sampler disconnected family trees #878

benlandrum opened this issue Jun 25, 2021 · 1 comment · Fixed by #879
Labels
bug Something isn't working
Milestone

Comments

@benlandrum
Copy link
Contributor

Describe your environment
Using a 1.0.0 release candidate.

Steps to reproduce
Create a parent-based sampler with a ratio-based delegate sampler and a 0.5 sampling ratio. Create multiple generations of spans. You'll find that instead of always getting either a single trace with all the spans in it (from a sampled ancestor) or no traces (from a non-sampled ancestor), you randomly get multiple traces (a disconnected family tree).

What is the expected behavior?
The parent-based sampler should delegate sampling decisions to a different sampler only if a sampling decision was not previously made for a parent context (i.e., if the parent context is invalid). Repeated use of the parent sampler on descendants of a non-sampled span should result in a single, non-exported trace, just as it results in a single, exported trace from a sampled ancestor.

What is the actual behavior?
Descendants of non-sampled spans have invalid context instead. This results in many "orphan spans" when the delegate sampler, which handles invalid span contexts, kicks in.

Additional context
I think the correct behavior should be to always create spans with valid IDs. Although I'm not so familiar with the Go and Java SDKs, it seems like they always generate spans with valid IDs.

@benlandrum benlandrum added the bug Something isn't working label Jun 25, 2021
@lalitb
Copy link
Member

lalitb commented Jun 27, 2021

Agree, this is what is expected as per the Specs too: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.0.0/specification/trace/sdk.md#sdk-span-creation

Generate a new span ID for the Span, independently of the sampling decision. This is done so other components (such as logs or exception handling) can rely on a unique span ID, even if the Span is a non-recording instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants