feat(span-first): Support before_send_span#6239
2 issues
code-review: Found 2 issues (1 medium, 1 low)
Medium
Exceptions from `before_send_span` propagate uncaught through `span.end()` and crash user code - `sentry_sdk/client.py:54`
Unlike before_send and before_send_transaction (wrapped in capture_internal_exceptions), the before_send_span call in _capture_telemetry is unguarded — if the callback raises, the exception propagates through scope._capture_span → StreamedSpan._end into user code, potentially crashing code that relies on with start_span(...): blocks exiting cleanly. Consider wrapping the before_send call in _capture_telemetry with capture_internal_exceptions().
Also found at:
tests/tracing/test_span_streaming.py:272
Low
`before_send_span` return type `Optional[SpanJSON]` is misleading — `None` does not drop the span - `sentry_sdk/consts.py:88-90`
The callback's return type in Experiments is declared as Optional[SpanJSON], which by convention (matching before_send_metric/before_send_log) suggests returning None drops the item. In reality Client._capture_telemetry treats None as an invalid return, logs Invalid return value from before_send_span. Keeping original span. and re-serializes the original span. Per the PR description and spec, spans cannot be dropped via before_send_span. The type annotation should be tightened to SpanJSON (non-optional) so the type system reflects the actual contract and users aren't misled into expecting drop semantics.
Also found at:
sentry_sdk/utils.py:2115-2121
⏱ 10m 14s · 3.8M in / 97.9k out · $3.97
Annotations
Check warning on line 54 in sentry_sdk/client.py
sentry-warden / warden: code-review
Exceptions from `before_send_span` propagate uncaught through `span.end()` and crash user code
Unlike `before_send` and `before_send_transaction` (wrapped in `capture_internal_exceptions`), the `before_send_span` call in `_capture_telemetry` is unguarded — if the callback raises, the exception propagates through `scope._capture_span → StreamedSpan._end` into user code, potentially crashing code that relies on `with start_span(...):` blocks exiting cleanly. Consider wrapping the `before_send` call in `_capture_telemetry` with `capture_internal_exceptions()`.
Check warning on line 272 in tests/tracing/test_span_streaming.py
sentry-warden / warden: code-review
[S8L-9QS] Exceptions from `before_send_span` propagate uncaught through `span.end()` and crash user code (additional location)
Unlike `before_send` and `before_send_transaction` (wrapped in `capture_internal_exceptions`), the `before_send_span` call in `_capture_telemetry` is unguarded — if the callback raises, the exception propagates through `scope._capture_span → StreamedSpan._end` into user code, potentially crashing code that relies on `with start_span(...):` blocks exiting cleanly. Consider wrapping the `before_send` call in `_capture_telemetry` with `capture_internal_exceptions()`.