feat(redis): Support streaming spans #6083
6 issues
Medium
Spans leak when Redis command throws exception - `sentry_sdk/integrations/redis/_sync_common.py:145-156`
In sentry_patched_execute_command, the db_span and cache_span are manually entered via __enter__() but if old_execute_command() raises an exception, the __exit__() calls on lines 152 and 156 are never reached. This causes spans to leak (not be properly closed), the error status to not be recorded on the span, and potentially corrupted scope state for StreamedSpan which tracks _previous_span_on_scope.
Test parameters `is_transaction`, `send_default_pii`, and `expected_first_ten` are unused in the streaming path - `tests/integrations/redis/test_redis.py:72-91`
The test_redis_pipeline function is parametrized with is_transaction, send_default_pii, and expected_first_ten, but when span_streaming=True, these parameters have no effect on the assertions. The streaming branch only asserts on span names, op, origin, and db.system.name, without verifying that different transaction/pii/command formatting behaviors work correctly. This means the streaming path runs multiple parameter combinations but doesn't actually test differentiated behavior, creating a false sense of test coverage.
Also found at:
sentry_sdk/integrations/redis/modules/queries.py:54
Assertion checks wrong span index for cache.hit absence - `tests/integrations/redis/test_redis_cache_module_async.py:293`
Line 293 checks spans[1]["data"] for the absence of cache.hit, but the surrounding context is asserting properties of spans[2] (the cache.put span). This appears to be a copy-paste error where the index should be spans[2]["data"] to check that the cache.put operation doesn't have a cache.hit attribute. The test will pass incorrectly if spans[1] (db.redis) happens to not have cache.hit, even if spans[2] has it erroneously.
Also found at:
tests/integrations/redis/test_redis_cache_module.py:330
Spans are never closed when Redis command raises an exception - `sentry_sdk/integrations/redis/_sync_common.py:145-150`
The code manually enters spans using __enter__() (lines 126, 145) and exits them using __exit__(None, None, None) (lines 152, 156), but if old_execute_command() (line 150) raises an exception, the __exit__ calls are never reached. This causes spans to remain open/unfinished, corrupts the scope's current span, and fails to record the error status on the spans.
Test assertion expects wrong value for db.system.name attribute - `tests/integrations/redis/cluster/test_redis_cluster.py:100`
The test asserts attrs["db.system.name"] == "redis-py" but the implementation in sentry_sdk/integrations/redis/modules/queries.py:54 sets span.set_attribute("db.system.name", "redis"). This test will fail at runtime. The value "redis-py" is actually used for DB_DRIVER_NAME, not db.system.name.
Assertion checks wrong span index for cache.hit attribute - `tests/integrations/redis/test_redis_cache_module.py:330`
Line 330 checks spans[1]["data"] for absence of cache.hit, but spans[1] is a db.redis span (confirmed at line 323). The assertion should check spans[2] which is the cache.put span (confirmed at line 325). This causes the test to always pass since db.redis spans never have cache.hit, masking potential bugs where cache.put spans incorrectly include this attribute.
Also found at:
tests/integrations/redis/test_redis_cache_module_async.py:293
4 skills analyzed
| Skill | Findings | Duration | Cost |
|---|---|---|---|
| code-review | 3 | 4m 16s | $3.40 |
| find-bugs | 3 | 10m 9s | $6.12 |
| skill-scanner | 0 | 12m 8s | $1.59 |
| security-review | 0 | 4m 8s | $1.66 |
Duration: 30m 40s · Tokens: 8.4M in / 96.2k out · Cost: $12.79 (+merge: $0.00, +fix_gate: $0.01, +dedup: $0.01, +extraction: $0.00)