[do not merge] feat: Span streaming & new span API #5551
5 issues
code-review: Found 5 issues (2 high, 2 medium, 1 low)
High
set_attribute method does not exist on Span class, will raise AttributeError - `sentry_sdk/integrations/openai_agents/utils.py:55-58`
The code assigns set_on_span = span.set_attribute when the span is an instance of Span, but the Span class (in sentry_sdk/tracing.py) does not have a set_attribute method. This will cause an AttributeError at runtime when _record_exception_on_span is called with a legacy Span object. The logic appears to be inverted - Span has set_data() while StreamedSpan has set_attribute().
Also found at:
sentry_sdk/integrations/sqlalchemy.py:102
Spans not properly closed on exception in async Redis command execution - `sentry_sdk/integrations/redis/_async_common.py:145-156`
In _sentry_execute_command, the db_span and cache_span are entered via __enter__() but their __exit__() calls (lines 152, 156) are not protected by a try/finally block. If old_execute_command raises an exception, the spans will never be closed, causing span leaks and incorrect timing data. The sync version in _sync_common.py correctly uses try/finally for this pattern.
Also found at:
sentry_sdk/integrations/redis/_sync_common.py:152
Medium
Streaming mode ignores http_methods_to_capture filter, creating spans for filtered methods - `sentry_sdk/integrations/asgi.py:238-241`
In the span streaming branch (lines 218-241), a span is always created via start_span() at line 238, even when ty == 'http' and the method is not in http_methods_to_capture. In contrast, the legacy branch correctly sets transaction = None and uses nullcontext() to skip span creation for filtered methods. This causes unintended spans to be created and sent for HTTP methods that should be excluded.
Also found at:
sentry_sdk/integrations/graphene.py:174-175
StreamedSpan is missing HTTP status code attribute - `sentry_sdk/integrations/httpx.py:116-118`
When using span streaming mode, the code sets span.status and span.set_attribute("reason", ...) but doesn't set the HTTP status code attribute (SPANDATA.HTTP_STATUS_CODE). The legacy Span.set_http_status() method sets both the status and http.response.status_code. This means StreamedSpans will be missing important HTTP status code telemetry data that the legacy spans include.
Also found at:
sentry_sdk/integrations/httpx.py:199-201sentry_sdk/integrations/stdlib.py:175-177
Low
Duplicate constant definitions will be immediately overwritten - `sentry_sdk/traces.py:41-46`
The constants BAGGAGE_HEADER_NAME and SENTRY_TRACE_HEADER_NAME are defined twice in succession (lines 41-42 and 45-46). While this doesn't cause a runtime error since both definitions have identical values, it indicates a likely copy-paste mistake and unnecessary code duplication.
Duration: 26m 8s · Tokens: 10.5M in / 118.0k out · Cost: $14.36 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.01)
Annotations
Check failure on line 58 in sentry_sdk/integrations/openai_agents/utils.py
sentry-warden / warden: code-review
set_attribute method does not exist on Span class, will raise AttributeError
The code assigns `set_on_span = span.set_attribute` when the span is an instance of `Span`, but the `Span` class (in sentry_sdk/tracing.py) does not have a `set_attribute` method. This will cause an `AttributeError` at runtime when `_record_exception_on_span` is called with a legacy `Span` object. The logic appears to be inverted - `Span` has `set_data()` while `StreamedSpan` has `set_attribute()`.
Check failure on line 102 in sentry_sdk/integrations/sqlalchemy.py
sentry-warden / warden: code-review
[LS6-SGE] set_attribute method does not exist on Span class, will raise AttributeError (additional location)
The code assigns `set_on_span = span.set_attribute` when the span is an instance of `Span`, but the `Span` class (in sentry_sdk/tracing.py) does not have a `set_attribute` method. This will cause an `AttributeError` at runtime when `_record_exception_on_span` is called with a legacy `Span` object. The logic appears to be inverted - `Span` has `set_data()` while `StreamedSpan` has `set_attribute()`.
Check failure on line 156 in sentry_sdk/integrations/redis/_async_common.py
sentry-warden / warden: code-review
Spans not properly closed on exception in async Redis command execution
In `_sentry_execute_command`, the `db_span` and `cache_span` are entered via `__enter__()` but their `__exit__()` calls (lines 152, 156) are not protected by a try/finally block. If `old_execute_command` raises an exception, the spans will never be closed, causing span leaks and incorrect timing data. The sync version in `_sync_common.py` correctly uses try/finally for this pattern.
Check failure on line 152 in sentry_sdk/integrations/redis/_sync_common.py
sentry-warden / warden: code-review
[Q5F-NNN] Spans not properly closed on exception in async Redis command execution (additional location)
In `_sentry_execute_command`, the `db_span` and `cache_span` are entered via `__enter__()` but their `__exit__()` calls (lines 152, 156) are not protected by a try/finally block. If `old_execute_command` raises an exception, the spans will never be closed, causing span leaks and incorrect timing data. The sync version in `_sync_common.py` correctly uses try/finally for this pattern.
Check warning on line 241 in sentry_sdk/integrations/asgi.py
sentry-warden / warden: code-review
Streaming mode ignores http_methods_to_capture filter, creating spans for filtered methods
In the span streaming branch (lines 218-241), a span is always created via `start_span()` at line 238, even when `ty == 'http'` and the method is not in `http_methods_to_capture`. In contrast, the legacy branch correctly sets `transaction = None` and uses `nullcontext()` to skip span creation for filtered methods. This causes unintended spans to be created and sent for HTTP methods that should be excluded.
Check warning on line 175 in sentry_sdk/integrations/graphene.py
sentry-warden / warden: code-review
[R2S-TGS] Streaming mode ignores http_methods_to_capture filter, creating spans for filtered methods (additional location)
In the span streaming branch (lines 218-241), a span is always created via `start_span()` at line 238, even when `ty == 'http'` and the method is not in `http_methods_to_capture`. In contrast, the legacy branch correctly sets `transaction = None` and uses `nullcontext()` to skip span creation for filtered methods. This causes unintended spans to be created and sent for HTTP methods that should be excluded.
Check warning on line 118 in sentry_sdk/integrations/httpx.py
sentry-warden / warden: code-review
StreamedSpan is missing HTTP status code attribute
When using span streaming mode, the code sets `span.status` and `span.set_attribute("reason", ...)` but doesn't set the HTTP status code attribute (`SPANDATA.HTTP_STATUS_CODE`). The legacy `Span.set_http_status()` method sets both the status and `http.response.status_code`. This means StreamedSpans will be missing important HTTP status code telemetry data that the legacy spans include.
Check warning on line 201 in sentry_sdk/integrations/httpx.py
sentry-warden / warden: code-review
[PJK-WDD] StreamedSpan is missing HTTP status code attribute (additional location)
When using span streaming mode, the code sets `span.status` and `span.set_attribute("reason", ...)` but doesn't set the HTTP status code attribute (`SPANDATA.HTTP_STATUS_CODE`). The legacy `Span.set_http_status()` method sets both the status and `http.response.status_code`. This means StreamedSpans will be missing important HTTP status code telemetry data that the legacy spans include.
Check warning on line 177 in sentry_sdk/integrations/stdlib.py
sentry-warden / warden: code-review
[PJK-WDD] StreamedSpan is missing HTTP status code attribute (additional location)
When using span streaming mode, the code sets `span.status` and `span.set_attribute("reason", ...)` but doesn't set the HTTP status code attribute (`SPANDATA.HTTP_STATUS_CODE`). The legacy `Span.set_http_status()` method sets both the status and `http.response.status_code`. This means StreamedSpans will be missing important HTTP status code telemetry data that the legacy spans include.