Skip to content

Commit

Permalink
feat(tracing): Record lost spans in client reports
Browse files Browse the repository at this point in the history
Also, update existing transport tests so they pass against the changes introduced in this commit.

Resolves #3229
  • Loading branch information
szokeasaurusrex committed Jul 8, 2024
1 parent 64b4a86 commit dbdba1e
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 dbdba1e

Please sign in to comment.