feat(propagation): emit and forward OTel consistent-probability ot tracestate member - #283
feat(propagation): emit and forward OTel consistent-probability ot tracestate member#283MilanGarnier wants to merge 7 commits into
Conversation
…umentation workspace
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b2b50fa3d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # TraceRootSamplingInfo::otel_consistent_sampling) is only on this | ||
| # libdatadog branch until libdd-sampling is released. | ||
| [patch.crates-io] | ||
| libdd-sampling = { git = "https://github.com/DataDog/libdatadog", branch = "milan.garnier/otel-consistent-sampling" } |
There was a problem hiding this comment.
The added patch replaces the crates.io libdd-sampling with a git branch that the comment itself marks as dev-only. Clean builds and publishing now depend on fetching that mutable branch instead of a released crate; in registry-only/offline or restricted-network environments, cargo check --locked fails before compilation because Cargo tries to update https://github.com/DataDog/libdatadog. Please wait for/bump to the released libdd-sampling version and remove this patch, including the duplicate instrumentation workspace patch.
Useful? React with 👍 / 👎.
| inbound: Option<String>, | ||
| ) -> Option<String> { | ||
| match (is_local_root, derived) { | ||
| (true, Some(ocs)) => ot_set_rv_th(inbound.as_deref(), Some(ocs.rv), Some(ocs.th)), |
There was a problem hiding this comment.
For a deferred remote parent (for example Datadog/B3 extracted without a sampling priority while W3C tracestate supplied ot=rv:...), inbound_ot reaches this probability branch, but the code replaces the inherited explicit randomness with ocs.rv derived from the trace id. OpenTelemetry uses rv as the stable randomness source for the trace, so changing it on the child can make downstream services compare a different R value and produce inconsistent keep/drop decisions; keep the inbound rv when present and only update th.
Useful? React with 👍 / 👎.
| let ot = propagation_data.ot.take().or_else(|| { | ||
| cx.get::<DatadogExtractData>() | ||
| .and_then(|extract_data| extract_data.ot.clone()) | ||
| }); |
There was a problem hiding this comment.
It seems the fallback could resurrect an erased ot:
get_trace_propagation_data returns EMPTY_PROPAGATION_DATA when the trace isn't registered, so ot: None is indistinguishable from "the local root resolved this to None on purpose":
- inbound traceparent + deferred
dd=+ot=th:e6666666666666(no rv) - local root's decision is non-probability - e.g. the rate limiter fired, which makes
otel_consistent_sampling()returnNoneviarl_effective_rate.is_some()(not a corner case) resolve_outbound_ot(true, None, Some("th:e666..."))>ot_extract_rv>None, ot_set_rv_thfilters the th out, no other subkeys > registry storesot: None- inject falls back to
DatadogExtractDataand emitsot=th:e6666666666666
2 participants
Summary
ottracestate member (rv/th) and forward it raw, rewriting only on injectionrv/thon inject based on the sampling decision, keeping unrecognized ot sub-keysTest plan