Conversation
…ync (LAB-3755)
The sync wrapper's miss-store record passes serializer="rust" and hit=False.
The two async miss-store sites (locked path and no-lock fallback) passed
neither, so the collector took its serializer default and Prometheus filed
every async set under cache_operations_total{serializer="unknown"} while sync
sets landed under "rust". Same operation, two label values, split series.
Both async sites now pass the same two keywords as the sync site. A unit test
drives an async miss through both paths via the real decorator stack (a
backend with acquire_lock selects the locked path) and asserts the record the
wrapper emits.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour. WalkthroughChangesAsync cache-set metric labels
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The async cache-set metric-label correction is covered for both execution paths and presents no current merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![kodus-27b[bot]](https://avatars.githubusercontent.com/u/223823690?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
This PR fixes a metrics labeling inconsistency between the async and sync cache decorator wrappers when recording cache miss-store operations (LAB-3755).
Problem
The sync wrapper records its miss-store operation with
serializer="rust"andhit=Falselabels. However, the async wrapper's two miss-store sites (the distributed-lock path and the no-lock fallback path) were recording thesetoperation without these labels. As a result, the Prometheus sink filed async set operations underserializer="unknown"with no hit marker, producing inconsistent metrics between sync and async code paths.Changes
src/cachekit/decorators/wrapper.py: Added theserializer="rust"andhit=Falselabels to both async miss-store recording sites (locked and no-lock fallback), bringing them into parity with the sync wrapper.tests/unit/test_async_set_record_labels.py: Added a new test that pins both async miss-store paths through the real decorator stack. It uses two byte-store backends:_ByteStore(noacquire_lock) to drive the no-lock miss path_LockableByteStore(withacquire_lock) to drive the locked miss pathThe test verifies that after a cache miss reaches L2, exactly one
setrecord is produced carryingserializer="rust"andhit=False, matching the sync behavior.Impact
Async cache set operations will now be correctly labeled in metrics/telemetry, ensuring consistent observability across sync and async cache usage.