diff --git a/sentry_sdk/_types.py b/sentry_sdk/_types.py index bd229977a5..14fa8d08c2 100644 --- a/sentry_sdk/_types.py +++ b/sentry_sdk/_types.py @@ -155,6 +155,7 @@ "profile_chunk", "metric_bucket", "monitor", + "span", ] SessionStatus = Literal["ok", "exited", "crashed", "abnormal"] diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py index 07cd39029b..f93aa935c2 100644 --- a/sentry_sdk/client.py +++ b/sentry_sdk/client.py @@ -448,6 +448,7 @@ def _prepare_event( if scope is not None: is_transaction = event.get("type") == "transaction" + spans_before = len(event.get("spans", [])) event_ = scope.apply_to_event(event, hint, self.options) # one of the event/error processors returned None @@ -457,10 +458,22 @@ def _prepare_event( "event_processor", data_category=("transaction" if is_transaction else "error"), ) + if is_transaction: + self.transport.record_lost_event( + "event_processor", + data_category="span", + quantity=spans_before + 1, # +1 for the transaction itself + ) return None event = event_ + spans_delta = spans_before - len(event.get("spans", [])) + if is_transaction and spans_delta > 0 and self.transport is not None: + self.transport.record_lost_event( + "event_processor", data_category="span", quantity=spans_delta + ) + if ( self.options["attach_stacktrace"] and "exception" not in event @@ -541,14 +554,27 @@ def _prepare_event( and event.get("type") == "transaction" ): new_event = None + spans_before = len(event.get("spans", [])) with capture_internal_exceptions(): new_event = before_send_transaction(event, hint or {}) if new_event is None: logger.info("before send transaction dropped event") if self.transport: self.transport.record_lost_event( - "before_send", data_category="transaction" + reason="before_send", data_category="transaction" + ) + self.transport.record_lost_event( + reason="before_send", + data_category="span", + quantity=spans_before + 1, # +1 for the transaction itself ) + else: + spans_delta = spans_before - len(new_event.get("spans", [])) + if spans_delta > 0 and self.transport is not None: + self.transport.record_lost_event( + reason="before_send", data_category="span", quantity=spans_delta + ) + event = new_event # type: ignore return event diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py index 6d206c3fa8..95a2d3469b 100644 --- a/sentry_sdk/tracing.py +++ b/sentry_sdk/tracing.py @@ -119,11 +119,9 @@ class TransactionKwargs(SpanKwargs, total=False): }, ) - BAGGAGE_HEADER_NAME = "baggage" SENTRY_TRACE_HEADER_NAME = "sentry-trace" - # Transaction source # see https://develop.sentry.dev/sdk/event-payloads/transaction/#transaction-annotations TRANSACTION_SOURCE_CUSTOM = "custom" @@ -869,6 +867,8 @@ def finish(self, hub=None, end_timestamp=None): 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 None if not self.name: diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py index 293dfc0e97..63bd1d9fb3 100644 --- a/sentry_sdk/transport.py +++ b/sentry_sdk/transport.py @@ -133,10 +133,23 @@ def record_lost_event( reason, # type: str data_category=None, # type: Optional[EventDataCategory] item=None, # type: Optional[Item] + *, + quantity=1, # type: int ): # type: (...) -> None """This increments a counter for event loss by reason and - data category. + data category by the given positive-int quantity (default 1). + + If an item is provided, the data category and quantity are + extracted from the item, and the values passed for + data_category and quantity are ignored. + + When recording a lost transaction via data_category="transaction", + the calling code should also record the lost spans via this method. + When recording lost spans, `quantity` should be set to the number + of contained spans, plus one for the transaction itself. When + passing an Item containing a transaction via the `item` parameter, + this method automatically records the lost spans. """ return None @@ -224,15 +237,26 @@ def record_lost_event( reason, # type: str data_category=None, # type: Optional[EventDataCategory] item=None, # type: Optional[Item] + *, + quantity=1, # type: int ): # type: (...) -> None if not self.options["send_client_reports"]: return - quantity = 1 if item is not None: data_category = item.data_category - if data_category == "attachment": + quantity = 1 # If an item is provided, we always count it as 1 (except for attachments, handled below). + + if data_category == "transaction": + # Also record the lost spans + event = item.get_transaction_event() or {} + + # +1 for the transaction itself + span_count = len(event.get("spans") or []) + 1 + self.record_lost_event(reason, "span", quantity=span_count) + + elif data_category == "attachment": # quantity of 0 is actually 1 as we do not want to count # empty attachments as actually empty. quantity = len(item.get_bytes()) or 1 diff --git a/tests/conftest.py b/tests/conftest.py index b043a849fb..eada3bdac7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -253,8 +253,8 @@ def inner(): calls = [] test_client = sentry_sdk.get_client() - def record_lost_event(reason, data_category=None, item=None): - calls.append((reason, data_category, item)) + def record_lost_event(reason, data_category=None, item=None, *, quantity=1): + calls.append((reason, data_category, item, quantity)) monkeypatch.setattr( test_client.transport, "record_lost_event", record_lost_event diff --git a/tests/profiler/test_transaction_profiler.py b/tests/profiler/test_transaction_profiler.py index b30faffc7c..ec506cfa67 100644 --- a/tests/profiler/test_transaction_profiler.py +++ b/tests/profiler/test_transaction_profiler.py @@ -162,7 +162,7 @@ def test_profiles_sample_rate( elif profile_count: assert record_lost_event_calls == [] else: - assert record_lost_event_calls == [("sample_rate", "profile", None)] + assert record_lost_event_calls == [("sample_rate", "profile", None, 1)] @pytest.mark.parametrize( @@ -231,7 +231,7 @@ def test_profiles_sampler( if profile_count: assert record_lost_event_calls == [] else: - assert record_lost_event_calls == [("sample_rate", "profile", None)] + assert record_lost_event_calls == [("sample_rate", "profile", None, 1)] def test_minimum_unique_samples_required( @@ -260,7 +260,7 @@ def test_minimum_unique_samples_required( # because we dont leave any time for the profiler to # take any samples, it should be not be sent assert len(items["profile"]) == 0 - assert record_lost_event_calls == [("insufficient_data", "profile", None)] + assert record_lost_event_calls == [("insufficient_data", "profile", None, 1)] @pytest.mark.forked diff --git a/tests/test_basics.py b/tests/test_basics.py index 391c1c418f..439215e013 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -570,7 +570,7 @@ def test_dedupe_event_processor_drop_records_client_report( assert event["level"] == "error" assert "exception" in event - assert lost_event_call == ("event_processor", "error", None) + assert lost_event_call == ("event_processor", "error", None, 1) def test_event_processor_drop_records_client_report( @@ -602,8 +602,9 @@ def foo(event, hint): # Using Counter because order of record_lost_event calls does not matter assert Counter(record_lost_event_calls) == Counter( [ - ("event_processor", "error", None), - ("event_processor", "transaction", None), + ("event_processor", "error", None, 1), + ("event_processor", "transaction", None, 1), + ("event_processor", "span", None, 1), ] ) diff --git a/tests/test_client.py b/tests/test_client.py index a2fea56202..3be8b1e64b 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -3,6 +3,7 @@ import subprocess import sys import time +from collections import Counter, defaultdict from collections.abc import Mapping from textwrap import dedent from unittest import mock @@ -1214,3 +1215,159 @@ def test_uwsgi_warnings(sentry_init, recwarn, opt, missing_flags): assert flag in str(record.message) else: assert not recwarn + + +class TestSpanClientReports: + """ + Tests for client reports related to spans. + """ + + @staticmethod + def span_dropper(spans_to_drop): + """ + Returns a function that can be used to drop spans from an event. + """ + + def drop_spans(event, _): + event["spans"] = event["spans"][spans_to_drop:] + return event + + return drop_spans + + @staticmethod + def mock_transaction_event(span_count): + """ + Returns a mock transaction event with the given number of spans. + """ + + return defaultdict( + mock.MagicMock, + type="transaction", + spans=[mock.MagicMock() for _ in range(span_count)], + ) + + def __init__(self, span_count): + """Configures a test case with the number of spans dropped and whether the transaction was dropped.""" + self.span_count = span_count + self.expected_record_lost_event_calls = Counter() + self.before_send = lambda event, _: event + self.event_processor = lambda event, _: event + + def _update_resulting_calls(self, reason, drops_transactions=0, drops_spans=0): + """ + Updates the expected calls with the given resulting calls. + """ + if drops_transactions > 0: + self.expected_record_lost_event_calls[ + (reason, "transaction", None, drops_transactions) + ] += 1 + + if drops_spans > 0: + self.expected_record_lost_event_calls[ + (reason, "span", None, drops_spans) + ] += 1 + + def with_before_send( + self, + before_send, + *, + drops_transactions=0, + drops_spans=0, + ): + self.before_send = before_send + self._update_resulting_calls( + "before_send", + drops_transactions, + drops_spans, + ) + + return self + + def with_event_processor( + self, + event_processor, + *, + drops_transactions=0, + drops_spans=0, + ): + self.event_processor = event_processor + self._update_resulting_calls( + "event_processor", + drops_transactions, + drops_spans, + ) + + return self + + def run(self, sentry_init, capture_record_lost_event_calls): + """Runs the test case with the configured parameters.""" + sentry_init(before_send_transaction=self.before_send) + record_lost_event_calls = capture_record_lost_event_calls() + + with sentry_sdk.isolation_scope() as scope: + scope.add_event_processor(self.event_processor) + event = self.mock_transaction_event(self.span_count) + sentry_sdk.get_client().capture_event(event, scope=scope) + + # We use counters to ensure that the calls are made the expected number of times, disregarding order. + assert Counter(record_lost_event_calls) == self.expected_record_lost_event_calls + + +@pytest.mark.parametrize( + "test_config", + ( + TestSpanClientReports(span_count=10), # No spans dropped + TestSpanClientReports(span_count=0).with_before_send( + lambda e, _: None, + drops_transactions=1, + drops_spans=1, + ), + TestSpanClientReports(span_count=10).with_before_send( + lambda e, _: None, + drops_transactions=1, + drops_spans=11, + ), + TestSpanClientReports(span_count=10).with_before_send( + TestSpanClientReports.span_dropper(3), + drops_spans=3, + ), + TestSpanClientReports(span_count=10).with_before_send( + TestSpanClientReports.span_dropper(10), + drops_spans=10, + ), + TestSpanClientReports(span_count=10).with_event_processor( + lambda e, _: None, + drops_transactions=1, + drops_spans=11, + ), + TestSpanClientReports(span_count=10).with_event_processor( + TestSpanClientReports.span_dropper(3), + drops_spans=3, + ), + TestSpanClientReports(span_count=10).with_event_processor( + TestSpanClientReports.span_dropper(10), + drops_spans=10, + ), + TestSpanClientReports(span_count=10) + .with_event_processor( + TestSpanClientReports.span_dropper(3), + drops_spans=3, + ) + .with_before_send( + TestSpanClientReports.span_dropper(5), + drops_spans=5, + ), + TestSpanClientReports(10) + .with_event_processor( + TestSpanClientReports.span_dropper(3), + drops_spans=3, + ) + .with_before_send( + lambda e, _: None, + drops_transactions=1, + drops_spans=8, # 3 of the 11 (incl. transaction) spans already dropped + ), + ), +) +def test_dropped_transaction(sentry_init, capture_record_lost_event_calls, test_config): + test_config.run(sentry_init, capture_record_lost_event_calls) diff --git a/tests/test_monitor.py b/tests/test_monitor.py index e15b3a7d08..03e415b5cc 100644 --- a/tests/test_monitor.py +++ b/tests/test_monitor.py @@ -1,4 +1,5 @@ import random +from collections import Counter from unittest import mock import sentry_sdk @@ -79,7 +80,12 @@ def test_transaction_uses_downsampled_rate( assert transaction.sampled is False assert transaction.sample_rate == 0.5 - assert record_lost_event_calls == [("backpressure", "transaction", None)] + assert Counter(record_lost_event_calls) == Counter( + [ + ("backpressure", "transaction", None, 1), + ("backpressure", "span", None, 1), + ] + ) def test_monitor_no_thread_on_shutdown_no_errors(sentry_init): diff --git a/tests/test_transport.py b/tests/test_transport.py index 4ed950533f..dc8e8073b5 100644 --- a/tests/test_transport.py +++ b/tests/test_transport.py @@ -86,6 +86,20 @@ def inner(**kwargs): return inner +def mock_transaction_envelope(span_count): + # type: (int) -> Envelope + event = defaultdict( + mock.MagicMock, + type="transaction", + spans=[mock.MagicMock() for _ in range(span_count)], + ) + + envelope = Envelope() + envelope.add_transaction(event) + + return envelope + + @pytest.mark.forked @pytest.mark.parametrize("debug", (True, False)) @pytest.mark.parametrize("client_flush_method", ["close", "flush"]) @@ -425,12 +439,17 @@ def intercepting_fetch(*args, **kwargs): discarded_events = report["discarded_events"] - assert len(discarded_events) == 2 + assert len(discarded_events) == 3 assert { "category": "transaction", "reason": "ratelimit_backoff", "quantity": 2, } in discarded_events + assert { + "category": "span", + "reason": "ratelimit_backoff", + "quantity": 2, + } in discarded_events assert { "category": "attachment", "reason": "ratelimit_backoff", @@ -454,9 +473,19 @@ def intercepting_fetch(*args, **kwargs): envelope = capturing_server.captured[1].envelope assert envelope.items[0].type == "client_report" report = parse_json(envelope.items[0].get_bytes()) - assert report["discarded_events"] == [ - {"category": "transaction", "reason": "ratelimit_backoff", "quantity": 1}, - ] + + discarded_events = report["discarded_events"] + assert len(discarded_events) == 2 + assert { + "category": "transaction", + "reason": "ratelimit_backoff", + "quantity": 1, + } in discarded_events + assert { + "category": "span", + "reason": "ratelimit_backoff", + "quantity": 1, + } in discarded_events @pytest.mark.parametrize("response_code", [200, 429]) @@ -613,3 +642,59 @@ class TestCustomHubClass(sentry_sdk.Hub): with pytest.deprecated_call(): assert transport.hub_cls is TestCustomHubClass + + +@pytest.mark.parametrize("quantity", (1, 2, 10)) +def test_record_lost_event_quantity(capturing_server, make_client, quantity): + client = make_client() + transport = client.transport + + transport.record_lost_event(reason="test", data_category="span", quantity=quantity) + client.flush() + + (captured,) = capturing_server.captured # Should only be one envelope + envelope = captured.envelope + (item,) = envelope.items # Envelope should only have one item + + assert item.type == "client_report" + + report = parse_json(item.get_bytes()) + + assert report["discarded_events"] == [ + {"category": "span", "reason": "test", "quantity": quantity} + ] + + +@pytest.mark.parametrize("span_count", (0, 1, 2, 10)) +def test_record_lost_event_transaction_item(capturing_server, make_client, span_count): + client = make_client() + transport = client.transport + + envelope = mock_transaction_envelope(span_count) + (transaction_item,) = envelope.items + + transport.record_lost_event(reason="test", item=transaction_item) + client.flush() + + (captured,) = capturing_server.captured # Should only be one envelope + envelope = captured.envelope + (item,) = envelope.items # Envelope should only have one item + + assert item.type == "client_report" + + report = parse_json(item.get_bytes()) + discarded_events = report["discarded_events"] + + assert len(discarded_events) == 2 + + assert { + "category": "transaction", + "reason": "test", + "quantity": 1, + } in discarded_events + + assert { + "category": "span", + "reason": "test", + "quantity": span_count + 1, + } in discarded_events diff --git a/tests/tracing/test_sampling.py b/tests/tracing/test_sampling.py index d9bb6ef4d8..491281fa67 100644 --- a/tests/tracing/test_sampling.py +++ b/tests/tracing/test_sampling.py @@ -264,7 +264,11 @@ def test_warns_and_sets_sampled_to_false_on_invalid_traces_sampler_return_value( "traces_sample_rate,sampled_output,expected_record_lost_event_calls", [ (None, False, []), - (0.0, False, [("sample_rate", "transaction", None)]), + ( + 0.0, + False, + [("sample_rate", "transaction", None, 1), ("sample_rate", "span", None, 1)], + ), (1.0, True, []), ], ) @@ -290,7 +294,11 @@ def test_records_lost_event_only_if_traces_sample_rate_enabled( "traces_sampler,sampled_output,expected_record_lost_event_calls", [ (None, False, []), - (lambda _x: 0.0, False, [("sample_rate", "transaction", None)]), + ( + lambda _x: 0.0, + False, + [("sample_rate", "transaction", None, 1), ("sample_rate", "span", None, 1)], + ), (lambda _x: 1.0, True, []), ], )