Skip to content

Commit 733711e

Browse files
bqbitsemmettbutler
andauthored
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 ## 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) --------- Co-authored-by: Emmett Butler <[email protected]>
1 parent 22ded2c commit 733711e

File tree

4 files changed

+48
-6
lines changed

4 files changed

+48
-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
@@ -57,9 +56,9 @@ class PriorityCategory(object):
5756
RULE_DYNAMIC = "rule_dynamic"
5857

5958

60-
# Use regex to validate trace tag value
61-
TRACE_TAG_RE = re.compile(r"^-([0-9])$")
62-
59+
SAMPLING_MECHANISM_CONSTANTS = {
60+
"-{}".format(value) for name, value in vars(SamplingMechanism).items() if name.isupper()
61+
}
6362

6463
SpanSamplingRules = TypedDict(
6564
"SpanSamplingRules",
@@ -79,7 +78,7 @@ def validate_sampling_decision(
7978
value = meta.get(SAMPLING_DECISION_TRACE_TAG_KEY)
8079
if value:
8180
# Skip propagating invalid sampling mechanism trace tag
82-
if TRACE_TAG_RE.match(value) is None:
81+
if value not in SAMPLING_MECHANISM_CONSTANTS:
8382
del meta[SAMPLING_DECISION_TRACE_TAG_KEY]
8483
meta["_dd.propagation_error"] = "decoding_error"
8584
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: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,45 @@ def test_sampling_with_rate_limit_3():
3333
tracer.trace("child").finish()
3434

3535

36+
def test_supported_sampling_mechanism():
37+
"""
38+
validate_sampling_decision should not give errors for supported sampling mechanisms
39+
"""
40+
from ddtrace.internal.constants import SAMPLING_DECISION_TRACE_TAG_KEY
41+
from ddtrace.internal.constants import SamplingMechanism
42+
from ddtrace.internal.sampling import validate_sampling_decision
43+
44+
# This list can grow over time so we should test all of them
45+
supported_mechanisms = {
46+
name: getattr(SamplingMechanism, name) for name in dir(SamplingMechanism) if not name.startswith("_")
47+
}
48+
49+
# The mechanisms we support should NOT return a decoding error
50+
for mechanism, value in supported_mechanisms.items():
51+
sampling_decision_validation = None
52+
meta = {}
53+
sampling_dm_value = "-" + str(value)
54+
meta = {SAMPLING_DECISION_TRACE_TAG_KEY: sampling_dm_value}
55+
sampling_decision_validation = validate_sampling_decision(meta)
56+
decoding_error_result = {"_dd.propagation_error": "decoding_error"}
57+
assert sampling_decision_validation != decoding_error_result, f"{mechanism} returned {decoding_error_result}"
58+
59+
60+
def test_unsupported_sampling_mechanism():
61+
"""
62+
Unsupported sampling mechanisms actually return a decoding error in validate_sampling_decision
63+
"""
64+
from ddtrace.internal.constants import SAMPLING_DECISION_TRACE_TAG_KEY
65+
from ddtrace.internal.sampling import validate_sampling_decision
66+
67+
meta = {SAMPLING_DECISION_TRACE_TAG_KEY: "-999999999999"}
68+
sampling_decision_validation = validate_sampling_decision(meta)
69+
decoding_error_result = {"_dd.propagation_error": "decoding_error"}
70+
assert (
71+
sampling_decision_validation == decoding_error_result
72+
), f"Instead of getting {decoding_error_result}, received {sampling_decision_validation}"
73+
74+
3675
@pytest.mark.snapshot()
3776
@pytest.mark.subprocess(env={"DD_TRACE_SAMPLING_RULES": json.dumps([{"sample_rate": 0, "resource": RESOURCE}])})
3877
def test_extended_sampling_resource():

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)