Skip to content

Commit 31cd944

Browse files
bqbitsemmettbutler
authored andcommitted
fix(sampling): validate_sampling_decision giving log warns on supported mechanisms (#13554)
#13516 https://gitlab.ddbuild.io/DataDog/apm-reliability/dd-trace-py/-/jobs/961877338#L460 =========================== short test summary info ============================ FAILED tests/integration/test_sampling.py::test_supported_sampling_mechanism - AssertionError: DATA_JOBS_MONITORING returned {'_dd.propagation_error': 'decoding_error'} assert {'_dd.propagation_error': 'decoding_error'} != {'_dd.propagation_error': 'decoding_error'} ====== 1 failed, 104 passed, 28 skipped, 6 xpassed, 14 warnings in 26.31s ====== Test failed with exit code 1 - [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)) - [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) --------- Co-authored-by: Emmett Butler <[email protected]> (cherry picked from commit 733711e)
1 parent 266ee38 commit 31cd944

File tree

4 files changed

+55
-6
lines changed

4 files changed

+55
-6
lines changed

ddtrace/internal/sampling.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import json
2-
import re
32
from typing import TYPE_CHECKING # noqa:F401
43
from typing import Any
54
from typing import Dict
@@ -71,9 +70,9 @@ class PriorityCategory(object):
7170
RULE_DYNAMIC = "rule_dynamic"
7271

7372

74-
# Use regex to validate trace tag value
75-
TRACE_TAG_RE = re.compile(r"^-([0-9])$")
76-
73+
SAMPLING_MECHANISM_CONSTANTS = {
74+
"-{}".format(value) for name, value in vars(SamplingMechanism).items() if name.isupper()
75+
}
7776

7877
SpanSamplingRules = TypedDict(
7978
"SpanSamplingRules",
@@ -93,7 +92,7 @@ def validate_sampling_decision(
9392
value = meta.get(SAMPLING_DECISION_TRACE_TAG_KEY)
9493
if value:
9594
# Skip propagating invalid sampling mechanism trace tag
96-
if TRACE_TAG_RE.match(value) is None:
95+
if value not in SAMPLING_MECHANISM_CONSTANTS:
9796
del meta[SAMPLING_DECISION_TRACE_TAG_KEY]
9897
meta["_dd.propagation_error"] = "decoding_error"
9998
log.warning("failed to decode _dd.p.dm: %r", value)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
tracing: This fix resolves an issue where the library fails to decode a supported sampling mechanism, resulting in the log line: "failed to decode _dd.p.dm: ..."

tests/integration/test_sampling.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,52 @@ def test_sampling_with_default_sample_rate_1_and_manual_drop(writer, tracer):
9292
span.set_tag(MANUAL_DROP_KEY)
9393

9494

95+
def test_supported_sampling_mechanism():
96+
"""
97+
validate_sampling_decision should not give errors for supported sampling mechanisms
98+
"""
99+
from ddtrace.internal.constants import SAMPLING_DECISION_TRACE_TAG_KEY
100+
from ddtrace.internal.constants import SamplingMechanism
101+
from ddtrace.internal.sampling import validate_sampling_decision
102+
103+
# This list can grow over time so we should test all of them
104+
supported_mechanisms = {
105+
name: getattr(SamplingMechanism, name) for name in dir(SamplingMechanism) if not name.startswith("_")
106+
}
107+
108+
# The mechanisms we support should NOT return a decoding error
109+
for mechanism, value in supported_mechanisms.items():
110+
sampling_decision_validation = None
111+
meta = {}
112+
sampling_dm_value = "-" + str(value)
113+
meta = {SAMPLING_DECISION_TRACE_TAG_KEY: sampling_dm_value}
114+
sampling_decision_validation = validate_sampling_decision(meta)
115+
decoding_error_result = {"_dd.propagation_error": "decoding_error"}
116+
assert sampling_decision_validation != decoding_error_result, f"{mechanism} returned {decoding_error_result}"
117+
118+
119+
def test_unsupported_sampling_mechanism():
120+
"""
121+
Unsupported sampling mechanisms actually return a decoding error in validate_sampling_decision
122+
"""
123+
from ddtrace.internal.constants import SAMPLING_DECISION_TRACE_TAG_KEY
124+
from ddtrace.internal.sampling import validate_sampling_decision
125+
126+
meta = {SAMPLING_DECISION_TRACE_TAG_KEY: "-999999999999"}
127+
sampling_decision_validation = validate_sampling_decision(meta)
128+
decoding_error_result = {"_dd.propagation_error": "decoding_error"}
129+
assert (
130+
sampling_decision_validation == decoding_error_result
131+
), f"Instead of getting {decoding_error_result}, received {sampling_decision_validation}"
132+
133+
134+
@pytest.mark.snapshot()
135+
@pytest.mark.subprocess(env={"DD_TRACE_SAMPLING_RULES": json.dumps([{"sample_rate": 0, "resource": RESOURCE}])})
136+
def test_extended_sampling_resource():
137+
from ddtrace.trace import tracer
138+
from tests.integration.test_sampling import RESOURCE
139+
140+
95141
@snapshot_parametrized_with_writers
96142
def test_sampling_with_default_sample_rate_1_and_manual_keep(writer, tracer):
97143
sampler = DatadogSampler(default_sample_rate=1)

tests/tracer/test_propagation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,7 @@ def test_extract_unicode(tracer): # noqa: F811
926926
("_dd.p.dm=-", {"_dd.propagation_error": "decoding_error"}),
927927
("_dd.p.dm=--1", {"_dd.propagation_error": "decoding_error"}),
928928
("_dd.p.dm=-1.0", {"_dd.propagation_error": "decoding_error"}),
929-
("_dd.p.dm=-10", {"_dd.propagation_error": "decoding_error"}),
929+
("_dd.p.dm=-13", {"_dd.propagation_error": "decoding_error"}),
930930
],
931931
)
932932
def test_extract_dm(x_datadog_tags, expected_trace_tags):

0 commit comments

Comments
 (0)