fix(sampling): skip _dd.p.dm tag when sampling priority is propagated#14308
Conversation
|
|
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 266 ± 3 ms. The average import time from base is: 268 ± 4 ms. The import time difference between this PR and base is: -1.7 ± 0.1 ms. Import time breakdownThe following import paths have shrunk:
|
mabdinur
left a comment
mabdinur
left a comment
There was a problem hiding this comment.
Can we add a simple unit test to test_propagation.py.
Performance SLOsCandidate: rachel.yang/sampling-tags (7a0b1b4) 🔵 No Baseline Data (24 suites)🔵 coreapiscenario - 12/12 (2 unstable)🔵 No baseline data available for this suite
|
rachelyangdog
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the trace sampling decision tag (_dd.p.dm) was being incorrectly set during header extraction even when a sampling priority was already propagated from upstream. The fix ensures that the local sampling decision tag is only applied when no head sampling decision exists.
Key Changes:
- Added logic to remove the
_dd.p.dmtag when a sampling priority is propagated via headers - Updated test expectations to reflect the corrected behavior where
_dd.p.dmshould not be present when sampling priority is propagated - Added comprehensive test coverage for the sampling decision tag behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ddtrace/propagation/http.py | Implemented the core fix by adding logic to remove LOCAL_USER_TRACE_SAMPLING_RULE tag when sampling priority is propagated |
| tests/tracer/test_propagation.py | Updated test expectations and added new test cases to verify the corrected sampling decision tag behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…#14308) Fixes an issue where the trace sampling decision tag (_dd.p.dm) was being incorrectly set during header extraction even when a sampling priority was already propagated. This issue was discovered when [new system-tests ](DataDog/system-tests#5014) were introduced. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
Fixes an issue where the trace sampling decision tag (_dd.p.dm) was being incorrectly set during header extraction even when a sampling priority was already propagated.
This issue was discovered when new system-tests were introduced.
Checklist
Reviewer Checklist