Skip to content

Commit

Permalink
feat(tracing): Record lost spans in client reports
Browse files Browse the repository at this point in the history
Resolves #3229
  • Loading branch information
szokeasaurusrex committed Jul 5, 2024
1 parent 8f8b7ef commit ed6f9f8
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 11 deletions.
1 change: 1 addition & 0 deletions sentry_sdk/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@
"profile_chunk",
"metric_bucket",
"monitor",
"span",
]
SessionStatus = Literal["ok", "exited", "crashed", "abnormal"]

Expand Down
30 changes: 25 additions & 5 deletions sentry_sdk/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions sentry_sdk/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand Down
61 changes: 58 additions & 3 deletions sentry_sdk/transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions tests/test_transport.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,7 @@ def intercepting_fetch(*args, **kwargs):
report = parse_json(envelope.items[1].get_bytes())
assert sorted(report["discarded_events"], key=lambda x: x["quantity"]) == [
{"category": "transaction", "reason": "ratelimit_backoff", "quantity": 2},
{"category": "span", "reason": "ratelimit_backoff", "quantity": 2},
{"category": "attachment", "reason": "ratelimit_backoff", "quantity": 11},
]
capturing_server.clear_captured()
Expand All @@ -453,6 +454,7 @@ def intercepting_fetch(*args, **kwargs):
report = parse_json(envelope.items[0].get_bytes())
assert report["discarded_events"] == [
{"category": "transaction", "reason": "ratelimit_backoff", "quantity": 1},
{"category": "span", "reason": "ratelimit_backoff", "quantity": 1},
]


Expand Down

0 comments on commit ed6f9f8

Please sign in to comment.