-
Notifications
You must be signed in to change notification settings - Fork 257
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
Add sampled flag to Span type #166
Comments
Isn't an unsampled but exported span an oxymoron? How could this happen in practice? |
There is work going on to do tail-based sampling in the collector. For this to work, all spans need to be exported to the collector. |
I am +1 on this |
@kbrockhoff Do you mind describing more concretely the use case you have in mind? I think with pure tail sampling, we would expect the SDK to do 100% sampling and that wouldn't need to be exported. Is the goal to support multiple sampling decisions, e.g., not sampled for trace, but sampled for logs or for secondary sampling of a subset of the trace? I guess instead of a sampling flag, we'd want a list of Also I think this would need to be propagated too for certain types of secondary sampling? |
There are several use cases for having a sampled flag.
In either case, adequate information can potentially be stored in the TraceState field but would then need to be parsed. I can be talked into either this approach or the SamplingDecision approach. |
@arminru from the bulk move of |
This can be added after-ga, if still desired, yes. |
Lack of a sampled flag in the Span type results in ambiguity. It implies that non-sampled traces are dropped at the instrumentation level and not forwarded through the system. If using tail-sampling decisions, this means all traces need to be marked sampled up front. In addition, it leaves no flexibility for different routing of traces. For example, routing all traces to a logging exporter and only sampled ones to a full-scale backend.
In addition, other protocols such as Zipkin have a sampled flag. The lack of this data makes it impossible to preserve full fidelity when converting between protocols.
The text was updated successfully, but these errors were encountered: