feat(sqlalchemy): Support span streaming #6132
2 issues
code-review: Found 2 issues (1 medium, 1 low)
Medium
Inconsistent handling of None query between streaming and non-streaming paths - `sentry_sdk/tracing_utils.py:224`
The streaming path (line 212) handles None query with a fallback to <unknown SQL query>, but the non-streaming path (line 224) passes query directly without the same check. This inconsistency means that if query is None, the streaming path will create a span named <unknown SQL query> while the non-streaming path will create a span with None as the name, leading to different behavior depending on the configuration.
Also found at:
tests/integrations/sqlalchemy/test_sqlalchemy.py:162tests/integrations/sqlalchemy/test_sqlalchemy.py:293
Low
Type annotation mismatch in _handle_error: declared as Optional[Span] but handles StreamedSpan - `sentry_sdk/integrations/sqlalchemy.py:107-113`
The type annotation on line 107 declares span as Optional[Span], but lines 110-111 use isinstance(span, StreamedSpan) to handle StreamedSpan objects. This inconsistency could confuse static analysis tools and developers maintaining the code. The annotation should be Optional[Union[Span, StreamedSpan]] to match the actual runtime behavior.
Also found at:
tests/conftest.py:479-507
Duration: 12m 32s · Tokens: 3.2M in / 42.0k out · Cost: $4.45 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.01)
Annotations
Check warning on line 224 in sentry_sdk/tracing_utils.py
sentry-warden / warden: code-review
Inconsistent handling of None query between streaming and non-streaming paths
The streaming path (line 212) handles `None` query with a fallback to `<unknown SQL query>`, but the non-streaming path (line 224) passes `query` directly without the same check. This inconsistency means that if `query` is `None`, the streaming path will create a span named `<unknown SQL query>` while the non-streaming path will create a span with `None` as the name, leading to different behavior depending on the configuration.
Check warning on line 162 in tests/integrations/sqlalchemy/test_sqlalchemy.py
sentry-warden / warden: code-review
[BEF-K5U] Inconsistent handling of None query between streaming and non-streaming paths (additional location)
The streaming path (line 212) handles `None` query with a fallback to `<unknown SQL query>`, but the non-streaming path (line 224) passes `query` directly without the same check. This inconsistency means that if `query` is `None`, the streaming path will create a span named `<unknown SQL query>` while the non-streaming path will create a span with `None` as the name, leading to different behavior depending on the configuration.
Check warning on line 293 in tests/integrations/sqlalchemy/test_sqlalchemy.py
sentry-warden / warden: code-review
[BEF-K5U] Inconsistent handling of None query between streaming and non-streaming paths (additional location)
The streaming path (line 212) handles `None` query with a fallback to `<unknown SQL query>`, but the non-streaming path (line 224) passes `query` directly without the same check. This inconsistency means that if `query` is `None`, the streaming path will create a span named `<unknown SQL query>` while the non-streaming path will create a span with `None` as the name, leading to different behavior depending on the configuration.