[do not merge] feat: Span streaming & new span API #5551
7 issues
code-review: Found 7 issues (3 high, 4 medium)
High
Accessing undefined `value` variable in finally block causes NameError - `sentry_sdk/integrations/redis/_sync_common.py:158`
When old_execute_command raises an exception at line 152, the value variable is never assigned. The finally block always executes, including line 158 which passes value to _set_cache_data. This causes an UnboundLocalError: local variable 'value' referenced before assignment. While wrapped in capture_internal_exceptions, this silently loses the ability to record cache span data on errors.
Also found at:
sentry_sdk/integrations/redis/_async_common.py:145-147
StreamedSpan lacks set_status() method causing AttributeError at runtime - `sentry_sdk/integrations/sqlalchemy.py:102`
The code calls span.set_status(SpanStatus.ERROR) on a StreamedSpan, but StreamedSpan class does not have a set_status() method. It only has a status property setter (lines 426-437 in traces.py). The legacy Span class in tracing.py has set_status() at line 600, but the new StreamedSpan does not. This will raise an AttributeError at runtime when SQLAlchemy query errors occur in streaming mode.
NoOpStreamedSpan._get_trace_context() will raise AttributeError due to _segment being None - `sentry_sdk/traces.py:583`
Setting self._segment = None at line 583 in NoOpStreamedSpan.__init__ will cause a runtime error when _get_trace_context() is called. The inherited _get_trace_context() method (line 537-552) calls self._dynamic_sampling_context(), which directly accesses self._segment._get_baggage() without a null check. This method is called from scope.get_trace_context() when the active span is a NoOpStreamedSpan, resulting in AttributeError: 'NoneType' object has no attribute '_get_baggage'.
Medium
Span streaming path creates spans for HTTP methods that should be ignored - `sentry_sdk/integrations/asgi.py:238-241`
In the span streaming path (lines 218-241), when ty is http but the method is not in self.http_methods_to_capture, the code still creates a span (line 238-240) without setting sentry.op or calling trace propagation functions. The legacy path correctly uses nullcontext() when the HTTP method should not be captured. This causes unintended spans to be created for requests like OPTIONS that are typically excluded.
HTTP status code not recorded on StreamedSpan - `sentry_sdk/integrations/httpx.py:116-118`
When span streaming is enabled, the HTTP response status code is not being recorded as an attribute on the span. The legacy set_http_status() method sets SPANDATA.HTTP_STATUS_CODE (http.response.status_code), but the StreamedSpan code path only sets span.status and reason. This means http.response.status_code will be missing from HTTP spans in streaming mode, causing data loss and inconsistent span attributes between streaming and non-streaming modes.
Also found at:
sentry_sdk/integrations/httpx.py:199-201sentry_sdk/integrations/stdlib.py:175-177
NoOpSpan may return formatted traceparent instead of empty string via private method alias - `sentry_sdk/scope.py:585`
The change from to_traceparent() to _to_traceparent() may alter behavior for NoOpSpan instances. NoOpSpan overrides to_traceparent() to return an empty string, but inherits _to_traceparent from Span which is bound to Span.to_traceparent. When a NoOpSpan is active on the scope, get_traceparent() may now return a formatted traceparent string instead of empty string, potentially causing unintended trace propagation.
Missing scope parameter in NoOpStreamedSpan for ignored child spans - `sentry_sdk/scope.py:1286-1288`
When creating a NoOpStreamedSpan for an ignored child span at line 1287, the scope=self parameter is missing. In contrast, when creating NoOpStreamedSpan for ignored segment spans (line 1250), the scope parameter is passed. Without the scope, _start() returns early and doesn't set the span on the scope, and _end() won't restore the previous span, potentially leaving the scope's span in an inconsistent state.
Also found at:
sentry_sdk/scope.py:1289-1292
Duration: 42m 42s · Tokens: 13.0M in / 153.3k out · Cost: $18.40 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.01)
Annotations
Check failure on line 158 in sentry_sdk/integrations/redis/_sync_common.py
sentry-warden / warden: code-review
Accessing undefined `value` variable in finally block causes NameError
When `old_execute_command` raises an exception at line 152, the `value` variable is never assigned. The `finally` block always executes, including line 158 which passes `value` to `_set_cache_data`. This causes an `UnboundLocalError: local variable 'value' referenced before assignment`. While wrapped in `capture_internal_exceptions`, this silently loses the ability to record cache span data on errors.
Check failure on line 147 in sentry_sdk/integrations/redis/_async_common.py
sentry-warden / warden: code-review
[GC4-L7S] Accessing undefined `value` variable in finally block causes NameError (additional location)
When `old_execute_command` raises an exception at line 152, the `value` variable is never assigned. The `finally` block always executes, including line 158 which passes `value` to `_set_cache_data`. This causes an `UnboundLocalError: local variable 'value' referenced before assignment`. While wrapped in `capture_internal_exceptions`, this silently loses the ability to record cache span data on errors.
Check failure on line 102 in sentry_sdk/integrations/sqlalchemy.py
sentry-warden / warden: code-review
StreamedSpan lacks set_status() method causing AttributeError at runtime
The code calls `span.set_status(SpanStatus.ERROR)` on a `StreamedSpan`, but `StreamedSpan` class does not have a `set_status()` method. It only has a `status` property setter (lines 426-437 in traces.py). The legacy `Span` class in tracing.py has `set_status()` at line 600, but the new `StreamedSpan` does not. This will raise an `AttributeError` at runtime when SQLAlchemy query errors occur in streaming mode.
Check failure on line 583 in sentry_sdk/traces.py
sentry-warden / warden: code-review
NoOpStreamedSpan._get_trace_context() will raise AttributeError due to _segment being None
Setting `self._segment = None` at line 583 in `NoOpStreamedSpan.__init__` will cause a runtime error when `_get_trace_context()` is called. The inherited `_get_trace_context()` method (line 537-552) calls `self._dynamic_sampling_context()`, which directly accesses `self._segment._get_baggage()` without a null check. This method is called from `scope.get_trace_context()` when the active span is a `NoOpStreamedSpan`, resulting in `AttributeError: 'NoneType' object has no attribute '_get_baggage'`.
Check warning on line 241 in sentry_sdk/integrations/asgi.py
sentry-warden / warden: code-review
Span streaming path creates spans for HTTP methods that should be ignored
In the span streaming path (lines 218-241), when `ty` is `http` but the method is not in `self.http_methods_to_capture`, the code still creates a span (line 238-240) without setting `sentry.op` or calling trace propagation functions. The legacy path correctly uses `nullcontext()` when the HTTP method should not be captured. This causes unintended spans to be created for requests like OPTIONS that are typically excluded.
Check warning on line 118 in sentry_sdk/integrations/httpx.py
sentry-warden / warden: code-review
HTTP status code not recorded on StreamedSpan
When span streaming is enabled, the HTTP response status code is not being recorded as an attribute on the span. The legacy `set_http_status()` method sets `SPANDATA.HTTP_STATUS_CODE` (http.response.status_code), but the StreamedSpan code path only sets `span.status` and `reason`. This means `http.response.status_code` will be missing from HTTP spans in streaming mode, causing data loss and inconsistent span attributes between streaming and non-streaming modes.
Check warning on line 201 in sentry_sdk/integrations/httpx.py
sentry-warden / warden: code-review
[BJQ-C4X] HTTP status code not recorded on StreamedSpan (additional location)
When span streaming is enabled, the HTTP response status code is not being recorded as an attribute on the span. The legacy `set_http_status()` method sets `SPANDATA.HTTP_STATUS_CODE` (http.response.status_code), but the StreamedSpan code path only sets `span.status` and `reason`. This means `http.response.status_code` will be missing from HTTP spans in streaming mode, causing data loss and inconsistent span attributes between streaming and non-streaming modes.
Check warning on line 177 in sentry_sdk/integrations/stdlib.py
sentry-warden / warden: code-review
[BJQ-C4X] HTTP status code not recorded on StreamedSpan (additional location)
When span streaming is enabled, the HTTP response status code is not being recorded as an attribute on the span. The legacy `set_http_status()` method sets `SPANDATA.HTTP_STATUS_CODE` (http.response.status_code), but the StreamedSpan code path only sets `span.status` and `reason`. This means `http.response.status_code` will be missing from HTTP spans in streaming mode, causing data loss and inconsistent span attributes between streaming and non-streaming modes.
Check warning on line 585 in sentry_sdk/scope.py
sentry-warden / warden: code-review
NoOpSpan may return formatted traceparent instead of empty string via private method alias
The change from `to_traceparent()` to `_to_traceparent()` may alter behavior for NoOpSpan instances. NoOpSpan overrides `to_traceparent()` to return an empty string, but inherits `_to_traceparent` from Span which is bound to `Span.to_traceparent`. When a NoOpSpan is active on the scope, `get_traceparent()` may now return a formatted traceparent string instead of empty string, potentially causing unintended trace propagation.
Check warning on line 1288 in sentry_sdk/scope.py
sentry-warden / warden: code-review
Missing scope parameter in NoOpStreamedSpan for ignored child spans
When creating a `NoOpStreamedSpan` for an ignored child span at line 1287, the `scope=self` parameter is missing. In contrast, when creating `NoOpStreamedSpan` for ignored segment spans (line 1250), the scope parameter is passed. Without the scope, `_start()` returns early and doesn't set the span on the scope, and `_end()` won't restore the previous span, potentially leaving the scope's span in an inconsistent state.
Check warning on line 1292 in sentry_sdk/scope.py
sentry-warden / warden: code-review
[AKA-MZF] Missing scope parameter in NoOpStreamedSpan for ignored child spans (additional location)
When creating a `NoOpStreamedSpan` for an ignored child span at line 1287, the `scope=self` parameter is missing. In contrast, when creating `NoOpStreamedSpan` for ignored segment spans (line 1250), the scope parameter is passed. Without the scope, `_start()` returns early and doesn't set the span on the scope, and `_end()` won't restore the previous span, potentially leaving the scope's span in an inconsistent state.