feat(sampling): Support for OpenTelemetry consistent tracestate sampling - #12397
feat(sampling): Support for OpenTelemetry consistent tracestate sampling#12397MilanGarnier wants to merge 12 commits into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: a0176c9 | Docs | View more details | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
3643548 to
2f90ba6
Compare
a168627 to
9e2422a
Compare
43e5493 to
ba21909
Compare
ba21909 to
ea27b06
Compare
7a1ddea to
a0176c9
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0176c99c0
ℹ️ 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".
| randomValue = threshold == 0 ? 0 : threshold - 1; | ||
| } | ||
|
|
||
| return create(randomValue, threshold, currentValue, originalMemberContributionSize, true); |
There was a problem hiding this comment.
Defer OTel state materialization until propagation
perf: Every new local root trace after agent rates load, and every trace matching a sampling rule, reaches this call and immediately allocates a StringBuilder plus backing storage, a String, and a retained OtelTraceState, even when the trace never performs W3C injection or OTLP export. This adds several allocations to the per-trace hot path; defer string encoding until headerValue(W3C) or gate state creation when no compatible exporter is enabled, and verify the improvement with an allocation profile.
AGENTS.md reference: AGENTS.md:L79-L79
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| long threshold = computeThreshold(sampleRate); | ||
| long randomValue = computeRandomValue(traceIdLowOrderBits); |
There was a problem hiding this comment.
Preserve inherited OTel randomness during resampling
When current contains a valid inherited rv but the trace priority is still unset—for example, default compound extraction first selects Datadog headers without a sampling-priority header and then copies matching W3C tracestate—a local sampling rule reaches this line and unconditionally replaces the upstream randomness with a trace-ID hash. Consistent probability sampling requires randomness to remain stable across a trace; replacing it makes upstream and downstream thresholds and adjusted-count weights describe different cohorts. Reuse current.randomValue when present and base the probability decision on it, generating a value only when none was inherited.
Useful? React with 👍 / 👎.
| } | ||
| if (samplingPriority != PrioritySampling.UNSET) { | ||
| setSamplingPriority(samplingPriority, SamplingMechanism.UNKNOWN); | ||
| if (this.propagationTags.getSamplingPriority() == samplingPriority) { |
There was a problem hiding this comment.
Preserve OTel state for compound Datadog extraction
With the default DATADOG, TRACECONTEXT extraction order, the first Datadog context owns the sampling priority while applyTraceContextToFirstContext copies only the later W3C tracestate. Its PropagationTags priority therefore remains UNSET, so this comparison fails and the UNKNOWN update reaches removeForNonProbabilityDecision(), deleting the inherited th even when the Datadog and W3C decisions agree. Dual headers emitted by one instrumented service consequently lose consistent-sampling state at the next Java hop; avoid reapplying extracted priorities as UNKNOWN or synchronize the merged propagation-tags priority first.
Useful? React with 👍 / 👎.
What Does This Do
Adds OpenTelemetry consistent probability-sampling behavior on top of the
ottracestate parsing introduced by #12405.rvandth.Motivation
Make Java tracer sampling decisions interoperable with OpenTelemetry
consistent probability sampling and provide the state needed for downstream
tracestate propagation.
Additional Notes
This is the second PR in a two-PR stack and depends on #12405.
This PR covers propagation and sampling only.
OTLP export (which should already work with this PR, but it can be optimized) will follow in a separate change.
Validation after rebasing the stack onto the current
master:dd-trace-corepropagation and sampling tests passed.:dd-trace-core:compileTraceAgentTestJavapassed.:dd-trace-core:spotlessCheckandgit diff --checkpassed.Prior validation of the feature branch:
:dd-trace-core:testrun passed 4,277 of 4,278 tests. The remainingtiming-sensitive
PendingTraceBufferTestcase passed in isolation.Contributor Checklist
type:andcomp:labels.Jira ticket: APMAPI-2171