Repository navigation
fix(trace): keep sampling priority numeric in SpanContextWrapper.fromTraceContext - #850
Open
SiddhithaBhoopathy wants to merge 2 commits into
Conversation
…TraceContext Co-authored-by: Cursor <cursoragent@cursor.com>
…-extractor snapshot Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
Hi @SiddhithaBhoopathy this change looks pretty straightforward and should work fine. I copied your changes into #854 to run the ci. Pending that, should be good to merge. |
Collaborator
|
@lucaspimentel asking since you've been working on sampling recently: would changing the type of sampling-priority (string -> number) break anything? |
SiddhithaBhoopathy
marked this pull request as ready for review
October 23, 2026 17:56
There was a problem hiding this comment.
More details
Keeping sampling priorities numeric matches the context type contract and dd-trace’s propagation requirements. The completed static review found no reportable regressions across the four sampling modes.
🤖 Bits Code Review · Commit 8f15251
This branch has not been deployed
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.
What does this PR do?
Keeps the sampling priority numeric in
SpanContextWrapper.fromTraceContext.fromTraceContextwas convertingsampleModeto a string ("2","-1") before passing it toDatadogSpanContext. dd-trace only injectsx-datadog-sampling-prioritywhen the priority is an integer (text_map.js_injectSamplingPriority), and only emits the_sampling_priority_v1metric when it is a number (span_format.js). Because child spans share the parent's_samplingobject and the priority sampler skips contexts that already have a priority, the string sticks for the whole trace and the sampling decision is dropped downstream.Motivation
Escalation SLES-3080: an API Gateway custom authorizer's sampling decision is not preserved by the integration Lambda. With no Datadog/W3C headers on the request, the authorizer falls back to X-Ray (
USER_KEEP/USER_REJECT), injects_datadogwithoutx-datadog-sampling-priority, and the integration Lambda (extract orderdatadog,tracecontext) re-samples independently. The customer confirmed that normalizing the priority to a number in the authorizer restores the header and preserves the upstream decision in all keep/reject × success/error cases.Testing Guidelines
fromTraceContextproduces an integer priority forUSER_KEEP,USER_REJECT,AUTO_KEEPandAUTO_REJECT.span-context-wrapper.spec.ts,xray-service.spec.ts,custom.spec.ts,extractor.spec.ts,sns.spec.ts,sqs.spec.ts,sns-sqs.spec.ts).cjs-custom-extractorintegration log snapshot: its spans now include"_sampling_priority_v1": 1, matching every other case (e.g.cjs-capture-payload). Before this fix, the custom-extractor case was the only one exported without a sampling priority, which is the bug itself showing up in the existing goldens.Additional Notes
Affected paths (all callers of
fromTraceContext):XrayService.extract)AWSTraceHeaderwith Datadog-injected IDs (XrayService.extraceDDContextFromAWSTraceHeader)CustomTraceExtractor)Regression introduced in #429 (W3C support). The
TraceContext.sampleModetype and thesampleMode(): numbergetter already expect a number.Types of Changes
Check all that apply