From 050f8da85d5f9d925a488f4d05635836e32f4e39 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Wed, 9 Sep 2026 16:52:52 +0200 Subject: [PATCH 1/2] feat(sentry): Replace the flat trace sample rate with a traces sampler The snuba project is moving off server-side dynamic sampling. Move the sampling rules into the SDK so the client makes the same decisions. A request that carries a sampling decision from the caller keeps it, so traces stay complete. Traces that start in snuba use the base rate, a separate rate for health check requests, and full sampling in development-style environments. The replay ID boost has no meaning for snuba and is left out. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01G5hviFo9NtMorc77sdcYRC --- snuba/environment.py | 31 ++++++++++++++- snuba/settings/__init__.py | 8 +++- tests/test_environment.py | 78 +++++++++++++++++++++++++++++++++++++- 3 files changed, 114 insertions(+), 3 deletions(-) diff --git a/snuba/environment.py b/snuba/environment.py index f689c3dcc51..662902621e5 100644 --- a/snuba/environment.py +++ b/snuba/environment.py @@ -1,5 +1,7 @@ import logging import os +import re +from typing import Any import sentry_sdk import structlog @@ -160,6 +162,33 @@ def before_send(event: Event, hint: Hint) -> Event | None: return event +_HEALTH_CHECK_PATH = re.compile(r"^/health(_envoy)?/?$") + + +def traces_sampler(sampling_context: dict[str, Any]) -> float: + """ + Decide the sample rate for a root span. This replaces the server-side + dynamic sampling rules for the snuba project. + + A request that carries a sampling decision from the caller keeps that + decision, so traces stay complete. Only traces that start in snuba get + a fresh decision. + """ + parent_sampled = sampling_context.get("parent_sampled") + if parent_sampled is not None: + return 1.0 if parent_sampled else 0.0 + + environment = sentry_sdk.get_client().options.get("environment") or "" + if any(marker in environment for marker in settings.SENTRY_ALWAYS_SAMPLED_ENVIRONMENTS): + return 1.0 + + path = sampling_context.get("wsgi_environ", {}).get("PATH_INFO", "") + if _HEALTH_CHECK_PATH.match(path): + return settings.SENTRY_HEALTH_CHECK_TRACE_SAMPLE_RATE + + return settings.SENTRY_TRACE_SAMPLE_RATE + + def setup_sentry() -> None: sentry_sdk.init( dsn=settings.SENTRY_DSN, @@ -179,7 +208,7 @@ def setup_sentry() -> None: # the value for release is also computed in rust-snuba, please keep the # logic in sync release=os.getenv("SNUBA_RELEASE"), - traces_sample_rate=settings.SENTRY_TRACE_SAMPLE_RATE, + traces_sampler=traces_sampler, profiles_sample_rate=settings.SNUBA_PROFILES_SAMPLE_RATE, # Stream spans as they finish. Disables the legacy tracing API # (start_span/start_transaction/update_current_span/scope.span). diff --git a/snuba/settings/__init__.py b/snuba/settings/__init__.py index f9225d3ef41..4fd1c6afa50 100644 --- a/snuba/settings/__init__.py +++ b/snuba/settings/__init__.py @@ -218,7 +218,13 @@ class RedisClusters(TypedDict): # Sentry Options SENTRY_DSN: str | None = None -SENTRY_TRACE_SAMPLE_RATE = 0 +# Sample rate for traces that start in snuba (no incoming sampling decision). +# Requests that carry a sampling decision from the caller inherit it. +SENTRY_TRACE_SAMPLE_RATE = 0.0 +# Sample rate for traces started by health check requests. +SENTRY_HEALTH_CHECK_TRACE_SAMPLE_RATE = 0.0 +# Environments whose traces are always sampled, matched as substrings. +SENTRY_ALWAYS_SAMPLED_ENVIRONMENTS = ("debug", "dev", "local", "qa", "test") # Snuba Admin Options SLACK_API_TOKEN = os.environ.get("SLACK_API_TOKEN") diff --git a/tests/test_environment.py b/tests/test_environment.py index e8359fc9d61..2f2a34ed2bb 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -1,3 +1,8 @@ +from collections.abc import Callable +from typing import Any + +import pytest +import sentry_sdk from arroyo.processing.strategies.run_task_with_multiprocessing import ( ChildProcessTerminated, ) @@ -6,7 +11,8 @@ from redis.exceptions import TimeoutError as RedisTimeoutError from sentry_sdk.types import Event, Hint -from snuba.environment import before_send +from snuba import settings +from snuba.environment import before_send, traces_sampler from snuba.query.allocation_policies import AllocationPolicyViolations from snuba.web.rpc.common.exceptions import RPCAllocationPolicyException @@ -144,3 +150,73 @@ def test_before_send_terminates_on_cyclic_cause_chain() -> None: except ValueError: err.__context__ = err assert before_send(event, _hint_for(err)) is event + + +@pytest.fixture +def sentry_environment(monkeypatch: pytest.MonkeyPatch) -> Callable[[str | None], None]: + def _set(environment: str | None) -> None: + monkeypatch.setattr(sentry_sdk.get_client(), "options", {"environment": environment}) + + _set(None) + return _set + + +@pytest.fixture +def sample_rates(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(settings, "SENTRY_TRACE_SAMPLE_RATE", 0.5) + monkeypatch.setattr(settings, "SENTRY_HEALTH_CHECK_TRACE_SAMPLE_RATE", 0.01) + + +def _request(path: str, parent_sampled: bool | None = None) -> dict[str, Any]: + return { + "transaction_context": {"name": "generic WSGI request"}, + "parent_sampled": parent_sampled, + "wsgi_environ": {"PATH_INFO": path}, + } + + +@pytest.mark.usefixtures("sentry_environment", "sample_rates") +def test_traces_sampler_inherits_parent_decision() -> None: + assert traces_sampler(_request("/health", parent_sampled=True)) == 1.0 + assert traces_sampler(_request("/query", parent_sampled=False)) == 0.0 + + +@pytest.mark.usefixtures("sentry_environment", "sample_rates") +def test_traces_sampler_uses_base_rate_for_root_requests() -> None: + assert traces_sampler(_request("/query")) == 0.5 + assert traces_sampler(_request("/rpc/EndpointTraceItemTable/v1")) == 0.5 + + +@pytest.mark.usefixtures("sentry_environment", "sample_rates") +def test_traces_sampler_uses_health_check_rate() -> None: + assert traces_sampler(_request("/health")) == 0.01 + assert traces_sampler(_request("/health/")) == 0.01 + assert traces_sampler(_request("/health_envoy")) == 0.01 + assert traces_sampler(_request("/healthy")) == 0.5 + + +@pytest.mark.usefixtures("sample_rates") +def test_traces_sampler_uses_base_rate_without_wsgi_environ( + sentry_environment: Callable[[str | None], None], +) -> None: + sentry_environment("us") + assert traces_sampler({"transaction_context": {"name": "[cli init] api"}}) == 0.5 + + +@pytest.mark.usefixtures("sample_rates") +@pytest.mark.parametrize("environment", ["dev", "local-simon", "test", "qa-eu", "debug"]) +def test_traces_sampler_samples_everything_in_development_environments( + sentry_environment: Callable[[str | None], None], environment: str +) -> None: + sentry_environment(environment) + assert traces_sampler(_request("/health")) == 1.0 + assert traces_sampler(_request("/query")) == 1.0 + + +@pytest.mark.usefixtures("sample_rates") +@pytest.mark.parametrize("environment", ["us", "de", "s4s2", "production"]) +def test_traces_sampler_keeps_rates_in_production_environments( + sentry_environment: Callable[[str | None], None], environment: str +) -> None: + sentry_environment(environment) + assert traces_sampler(_request("/query")) == 0.5 From 8a5ac322bd778d614efae953ad30f6a5eebd5bc7 Mon Sep 17 00:00:00 2001 From: Simon Hellmayr Date: Mon, 14 Sep 2026 13:32:44 +0200 Subject: [PATCH 2/2] feat(sentry): Keep only a fraction of the traces that callers sampled Nearly all snuba requests carry a sampling decision from the caller, and the traces_sampler kept every sampled one. That volume is set by the callers' sample rates, so snuba had no lever of its own on the size of its project. Keep a caller-sampled trace at SENTRY_INHERITED_TRACE_SAMPLE_RATE. The decision is derived from the trace id, so every snuba request of one trace gets the same answer, and the bucketing matches the one sentry uses in its transport so both services keep the same traces when they run with the same rate. Co-Authored-By: Claude Fable 5.1 --- snuba/environment.py | 23 +++++++++++++++++++---- snuba/settings/__init__.py | 7 ++++++- tests/test_environment.py | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/snuba/environment.py b/snuba/environment.py index 662902621e5..71618ccca68 100644 --- a/snuba/environment.py +++ b/snuba/environment.py @@ -165,18 +165,33 @@ def before_send(event: Event, hint: Hint) -> Event | None: _HEALTH_CHECK_PATH = re.compile(r"^/health(_envoy)?/?$") +def _inherited_sample_rate(sampling_context: dict[str, Any]) -> float: + """ + Keep a fraction of the traces that the caller sampled, chosen by trace + id so that every snuba request of one trace gets the same decision. + + The bucketing matches the one sentry applies in its own transport, so + both services keep the same traces when they run with the same rate. + """ + rate = settings.SENTRY_INHERITED_TRACE_SAMPLE_RATE + trace_id = sampling_context.get("transaction_context", {}).get("trace_id") + if rate >= 1.0 or not trace_id: + return rate + return 1.0 if int(trace_id[:8], 16) % 100 < int(rate * 100) else 0.0 + + def traces_sampler(sampling_context: dict[str, Any]) -> float: """ Decide the sample rate for a root span. This replaces the server-side dynamic sampling rules for the snuba project. - A request that carries a sampling decision from the caller keeps that - decision, so traces stay complete. Only traces that start in snuba get - a fresh decision. + A request that the caller did not sample is never sampled. A request + that the caller sampled is kept at `SENTRY_INHERITED_TRACE_SAMPLE_RATE`. + Only traces that start in snuba get a fresh decision. """ parent_sampled = sampling_context.get("parent_sampled") if parent_sampled is not None: - return 1.0 if parent_sampled else 0.0 + return _inherited_sample_rate(sampling_context) if parent_sampled else 0.0 environment = sentry_sdk.get_client().options.get("environment") or "" if any(marker in environment for marker in settings.SENTRY_ALWAYS_SAMPLED_ENVIRONMENTS): diff --git a/snuba/settings/__init__.py b/snuba/settings/__init__.py index 4fd1c6afa50..12189148b92 100644 --- a/snuba/settings/__init__.py +++ b/snuba/settings/__init__.py @@ -219,8 +219,13 @@ class RedisClusters(TypedDict): # Sentry Options SENTRY_DSN: str | None = None # Sample rate for traces that start in snuba (no incoming sampling decision). -# Requests that carry a sampling decision from the caller inherit it. SENTRY_TRACE_SAMPLE_RATE = 0.0 +# Fraction of the traces sampled by the caller that snuba keeps. Nearly all +# snuba traffic carries a caller decision, so this is the main lever on the +# volume of the snuba project. +SENTRY_INHERITED_TRACE_SAMPLE_RATE = float( + os.environ.get("SENTRY_INHERITED_TRACE_SAMPLE_RATE", 0.1) +) # Sample rate for traces started by health check requests. SENTRY_HEALTH_CHECK_TRACE_SAMPLE_RATE = 0.0 # Environments whose traces are always sampled, matched as substrings. diff --git a/tests/test_environment.py b/tests/test_environment.py index 2f2a34ed2bb..6b74396aa6f 100644 --- a/tests/test_environment.py +++ b/tests/test_environment.py @@ -165,11 +165,17 @@ def _set(environment: str | None) -> None: def sample_rates(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(settings, "SENTRY_TRACE_SAMPLE_RATE", 0.5) monkeypatch.setattr(settings, "SENTRY_HEALTH_CHECK_TRACE_SAMPLE_RATE", 0.01) + monkeypatch.setattr(settings, "SENTRY_INHERITED_TRACE_SAMPLE_RATE", 1.0) -def _request(path: str, parent_sampled: bool | None = None) -> dict[str, Any]: +def _request( + path: str, parent_sampled: bool | None = None, trace_id: str | None = None +) -> dict[str, Any]: + transaction_context: dict[str, Any] = {"name": "generic WSGI request"} + if trace_id is not None: + transaction_context["trace_id"] = trace_id return { - "transaction_context": {"name": "generic WSGI request"}, + "transaction_context": transaction_context, "parent_sampled": parent_sampled, "wsgi_environ": {"PATH_INFO": path}, } @@ -181,6 +187,28 @@ def test_traces_sampler_inherits_parent_decision() -> None: assert traces_sampler(_request("/query", parent_sampled=False)) == 0.0 +@pytest.mark.usefixtures("sentry_environment", "sample_rates") +def test_traces_sampler_keeps_a_fraction_of_sampled_traces_by_trace_id( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "SENTRY_INHERITED_TRACE_SAMPLE_RATE", 0.1) + + # int("00000005", 16) % 100 == 5 < 10, int("00000063", 16) % 100 == 99 + kept = "00000005" + "0" * 24 + dropped = "00000063" + "0" * 24 + assert traces_sampler(_request("/query", parent_sampled=True, trace_id=kept)) == 1.0 + assert traces_sampler(_request("/query", parent_sampled=True, trace_id=dropped)) == 0.0 + assert traces_sampler(_request("/query", parent_sampled=False, trace_id=kept)) == 0.0 + + +@pytest.mark.usefixtures("sentry_environment", "sample_rates") +def test_traces_sampler_falls_back_to_the_inherited_rate_without_trace_id( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "SENTRY_INHERITED_TRACE_SAMPLE_RATE", 0.1) + assert traces_sampler(_request("/query", parent_sampled=True)) == 0.1 + + @pytest.mark.usefixtures("sentry_environment", "sample_rates") def test_traces_sampler_uses_base_rate_for_root_requests() -> None: assert traces_sampler(_request("/query")) == 0.5