Skip to content

Commit ea64efc

Browse files
authored
fix(tracing): do not incorrectly match on None with ?* (#13100)
Fixes #12775 ## 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)
1 parent f5209c5 commit ea64efc

File tree

3 files changed

+22
-8
lines changed

3 files changed

+22
-8
lines changed

ddtrace/_trace/sampling_rule.py

+7-8
Original file line numberDiff line numberDiff line change
@@ -153,26 +153,25 @@ def check_tags(self, meta, metrics):
153153
tag_match = False
154154
for tag_key in self._tag_value_matchers.keys():
155155
value = meta.get(tag_key)
156-
tag_match = self._tag_value_matchers[tag_key].match(str(value))
157-
# If the value doesn't match in meta, check the metrics
158-
if tag_match is False:
156+
# it's because we're not checking metrics first before continuing
157+
if value is None:
159158
value = metrics.get(tag_key)
159+
if value is None:
160+
continue
160161
# Floats: Matching floating point values with a non-zero decimal part is not supported.
161162
# For floating point values with a non-zero decimal part, any all * pattern always returns true.
162163
# Other patterns always return false.
163164
if isinstance(value, float):
164165
if not value.is_integer():
165-
if self._tag_value_matchers[tag_key].pattern == "*":
166+
if all(c == "*" for c in self._tag_value_matchers[tag_key].pattern):
166167
tag_match = True
168+
continue
167169
else:
168170
return False
169-
continue
170171
else:
171172
value = int(value)
172173

173-
tag_match = self._tag_value_matchers[tag_key].match(str(value))
174-
else:
175-
continue
174+
tag_match = self._tag_value_matchers[tag_key].match(str(value))
176175
# if we don't match with all specified tags for a rule, it's not a match
177176
if tag_match is False:
178177
return False
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
fixes:
3+
- |
4+
fix(tracer):Fixed a bug in the sampling rule matcher where the pattern ``?*`` was not being matched correctly
5+
for ``DD_TRACE_SAMPLING_RULES`` tags, due to it matching on spans with no tag matching the specified key.

tests/tracer/test_sampler.py

+10
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,16 @@ def sample(self, span):
729729
0,
730730
None,
731731
),
732+
( # Regression test for https://github.com/DataDog/dd-trace-py/issues/12775
733+
# We should not match None values with ?* patterns.
734+
DatadogSampler(
735+
rules=[SamplingRule(sample_rate=0, name="?*", resource="?*", service="?*", tags={"key": "?*"})],
736+
),
737+
AUTO_KEEP,
738+
SamplingMechanism.DEFAULT,
739+
0,
740+
None,
741+
),
732742
],
733743
)
734744
def test_datadog_sampler_sample_rules(sampler, sampling_priority, sampling_mechanism, rule, limit, dummy_tracer):

0 commit comments

Comments
 (0)