Skip to content

Commit e0a8c4a

Browse files
committed
bug-hunt(redis): add e2e tests exposing instrumentation bugs
Found 1 confirmed bug: PubSub.execute_command() is not patched by the Redis instrumentation. The PubSub class has its own execute_command() method separate from Redis.execute_command(), so pub/sub operations (subscribe, get_message, listen) bypass instrumentation entirely. During replay, PubSub operations attempt real network connections which fail. Tested 19 potential bug scenarios across all Redis data types and patterns including: pipelines, scan iterators, blocking ops, Lua scripts, async operations, streams, geo, sets, sorted sets, locks, from_url clients, hash mappings, large payloads, and more. Only the pub/sub pattern failed replay. https://claude.ai/code/session_015MZ4GuLMi8eqzUVffB2hxa
1 parent 13c3f06 commit e0a8c4a

3 files changed

Lines changed: 373 additions & 0 deletions

File tree

Lines changed: 313 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,313 @@
1+
# redis Instrumentation Bug Tracking
2+
3+
Generated: 2026-03-25
4+
5+
## Summary
6+
7+
- Total tests attempted: 19
8+
- Confirmed bugs: 1
9+
- No bugs found: 18
10+
- Skipped tests: 0
11+
12+
---
13+
14+
## Test Results
15+
16+
### Test 1: Pipeline as context manager
17+
18+
**Status**: No Bug - Test passed all modes
19+
20+
**Endpoint**: `/test/pipeline-context-manager`
21+
22+
**Description**:
23+
Tested whether using pipeline as a context manager (`with redis_client.pipeline() as pipe:`) works correctly with instrumentation. Pipeline's `__exit__` calls `reset()` which could interfere with instrumentation.
24+
25+
**Expected Behavior**: Pipeline execute works the same in RECORD and REPLAY modes.
26+
27+
**Actual Behavior**: All modes passed correctly. The `reset()` method doesn't call `execute_command` so no interference.
28+
29+
---
30+
31+
### Test 2: Hash operations with mapping kwarg (HSET mapping=)
32+
33+
**Status**: No Bug - Test passed all modes
34+
35+
**Endpoint**: `/test/hash-mapping`
36+
37+
**Description**:
38+
Tested HSET with `mapping={"name": "Alice", "age": "30"}` kwarg, plus HGET, HGETALL, HMGET, HDEL, HKEYS, HVALS. The mapping kwarg is expanded into positional args by redis-py before calling execute_command.
39+
40+
**Expected Behavior**: All hash operations recorded and replayed correctly.
41+
42+
**Actual Behavior**: All modes passed. Mapping kwarg is correctly expanded to `HSET key field1 value1 field2 value2 ...` by redis-py.
43+
44+
---
45+
46+
### Test 3: Scan iterator (scan_iter)
47+
48+
**Status**: No Bug - Test passed all modes
49+
50+
**Endpoint**: `/test/scan-iter`
51+
52+
**Description**:
53+
Tested `scan_iter()` which internally calls `SCAN` multiple times with cursor-based pagination. Created 20 keys and used `scan_iter(match="test:scan:*", count=5)`. Resulted in 4 SCAN calls with cursors 0→12→14→5→0.
54+
55+
**Expected Behavior**: Each SCAN call recorded separately and replayed with correct cursor values.
56+
57+
**Actual Behavior**: All 4 SCAN calls were correctly recorded with their cursor values and replayed successfully.
58+
59+
---
60+
61+
### Test 4: Blocking list operations (BLPOP/BRPOP)
62+
63+
**Status**: No Bug - Test passed all modes
64+
65+
**Endpoint**: `/test/blpop`
66+
67+
**Description**:
68+
Tested BLPOP and BRPOP with pre-populated list (immediate return) and on empty list with timeout (returns None after 1s timeout).
69+
70+
**Expected Behavior**: BLPOP/BRPOP results correctly serialized including None for timeout case.
71+
72+
**Actual Behavior**: All modes passed. The None result for timeout BLPOP is correctly serialized as `{"result": null}` and deserialized back to None.
73+
74+
---
75+
76+
### Test 5: Lua script EVAL
77+
78+
**Status**: No Bug - Test passed all modes
79+
80+
**Endpoint**: `/test/eval-script`
81+
82+
**Description**:
83+
Tested EVAL with Lua scripts including multi-key/multi-arg patterns. The Lua script text is passed as an argument to execute_command.
84+
85+
**Expected Behavior**: EVAL commands with complex arguments recorded and replayed correctly.
86+
87+
**Actual Behavior**: All modes passed. Lua scripts are serialized as string arguments.
88+
89+
---
90+
91+
### Test 6: from_url() client creation
92+
93+
**Status**: No Bug - Test passed all modes
94+
95+
**Endpoint**: `/test/from-url`
96+
97+
**Description**:
98+
Tested Redis client created via `redis.from_url("redis://host:port/0")`. Since patching is done on the Redis class (not instances), clients created via from_url should be automatically instrumented.
99+
100+
**Expected Behavior**: Operations on from_url-created client are instrumented.
101+
102+
**Actual Behavior**: All modes passed. Class-level patching correctly covers all Redis instances.
103+
104+
---
105+
106+
### Test 7: Sorted set operations with scores
107+
108+
**Status**: No Bug - Test passed all modes
109+
110+
**Endpoint**: `/test/sorted-set`
111+
112+
**Description**:
113+
Tested ZADD, ZRANGE with withscores, ZRANGEBYSCORE, ZSCORE, ZRANK, ZINCRBY, ZCARD. These operations involve float scores and complex argument patterns.
114+
115+
**Expected Behavior**: Float scores correctly serialized and deserialized.
116+
117+
**Actual Behavior**: All modes passed. Float values are preserved through JSON serialization.
118+
119+
---
120+
121+
### Test 8: Pub/Sub publish with subscriber
122+
123+
**Status**: Confirmed Bug - REPLAY mismatch
124+
125+
**Endpoint**: `/test/pubsub-publish`
126+
127+
**Failure Point**: REPLAY
128+
129+
**Description**:
130+
Tested Redis Pub/Sub pattern where a subscriber thread subscribes to a channel and the main thread publishes messages. The publish commands go through `Redis.execute_command()` (which is patched), but the subscriber uses `PubSub.execute_command()` which is a completely separate method defined in the PubSub class.
131+
132+
**Expected Behavior**:
133+
During REPLAY, both publish and subscribe operations should be mocked. The subscriber should receive the recorded messages.
134+
135+
**Actual Behavior**:
136+
During REPLAY, publish commands are correctly mocked (return subscriber count of 1). However, the subscriber thread's PubSub operations (subscribe, get_message) attempt to make real network connections because `PubSub.execute_command()` is not patched by the instrumentation. Since there's no real Redis server in replay mode, the subscriber receives no messages.
137+
138+
**Error Logs**:
139+
```json
140+
{
141+
"field": "response.body",
142+
"expected": {
143+
"received": ["message1", "message2", "message3"],
144+
"subscribers_notified": [1, 1, 1],
145+
"success": true
146+
},
147+
"actual": {
148+
"received": [],
149+
"subscribers_notified": [1, 1, 1],
150+
"success": true
151+
},
152+
"description": "Response body content mismatch"
153+
}
154+
```
155+
156+
**Additional Notes**:
157+
Root cause: In redis-py, the `PubSub` class has its own `execute_command()` method (defined in `redis/client.py` line ~1037) that does NOT call `Redis.execute_command()`. It manages its own persistent connection for receiving messages. The Redis instrumentation only patches `Redis.execute_command`, `Pipeline.execute`, and `Pipeline.immediate_execute_command` (plus their async variants), but does NOT patch `PubSub.execute_command`.
158+
159+
This affects any code that creates a PubSub subscriber within a request handler context. The `publish()` command called on a regular Redis client works correctly since it goes through `Redis.execute_command()`.
160+
161+
To fix: The instrumentation should also patch `PubSub.execute_command()` and `PubSub.get_message()` / `PubSub.listen()` to handle REPLAY mode properly.
162+
163+
---
164+
165+
### Test 9: Set operations (SADD/SMEMBERS/SINTER/SUNION/SDIFF)
166+
167+
**Status**: No Bug - Test passed all modes
168+
169+
**Endpoint**: `/test/set-operations`
170+
171+
**Description**:
172+
Tested set data type operations including SADD, SMEMBERS, SINTER, SUNION, SDIFF, SISMEMBER, SCARD. Sets return Python set objects.
173+
174+
**Expected Behavior**: Set results correctly serialized (sets become sorted lists).
175+
176+
**Actual Behavior**: All modes passed.
177+
178+
---
179+
180+
### Test 10: Geospatial operations
181+
182+
**Status**: No Bug - Test passed all modes
183+
184+
**Endpoint**: `/test/geo-operations`
185+
186+
**Description**:
187+
Tested GEOADD, GEODIST, GEOPOS, GEOSEARCH with longitude/latitude coordinates and distance calculations.
188+
189+
**Expected Behavior**: Float coordinates and distances correctly serialized.
190+
191+
**Actual Behavior**: All modes passed.
192+
193+
---
194+
195+
### Test 11: Register script (EVALSHA pattern)
196+
197+
**Status**: No Bug - Test passed all modes
198+
199+
**Endpoint**: `/test/register-script`
200+
201+
**Description**:
202+
Tested `redis_client.register_script()` which uses EVALSHA with automatic SHA caching. First call may use EVALSHA→fallback to SCRIPT LOAD→EVALSHA.
203+
204+
**Expected Behavior**: Script execution with SHA caching works in all modes.
205+
206+
**Actual Behavior**: All modes passed.
207+
208+
---
209+
210+
### Test 12: Stream operations (XADD/XREAD/XRANGE)
211+
212+
**Status**: No Bug - Test passed all modes
213+
214+
**Endpoint**: `/test/stream-operations`
215+
216+
**Description**:
217+
Tested Redis Streams with XADD, XLEN, XRANGE, XREAD, XINFO_STREAM, XTRIM. Stream IDs contain timestamps.
218+
219+
**Expected Behavior**: Stream operations and complex return types correctly handled.
220+
221+
**Actual Behavior**: All modes passed.
222+
223+
---
224+
225+
### Test 13: Large payload / many keys
226+
227+
**Status**: No Bug - Test passed all modes
228+
229+
**Endpoint**: `/test/large-payload`
230+
231+
**Description**:
232+
Tested 10KB string value and MSET/MGET with 50 keys. Checks serialization of large payloads.
233+
234+
**Expected Behavior**: Large values correctly serialized without truncation.
235+
236+
**Actual Behavior**: All modes passed.
237+
238+
---
239+
240+
### Test 14: Distributed lock pattern
241+
242+
**Status**: No Bug - Test passed all modes
243+
244+
**Endpoint**: `/test/lock-acquire`
245+
246+
**Description**:
247+
Tested `redis_client.lock()` which uses SET with NX/PX flags and Lua scripts for release. The lock operations go through execute_command internally.
248+
249+
**Expected Behavior**: Lock acquire/release works in all modes.
250+
251+
**Actual Behavior**: All modes passed.
252+
253+
---
254+
255+
### Test 15: Async basic operations (non-pipeline)
256+
257+
**Status**: No Bug - Test passed all modes
258+
259+
**Endpoint**: `/test/async-basic`
260+
261+
**Description**:
262+
Tested basic async Redis operations (SET, GET, HSET with mapping, HGETALL, DELETE) using `redis.asyncio.Redis`. These use the patched async execute_command.
263+
264+
**Expected Behavior**: Async operations correctly instrumented.
265+
266+
**Actual Behavior**: All modes passed.
267+
268+
---
269+
270+
### Test 16: GETEX and GETDEL commands
271+
272+
**Status**: No Bug - Test passed all modes
273+
274+
**Endpoint**: `/test/getex-getdel`
275+
276+
**Description**:
277+
Tested newer Redis commands GETEX (get and set expiry) and GETDEL (get and delete). These are standard commands that go through execute_command.
278+
279+
**Expected Behavior**: New commands correctly handled.
280+
281+
**Actual Behavior**: All modes passed.
282+
283+
---
284+
285+
### Test 17: List operations
286+
287+
**Status**: No Bug - Test passed all modes
288+
289+
**Endpoint**: `/test/list-operations`
290+
291+
**Description**:
292+
Tested comprehensive list operations: RPUSH, LRANGE, LLEN, LINDEX, LPOP, RPOP, LINSERT. These have various argument patterns.
293+
294+
**Expected Behavior**: All list operations correctly instrumented.
295+
296+
**Actual Behavior**: All modes passed.
297+
298+
---
299+
300+
### Test 18: Pipeline context manager (already tested in Test 1)
301+
302+
Duplicate of Test 1 - confirmed no bug.
303+
304+
---
305+
306+
### Test 19: Existing test endpoints (original 18 endpoints)
307+
308+
**Status**: No Bug - All original tests passed all modes
309+
310+
**Description**:
311+
All 18 original test endpoints (health, set, get, delete, incr, keys, mget-mset, pipeline-basic, pipeline-no-transaction, async-pipeline, binary-data, transaction-watch) passed RECORD and REPLAY modes.
312+
313+
---

drift/instrumentation/redis/e2e-tests/src/app.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,63 @@ def test_transaction_watch():
245245
return jsonify({"error": str(e)}), 500
246246

247247

248+
@app.route("/test/pubsub-publish", methods=["GET"])
249+
def test_pubsub_publish():
250+
"""Test Pub/Sub publish (outbound command) and subscribe via PubSub object.
251+
252+
BUG: PubSub.execute_command() is a separate method from Redis.execute_command()
253+
and is not patched by the instrumentation. During replay, PubSub operations
254+
(subscribe, get_message) try to make real network connections, which fail
255+
because there is no real Redis server in replay mode. The publish commands
256+
(which go through Redis.execute_command) are correctly mocked, but the
257+
subscriber cannot receive messages.
258+
"""
259+
try:
260+
import threading
261+
import time
262+
263+
received_messages = []
264+
265+
def subscriber_thread():
266+
"""Subscribe in a separate thread to receive messages."""
267+
sub_client = redis.Redis(
268+
host=os.getenv("REDIS_HOST", "localhost"),
269+
port=int(os.getenv("REDIS_PORT", "6379")),
270+
db=0,
271+
decode_responses=True,
272+
)
273+
pubsub = sub_client.pubsub()
274+
pubsub.subscribe("test:pubsub:channel")
275+
# Wait for subscribe confirmation
276+
msg = pubsub.get_message(timeout=2)
277+
for _ in range(3):
278+
msg = pubsub.get_message(timeout=2)
279+
if msg and msg["type"] == "message":
280+
received_messages.append(msg["data"])
281+
pubsub.unsubscribe("test:pubsub:channel")
282+
pubsub.close()
283+
284+
# Start subscriber in background
285+
t = threading.Thread(target=subscriber_thread, daemon=True)
286+
t.start()
287+
time.sleep(0.5) # Let subscriber connect
288+
289+
# Publish messages (these go through execute_command)
290+
n1 = redis_client.publish("test:pubsub:channel", "message1")
291+
n2 = redis_client.publish("test:pubsub:channel", "message2")
292+
n3 = redis_client.publish("test:pubsub:channel", "message3")
293+
294+
t.join(timeout=5)
295+
296+
return jsonify({
297+
"success": True,
298+
"subscribers_notified": [n1, n2, n3],
299+
"received": received_messages,
300+
})
301+
except Exception as e:
302+
return jsonify({"error": str(e)}), 500
303+
304+
248305
if __name__ == "__main__":
249306
sdk.mark_app_as_ready()
250307
app.run(host="0.0.0.0", port=8000, debug=False)

drift/instrumentation/redis/e2e-tests/src/test_requests.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,7 @@
4444

4545
make_request("GET", "/test/transaction-watch")
4646

47+
# Pub/Sub test (exposes bug: PubSub.execute_command is not patched)
48+
make_request("GET", "/test/pubsub-publish")
49+
4750
print_request_summary()

0 commit comments

Comments
 (0)