[SLES-2971] feat(traces): gate X-Ray-driven sampling behind DD_MERGE_XRAY_TRACES - #1337
Draft
lym953 wants to merge 1 commit into
Draft
Conversation
…XRAY_TRACES Adopting an AWS-generated X-Ray `Sampled` flag as a Datadog sampling decision is opt-in behavior in the tracer libraries, where `DD_MERGE_XRAY_TRACES` defaults to false. bottlecap never read it, so there was no way to decline. Add the config and gate the flag on it. A header a Datadog library planted is never gated -- its `Sampled` is a Datadog decision regardless of the setting, and gating it would break dd-trace-java's SQS propagation, which uses this same header and has nothing to do with X-Ray. Scope is the sampling verdict only. Trace and parent IDs are still taken from an AWS-generated header either way, so correlation is unchanged by this flag; that is pre-existing v88+ behavior and gating it is a separate argument. The flag is read off `DatadogCompositePropagator`, which already carries the config to every extraction site -- threading `Arc<Config>` through the listener, LWA proxy, and interceptor state instead would have touched five more files.
|
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.
Stacked on #1325 — review that first; this diff is only the gate.
Why
#1325 stops an AWS-generated
Sampled=0from dropping Datadog traces, but an AWS-generatedSampled=1still becomes an explicit priority1— X-Ray still drives Datadog sampling, in the direction that over-retains rather than loses data.The tracer libraries make this opt-in through
DD_MERGE_XRAY_TRACES(DD_MERGE_DATADOG_XRAY_TRACESin Ruby), default false. bottlecap never read it, so there was no way to decline.What
Adds
merge_xray_tracestoLambdaConfig(default false) and gates the flag on it:Sampled…-00000000<16 hex>(Datadog-planted)00…-00000000<16 hex>11011Three scope decisions:
Datadog-planted headers are never gated.
dd-trace-javaplants context in this sameAWSTraceHeaderfor SQS, and it is the only carrier for JMS→SQS. Gating it on an X-Ray flag would break Datadog-to-Datadog propagation for users with no X-Ray involvement.Correlation is not gated. Trace and parent ids are taken from an AWS-generated header in both states — pre-existing v88+ behavior that loses no data, and gating it would cost the reporting customer their producer→consumer link. Gating it is separately arguable.
merge onhonors an AWS-generatedSampled=0as a drop, reinstating the reported behavior for anyone who asks for it: the flag means X-Ray's decisions count. If reviewers prefer the flag to govern correlation only and never sampling, that's a three-line change.The flag is read off
DatadogCompositePropagator, which already carriesArc<Config>to every extraction site. ThreadingArc<Config>explicitly would touch five more files — the listener, LWA and interceptor state tuples,processor_service, andmain.rs. Happy to switch; it's mechanical.Open question
The docs scope trace merging to Node.js and Python, and describe it as merging X-Ray's own spans via the invocation's
_X_AMZN_TRACE_ID— not as governing an SQS message system attribute. The reported case is Go. Reuse this variable and broaden its meaning, or add a distinct one? The docs need a line either way.Testing
549 tests pass,
clippy -D warningsandfmtclean — opt-in honoring a drop, Datadog-planted headers identical in both states, ids preserved in both.Verified live on the SLES-2971 repro (Go, arm64,
provided.al2023, SQS trigger), all four shapes in both flag states:Sampled10110011Only the AWS-generated
Sampled=0row moves with the flag. A trace id was present in all eight runs, confirming correlation is unaffected either way.🤖 Generated with Claude Code