From 85de7813991194da1a54ac07fba5487f438b2041 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Sep 2024 15:56:27 -0400 Subject: [PATCH] only apply rate limits to tracer sampled spans --- ddtrace/internal/rate_limiter.py | 5 -- ddtrace/sampler.py | 25 ++++--- docs/configuration.rst | 5 +- ...it-on-sampling-rules-6f807defa7d0f5f1.yaml | 65 +++++++++++++++++++ 4 files changed, 79 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/only-rate-limit-on-sampling-rules-6f807defa7d0f5f1.yaml diff --git a/ddtrace/internal/rate_limiter.py b/ddtrace/internal/rate_limiter.py index 63312911ecc..981701cd51b 100644 --- a/ddtrace/internal/rate_limiter.py +++ b/ddtrace/internal/rate_limiter.py @@ -12,7 +12,6 @@ from ddtrace.vendor.debtcollector import deprecate from ..internal import compat -from ..internal.constants import DEFAULT_SAMPLING_RATE_LIMIT class RateLimiter(object): @@ -59,10 +58,6 @@ def __init__(self, rate_limit: int, time_window: float = 1e9): self._lock = threading.Lock() - @property - def _has_been_configured(self): - return self.rate_limit != DEFAULT_SAMPLING_RATE_LIMIT - def is_allowed(self, timestamp_ns: Optional[int] = None) -> bool: """ Check whether the current request is allowed or not diff --git a/ddtrace/sampler.py b/ddtrace/sampler.py index c5fd896acfe..e59718baf4d 100644 --- a/ddtrace/sampler.py +++ b/ddtrace/sampler.py @@ -228,9 +228,10 @@ def __init__( # Use default sample rate of 1.0 super(DatadogSampler, self).__init__() self.default_sample_rate = default_sample_rate + effective_sample_rate = default_sample_rate if default_sample_rate is None: if ddconfig._get_source("_trace_sample_rate") != "default": - default_sample_rate = float(ddconfig._trace_sample_rate) + effective_sample_rate = float(ddconfig._trace_sample_rate) if rate_limit is None: rate_limit = int(ddconfig._trace_rate_limit) @@ -251,9 +252,9 @@ def __init__( elif config._raise: raise TypeError("Rule {!r} must be a sub-class of type ddtrace.sampler.SamplingRules".format(rule)) - # DEV: Default sampling rule must come last - if default_sample_rate is not None: - self.rules.append(SamplingRule(sample_rate=default_sample_rate)) + # DEV: sampling rule must come last + if effective_sample_rate is not None: + self.rules.append(SamplingRule(sample_rate=effective_sample_rate)) # Configure rate limiter self.limiter = RateLimiter(rate_limit) @@ -315,17 +316,18 @@ def sample(self, span): sampler = self._default_sampler # type: BaseSampler sample_rate = self.sample_rate if matched_rule: + # Client based sampling sampled = matched_rule.sample(span) sample_rate = matched_rule.sample_rate + # Apply tracer rate limit ONLY if sampling rule is defined + if sampled: + sampled = self.limiter.is_allowed() + span.set_metric(SAMPLING_LIMIT_DECISION, self.limiter.effective_rate) else: + # Agent based sampling sampled, sampler = super(DatadogSampler, self)._make_sampling_decision(span) if isinstance(sampler, RateSampler): sample_rate = sampler.sample_rate - # Apply rate limit - if sampled: - sampled = self.limiter.is_allowed() - if self.limiter._has_been_configured: - span.set_metric(SAMPLING_LIMIT_DECISION, self.limiter.effective_rate) _set_sampling_tags( span, @@ -346,9 +348,4 @@ def _choose_priority_category_with_rule(self, rule, sampler): return _PRIORITY_CATEGORY.RULE_DYNAMIC return _PRIORITY_CATEGORY.RULE_DEF - if self.limiter._has_been_configured: - # If the default rate limiter is NOT used to sample traces - # the sampling priority must be set to manual keep/drop. - # This will disable agent based sample rates. - return _PRIORITY_CATEGORY.USER return super(DatadogSampler, self)._choose_priority_category(sampler) diff --git a/docs/configuration.rst b/docs/configuration.rst index cb707117b68..4e647e084fa 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -240,10 +240,11 @@ The following environment variables for the tracer are supported: type: int default: 100 description: | - Maximum number of traces per second to sample. Set a rate limit to avoid the ingestion volume overages in the case of traffic spikes. - + Maximum number of traces per second to sample. Set a rate limit to avoid the ingestion volume overages in the case of traffic spikes. This configuration + is only applied when client based sampling is configured, otherwise agent based rate limits are used. version_added: v0.33.0: + v2.14.0: Only applied when DD_TRACE_SAMPLE_RATE, DD_TRACE_SAMPLING_RULES, or DD_SPAN_SAMPLING_RULE are set. DD_TRACE_SAMPLING_RULES: type: JSON array diff --git a/releasenotes/notes/only-rate-limit-on-sampling-rules-6f807defa7d0f5f1.yaml b/releasenotes/notes/only-rate-limit-on-sampling-rules-6f807defa7d0f5f1.yaml new file mode 100644 index 00000000000..47514409f25 --- /dev/null +++ b/releasenotes/notes/only-rate-limit-on-sampling-rules-6f807defa7d0f5f1.yaml @@ -0,0 +1,65 @@ +--- +#instructions: +# The style guide below provides explanations, instructions, and templates to write your own release note. +# Once finished, all irrelevant sections (including this instruction section) should be removed, +# and the release note should be committed with the rest of the changes. +# +# The main goal of a release note is to provide a brief overview of a change and provide actionable steps to the user. +# The release note should clearly communicate what the change is, why the change was made, and how a user can migrate their code. +# +# The release note should also clearly distinguish between announcements and user instructions. Use: +# * Past tense for previous/existing behavior (ex: ``resulted, caused, failed``) +# * Third person present tense for the change itself (ex: ``adds, fixes, upgrades``) +# * Active present infinitive for user instructions (ex: ``set, use, add``) +# +# Release notes should: +# * Use plain language +# * Be concise +# * Include actionable steps with the necessary code changes +# * Include relevant links (bug issues, upstream issues or release notes, documentation pages) +# * Use full sentences with sentence-casing and punctuation. +# * Before using Datadog specific acronyms/terminology, a release note must first introduce them with a definition. +# +# Release notes should not: +# * Be vague. Example: ``fixes an issue in tracing``. +# * Use overly technical language +# * Use dynamic links (``stable/latest/1.x`` URLs). Instead, use static links (specific version, commit hash) whenever possible so that they don't break in the future. +prelude: > + Usually in tandem with a new feature or major change, meant to provide context or background for a major change. + No specific format other than a required scope is provided and the author is requested to use their best judgment. + Format: : . +features: + - | + For new features such as a new integration or component. Use present tense with the following format: + Format: : This introduces . +issues: + - | + For known issues. Use present tense with the following format: + Format: : There is a known issue with . + . +upgrade: + - | + For enhanced functionality or if package dependencies are upgraded. If applicable, include instructions + for how a user can migrate their code. + Use present tense with the following formats, respectively for enhancements or removals: + Format: : This upgrades . With this upgrade, you can . + - | + Format: : is now removed. As an alternative to , you can use instead. +deprecations: + - | + Warning of a component or member of the public API being removed in the future. + Use present tense for when deprecation actually happens and future tense for when removal is planned to happen. + Include deprecation/removal timeline, as well as workarounds and alternatives in the following format: + Format: : is deprecated and will be removed in . + As an alternative to , you can use instead. +fixes: + - | + For reporting bug fixes. + Use past tense for the problem and present tense for the fix and solution in the following format: + Format: : This fix resolves an issue where caused . +other: + - | + For any change which does not fall into any of the above categories. Since changes falling into this category are + likely rare and not very similar to each other, no specific format other than a required scope is provided. + The author is requested to use their best judgment to ensure a quality release note. + Format: : .