feat(redis): Support streaming spans #6083
4 issues
find-bugs: Found 4 issues (1 high, 3 medium)
High
Spans leak and miss error status when Redis commands raise exceptions - `sentry_sdk/integrations/redis/_sync_common.py:145-158`
The sentry_patched_execute_command function manually calls __enter__() and __exit__(None, None, None) on spans without try/finally protection. When old_execute_command() raises an exception (e.g., connection error, timeout), the spans are never closed because __exit__() is skipped. Additionally, even on normal exit, __exit__ is called with (None, None, None) instead of exception info, preventing the span's error status from being set. This results in resource leaks (unclosed spans) and missing error tracking when Redis operations fail.
Also found at:
sentry_sdk/integrations/redis/_async_common.py:147-148
Medium
Pipeline command data is computed but never set for StreamedSpan - `sentry_sdk/integrations/redis/utils.py:127-134`
In _set_pipeline_data, the commands list is built by iterating over commands_seq (lines 119-125), but for StreamedSpan instances, this data is never set on the span. The check at line 127 if not isinstance(span, StreamedSpan) causes the redis.commands data to be skipped entirely. This results in wasted computation and missing telemetry data for streaming spans.
Also found at:
sentry_sdk/integrations/redis/utils.py:115-117
Test doesn't verify PII filtering for streaming spans - `tests/integrations/redis/test_redis.py:72-90`
The test_redis_pipeline test parametrizes send_default_pii and expected_first_ten but the streaming branch (lines 72-90) completely ignores these parameters. When span_streaming=True, the test only verifies the span name and op, but doesn't check whether sensitive data (like the values in SET commands) is properly filtered based on the send_default_pii setting. This means the PII filtering behavior could be broken in streaming mode without any test failure.
Test assertion checks wrong span index, always passes even if cache.put spans have cache.hit - `tests/integrations/redis/test_redis_cache_module.py:330`
Line 330 asserts "cache.hit" not in spans[1]["data"] but spans[1] is a db.redis span, not the cache.put span being tested. The surrounding assertions (lines 325-341) all check spans[2], which is the cache.put span. This test will always pass because db.redis spans never have cache.hit, making it unable to catch regressions where cache.put spans incorrectly include cache.hit.
Also found at:
tests/integrations/redis/test_redis_cache_module_async.py:293
Duration: 7m 40s · Tokens: 4.2M in / 44.7k out · Cost: $6.82 (+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: find-bugs
Spans leak and miss error status when Redis commands raise exceptions
The `sentry_patched_execute_command` function manually calls `__enter__()` and `__exit__(None, None, None)` on spans without try/finally protection. When `old_execute_command()` raises an exception (e.g., connection error, timeout), the spans are never closed because `__exit__()` is skipped. Additionally, even on normal exit, `__exit__` is called with `(None, None, None)` instead of exception info, preventing the span's error status from being set. This results in resource leaks (unclosed spans) and missing error tracking when Redis operations fail.
Check failure on line 148 in sentry_sdk/integrations/redis/_async_common.py
sentry-warden / warden: find-bugs
[6LE-2VT] Spans leak and miss error status when Redis commands raise exceptions (additional location)
The `sentry_patched_execute_command` function manually calls `__enter__()` and `__exit__(None, None, None)` on spans without try/finally protection. When `old_execute_command()` raises an exception (e.g., connection error, timeout), the spans are never closed because `__exit__()` is skipped. Additionally, even on normal exit, `__exit__` is called with `(None, None, None)` instead of exception info, preventing the span's error status from being set. This results in resource leaks (unclosed spans) and missing error tracking when Redis operations fail.
Check warning on line 134 in sentry_sdk/integrations/redis/utils.py
sentry-warden / warden: find-bugs
Pipeline command data is computed but never set for StreamedSpan
In `_set_pipeline_data`, the `commands` list is built by iterating over `commands_seq` (lines 119-125), but for `StreamedSpan` instances, this data is never set on the span. The check at line 127 `if not isinstance(span, StreamedSpan)` causes the `redis.commands` data to be skipped entirely. This results in wasted computation and missing telemetry data for streaming spans.
Check warning on line 117 in sentry_sdk/integrations/redis/utils.py
sentry-warden / warden: find-bugs
[8V5-4BW] Pipeline command data is computed but never set for StreamedSpan (additional location)
In `_set_pipeline_data`, the `commands` list is built by iterating over `commands_seq` (lines 119-125), but for `StreamedSpan` instances, this data is never set on the span. The check at line 127 `if not isinstance(span, StreamedSpan)` causes the `redis.commands` data to be skipped entirely. This results in wasted computation and missing telemetry data for streaming spans.
Check warning on line 90 in tests/integrations/redis/test_redis.py
sentry-warden / warden: find-bugs
Test doesn't verify PII filtering for streaming spans
The `test_redis_pipeline` test parametrizes `send_default_pii` and `expected_first_ten` but the streaming branch (lines 72-90) completely ignores these parameters. When `span_streaming=True`, the test only verifies the span name and op, but doesn't check whether sensitive data (like the values in SET commands) is properly filtered based on the `send_default_pii` setting. This means the PII filtering behavior could be broken in streaming mode without any test failure.
Check warning on line 330 in tests/integrations/redis/test_redis_cache_module.py
sentry-warden / warden: find-bugs
Test assertion checks wrong span index, always passes even if cache.put spans have cache.hit
Line 330 asserts `"cache.hit" not in spans[1]["data"]` but `spans[1]` is a `db.redis` span, not the `cache.put` span being tested. The surrounding assertions (lines 325-341) all check `spans[2]`, which is the cache.put span. This test will always pass because db.redis spans never have cache.hit, making it unable to catch regressions where cache.put spans incorrectly include cache.hit.
Check warning on line 293 in tests/integrations/redis/test_redis_cache_module_async.py
sentry-warden / warden: find-bugs
[86Q-QBF] Test assertion checks wrong span index, always passes even if cache.put spans have cache.hit (additional location)
Line 330 asserts `"cache.hit" not in spans[1]["data"]` but `spans[1]` is a `db.redis` span, not the `cache.put` span being tested. The surrounding assertions (lines 325-341) all check `spans[2]`, which is the cache.put span. This test will always pass because db.redis spans never have cache.hit, making it unable to catch regressions where cache.put spans incorrectly include cache.hit.