diff --git a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py index 63a9acb9db..ecd70a7a76 100644 --- a/sentry_sdk/integrations/opentelemetry/potel_span_processor.py +++ b/sentry_sdk/integrations/opentelemetry/potel_span_processor.py @@ -59,11 +59,12 @@ def on_end(self, span): if is_sentry_span(span): return - if span.parent and not span.parent.is_remote: - self._children_spans[span.parent.span_id].append(span) - else: + is_root_span = not (span.parent and not span.parent.is_remote) + if is_root_span: # if have a root span ending, we build a transaction and send it self._flush_root_span(span) + else: + self._children_spans[span.parent.span_id].append(span) # TODO-neel-potel not sure we need a clear like JS def shutdown(self): diff --git a/sentry_sdk/integrations/opentelemetry/sampler.py b/sentry_sdk/integrations/opentelemetry/sampler.py index 404957f028..2033addf09 100644 --- a/sentry_sdk/integrations/opentelemetry/sampler.py +++ b/sentry_sdk/integrations/opentelemetry/sampler.py @@ -1,5 +1,5 @@ +import random from typing import cast -from random import random from opentelemetry import trace @@ -54,6 +54,20 @@ def dropped_result(span_context, attributes, sample_rate=None): if sample_rate: trace_state = trace_state.add(TRACESTATE_SAMPLE_RATE_KEY, str(sample_rate)) + 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() + if client.monitor and client.monitor.downsample_factor > 0: + reason = "backpressure" + else: + reason = "sample_rate" + + client.transport.record_lost_event(reason, data_category="transaction") + + # Only one span (the transaction itself) is discarded, since we did not record any spans here. + client.transport.record_lost_event(reason, data_category="span") + return SamplingResult( Decision.DROP, attributes=attributes, @@ -124,6 +138,11 @@ def should_sample( # Check if there is a traces_sample_rate sample_rate = client.options.get("traces_sample_rate") + # Down-sample in case of back pressure monitor says so + # TODO: this should only be done for transactions (aka root spans) + if client.monitor: + sample_rate /= 2**client.monitor.downsample_factor + # If the sample rate is invalid, drop the span if not is_valid_sample_rate(sample_rate, source=self.__class__.__name__): logger.warning( @@ -133,7 +152,7 @@ def should_sample( # Roll the dice on sample rate sample_rate = float(cast("Union[bool, float, int]", sample_rate)) - sampled = random() < sample_rate + sampled = random.random() < sample_rate if sampled: return sampled_result(parent_span_context, attributes, sample_rate) diff --git a/tests/test_monitor.py b/tests/test_monitor.py index 03e415b5cc..ccc97037ce 100644 --- a/tests/test_monitor.py +++ b/tests/test_monitor.py @@ -56,13 +56,15 @@ def test_monitor_unhealthy(sentry_init): def test_transaction_uses_downsampled_rate( - sentry_init, capture_record_lost_event_calls, monkeypatch + sentry_init, capture_envelopes, capture_record_lost_event_calls, monkeypatch ): sentry_init( traces_sample_rate=1.0, transport=UnhealthyTestTransport(), ) + envelopes = capture_envelopes() + record_lost_event_calls = capture_record_lost_event_calls() monitor = sentry_sdk.get_client().monitor @@ -77,13 +79,32 @@ def test_transaction_uses_downsampled_rate( assert monitor.downsample_factor == 1 with sentry_sdk.start_transaction(name="foobar") as transaction: + with sentry_sdk.start_span(name="foospan"): + with sentry_sdk.start_span(name="foospan2"): + with sentry_sdk.start_span(name="foospan3"): + ... + assert transaction.sampled is False - assert transaction.sample_rate == 0.5 + assert ( + transaction.sample_rate == 0.5 + ) # TODO: this fails until we put the sample_rate in the POTelSpan + + assert len(envelopes) == 0 assert Counter(record_lost_event_calls) == Counter( [ - ("backpressure", "transaction", None, 1), - ("backpressure", "span", None, 1), + ( + "backpressure", + "transaction", + None, + 1, + ), + ( + "backpressure", + "span", + None, + 1, + ), # Only one span (the transaction itself) is counted, since we did not record any spans in the first place. ] )