Skip to content

Commit

Permalink
only apply rate limits to tracer sampled spans
Browse files Browse the repository at this point in the history
  • Loading branch information
mabdinur committed Sep 23, 2024
1 parent 5546c23 commit 85de781
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 21 deletions.
5 changes: 0 additions & 5 deletions ddtrace/internal/rate_limiter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down
25 changes: 11 additions & 14 deletions ddtrace/sampler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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)
5 changes: 3 additions & 2 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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: <scope>: <add_prelude_and_context_here>.
features:
- |
For new features such as a new integration or component. Use present tense with the following format:
Format: <scope>: This introduces <new_feature_or_component>.
issues:
- |
For known issues. Use present tense with the following format:
Format: <scope>: There is a known <symptom_of_issue> issue with <affected_code>.
<provide_actionable_workaround_here>.
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: <scope>: This upgrades <present_tense_explanation>. With this upgrade, you can <actionable_step_for_user>.
- |
Format: <scope>: <affected_code> is now removed. As an alternative to <affected_code>, you can use <alternative> 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: <scope>: <affected_code> is deprecated and will be removed in <version_to_be_removed>.
As an alternative to <affected_code>, you can use <alternative> 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: <scope>: This fix resolves an issue where <ABC_bug> caused <XYZ_situation>.
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: <scope>: <add_release_note_here>.

0 comments on commit 85de781

Please sign in to comment.