Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backpressure: only downsample a max of 10 times #2347

Merged
merged 1 commit into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions sentry_sdk/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@
from typing import Optional


MAX_DOWNSAMPLE_FACTOR = 10


class Monitor(object):
"""
Performs health checks in a separate thread once every interval seconds
Expand All @@ -25,7 +28,7 @@ def __init__(self, transport, interval=10):
self.interval = interval # type: float

self._healthy = True
self._downsample_factor = 1 # type: int
self._downsample_factor = 0 # type: int

self._thread = None # type: Optional[Thread]
self._thread_lock = Lock()
Expand Down Expand Up @@ -64,13 +67,14 @@ def run(self):
def set_downsample_factor(self):
# type: () -> None
if self._healthy:
if self._downsample_factor > 1:
if self._downsample_factor > 0:
logger.debug(
"[Monitor] health check positive, reverting to normal sampling"
)
self._downsample_factor = 1
self._downsample_factor = 0
else:
self._downsample_factor *= 2
if self.downsample_factor < MAX_DOWNSAMPLE_FACTOR:
self._downsample_factor += 1
logger.debug(
"[Monitor] health check negative, downsampling with a factor of %d",
self._downsample_factor,
Expand Down
4 changes: 2 additions & 2 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -595,7 +595,7 @@ def finish(self, hub=None, end_timestamp=None):
# exclusively based on sample rate but also traces sampler, but
# we handle this the same here.
if client.transport and has_tracing_enabled(client.options):
if client.monitor and client.monitor.downsample_factor > 1:
if client.monitor and client.monitor.downsample_factor > 0:
reason = "backpressure"
else:
reason = "sample_rate"
Expand Down Expand Up @@ -758,7 +758,7 @@ def _set_initial_sampling_decision(self, sampling_context):
self.sample_rate = float(sample_rate)

if client.monitor:
self.sample_rate /= client.monitor.downsample_factor
self.sample_rate /= 2**client.monitor.downsample_factor

# if the function returned 0 (or false), or if `traces_sample_rate` is
# 0, it's a sign the transaction should be dropped
Expand Down
14 changes: 7 additions & 7 deletions tests/test_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_monitor_if_enabled(sentry_init):
assert monitor._thread is None

assert monitor.is_healthy() is True
assert monitor.downsample_factor == 1
assert monitor.downsample_factor == 0
assert monitor._thread is not None
assert monitor._thread.name == "sentry.monitor"

Expand All @@ -49,11 +49,11 @@ def test_monitor_unhealthy(sentry_init):
monitor.interval = 0.1

assert monitor.is_healthy() is True
monitor.run()
assert monitor.is_healthy() is False
assert monitor.downsample_factor == 2
monitor.run()
assert monitor.downsample_factor == 4

for i in range(15):
monitor.run()
assert monitor.is_healthy() is False
assert monitor.downsample_factor == (i + 1 if i < 10 else 10)


def test_transaction_uses_downsampled_rate(
Expand All @@ -75,7 +75,7 @@ def test_transaction_uses_downsampled_rate(
assert monitor.is_healthy() is True
monitor.run()
assert monitor.is_healthy() is False
assert monitor.downsample_factor == 2
assert monitor.downsample_factor == 1

with start_transaction(name="foobar") as transaction:
assert transaction.sampled is False
Expand Down