Probability sampling in tracestate specification#2047
Probability sampling in tracestate specification#2047carlosalberto merged 67 commits intoopen-telemetry:mainfrom
tracestate specification#2047Conversation
|
@oertl @dyladan @carlosalberto Please take a look at this draft. I'm expecting at least the two of us vendors to supply implementations of this, while we continue to explore ways of adding sampling support into the |
…ication into jmacd/sampling_spec2
…ication into jmacd/sampling_spec2
…-specification into jmacd/sampling_spec2
|
@open-telemetry/specs-trace-approvers the two optional samplers specified here are implemented by this PR in opentelemetry-go-contrib: open-telemetry/opentelemetry-go-contrib#1379 |
ocelotl
left a comment
There was a problem hiding this comment.
Changing to comment instead of requesting changes since these are not critical changes needed.
…ication into jmacd/sampling_spec2
…-specification into jmacd/sampling_spec2
|
@open-telemetry/specs-trace-approvers This meets our minimum criteria for merging. I have been trying to get more stamps of approval, but it's not happening (or not happening quickly). I propose to merge this PR. |
mtwo
left a comment
There was a problem hiding this comment.
Looks good, a bit long but that's likely necessary given the topic. I couldn't find an example for what I suspect might be a common scenario: when an edge service uses a probability-based sampler but a downstream service decides to sample when it sees a trace with a sampled flag OR some other condition is satisfied (an error, for example).
I think (?) that this is similar to the "interoperability with TraceIDRatioBased sampler" example, though the second sampler in that case uses different criteria.
|
@mtwo There isn't an exact example corresponding to your question. The section "Example: Probability and non-probability sampler in a root context" comes close, and there is a related discussion in #2179. As discussed in the issue, a better Delegating sampler API would be nice, but is not critical to the answer here. Sampler API details aside (it's a bit messy, we could use a v2 API proposal to clear things up--see #2179), the child span is expected to have adjusted count equal to the parent when the span is sampled, and adjusted count 0 when the parent was not sampled but there were errors ( |
|
We have more than enough reviews and no standing questions. Let's merge this now and address any improvements in future PRs. |
Changes
Specifies how to interpret and use p-value and r-value for consistent probability sampling decisions, including two optional samplers named
ConsistentProbabilityBasedandConsistentParentProbabilityBased.Does not modify existing built-in Sampler definitions.
This is ready for general review.
Implemented in open-telemetry/opentelemetry-go-contrib#1379
OTEPs
https://github.com/open-telemetry/oteps/blob/main/text/trace/0168-sampling-propagation.md
https://github.com/open-telemetry/oteps/blob/main/text/trace/0170-sampling-probability.md
Part of #1414
Part of #1412
Part of #1524
Part of #570
Another take on #1899
Fixes #2224
Part of #2179