diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 3d807d795a..cc35f9134b 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -157,6 +157,7 @@ Looking to upgrade from Sentry SDK 2.x to 3.x? Here's a comprehensive list of wh - `profiles_sample_rate` and `profiler_mode` were removed from options available via `_experiments`. Use the top-level `profiles_sample_rate` and `profiler_mode` options instead. - `Transport.capture_event` has been removed. Use `Transport.capture_envelope` instead. - Function transports are no longer supported. Subclass the `Transport` instead. +- `start_transaction` (`start_span`) no longer takes a `baggage` argument. Use the `continue_trace()` context manager instead to propagate baggage. ### Deprecated diff --git a/sentry_sdk/integrations/opentelemetry/consts.py b/sentry_sdk/integrations/opentelemetry/consts.py index 1585e8d893..d4b2b47768 100644 --- a/sentry_sdk/integrations/opentelemetry/consts.py +++ b/sentry_sdk/integrations/opentelemetry/consts.py @@ -12,9 +12,12 @@ SENTRY_USE_CURRENT_SCOPE_KEY = create_key("sentry_use_current_scope") SENTRY_USE_ISOLATION_SCOPE_KEY = create_key("sentry_use_isolation_scope") +# trace state keys TRACESTATE_SAMPLED_KEY = Baggage.SENTRY_PREFIX + "sampled" TRACESTATE_SAMPLE_RATE_KEY = Baggage.SENTRY_PREFIX + "sample_rate" +TRACESTATE_SAMPLE_RAND_KEY = Baggage.SENTRY_PREFIX + "sample_rand" +# misc OTEL_SENTRY_CONTEXT = "otel" SPAN_ORIGIN = "auto.otel" diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index a257f76f1e..83d647f1d8 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -1,4 +1,4 @@ -import random +from decimal import Decimal from typing import cast from opentelemetry import trace @@ -6,10 +6,14 @@ from opentelemetry.trace.span import TraceState import sentry_sdk -from sentry_sdk.tracing_utils import has_tracing_enabled +from sentry_sdk.tracing_utils import ( + _generate_sample_rand, + has_tracing_enabled, +) from sentry_sdk.utils import is_valid_sample_rate, logger from sentry_sdk.integrations.opentelemetry.consts import ( TRACESTATE_SAMPLED_KEY, + TRACESTATE_SAMPLE_RAND_KEY, TRACESTATE_SAMPLE_RATE_KEY, SentrySpanAttribute, ) @@ -70,23 +74,40 @@ def get_parent_sample_rate(parent_context, trace_id): return None -def dropped_result(parent_span_context, attributes, sample_rate=None): - # type: (SpanContext, Attributes, Optional[float]) -> SamplingResult - # these will only be added the first time in a root span sampling decision - # if sample_rate is provided, it'll be updated in trace state - trace_state = parent_span_context.trace_state +def get_parent_sample_rand(parent_context, trace_id): + # type: (Optional[SpanContext], int) -> Optional[Decimal] + if parent_context is None: + return None - if TRACESTATE_SAMPLED_KEY not in trace_state: - trace_state = trace_state.add(TRACESTATE_SAMPLED_KEY, "false") - elif trace_state.get(TRACESTATE_SAMPLED_KEY) == "deferred": - trace_state = trace_state.update(TRACESTATE_SAMPLED_KEY, "false") + is_span_context_valid = parent_context is not None and parent_context.is_valid - if sample_rate is not None: - trace_state = trace_state.update(TRACESTATE_SAMPLE_RATE_KEY, str(sample_rate)) + if is_span_context_valid and parent_context.trace_id == trace_id: + parent_sample_rand = parent_context.trace_state.get(TRACESTATE_SAMPLE_RAND_KEY) + if parent_sample_rand is None: + return None - is_root_span = not ( - parent_span_context.is_valid and not parent_span_context.is_remote + return Decimal(parent_sample_rand) + + return None + + +def dropped_result(span_context, attributes, sample_rate=None, sample_rand=None): + # type: (SpanContext, Attributes, Optional[float], Optional[Decimal]) -> SamplingResult + """ + React to a span getting unsampled and return a DROP SamplingResult. + + Update the trace_state with the effective sampled, sample_rate and sample_rand, + record that we dropped the event for client report purposes, and return + an OTel SamplingResult with Decision.DROP. + + See for more info about OTel sampling: + https://opentelemetry-python.readthedocs.io/en/latest/sdk/trace.sampling.html + """ + trace_state = _update_trace_state( + span_context, sampled=False, sample_rate=sample_rate, sample_rand=sample_rand ) + + is_root_span = not (span_context.is_valid and not span_context.is_remote) if is_root_span: # Tell Sentry why we dropped the transaction/root-span client = sentry_sdk.get_client() @@ -108,19 +129,20 @@ def dropped_result(parent_span_context, attributes, sample_rate=None): ) -def sampled_result(span_context, attributes, sample_rate): - # type: (SpanContext, Attributes, Optional[float]) -> SamplingResult - # these will only be added the first time in a root span sampling decision - # if sample_rate is provided, it'll be updated in trace state - trace_state = span_context.trace_state +def sampled_result(span_context, attributes, sample_rate=None, sample_rand=None): + # type: (SpanContext, Attributes, Optional[float], Optional[Decimal]) -> SamplingResult + """ + React to a span being sampled and return a sampled SamplingResult. - if TRACESTATE_SAMPLED_KEY not in trace_state: - trace_state = trace_state.add(TRACESTATE_SAMPLED_KEY, "true") - elif trace_state.get(TRACESTATE_SAMPLED_KEY) == "deferred": - trace_state = trace_state.update(TRACESTATE_SAMPLED_KEY, "true") + Update the trace_state with the effective sampled, sample_rate and sample_rand, + and return an OTel SamplingResult with Decision.RECORD_AND_SAMPLE. - if sample_rate is not None: - trace_state = trace_state.update(TRACESTATE_SAMPLE_RATE_KEY, str(sample_rate)) + See for more info about OTel sampling: + https://opentelemetry-python.readthedocs.io/en/latest/sdk/trace.sampling.html + """ + trace_state = _update_trace_state( + span_context, sampled=True, sample_rate=sample_rate, sample_rand=sample_rand + ) return SamplingResult( Decision.RECORD_AND_SAMPLE, @@ -129,6 +151,27 @@ def sampled_result(span_context, attributes, sample_rate): ) +def _update_trace_state(span_context, sampled, sample_rate=None, sample_rand=None): + # type: (SpanContext, bool, Optional[float], Optional[Decimal]) -> TraceState + trace_state = span_context.trace_state + + sampled = "true" if sampled else "false" + if TRACESTATE_SAMPLED_KEY not in trace_state: + trace_state = trace_state.add(TRACESTATE_SAMPLED_KEY, sampled) + elif trace_state.get(TRACESTATE_SAMPLED_KEY) == "deferred": + trace_state = trace_state.update(TRACESTATE_SAMPLED_KEY, sampled) + + if sample_rate is not None: + trace_state = trace_state.update(TRACESTATE_SAMPLE_RATE_KEY, str(sample_rate)) + + if sample_rand is not None: + trace_state = trace_state.update( + TRACESTATE_SAMPLE_RAND_KEY, f"{sample_rand:.6f}" # noqa: E231 + ) + + return trace_state + + class SentrySampler(Sampler): def should_sample( self, @@ -156,6 +199,18 @@ def should_sample( sample_rate = None + parent_sampled = get_parent_sampled(parent_span_context, trace_id) + parent_sample_rate = get_parent_sample_rate(parent_span_context, trace_id) + parent_sample_rand = get_parent_sample_rand(parent_span_context, trace_id) + + if parent_sample_rand is not None: + # We have a sample_rand on the incoming trace or we already backfilled + # it in PropagationContext + sample_rand = parent_sample_rand + else: + # We are the head SDK and we need to generate a new sample_rand + sample_rand = cast(Decimal, _generate_sample_rand(str(trace_id), (0, 1))) + # Explicit sampled value provided at start_span custom_sampled = cast( "Optional[bool]", attributes.get(SentrySpanAttribute.CUSTOM_SAMPLED) @@ -165,11 +220,17 @@ def should_sample( sample_rate = float(custom_sampled) if sample_rate > 0: return sampled_result( - parent_span_context, attributes, sample_rate=sample_rate + parent_span_context, + attributes, + sample_rate=sample_rate, + sample_rand=sample_rand, ) else: return dropped_result( - parent_span_context, attributes, sample_rate=sample_rate + parent_span_context, + attributes, + sample_rate=sample_rate, + sample_rand=sample_rand, ) else: logger.debug( @@ -190,8 +251,6 @@ def should_sample( sample_rate_to_propagate = sample_rate else: # Check if there is a parent with a sampling decision - parent_sampled = get_parent_sampled(parent_span_context, trace_id) - parent_sample_rate = get_parent_sample_rate(parent_span_context, trace_id) if parent_sampled is not None: sample_rate = bool(parent_sampled) sample_rate_to_propagate = ( @@ -215,17 +274,23 @@ def should_sample( if client.monitor.downsample_factor > 0: sample_rate_to_propagate = sample_rate - # Roll the dice on sample rate + # Compare sample_rand to sample_rate to make the final sampling decision sample_rate = float(cast("Union[bool, float, int]", sample_rate)) - sampled = random.random() < sample_rate + sampled = sample_rand < sample_rate if sampled: return sampled_result( - parent_span_context, attributes, sample_rate=sample_rate_to_propagate + parent_span_context, + attributes, + sample_rate=sample_rate_to_propagate, + sample_rand=None if sample_rand == parent_sample_rand else sample_rand, ) else: return dropped_result( - parent_span_context, attributes, sample_rate=sample_rate_to_propagate + parent_span_context, + attributes, + sample_rate=sample_rate_to_propagate, + sample_rand=None if sample_rand == parent_sample_rand else sample_rand, ) def get_description(self) -> str: diff --git a/sentry_sdk/integrations/opentelemetry/scope.py b/sentry_sdk/integrations/opentelemetry/scope.py index 56dd129a7f..2cd734bcdd 100644 --- a/sentry_sdk/integrations/opentelemetry/scope.py +++ b/sentry_sdk/integrations/opentelemetry/scope.py @@ -30,7 +30,6 @@ from sentry_sdk.integrations.opentelemetry.utils import trace_state_from_baggage from sentry_sdk.scope import Scope, ScopeType from sentry_sdk.tracing import Span -from sentry_sdk.utils import logger from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 0812d31c67..49313bb0a5 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -73,7 +73,7 @@ def putrequest(self, method, url, *args, **kwargs): client = sentry_sdk.get_client() if client.get_integration(StdlibIntegration) is None or is_sentry_url( - client, f"{host}:{port}" + client, f"{host}:{port}" # noqa: E231 ): return real_putrequest(self, method, url, *args, **kwargs) diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 0e31ad4ff5..010f2a3d2a 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -20,7 +20,6 @@ from sentry_sdk.utils import ( _serialize_span_attribute, get_current_thread_meta, - logger, should_be_treated_as_error, ) diff --git a/sentry_sdk/tracing_utils.py b/sentry_sdk/tracing_utils.py index 07f8373c68..4bc7c6aeff 100644 --- a/sentry_sdk/tracing_utils.py +++ b/sentry_sdk/tracing_utils.py @@ -1,4 +1,5 @@ import contextlib +import decimal import inspect import os import re @@ -392,6 +393,9 @@ def from_incoming_data(cls, incoming_data): propagation_context = PropagationContext() propagation_context.update(sentrytrace_data) + if propagation_context is not None: + propagation_context._fill_sample_rand() + return propagation_context @property @@ -433,6 +437,78 @@ def update(self, other_dict): except AttributeError: pass + def _fill_sample_rand(self): + # type: () -> None + """ + Ensure that there is a valid sample_rand value in the baggage. + + If there is a valid sample_rand value in the baggage, we keep it. + Otherwise, we generate a sample_rand value according to the following: + + - If we have a parent_sampled value and a sample_rate in the DSC, we compute + a sample_rand value randomly in the range: + - [0, sample_rate) if parent_sampled is True, + - or, in the range [sample_rate, 1) if parent_sampled is False. + + - If either parent_sampled or sample_rate is missing, we generate a random + value in the range [0, 1). + + The sample_rand is deterministically generated from the trace_id, if present. + + This function does nothing if there is no dynamic_sampling_context. + """ + if self.dynamic_sampling_context is None or self.baggage is None: + return + + sentry_baggage = self.baggage.sentry_items + + sample_rand = None + if sentry_baggage.get("sample_rand"): + try: + sample_rand = Decimal(sentry_baggage["sample_rand"]) + except Exception: + logger.debug( + f"Failed to convert incoming sample_rand to Decimal: {sample_rand}" + ) + + if sample_rand is not None and 0 <= sample_rand < 1: + # sample_rand is present and valid, so don't overwrite it + return + + sample_rate = None + if sentry_baggage.get("sample_rate"): + try: + sample_rate = float(sentry_baggage["sample_rate"]) + except Exception: + logger.debug( + f"Failed to convert incoming sample_rate to float: {sample_rate}" + ) + + lower, upper = _sample_rand_range(self.parent_sampled, sample_rate) + + try: + sample_rand = _generate_sample_rand(self.trace_id, interval=(lower, upper)) + except ValueError: + # ValueError is raised if the interval is invalid, i.e. lower >= upper. + # lower >= upper might happen if the incoming trace's sampled flag + # and sample_rate are inconsistent, e.g. sample_rate=0.0 but sampled=True. + # We cannot generate a sensible sample_rand value in this case. + logger.debug( + f"Could not backfill sample_rand, since parent_sampled={self.parent_sampled} " + f"and sample_rate={sample_rate}." + ) + return + + self.baggage.sentry_items["sample_rand"] = f"{sample_rand:.6f}" # noqa: E231 + + def _sample_rand(self): + # type: () -> Optional[str] + """Convenience method to get the sample_rand value from the baggage.""" + if self.baggage is None: + return None + + return self.baggage.sentry_items.get("sample_rand") + def __repr__(self): # type: (...) -> str return "".format( @@ -684,13 +760,11 @@ def get_current_span(scope=None): return current_span -# XXX-potel-ivana: use this def _generate_sample_rand( trace_id, # type: Optional[str] - *, interval=(0.0, 1.0), # type: tuple[float, float] ): - # type: (...) -> Any + # type: (...) -> Optional[decimal.Decimal] """Generate a sample_rand value from a trace ID. The generated value will be pseudorandomly chosen from the provided @@ -709,15 +783,11 @@ def _generate_sample_rand( while sample_rand >= upper: sample_rand = rng.uniform(lower, upper) - # Round down to exactly six decimal-digit precision. - # Setting the context is needed to avoid an InvalidOperation exception - # in case the user has changed the default precision. return Decimal(sample_rand).quantize( Decimal("0.000001"), rounding=ROUND_DOWN, context=Context(prec=6) ) -# XXX-potel-ivana: use this def _sample_rand_range(parent_sampled, sample_rate): # type: (Optional[bool], Optional[float]) -> tuple[float, float] """ diff --git a/tests/integrations/opentelemetry/test_propagator.py b/tests/integrations/opentelemetry/test_propagator.py index 46f4250771..b18e3bc400 100644 --- a/tests/integrations/opentelemetry/test_propagator.py +++ b/tests/integrations/opentelemetry/test_propagator.py @@ -1,6 +1,6 @@ -import pytest +from unittest.mock import MagicMock, patch -from unittest.mock import MagicMock +import pytest from opentelemetry.trace.propagation import get_current_span from opentelemetry.propagators.textmap import DefaultSetter @@ -139,11 +139,47 @@ def test_inject_continue_trace(sentry_init): "HTTP_BAGGAGE": baggage, } + expected_baggage = baggage + ",sentry-sample_rand=0.001111" + + with patch( + "sentry_sdk.tracing_utils.Random.uniform", + return_value=0.001111, + ): + with sentry_sdk.continue_trace(incoming_headers): + with sentry_sdk.start_span(name="foo") as span: + SentryPropagator().inject(carrier, setter=setter) + assert carrier["sentry-trace"] == f"{trace_id}-{span.span_id}-1" + assert carrier["baggage"] == SortedBaggage(expected_baggage) + + +def test_inject_continue_trace_incoming_sample_rand(sentry_init): + sentry_init(traces_sample_rate=1.0) + + carrier = {} + setter = DefaultSetter() + + trace_id = "771a43a4192642f0b136d5159a501700" + sentry_trace = "771a43a4192642f0b136d5159a501700-1234567890abcdef-1" + baggage = ( + "sentry-trace_id=771a43a4192642f0b136d5159a501700," + "sentry-public_key=frontendpublickey," + "sentry-sample_rate=0.01337," + "sentry-sampled=true," + "sentry-release=myfrontend," + "sentry-environment=bird," + "sentry-transaction=bar," + "sentry-sample_rand=0.002849" + ) + incoming_headers = { + "HTTP_SENTRY_TRACE": sentry_trace, + "HTTP_BAGGAGE": baggage, + } + with sentry_sdk.continue_trace(incoming_headers): with sentry_sdk.start_span(name="foo") as span: SentryPropagator().inject(carrier, setter=setter) - assert (carrier["sentry-trace"]) == f"{trace_id}-{span.span_id}-1" - assert (carrier["baggage"]) == SortedBaggage(baggage) + assert carrier["sentry-trace"] == f"{trace_id}-{span.span_id}-1" + assert carrier["baggage"] == SortedBaggage(baggage) def test_inject_head_sdk(sentry_init): @@ -152,9 +188,23 @@ def test_inject_head_sdk(sentry_init): carrier = {} setter = DefaultSetter() - with sentry_sdk.start_span(name="foo") as span: - SentryPropagator().inject(carrier, setter=setter) - assert (carrier["sentry-trace"]) == f"{span.trace_id}-{span.span_id}-1" - assert (carrier["baggage"]) == SortedBaggage( - f"sentry-transaction=foo,sentry-release=release,sentry-environment=production,sentry-trace_id={span.trace_id},sentry-sample_rate=1.0,sentry-sampled=true" # noqa: E231 - ) + expected_baggage = ( + "sentry-transaction=foo," + "sentry-release=release," + "sentry-environment=production," + "sentry-trace_id={trace_id}," + "sentry-sample_rate=1.0," + "sentry-sampled=true," + "sentry-sample_rand=0.111111" + ) + + with patch( + "sentry_sdk.tracing_utils.Random.uniform", + return_value=0.111111, + ): + with sentry_sdk.start_span(name="foo") as span: + SentryPropagator().inject(carrier, setter=setter) + assert carrier["sentry-trace"] == f"{span.trace_id}-{span.span_id}-1" + assert carrier["baggage"] == SortedBaggage( + expected_baggage.format(trace_id=span.trace_id) + ) diff --git a/tests/integrations/opentelemetry/test_sampler.py b/tests/integrations/opentelemetry/test_sampler.py index 9e67eb7921..8cccab05be 100644 --- a/tests/integrations/opentelemetry/test_sampler.py +++ b/tests/integrations/opentelemetry/test_sampler.py @@ -71,13 +71,17 @@ def test_sampling_traces_sample_rate_50(sentry_init, capture_envelopes): envelopes = capture_envelopes() - with mock.patch("random.random", return_value=0.2): # drop + with mock.patch( + "sentry_sdk.tracing_utils.Random.uniform", return_value=0.2 + ): # drop with sentry_sdk.start_span(description="request a"): with sentry_sdk.start_span(description="cache a"): with sentry_sdk.start_span(description="db a"): ... - with mock.patch("random.random", return_value=0.7): # keep + with mock.patch( + "sentry_sdk.tracing_utils.Random.uniform", return_value=0.7 + ): # keep with sentry_sdk.start_span(description="request b"): with sentry_sdk.start_span(description="cache b"): with sentry_sdk.start_span(description="db b"): diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index baf12ca7d2..5f6d57998b 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -206,7 +206,7 @@ def test_outgoing_trace_headers( "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, sentry-sample_rand=0.132521102938283, other-vendor-value-2=foo;bar;" + "sentry-user_id=Am%C3%A9lie, sentry-sample_rand=0.003370, other-vendor-value-2=foo;bar;" ), } @@ -231,9 +231,10 @@ def test_outgoing_trace_headers( expected_outgoing_baggage = ( "sentry-trace_id=771a43a4192642f0b136d5159a501700," "sentry-public_key=49d0f7386ad645858ae85020e393bef3," - "sentry-sample_rate=1.0," + "sentry-sample_rate=0.01337," "sentry-user_id=Am%C3%A9lie," - "sentry-sample_rand=0.132521102938283" + "sentry-sample_rand=0.003370," + "sentry-sampled=true" ) assert request_headers["baggage"] == SortedBaggage(expected_outgoing_baggage) diff --git a/tests/test_api.py b/tests/test_api.py index 612a092a73..ae88791f96 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,6 +1,5 @@ import pytest -import re from unittest import mock from sentry_sdk import ( @@ -94,11 +93,10 @@ def test_baggage_with_tracing_disabled(sentry_init): @pytest.mark.forked def test_baggage_with_tracing_enabled(sentry_init): sentry_init(traces_sample_rate=1.0, release="1.0.0", environment="dev") - with start_span(name="foo") as span: - expected_baggage_re = r"^sentry-transaction=foo,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( - span.trace_id, "true" if span.sampled else "false" - ) - assert re.match(expected_baggage_re, get_baggage()) + with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.111111): + with start_span(name="foo") as span: + expected_baggage = f"sentry-transaction=foo,sentry-trace_id={span.trace_id},sentry-sample_rand=0.111111,sentry-environment=dev,sentry-release=1.0.0,sentry-sample_rate=1.0,sentry-sampled=true" # noqa: E231 + assert get_baggage() == SortedBaggage(expected_baggage) @pytest.mark.forked @@ -112,7 +110,7 @@ def test_continue_trace(sentry_init): with continue_trace( { "sentry-trace": "{}-{}-{}".format(trace_id, parent_span_id, parent_sampled), - "baggage": "sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-sample_rand=0.123456", + "baggage": "sentry-trace_id=566e3688a61d4bc888951642d6f14a19,sentry-sample_rand=0.123456", # noqa: E231 }, ): with start_span(name="some name") as span: diff --git a/tests/test_dsc.py b/tests/test_dsc.py index 9698bcd8d0..ea3c0b8988 100644 --- a/tests/test_dsc.py +++ b/tests/test_dsc.py @@ -8,7 +8,6 @@ This is not tested in this file. """ -import random from unittest import mock import pytest @@ -176,10 +175,10 @@ def my_traces_sampler(sampling_context): } # We continue the incoming trace and start a new transaction - monkeypatch.setattr(random, "random", lambda: 0.125) - with sentry_sdk.continue_trace(incoming_http_headers): - with sentry_sdk.start_span(name="foo"): - pass + with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.125): + with sentry_sdk.continue_trace(incoming_http_headers): + with sentry_sdk.start_span(name="foo"): + pass assert len(envelopes) == 1 @@ -231,7 +230,7 @@ def my_traces_sampler(sampling_context): # The result of the local traces sampler. # "local_traces_sample_rate": # The `traces_sample_rate` setting in the local `sentry_init` call. - ( + ( # 1 traces_sample_rate does not override incoming { "incoming_sample_rate": 1.0, "incoming_sampled": "true", @@ -243,7 +242,7 @@ def my_traces_sampler(sampling_context): 1.0, # expected_sample_rate "true", # expected_sampled ), - ( + ( # 2 traces_sampler overrides incoming { "incoming_sample_rate": 1.0, "incoming_sampled": "true", @@ -255,7 +254,7 @@ def my_traces_sampler(sampling_context): 0.5, # expected_sample_rate "true", # expected_sampled ), - ( + ( # 3 traces_sample_rate does not overrides incoming sample rate or parent (incoming not sampled) { "incoming_sample_rate": 1.0, "incoming_sampled": "false", @@ -267,19 +266,19 @@ def my_traces_sampler(sampling_context): None, # expected_sample_rate "tracing-disabled-no-transactions-should-be-sent", # expected_sampled (because the parent sampled is 0) ), - ( + ( # 4 traces_sampler overrides incoming (incoming not sampled) { - "incoming_sample_rate": 1.0, + "incoming_sample_rate": 0.3, "incoming_sampled": "false", "sentry_trace_header_parent_sampled": 0, "use_local_traces_sampler": True, - "local_traces_sampler_result": 0.5, + "local_traces_sampler_result": 0.25, "local_traces_sample_rate": 0.7, }, - 0.5, # expected_sample_rate + 0.25, # expected_sample_rate "false", # expected_sampled (traces sampler can override parent sampled) ), - ( + ( # 5 forwarding incoming (traces_sample_rate not set) { "incoming_sample_rate": 1.0, "incoming_sampled": "true", @@ -291,7 +290,7 @@ def my_traces_sampler(sampling_context): None, # expected_sample_rate "tracing-disabled-no-transactions-should-be-sent", # expected_sampled (traces_sample_rate=None disables all transaction creation) ), - ( + ( # 6 traces_sampler overrides incoming (traces_sample_rate not set) { "incoming_sample_rate": 1.0, "incoming_sampled": "true", @@ -303,7 +302,7 @@ def my_traces_sampler(sampling_context): 0.5, # expected_sample_rate "true", # expected_sampled (traces sampler overrides the traces_sample_rate setting, so transactions are created) ), - ( + ( # 7 forwarding incoming (traces_sample_rate not set) (incoming not sampled) { "incoming_sample_rate": 1.0, "incoming_sampled": "false", @@ -315,19 +314,19 @@ def my_traces_sampler(sampling_context): None, # expected_sample_rate "tracing-disabled-no-transactions-should-be-sent", # expected_sampled (traces_sample_rate=None disables all transaction creation) ), - ( + ( # 8 traces_sampler overrides incoming (traces_sample_rate not set) (incoming not sampled) { - "incoming_sample_rate": 1.0, + "incoming_sample_rate": 0.3, "incoming_sampled": "false", "sentry_trace_header_parent_sampled": 0, "use_local_traces_sampler": True, - "local_traces_sampler_result": 0.5, + "local_traces_sampler_result": 0.25, "local_traces_sample_rate": None, }, - 0.5, # expected_sample_rate + 0.25, # expected_sample_rate "false", # expected_sampled ), - ( + ( # 9 traces_sample_rate overrides incoming (upstream deferred sampling decision) { "incoming_sample_rate": 1.0, "incoming_sampled": None, @@ -405,7 +404,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.2): with sentry_sdk.continue_trace(incoming_http_headers): with sentry_sdk.start_span(name="foo"): pass @@ -413,6 +412,7 @@ def my_traces_sampler(sampling_context): if expected_sampled == "tracing-disabled-no-transactions-should-be-sent": assert len(envelopes) == 0 else: + assert len(envelopes) == 1 transaction_envelope = envelopes[0] dsc_in_envelope_header = transaction_envelope.headers["trace"] diff --git a/tests/test_propagationcontext.py b/tests/test_propagationcontext.py index c8749027e4..797a18cecd 100644 --- a/tests/test_propagationcontext.py +++ b/tests/test_propagationcontext.py @@ -104,11 +104,11 @@ def test_update(): def test_existing_sample_rand_kept(): ctx = PropagationContext( trace_id="00000000000000000000000000000000", - dynamic_sampling_context={"sample_rand": "0.5"}, + baggage=Baggage(sentry_items={"sample_rand": "0.5"}), ) - # If sample_rand was regenerated, the value would be 0.919221 based on the trace_id assert ctx.dynamic_sampling_context["sample_rand"] == "0.5" + assert ctx.baggage.sentry_items["sample_rand"] == "0.5" @pytest.mark.parametrize( @@ -158,7 +158,7 @@ def mock_random_class(seed): ) assert ( - ctx.dynamic_sampling_context["sample_rand"] + ctx.dynamic_sampling_context.get("sample_rand") == f"{expected_interval[0]:.6f}" # noqa: E231 ) assert mock_uniform.call_count == 1 diff --git a/tests/tracing/test_integration_tests.py b/tests/tracing/test_integration_tests.py index e6e436dbd2..df6cf57e29 100644 --- a/tests/tracing/test_integration_tests.py +++ b/tests/tracing/test_integration_tests.py @@ -227,7 +227,7 @@ def test_trace_propagation_meta_head_sdk(sentry_init): assert 'meta name="baggage"' in baggage baggage_content = re.findall('content="([^"]*)"', baggage)[0] - assert baggage_content == root_span.get_baggage().serialize() + assert SortedBaggage(baggage_content) == root_span.get_baggage().serialize() @pytest.mark.parametrize( diff --git a/tests/tracing/test_sample_rand.py b/tests/tracing/test_sample_rand.py index fc7d0e2404..38a0fe05a2 100644 --- a/tests/tracing/test_sample_rand.py +++ b/tests/tracing/test_sample_rand.py @@ -4,7 +4,7 @@ import pytest import sentry_sdk -from sentry_sdk.tracing_utils import Baggage +from sentry_sdk.tracing import BAGGAGE_HEADER_NAME, SENTRY_TRACE_HEADER_NAME @pytest.mark.parametrize("sample_rand", (0.0, 0.25, 0.5, 0.75)) @@ -40,16 +40,20 @@ def test_transaction_uses_incoming_sample_rand( """ Test that the transaction uses the sample_rand value from the incoming baggage. """ - baggage = Baggage(sentry_items={"sample_rand": f"{sample_rand:.6f}"}) # noqa: E231 - sentry_init(traces_sample_rate=sample_rate) events = capture_events() - with sentry_sdk.start_span(baggage=baggage) as root_span: - assert ( - root_span.get_baggage().sentry_items["sample_rand"] - == f"{sample_rand:.6f}" # noqa: E231 - ) + baggage = f"sentry-sample_rand={sample_rand:.6f},sentry-trace_id=771a43a4192642f0b136d5159a501700" # noqa: E231 + sentry_trace = "771a43a4192642f0b136d5159a501700-1234567890abcdef" + + with sentry_sdk.continue_trace( + {BAGGAGE_HEADER_NAME: baggage, SENTRY_TRACE_HEADER_NAME: sentry_trace} + ): + with sentry_sdk.start_span() as root_span: + assert ( + root_span.get_baggage().sentry_items["sample_rand"] + == f"{sample_rand:.6f}" # noqa: E231 + ) # Transaction event captured if sample_rand < sample_rate, indicating that # sample_rand is used to make the sampling decision. @@ -71,11 +75,93 @@ def test_decimal_context(sentry_init, capture_events): with mock.patch( "sentry_sdk.tracing_utils.Random.uniform", return_value=0.123456789 ): - with sentry_sdk.start_transaction() as transaction: - assert ( - transaction.get_baggage().sentry_items["sample_rand"] == "0.123456" - ) + with sentry_sdk.start_span() as root_span: + assert root_span.get_baggage().sentry_items["sample_rand"] == "0.123456" finally: decimal.getcontext().prec = old_prec assert len(events) == 1 + + +@pytest.mark.parametrize( + "incoming_sample_rand,expected_sample_rand", + ( + ("0.0100015", "0.0100015"), + ("0.1", "0.1"), + ), +) +def test_unexpected_incoming_sample_rand_precision( + sentry_init, capture_events, incoming_sample_rand, expected_sample_rand +): + """ + Test that incoming sample_rand is correctly interpreted even if it looks unexpected. + + We shouldn't be getting arbitrary precision sample_rand in incoming headers, + but if we do for some reason, check that we don't tamper with it. + """ + sentry_init(traces_sample_rate=1.0) + events = capture_events() + + baggage = f"sentry-sample_rand={incoming_sample_rand},sentry-trace_id=771a43a4192642f0b136d5159a501700" # noqa: E231 + sentry_trace = "771a43a4192642f0b136d5159a501700-1234567890abcdef" + + with sentry_sdk.continue_trace( + {BAGGAGE_HEADER_NAME: baggage, SENTRY_TRACE_HEADER_NAME: sentry_trace} + ): + with sentry_sdk.start_span() as root_span: + assert ( + root_span.get_baggage().sentry_items["sample_rand"] + == expected_sample_rand + ) + + assert len(events) == 1 + + +@pytest.mark.parametrize( + "incoming_sample_rand", + ("abc", "null", "47"), +) +def test_invalid_incoming_sample_rand(sentry_init, incoming_sample_rand): + """Test that we handle malformed incoming sample_rand.""" + sentry_init(traces_sample_rate=1.0) + + baggage = f"sentry-sample_rand={incoming_sample_rand},sentry-trace_id=771a43a4192642f0b136d5159a501700" # noqa: E231 + sentry_trace = "771a43a4192642f0b136d5159a501700-1234567890abcdef" + + with sentry_sdk.continue_trace( + {BAGGAGE_HEADER_NAME: baggage, SENTRY_TRACE_HEADER_NAME: sentry_trace} + ): + with sentry_sdk.start_span(): + pass + + # The behavior here is undefined since we got a broken incoming trace, + # so as long as the SDK doesn't produce an error we consider this + # testcase a success. + + +@pytest.mark.parametrize("incoming", ((0.0, "true"), (1.0, "false"))) +def test_invalid_incoming_sampled_and_sample_rate(sentry_init, incoming): + """ + Test that we don't error out in case we can't generate a sample_rand that + would respect the incoming sampled and sample_rate. + """ + sentry_init(traces_sample_rate=1.0) + + sample_rate, sampled = incoming + + baggage = ( + f"sentry-sample_rate={sample_rate}," # noqa: E231 + f"sentry-sampled={sampled}," # noqa: E231 + "sentry-trace_id=771a43a4192642f0b136d5159a501700" + ) + sentry_trace = f"771a43a4192642f0b136d5159a501700-1234567890abcdef-{1 if sampled == 'true' else 0}" + + with sentry_sdk.continue_trace( + {BAGGAGE_HEADER_NAME: baggage, SENTRY_TRACE_HEADER_NAME: sentry_trace} + ): + with sentry_sdk.start_span(): + pass + + # The behavior here is undefined since we got a broken incoming trace, + # so as long as the SDK doesn't produce an error we consider this + # testcase a success. diff --git a/tests/tracing/test_sample_rand_propagation.py b/tests/tracing/test_sample_rand_propagation.py index ea3ea548ff..f598b24154 100644 --- a/tests/tracing/test_sample_rand_propagation.py +++ b/tests/tracing/test_sample_rand_propagation.py @@ -7,37 +7,38 @@ """ from unittest import mock -from unittest.mock import Mock import sentry_sdk -def test_continue_trace_with_sample_rand(): +def test_continue_trace_with_sample_rand(sentry_init): """ Test that an incoming sample_rand is propagated onto the transaction's baggage. """ + sentry_init() + headers = { "sentry-trace": "00000000000000000000000000000000-0000000000000000-0", "baggage": "sentry-sample_rand=0.1,sentry-sample_rate=0.5", } - transaction = sentry_sdk.continue_trace(headers) - assert transaction.get_baggage().sentry_items["sample_rand"] == "0.1" + with sentry_sdk.continue_trace(headers): + with sentry_sdk.start_span(name="root-span") as root_span: + assert root_span.get_baggage().sentry_items["sample_rand"] == "0.1" -def test_continue_trace_missing_sample_rand(): +def test_continue_trace_missing_sample_rand(sentry_init): """ Test that a missing sample_rand is filled in onto the transaction's baggage. """ + sentry_init() headers = { "sentry-trace": "00000000000000000000000000000000-0000000000000000", "baggage": "sentry-placeholder=asdf", } - mock_uniform = Mock(return_value=0.5) - - with mock.patch("sentry_sdk.tracing_utils.Random.uniform", mock_uniform): - transaction = sentry_sdk.continue_trace(headers) - - assert transaction.get_baggage().sentry_items["sample_rand"] == "0.500000" + with mock.patch("sentry_sdk.tracing_utils.Random.uniform", return_value=0.5): + with sentry_sdk.continue_trace(headers): + with sentry_sdk.start_span(name="root-span") as root_span: + assert root_span.get_baggage().sentry_items["sample_rand"] == "0.500000" diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index b418e5a572..59780729b7 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -6,7 +6,7 @@ import sentry_sdk from sentry_sdk import start_span, capture_exception -from sentry_sdk.tracing_utils import Baggage +from sentry_sdk.tracing import BAGGAGE_HEADER_NAME, SENTRY_TRACE_HEADER_NAME from sentry_sdk.utils import logger @@ -59,9 +59,14 @@ def test_uses_traces_sample_rate_correctly( ): sentry_init(traces_sample_rate=traces_sample_rate) - baggage = Baggage(sentry_items={"sample_rand": "0.500000"}) - root_span = start_span(name="dogpark", baggage=baggage) - assert root_span.sampled is expected_decision + with sentry_sdk.continue_trace( + { + BAGGAGE_HEADER_NAME: "sentry-sample_rand=0.500000,sentry-trace_id=397f36434d07b20135324b2e6ae70c77", + SENTRY_TRACE_HEADER_NAME: "397f36434d07b20135324b2e6ae70c77-1234567890abcdef", + } + ): + with start_span(name="dogpark") as root_span: + assert root_span.sampled is expected_decision @pytest.mark.parametrize( @@ -75,9 +80,14 @@ def test_uses_traces_sampler_return_value_correctly( ): sentry_init(traces_sampler=mock.Mock(return_value=traces_sampler_return_value)) - baggage = Baggage(sentry_items={"sample_rand": "0.500000"}) - root_span = start_span(name="dogpark", baggage=baggage) - assert root_span.sampled is expected_decision + with sentry_sdk.continue_trace( + { + BAGGAGE_HEADER_NAME: "sentry-sample_rand=0.500000,sentry-trace_id=397f36434d07b20135324b2e6ae70c77", + SENTRY_TRACE_HEADER_NAME: "397f36434d07b20135324b2e6ae70c77-1234567890abcdef", + } + ): + with start_span(name="dogpark") as root_span: + assert root_span.sampled is expected_decision @pytest.mark.parametrize("traces_sampler_return_value", [True, False])