[SLES-2971] fix(traces): only trust the X-Ray Sampled flag on Datadog-planted headers - #1325
Draft
lym953 wants to merge 1 commit into
Draft
[SLES-2971] fix(traces): only trust the X-Ray Sampled flag on Datadog-planted headers#1325lym953 wants to merge 1 commit into
lym953 wants to merge 1 commit into
Conversation
|
lym953
force-pushed
the
yiming.luo/sles-2971-xray-sampled-priority
branch
2 times, most recently
from
August 17, 2026 20:38
00c42b5 to
4950cd1
Compare
…-planted headers The SQS AWSTraceHeader fallback accepted any `Root=`-prefixed value and mapped `Sampled != 1` to sampling priority 0, which the tracer then applied to its whole trace. On an AWS-generated root ID that flag is X-Ray's decision, not Datadog's -- and when X-Ray tracing is off it is not a decision at all, just the default the AWS SDK's recursion-detection header carries. Gate the priority on the ID shape instead: Datadog libraries zero the high 32 bits of the ID section (dd-trace-java's `XRayHttpCodec.TRACE_ID_PADDING`, and the Go extension's `rootRegex` asserted the same), so only then is `Sampled` ours to honor. For an AWS-generated ID keep the IDs for correlation and leave the priority unset so the tracer decides. Also switch the root-ID slice to a checked `get`, which the surrounding code needs anyway: a truncated `Root=` part would panic on the old fixed index.
lym953
force-pushed
the
yiming.luo/sles-2971-xray-sampled-priority
branch
from
August 18, 2026 20:37
4950cd1 to
e93c41c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://datadoghq.atlassian.net/browse/SLES-2971
Overview
A reported Go Lambda dropped from ~14 spans per invocation to 2 after moving from extension
v87+DD_EXTENSION_VERSION=compatibilitytov98. The spans weren't lost in transit — the tracer built the full tree, but the whole trace was marked sampled-out.With no readable
_datadogattribute on the SQS record,extract_span_contextfalls back to the X-RayAWSTraceHeaderand mapped itsSampledflag straight onto a Datadog priority:Sampledwas not1, so priority0went into the start-invocation response (listener.rs:216-224), dd-trace-go adopted it for its whole trace, and the backend dropped that chunk. The extension's own inferred + invocation spans survive because a priority is only read for them whentrace_id == 0(processor.rs:1401-1419) — hence exactly 2 spans left.v87avoided this because the Go extension guards the same fallback withrootRegex = "Root=1-[0-9a-fA-F]{8}-00000000[0-9a-fA-F]{16}"(datadog-agent,carriers.go:37), rejecting AWS-generated ids and returning no headers, so the tracer sampled for itself.Why that flag isn't ours to honor
Two findings that shaped this version of the fix:
The header gets planted with no X-Ray involvement at all. The AWS SDKs' recursion-detection middleware copies
_X_AMZN_TRACE_IDonto every outbound request inside Lambda (aws-sdk-go-v2/aws/middleware/recursion_detection.go:39-50), and SQS stores it as theAWSTraceHeadersystem attribute. So any Lambda→SQS→Lambda chain carries one, no X-Ray SDK and no tracing config required — and itsSampledis0precisely when the producer has X-Ray tracing off, which is the default. Measured on a live pair: producerPassThrough→Sampled=0→ consumer's whole trace p0; producerActive→Sampled=1→ kept. The consuming function's own tracing mode is irrelevant, since the value is written by the producer atSendMessagetime.dd-trace-javagates the same path on the id shape. It injectsRoot=1-<epoch>-00000000<16 hex>(XRayHttpCodec.java:41,TRACE_ID_PADDING) and on extraction ignores any header lacking that padding (:200-205, "header doesn't match our padded version"). Only after that check does it mapSampled != 1to a drop. The Go extension's regex is the same assertion.So a padded root id carries a Datadog decision, and
Sampled=0there should still drop. An unpadded one carries X-Ray's decision — or, with tracing off, nobody's.Fix
Gate the priority on the id shape instead of discarding it wholesale:
Sampled…-00000000<16 hex>(Datadog-planted)00— our own decision, honored…-00000000<16 hex>11110Trace ids still come from the header in every row, so correlation is unchanged. This keeps
v87/dd-trace-javabehavior for Datadog-planted headers while fixing the reported drop, and unlike porting the regex it leaves #452's AWS-native correlation intact.Also switches the root-id slice to a checked
get: a truncatedRoot=part would have panicked on the old fixed index.Open questions
Sampled=1be honored as a keep at all (row 3)? It only over-retains, but it is still X-Ray driving Datadog sampling. ADD_MERGE_XRAY_TRACES-style gate would cover that, and also cover adopting the AWS trace id for correlation in the first place. Neither is gated today:xrayisn't aTracePropagationStyleindd-trace-rs, and this fallback sits outside the propagator, so noDD_TRACE_PROPAGATION_STYLEsetting reaches it —extract_generated_span_contexttakes no config argument.Correcting my earlier note on this PR: I said porting Go's regex would regress #452's deliberate acceptance of AWS-native ids. Having read
XRayHttpCodec, that fixture's unpadded root id cannot have come from dd-trace-java's injector, so the missing guard looks like an oversight rather than intent.Testing
cargo test --lib546 pass,clippy -D warningsclean,fmtclean. One case per row of the table above, plus a truncated-header case for the slice.Verified on a live Lambda (Go, arm64,
provided.al2023, SQS trigger, us-east-2,datadog-lambda-go v1.32.0+dd-trace-go/v2 v2.9.2to match the reported setup), sending each header shape with no_datadogattribute. Counts are distinct spans — the repro trace has 7, andDD_TRACE_DEBUGlogs each twice.SampledDatadog-Extension-ARM:98…-00000000<16 hex>0prio=0prio=0(unchanged)…-00000000<16 hex>1prio=1prio=1(unchanged)0prio=0prio=11prio=1prio=1(unchanged)All four log
Returning generated span context, and the tracer's trace id equals the header's low 64 bits in every row, so the fallback still runs and correlation is intact:Row 1 is the case the first version of this PR got wrong: it sent priority
1there, discarding a legitimate Datadog drop. Row 3 is the reported bug. Every row still logsReturning generated span context, so the fallback runs throughout, and the trace id still equals the header's low 64 bits.Not re-verified: the resulting backend span count (the reported "14 → 2"). Traces flushed fine, but the sandbox API key's org isn't one I can query, so I could not count spans in the UI. That step follows from p0 traces being dropped rather than from a measurement here.
🤖 Generated with Claude Code