feat(tracing): Record lost spans in client reports

Also, update existing transport tests so they pass against the changes introduced in this commit.

Resolves #3229

Author: szokeasaurusrex

sentry_sdk/_types.py

@@ -155,6 +155,7 @@
         "profile_chunk",
         "metric_bucket",
         "monitor",
+        "span",
     ]
     SessionStatus = Literal["ok", "exited", "crashed", "abnormal"]
 

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

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"
@@ -858,6 +856,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:

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

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

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

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),
             ]
         )
 

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):

tests/test_transport.py

@@ -425,12 +425,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 +459,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])

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, []),
     ],
 )