feat(redis): Support streaming spans #6083
5 issues
Medium
Test assertion checks wrong span index - `tests/integrations/redis/test_redis_cache_module_async.py:293`
Line 293 asserts "cache.hit" not in spans[1]["data"] but spans[1] is a db.redis span (as confirmed by line 286), not the cache.put span. The assertion should check spans[2] to properly verify that cache.put operations don't have a cache.hit attribute. This bug makes the test less effective since it's not validating the intended behavior.
Also found at:
tests/integrations/redis/test_redis_cache_module.py:330
Spans not properly closed on exception in _sentry_execute_command - `sentry_sdk/integrations/redis/_async_common.py:126-156`
In _sentry_execute_command, spans are opened via manual __enter__() calls (lines 126, 145) but their corresponding __exit__() calls (lines 152, 156) are not wrapped in try/finally. If old_execute_command raises an exception at line 150, both db_span and cache_span will never be closed, resulting in spans not being sent to Sentry, error status not being recorded, and the scope's span reference not being restored.
Also found at:
sentry_sdk/integrations/redis/_sync_common.py:145-147
Test assertion checks wrong span index - verifies db.redis instead of cache.put - `tests/integrations/redis/test_redis_cache_module.py:330`
Line 330 asserts "cache.hit" not in spans[1]["data"] but spans[1] is the db.redis span (confirmed by line 323), not the cache.put span at spans[2]. The surrounding code context (lines 325-331) is testing the cache.put span properties, but this assertion mistakenly verifies the db.redis span. This causes the test to not actually verify that cache.put spans lack the cache.hit attribute.
Also found at:
tests/integrations/redis/test_redis_cache_module_async.py:293
Low
Unnecessary computation of commands list for StreamedSpan - `sentry_sdk/integrations/redis/utils.py:119-125`
The commands list is built by iterating over commands_seq (up to _MAX_NUM_COMMANDS items) and calling get_command_args_fn and _get_safe_command on each. However, when span is a StreamedSpan, this list is never used since the span.set_data call is skipped. This causes unnecessary CPU cycles for every Redis pipeline execution in streaming mode.
Test parameter `expected_first_ten` unused in span_streaming path creates redundant test runs - `tests/integrations/redis/cluster/test_redis_cluster.py:146-155`
The expected_first_ten parameter is only used in the non-streaming branch (lines 205-210), but the test is parameterized with @pytest.mark.parametrize("span_streaming", [True, False]) combined with the expected_first_ten parametrization. This creates 4 test cases when only 3 are meaningful (2 for static + 1 for streaming), resulting in redundant test execution for span_streaming=True.
4 skills analyzed
| Skill | Findings | Duration | Cost |
|---|---|---|---|
| code-review | 3 | 8m 55s | $4.00 |
| find-bugs | 2 | 4m 15s | $6.75 |
| skill-scanner | 0 | 9m 26s | $1.00 |
| security-review | 0 | 5m 10s | $1.10 |
Duration: 27m 45s · Tokens: 9.1M in / 99.5k out · Cost: $12.89 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.01, +dedup: $0.01)