Skip to content

Commit

Permalink
feat(tracing): Use sample_rand for sampling decisions (#4044)
Browse files Browse the repository at this point in the history
Use the `sample_rand` value from an incoming trace to make sampling
decisions, rather than generating a random value. When we are the head
SDK starting a new trace, save our randomly-generated value as the
`sample_rand`, and also change the random generation logic so that the
`sample_rand` is computed deterministically based on the `trace_id`.

Depends on:
  - #4040 
  - #4038 

Closes #3998
  • Loading branch information
szokeasaurusrex authored Feb 26, 2025
1 parent 61ae6e7 commit ccb969e
Show file tree
Hide file tree
Showing 12 changed files with 168 additions and 82 deletions.
15 changes: 12 additions & 3 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import uuid
import random
import warnings
from datetime import datetime, timedelta, timezone
from enum import Enum
Expand Down Expand Up @@ -785,6 +784,7 @@ class Transaction(Span):
"_profile",
"_continuous_profile",
"_baggage",
"_sample_rand",
)

def __init__( # type: ignore[misc]
Expand All @@ -809,6 +809,14 @@ def __init__( # type: ignore[misc]
self._continuous_profile = None # type: Optional[ContinuousProfile]
self._baggage = baggage

baggage_sample_rand = (
None if self._baggage is None else self._baggage._sample_rand()
)
if baggage_sample_rand is not None:
self._sample_rand = baggage_sample_rand
else:
self._sample_rand = _generate_sample_rand(self.trace_id)

def __repr__(self):
# type: () -> str
return (
Expand Down Expand Up @@ -1179,10 +1187,10 @@ def _set_initial_sampling_decision(self, sampling_context):
self.sampled = False
return

# Now we roll the dice. random.random is inclusive of 0, but not of 1,
# Now we roll the dice. self._sample_rand is inclusive of 0, but not of 1,
# so strict < is safe here. In case sample_rate is a boolean, cast it
# to a float (True becomes 1.0 and False becomes 0.0)
self.sampled = random.random() < self.sample_rate
self.sampled = self._sample_rand < self.sample_rate

if self.sampled:
logger.debug(
Expand Down Expand Up @@ -1339,6 +1347,7 @@ async def my_async_function():
Baggage,
EnvironHeaders,
extract_sentrytrace_data,
_generate_sample_rand,
has_tracing_enabled,
maybe_create_breadcrumbs_from_span,
)
Expand Down
15 changes: 15 additions & 0 deletions sentry_sdk/tracing_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ def populate_from_transaction(cls, transaction):
options = client.options or {}

sentry_items["trace_id"] = transaction.trace_id
sentry_items["sample_rand"] = str(transaction._sample_rand)

if options.get("environment"):
sentry_items["environment"] = options["environment"]
Expand Down Expand Up @@ -717,6 +718,20 @@ def strip_sentry_baggage(header):
)
)

def _sample_rand(self):
# type: () -> Optional[Decimal]
"""Convenience method to get the sample_rand value from the sentry_items.
We validate the value and parse it as a Decimal before returning it. The value is considered
valid if it is a Decimal in the range [0, 1).
"""
sample_rand = try_convert(Decimal, self.sentry_items.get("sample_rand"))

if sample_rand is not None and Decimal(0) <= sample_rand < Decimal(1):
return sample_rand

return None

def __repr__(self):
# type: () -> str
return f'<Baggage "{self.serialize(include_third_party=True)}", mutable={self.mutable}>'
Expand Down
25 changes: 13 additions & 12 deletions tests/integrations/aiohttp/test_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -626,18 +626,19 @@ async def handler(request):

raw_server = await aiohttp_raw_server(handler)

with start_transaction(
name="/interactions/other-dogs/new-dog",
op="greeting.sniff",
trace_id="0123456789012345678901234567890",
):
client = await aiohttp_client(raw_server)
resp = await client.get("/", headers={"bagGage": "custom=value"})

assert (
resp.request_info.headers["baggage"]
== "custom=value,sentry-trace_id=0123456789012345678901234567890,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true"
)
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.5):
with start_transaction(
name="/interactions/other-dogs/new-dog",
op="greeting.sniff",
trace_id="0123456789012345678901234567890",
):
client = await aiohttp_client(raw_server)
resp = await client.get("/", headers={"bagGage": "custom=value"})

assert (
resp.request_info.headers["baggage"]
== "custom=value,sentry-trace_id=0123456789012345678901234567890,sentry-sample_rand=0.500000,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true"
)


@pytest.mark.asyncio
Expand Down
35 changes: 19 additions & 16 deletions tests/integrations/celery/test_celery.py
Original file line number Diff line number Diff line change
Expand Up @@ -509,22 +509,25 @@ def test_baggage_propagation(init_celery):
def dummy_task(self, x, y):
return _get_headers(self)

with start_transaction() as transaction:
result = dummy_task.apply_async(
args=(1, 0),
headers={"baggage": "custom=value"},
).get()

assert sorted(result["baggage"].split(",")) == sorted(
[
"sentry-release=abcdef",
"sentry-trace_id={}".format(transaction.trace_id),
"sentry-environment=production",
"sentry-sample_rate=1.0",
"sentry-sampled=true",
"custom=value",
]
)
# patch random.uniform to return a predictable sample_rand value
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.5):
with start_transaction() as transaction:
result = dummy_task.apply_async(
args=(1, 0),
headers={"baggage": "custom=value"},
).get()

assert sorted(result["baggage"].split(",")) == sorted(
[
"sentry-release=abcdef",
"sentry-trace_id={}".format(transaction.trace_id),
"sentry-environment=production",
"sentry-sample_rand=0.500000",
"sentry-sample_rate=1.0",
"sentry-sampled=true",
"custom=value",
]
)


def test_sentry_propagate_traces_override(init_celery):
Expand Down
48 changes: 25 additions & 23 deletions tests/integrations/httpx/test_httpx.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,30 +170,32 @@ def test_outgoing_trace_headers_append_to_baggage(

url = "http://example.com/"

with start_transaction(
name="/interactions/other-dogs/new-dog",
op="greeting.sniff",
trace_id="01234567890123456789012345678901",
) as transaction:
if asyncio.iscoroutinefunction(httpx_client.get):
response = asyncio.get_event_loop().run_until_complete(
httpx_client.get(url, headers={"baGGage": "custom=data"})
# patch random.uniform to return a predictable sample_rand value
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.5):
with start_transaction(
name="/interactions/other-dogs/new-dog",
op="greeting.sniff",
trace_id="01234567890123456789012345678901",
) as transaction:
if asyncio.iscoroutinefunction(httpx_client.get):
response = asyncio.get_event_loop().run_until_complete(
httpx_client.get(url, headers={"baGGage": "custom=data"})
)
else:
response = httpx_client.get(url, headers={"baGGage": "custom=data"})

request_span = transaction._span_recorder.spans[-1]
assert response.request.headers[
"sentry-trace"
] == "{trace_id}-{parent_span_id}-{sampled}".format(
trace_id=transaction.trace_id,
parent_span_id=request_span.span_id,
sampled=1,
)
assert (
response.request.headers["baggage"]
== "custom=data,sentry-trace_id=01234567890123456789012345678901,sentry-sample_rand=0.500000,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true"
)
else:
response = httpx_client.get(url, headers={"baGGage": "custom=data"})

request_span = transaction._span_recorder.spans[-1]
assert response.request.headers[
"sentry-trace"
] == "{trace_id}-{parent_span_id}-{sampled}".format(
trace_id=transaction.trace_id,
parent_span_id=request_span.span_id,
sampled=1,
)
assert (
response.request.headers["baggage"]
== "custom=data,sentry-trace_id=01234567890123456789012345678901,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true"
)


@pytest.mark.parametrize(
Expand Down
13 changes: 6 additions & 7 deletions tests/integrations/stdlib/test_httplib.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import random
from http.client import HTTPConnection, HTTPSConnection
from socket import SocketIO
from urllib.error import HTTPError
Expand Down Expand Up @@ -189,7 +188,7 @@ def test_outgoing_trace_headers(sentry_init, monkeypatch):
"baggage": (
"other-vendor-value-1=foo;bar;baz, sentry-trace_id=771a43a4192642f0b136d5159a501700, "
"sentry-public_key=49d0f7386ad645858ae85020e393bef3, sentry-sample_rate=0.01337, "
"sentry-user_id=Am%C3%A9lie, other-vendor-value-2=foo;bar;"
"sentry-user_id=Am%C3%A9lie, sentry-sample_rand=0.132521102938283, other-vendor-value-2=foo;bar;"
),
}

Expand Down Expand Up @@ -222,7 +221,8 @@ def test_outgoing_trace_headers(sentry_init, monkeypatch):
"sentry-trace_id=771a43a4192642f0b136d5159a501700,"
"sentry-public_key=49d0f7386ad645858ae85020e393bef3,"
"sentry-sample_rate=1.0,"
"sentry-user_id=Am%C3%A9lie"
"sentry-user_id=Am%C3%A9lie,"
"sentry-sample_rand=0.132521102938283"
)

assert request_headers["baggage"] == expected_outgoing_baggage
Expand All @@ -235,11 +235,9 @@ def test_outgoing_trace_headers_head_sdk(sentry_init, monkeypatch):
mock_send = mock.Mock()
monkeypatch.setattr(HTTPSConnection, "send", mock_send)

# make sure transaction is always sampled
monkeypatch.setattr(random, "random", lambda: 0.1)

sentry_init(traces_sample_rate=0.5, release="foo")
transaction = Transaction.continue_from_headers({})
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.25):
transaction = Transaction.continue_from_headers({})

with start_transaction(transaction=transaction, name="Head SDK tx") as transaction:
HTTPSConnection("www.squirrelchasers.com").request("GET", "/top-chasers")
Expand All @@ -261,6 +259,7 @@ def test_outgoing_trace_headers_head_sdk(sentry_init, monkeypatch):

expected_outgoing_baggage = (
"sentry-trace_id=%s,"
"sentry-sample_rand=0.250000,"
"sentry-environment=production,"
"sentry-release=foo,"
"sentry-sample_rate=0.5,"
Expand Down
6 changes: 4 additions & 2 deletions tests/test_api.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import pytest

import re
from unittest import mock

import sentry_sdk
Expand Down Expand Up @@ -95,10 +97,10 @@ def test_baggage_with_tracing_disabled(sentry_init):
def test_baggage_with_tracing_enabled(sentry_init):
sentry_init(traces_sample_rate=1.0, release="1.0.0", environment="dev")
with start_transaction() as transaction:
expected_baggage = "sentry-trace_id={},sentry-environment=dev,sentry-release=1.0.0,sentry-sample_rate=1.0,sentry-sampled={}".format(
expected_baggage_re = r"^sentry-trace_id={},sentry-sample_rand=0\.\d{{6}},sentry-environment=dev,sentry-release=1\.0\.0,sentry-sample_rate=1\.0,sentry-sampled={}$".format(
transaction.trace_id, "true" if transaction.sampled else "false"
)
assert get_baggage() == expected_baggage
assert re.match(expected_baggage_re, get_baggage())


@pytest.mark.forked
Expand Down
3 changes: 1 addition & 2 deletions tests/test_dsc.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
This is not tested in this file.
"""

import random
from unittest import mock

import pytest
Expand Down Expand Up @@ -176,7 +175,7 @@ def my_traces_sampler(sampling_context):
}

# We continue the incoming trace and start a new transaction
with mock.patch.object(random, "random", return_value=0.2):
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.125):
transaction = sentry_sdk.continue_trace(incoming_http_headers)
with sentry_sdk.start_transaction(transaction, name="foo"):
pass
Expand Down
12 changes: 5 additions & 7 deletions tests/test_monitor.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import random
from collections import Counter
from unittest import mock

Expand Down Expand Up @@ -68,17 +67,16 @@ def test_transaction_uses_downsampled_rate(
monitor = sentry_sdk.get_client().monitor
monitor.interval = 0.1

# make sure rng doesn't sample
monkeypatch.setattr(random, "random", lambda: 0.9)

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

with sentry_sdk.start_transaction(name="foobar") as transaction:
assert transaction.sampled is False
assert transaction.sample_rate == 0.5
# make sure we don't sample the transaction
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.75):
with sentry_sdk.start_transaction(name="foobar") as transaction:
assert transaction.sampled is False
assert transaction.sample_rate == 0.5

assert Counter(record_lost_event_calls) == Counter(
[
Expand Down
10 changes: 6 additions & 4 deletions tests/tracing/test_integration_tests.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import gc
import random
import re
import sys
import weakref
from unittest import mock

import pytest

Expand Down Expand Up @@ -169,9 +169,8 @@ def test_dynamic_sampling_head_sdk_creates_dsc(
envelopes = capture_envelopes()

# make sure transaction is sampled for both cases
monkeypatch.setattr(random, "random", lambda: 0.1)

transaction = Transaction.continue_from_headers({}, name="Head SDK tx")
with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.25):
transaction = Transaction.continue_from_headers({}, name="Head SDK tx")

# will create empty mutable baggage
baggage = transaction._baggage
Expand All @@ -196,12 +195,14 @@ def test_dynamic_sampling_head_sdk_creates_dsc(
"release": "foo",
"sample_rate": str(sample_rate),
"sampled": "true" if transaction.sampled else "false",
"sample_rand": "0.250000",
"transaction": "Head SDK tx",
"trace_id": trace_id,
}

expected_baggage = (
"sentry-trace_id=%s,"
"sentry-sample_rand=0.250000,"
"sentry-environment=production,"
"sentry-release=foo,"
"sentry-transaction=Head%%20SDK%%20tx,"
Expand All @@ -217,6 +218,7 @@ def test_dynamic_sampling_head_sdk_creates_dsc(
"environment": "production",
"release": "foo",
"sample_rate": str(sample_rate),
"sample_rand": "0.250000",
"sampled": "true" if transaction.sampled else "false",
"transaction": "Head SDK tx",
"trace_id": trace_id,
Expand Down
Loading

0 comments on commit ccb969e

Please sign in to comment.