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

Revisit Default Sampler and Interaction With TraceFlags #728

Closed
MitchellDumovic opened this issue Jul 23, 2020 · 2 comments · Fixed by #750
Closed

Revisit Default Sampler and Interaction With TraceFlags #728

MitchellDumovic opened this issue Jul 23, 2020 · 2 comments · Fixed by #750
Assignees
Labels
area:sampling Related to trace sampling area:sdk Related to the SDK priority:p2 Medium 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

@MitchellDumovic
Copy link
Contributor

MitchellDumovic commented Jul 23, 2020

Should the Default Sampler be changed from AlwaysOn to ParentOrElse(AlwaysOn)?

The spec suggests that AlwaysOn is the default - currently this is not truly enforced consistently across languages:
In Go (this might be a bug), the span's parent is always followed unless a different sampler is specified:

https://github.com/open-telemetry/opentelemetry-go/blob/master/sdk/trace/span.go#L394

One thing that I think adds some confusion (at least for me) here is the interaction between the sampler and the Span context's TraceFlags. The spec suggests that TraceFlags should be passed down from parent to child here, but by always sampling by default this may not actually happen in practice.

@MitchellDumovic MitchellDumovic added the spec:trace Related to the specification/trace directory label Jul 23, 2020
@anuraaga
Copy link
Contributor

Thanks for separating out this issue. +1 to changing default to ParentOrElse(AlwaysOn), it seems the least surprising. With all nodes using the default, they'll be effectively AlwaysOn but changing the sampling can be done at one node, the frontend, without worrying about incomplete traces.

@MitchellDumovic
Copy link
Contributor Author

Discussed at the Spec Sig where there was general agreement that this is a good idea (changing the default sampler from AlwaysOn to ParentOrElse(AlwaysOn)). Will push up a PR to change this in the spec.

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:p2 Medium 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
Development

Successfully merging a pull request may close this issue.

4 participants