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

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,19 +448,31 @@ 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
             if event_ is None:
                 if self.transport:
-                    self.transport.record_lost_event(
-                        "event_processor",
-                        data_category=("transaction" if is_transaction else "error"),
-                    )
+                    if is_transaction:
+                        self.transport.record_lost_transaction(
+                            "event_processor", len(event.get("spans", []))
+                        )
+                    else:
+                        self.transport.record_lost_event(
+                            "event_processor",
+                            data_category="error",
+                        )
                 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 +553,22 @@ 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_transaction(
+                        reason="before_send", span_count=len(event.get("spans", []))
+                    )
+            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(
-                        "before_send", data_category="transaction"
+                        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"
@@ -856,7 +854,8 @@ def finish(self, hub=None, end_timestamp=None):
                 else:
                     reason = "sample_rate"
 
-                client.transport.record_lost_event(reason, data_category="transaction")
+                # No spans are discarded here because they were never recorded in the first place
+                client.transport.record_lost_transaction(reason, span_count=0)
 
             return None
 

sentry_sdk/transport.py

@@ -133,10 +133,33 @@ 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.
+
+        data_category="transaction" should use record_lost_transaction,
+        instead. Items containing a transaction are okay to pass, since
+        we can get the number of spans from the transaction.
+        """
+        return None
+
+    def record_lost_transaction(
+        self,
+        reason,  # type: str
+        span_count,  # type: int
+    ):
+        # type: (...) -> None
+        """This increments a counter for loss of a transaction and the transaction's spans.
+
+        The span_count should only include the number of spans contained in the transaction, EXCLUDING the transaction
+        itself.
         """
         return None
 
@@ -224,24 +247,56 @@ 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 data_category == "transaction":
+            warnings.warn(
+                "Use record_lost_transaction for transactions to ensure lost spans are counted.",
+                stacklevel=2,
+            )
+
         if item is not None:
             data_category = item.data_category
-            if data_category == "attachment":
+
+            if data_category == "transaction":
+                # Handle transactions through record_lost_transaction.
+                event = item.get_transaction_event() or {}
+                span_count = len(event.get("spans") or [])
+                self.record_lost_transaction(reason, span_count)
+                return
+
+            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
 
+            else:
+                # Any other item data category should be counted as 1, since the one item corresponds to one event.
+                quantity = 1
+
         elif data_category is None:
             raise TypeError("data category not provided")
 
         self._discarded_events[data_category, reason] += quantity
 
+    def record_lost_transaction(
+        self,
+        reason,  # type: str
+        span_count,  # type: int
+    ):  # type: (...) -> None
+        if not self.options["send_client_reports"]:
+            return
+
+        self._discarded_events["transaction", reason] += 1
+        self._discarded_events["span", reason] += (
+            span_count + 1
+        )  # Also count the transaction itself
+
     def _update_rate_limits(self, response):
         # type: (urllib3.BaseHTTPResponse) -> None
 

tests/test_transport.py

@@ -433,12 +433,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",
@@ -462,9 +467,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])