Skip to content

add comment

47f5a6b
Select commit
Loading
Failed to load commit list.
Merged

feat(sqlalchemy): Support span streaming #6132

add comment
47f5a6b
Select commit
Loading
Failed to load commit list.
@sentry/warden / warden: code-review completed Apr 28, 2026 in 1m 45s

3 issues

code-review: Found 3 issues (1 medium, 2 low)

Medium

Streaming branch drops db.params/db.paramstyle/db.executemany/db.cursor span data - `sentry_sdk/tracing_utils.py:213-220`

In the has_span_streaming_enabled branch, the data dict is built (lines 198-207) and used for the breadcrumb, but is never attached to the span — only sentry.origin and sentry.op are passed as attributes. The non-streaming branch attaches all of data via span.set_data(k, v). As a result, when span streaming is enabled, SQL spans will lose db.params, db.paramstyle, db.executemany, and db.cursor attributes, which is a behavioral/backwards-compatibility regression in telemetry data.

Low

Type annotation for `span` in `_handle_error` excludes `StreamedSpan` but code branches on it - `sentry_sdk/integrations/sqlalchemy.py:107-113`

On line 107, span is annotated as Optional[Span], yet the new branch on line 110 calls isinstance(span, StreamedSpan). Because _before_cursor_execute now stores either a Span or StreamedSpan in context._sentry_sql_span, the annotation is no longer accurate and is inconsistent with _after_cursor_execute (line 86), which correctly types the variable as Optional[Union[Span, StreamedSpan]]. This is only a type-hint inaccuracy — runtime behavior is correct — but it can mislead readers and static type checkers.

`` fallback is unreachable because _format_sql never returns None - `sentry_sdk/tracing_utils.py:214`

The streaming branch uses name="<unknown SQL query>" if query is None else query, but query is reassigned just above via query = _format_sql(cursor, query). Per _format_sql (line 498), the return is real_sql or to_string(sql), which always returns a string (never None). The fallback name therefore can never trigger, so spans generated from a None original description will be named "None" (from to_string(None)) instead of <unknown SQL query>, defeating the stated PR goal.


Duration: 1m 42s · Tokens: 428.8k in / 4.4k out · Cost: $2.43 (+merge: $0.00)

Annotations

Check warning on line 220 in sentry_sdk/tracing_utils.py

See this annotation in the file changed.

@sentry-warden sentry-warden / warden: code-review

Streaming branch drops db.params/db.paramstyle/db.executemany/db.cursor span data

In the `has_span_streaming_enabled` branch, the `data` dict is built (lines 198-207) and used for the breadcrumb, but is never attached to the span — only `sentry.origin` and `sentry.op` are passed as attributes. The non-streaming branch attaches all of `data` via `span.set_data(k, v)`. As a result, when span streaming is enabled, SQL spans will lose `db.params`, `db.paramstyle`, `db.executemany`, and `db.cursor` attributes, which is a behavioral/backwards-compatibility regression in telemetry data.