Skip to content

Commit 9b96fd2

Browse files
27Bslash6Mark S
andauthored
fix(decorators): async get hits record serializer/size/hit like the sync path (LAB-3765) (#297)
* fix(decorators): async get hits record serializer/size/hit like the sync path (LAB-3765) The async wrapper's L1-hit and L2-hit record_cache_operation calls omitted serializer, size_bytes and hit, so FeatureOrchestrator's serializer="unknown" default swallowed every async get hit. The sync path labels the same tiers l1_memory / rust with the served size and hit=True. Pass the same kwargs at both async sites; size_bytes for the L2 hit is the raw envelope the async handler already returns, so L1 and L2 hits on one entry report the same size. Test drives both tiers through the real decorator stack and captures at the orchestrator, not the collector, because record_success() also forwards an unlabelled record at these sites. * fix(decorators): drop unreachable size guard, name the sync L2 divergence, tighten test docstrings (LAB-3765) Panel trims: the L2 hit tuple is only built after deserialize_data succeeded on cached_data, so the else-0 branch could not run; the comment now says why this site sizes the envelope where the sync L2 site sizes str(value); the test docstring no longer calls the miss-store a backfill and bounds its claim to the L1 and uncontended L2 sites; the fixture docstring says what the override actually buys (the L1 clear the unit conftest dropped). * docs(metrics): drop the sync-site aside from the async L2 hit comment (LAB-3765) The sync L2 site measures the served envelope as of #298; this comment described the state it was written against and would read as a lie once both land. The remaining sentence is about this site only. * test(decorators): assert exact served size in async hit-record test (LAB-3765) The positive-only size_bytes assertion would pass on a wrong constant (e.g. size_bytes=1). Pin it to the exact served envelope length: both hit sites record the raw serialized envelope, and the L1 backfill stores the same bytes L2 returned, so the single L2 store value is ground truth for either tier. Addresses the CodeRabbit review nitpick. * fix: address coderabbit review — measure async L2 str envelopes in UTF-8 bytes The async L2 hit-record metric measured size_bytes with len(cached_data), which counts Unicode characters when the envelope is a str, under-reporting non-ASCII payloads. _l1_backfill_from_l2 already UTF-8-encodes str envelopes before storing, and the L1 site's len(l1_bytes) measures bytes; measure the same encoded envelope here so all three agree. CodeRabbit-Resolved: wrapper.py:1716:Measure string envelopes in byt --------- Co-authored-by: Mark S <ray@insighttimer.com>
1 parent 705640b commit 9b96fd2

2 files changed

Lines changed: 100 additions & 0 deletions

File tree

‎src/cachekit/decorators/wrapper.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1629,8 +1629,11 @@ async def async_wrapper(*args: Any, **kwargs: Any) -> Any:
16291629
features.record_cache_operation(
16301630
operation="get",
16311631
namespace=namespace or "default",
1632+
serializer="l1_memory",
16321633
success=True,
16331634
duration_ms=0.001, # Sub-microsecond
1635+
size_bytes=len(l1_bytes),
1636+
hit=True,
16341637
)
16351638

16361639
# Record L1 hit for cache_info()
@@ -1726,11 +1729,18 @@ async def async_wrapper(*args: Any, **kwargs: Any) -> Any:
17261729
features.record_success()
17271730

17281731
if features.collect_stats:
1732+
# size_bytes: the encoded envelope, matching the bytes _l1_backfill_from_l2
1733+
# stores and the L1 site's len(l1_bytes). A str envelope is UTF-8 encoded
1734+
# first, so non-ASCII payloads report byte length, not character count.
1735+
_l2_envelope = cached_data.encode("utf-8") if isinstance(cached_data, str) else cached_data
17291736
features.record_cache_operation(
17301737
operation="get",
17311738
namespace=namespace or "default",
1739+
serializer="rust",
17321740
success=True,
17331741
duration_ms=get_duration_ms,
1742+
size_bytes=len(_l2_envelope),
1743+
hit=True,
17341744
)
17351745

17361746
# Update L1 cache with the L2 value (serialized bytes) for subsequent
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
"""Async hit-record stats parity (LAB-3765).
2+
3+
The sync wrapper records an L1 hit as ``serializer="l1_memory", hit=True`` and an L2
4+
hit as ``serializer="rust", hit=True``, both with the served size; the async L1 and
5+
uncontended L2 hit sites recorded none of those, so async hits filed under
6+
``serializer="unknown"``. A miss primes L2 (and L1 via the miss-store); clearing L1
7+
before the second call forces it past L1 to the L2 site.
8+
"""
9+
10+
from __future__ import annotations
11+
12+
from collections.abc import Iterator
13+
from typing import Any
14+
15+
import pytest
16+
17+
from cachekit import cache
18+
from cachekit.decorators.orchestrator import FeatureOrchestrator
19+
from cachekit.l1_cache import get_l1_cache_manager
20+
21+
22+
class _ByteStore:
23+
"""Plain in-memory byte store standing in for L2."""
24+
25+
def __init__(self) -> None:
26+
self.store: dict[str, bytes] = {}
27+
28+
def get(self, key: str) -> bytes | None:
29+
return self.store.get(key)
30+
31+
def set(self, key: str, value: bytes, ttl: int | None = None) -> None:
32+
self.store[key] = value
33+
34+
def delete(self, key: str) -> bool:
35+
return self.store.pop(key, None) is not None
36+
37+
def exists(self, key: str) -> bool:
38+
return key in self.store
39+
40+
def health_check(self) -> tuple[bool, dict[str, Any]]:
41+
return True, {}
42+
43+
44+
@pytest.fixture(autouse=True)
45+
def setup_di_for_redis_isolation() -> Iterator[None]:
46+
"""L1 hygiene between the two cases: tests/unit/conftest.py's no-op override of the root
47+
fixture dropped its clear_all(), and a leaked entry would turn the L2 case's priming
48+
miss into an L1 hit."""
49+
yield
50+
get_l1_cache_manager().clear_all()
51+
52+
53+
@pytest.fixture
54+
def recorded(monkeypatch: pytest.MonkeyPatch) -> list[dict[str, Any]]:
55+
"""Capture the wrapper's explicit features.record_cache_operation(...) calls, keyword args as passed.
56+
57+
Patched at the orchestrator, not the collector: record_success() also forwards an
58+
operation-context record to the collector, which would shadow the labels under test.
59+
"""
60+
calls: list[dict[str, Any]] = []
61+
monkeypatch.setattr(FeatureOrchestrator, "record_cache_operation", lambda self, **kw: calls.append(kw))
62+
return calls
63+
64+
65+
@pytest.mark.unit
66+
@pytest.mark.parametrize("tier", ["l1_memory", "rust"], ids=["l1-hit", "l2-hit"])
67+
async def test_async_get_hit_records_sync_labels(recorded: list[dict[str, Any]], tier: str) -> None:
68+
backend = _ByteStore()
69+
70+
@cache(backend=backend, ttl=60, namespace="async-get-labels")
71+
async def compute() -> dict[str, int]:
72+
return {"answer": 42}
73+
74+
assert await compute() == {"answer": 42} # miss: primes L2, and L1 via the miss-store
75+
assert backend.store
76+
if tier == "rust":
77+
get_l1_cache_manager().clear_all() # force the hit past L1 to the L2 site
78+
recorded.clear()
79+
80+
assert await compute() == {"answer": 42}
81+
82+
gets = [c for c in recorded if c["operation"] == "get"]
83+
assert len(gets) == 1
84+
assert (gets[0].get("serializer"), gets[0].get("hit")) == (tier, True) # what the sync hit sites pass
85+
# Exact served size, not just positive: both hit sites record the raw serialized
86+
# envelope's length — the L1 backfill stores the same bytes L2 returned, so the
87+
# single L2 store value is the ground truth for either tier. A `> 0` assertion
88+
# would pass on a wrong constant (e.g. size_bytes=1); this pins the real value.
89+
expected_size = len(next(iter(backend.store.values())))
90+
assert gets[0].get("size_bytes") == expected_size

0 commit comments

Comments
 (0)