Skip to content

test(redis): Run tracing tests in both span streaming and legacy modes

2cab0f0
Select commit
Loading
Failed to load commit list.
Merged

feat(redis): Support streaming spans #6083

test(redis): Run tracing tests in both span streaming and legacy modes
2cab0f0
Select commit
Loading
Failed to load commit list.
@sentry/warden / warden: code-review completed Apr 22, 2026 in 3m 57s

3 issues

code-review: Found 3 issues (3 medium)

Medium

Spans not properly closed if Redis command raises exception - `sentry_sdk/integrations/redis/_async_common.py:145-156`

In _sentry_execute_command, when old_execute_command raises an exception, db_span.__exit__() (line 152) and cache_span.__exit__() (line 156) will never be called because they are executed after the await statement without exception handling. This causes span leaks and corrupted span trees where spans remain open indefinitely. The same pattern exists in the sync version.

Also found at:

  • sentry_sdk/integrations/redis/utils.py:119-125
Span streaming test for pipeline misses redis.commands assertions - `tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py:160-179`

In test_async_redis_pipeline, when span_streaming=True, the test does not verify the redis.commands data (count and first_ten commands). The expected_first_ten parameter from the parametrize decorator is unused in the streaming branch. This creates a test coverage gap where the pipeline command details are not validated for streaming spans.

Also found at:

  • tests/integrations/redis/test_redis.py:72-90
  • tests/integrations/redis/cluster/test_redis_cluster.py:152-153
Test checks wrong span index for cache.hit assertion - `tests/integrations/redis/test_redis_cache_module_async.py:293`

Line 293 checks assert "cache.hit" not in spans[1]["data"] but spans[1] is the db.redis span (as stated in line 286), not the cache.put span. The assertion should check spans[2]["data"] since that's the cache.put span being validated in the surrounding assertions (lines 288-304). This test will pass but doesn't actually verify the intended behavior - that cache.put spans don't have a cache.hit attribute.

Also found at:

  • tests/integrations/redis/test_redis_cache_module.py:330

Duration: 3m 50s · Tokens: 2.6M in / 32.0k out · Cost: $4.20 (+merge: $0.00, +fix_gate: $0.00)

Annotations

Check warning on line 156 in sentry_sdk/integrations/redis/_async_common.py

See this annotation in the file changed.

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

Spans not properly closed if Redis command raises exception

In `_sentry_execute_command`, when `old_execute_command` raises an exception, `db_span.__exit__()` (line 152) and `cache_span.__exit__()` (line 156) will never be called because they are executed after the await statement without exception handling. This causes span leaks and corrupted span trees where spans remain open indefinitely. The same pattern exists in the sync version.

Check warning on line 125 in sentry_sdk/integrations/redis/utils.py

See this annotation in the file changed.

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

[AR8-4BE] Spans not properly closed if Redis command raises exception (additional location)

In `_sentry_execute_command`, when `old_execute_command` raises an exception, `db_span.__exit__()` (line 152) and `cache_span.__exit__()` (line 156) will never be called because they are executed after the await statement without exception handling. This causes span leaks and corrupted span trees where spans remain open indefinitely. The same pattern exists in the sync version.

Check warning on line 179 in tests/integrations/redis/cluster_asyncio/test_redis_cluster_asyncio.py

See this annotation in the file changed.

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

Span streaming test for pipeline misses redis.commands assertions

In `test_async_redis_pipeline`, when `span_streaming=True`, the test does not verify the `redis.commands` data (count and first_ten commands). The `expected_first_ten` parameter from the parametrize decorator is unused in the streaming branch. This creates a test coverage gap where the pipeline command details are not validated for streaming spans.

Check warning on line 90 in tests/integrations/redis/test_redis.py

See this annotation in the file changed.

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

[XLD-33B] Span streaming test for pipeline misses redis.commands assertions (additional location)

In `test_async_redis_pipeline`, when `span_streaming=True`, the test does not verify the `redis.commands` data (count and first_ten commands). The `expected_first_ten` parameter from the parametrize decorator is unused in the streaming branch. This creates a test coverage gap where the pipeline command details are not validated for streaming spans.

Check warning on line 153 in tests/integrations/redis/cluster/test_redis_cluster.py

See this annotation in the file changed.

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

[XLD-33B] Span streaming test for pipeline misses redis.commands assertions (additional location)

In `test_async_redis_pipeline`, when `span_streaming=True`, the test does not verify the `redis.commands` data (count and first_ten commands). The `expected_first_ten` parameter from the parametrize decorator is unused in the streaming branch. This creates a test coverage gap where the pipeline command details are not validated for streaming spans.

Check warning on line 293 in tests/integrations/redis/test_redis_cache_module_async.py

See this annotation in the file changed.

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

Test checks wrong span index for cache.hit assertion

Line 293 checks `assert "cache.hit" not in spans[1]["data"]` but `spans[1]` is the `db.redis` span (as stated in line 286), not the `cache.put` span. The assertion should check `spans[2]["data"]` since that's the cache.put span being validated in the surrounding assertions (lines 288-304). This test will pass but doesn't actually verify the intended behavior - that cache.put spans don't have a cache.hit attribute.

Check warning on line 330 in tests/integrations/redis/test_redis_cache_module.py

See this annotation in the file changed.

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

[XLS-Y8L] Test checks wrong span index for cache.hit assertion (additional location)

Line 293 checks `assert "cache.hit" not in spans[1]["data"]` but `spans[1]` is the `db.redis` span (as stated in line 286), not the `cache.put` span. The assertion should check `spans[2]["data"]` since that's the cache.put span being validated in the surrounding assertions (lines 288-304). This test will pass but doesn't actually verify the intended behavior - that cache.put spans don't have a cache.hit attribute.