feat(metrics)!: classify cache misses by bounded reason - #130
Open
lan17 wants to merge 5 commits into
Open
Conversation
Add a reason label to the miss metric so watermark-fenced misses are separable from cold keys. Decoders and DialCacheRedisClient.read() now return a discriminated RedisReadOutcome instead of payload-or-null, with four bounded read-miss reasons (not_found, frame_unsupported, watermark_unreadable, watermark_invalidated) plus a metrics-level deserialization_failed for serializer.load failures. Local layers always emit not_found. A runtime outcome guard routes malformed client results through the existing fail-open cache_read error path so the label vocabulary stays bounded. BREAKING CHANGE: DialCacheRedisClient.read() and the exported decodeRedisFrame/decodeTrackedRedisFrame return RedisReadOutcome instead of RedisCachePayload | null, and dialcache_miss_counter gains a reason label, so an old-schema collector in the same Prometheus registry now fails adapter construction.
- Migrate all four benchmark fake clients to RedisReadOutcome shapes; the legacy payload/null returns tripped the new outcome guard, failing three scenarios on their own asserts and hanging the dark-fill one - Single-source the guard's reason set via satisfies Record<RedisReadMissReason, true> so a future reason cannot drift out of it, and pin that a client-supplied deserialization_failed is rejected as malformed - Pin frame-before-watermark precedence with competing-bad-state decoder assertions and a real-Redis mirror - Delegate FakeRedis read classification to the exported decoders on a copied frame instead of hand-mirroring the ladder - Fix the stale shadow-spec sentence still defining a clean miss as a null read; scope deserialization_failed to the remote layer; note that only the first fenced read per invalidation window labels watermark_invalidated; qualify not_found's wrong-type doc as tracked-MGET-only
Collapse the decoder singletons, the guard's exhaustive flags object, and its derived Set into a single REDIS_READ_MISS_OUTCOMES table: one frozen outcome per bounded reason, returned by the decoders and now also the normal form the core collapses every client miss onto. The read guard (renamed normalizeRedisReadOutcome) reads each client-owned property exactly once, recaptures hit payloads, and returns canonical singletons for misses, so accessor-backed or otherwise unstable client objects can never flip answers between validation and metric emission. Rejections now carry a bounded shape fingerprint naming the malformed class - legacy payload-or-null, non-object value, unknown status, unbounded or core-owned reason, payload-less hit - so a stale-client migration storm names its own cause. Also: unify the two identical started-read interfaces as StartedRead<T>; pin the accessor-flip behavior in both directions plus the toString prototype-chain forgery; add the missing negative type pin that deserialization_failed is not assignable to RedisReadMissReason.
The decoders and the read normalizer alias one frozen outcome per miss reason, so a mutable entry would let a single consumer corrupt every later miss and its bounded metric label process-wide; pin identity, frozenness, and mutation rejection. Also narrow not_found's doc to the tracked MGET value member - a wrong-type watermark member reads as nil and classifies watermark_unreadable, as the README and integration tests already state.
…ication Conflicts were the adjacent bounded-vocabulary blocks in the adapter tests (MISS_REASONS vs COMPRESSION_OUTCOMES / ERROR_KINDS with the new compression kind) - resolved by keeping both. Compression operates on serializer output inside the frame payload, one layer below the read outcome classification, so the two features compose without semantic changes; a rejected compression envelope surfaces as deserialization_failed alongside its fallback_raw/read_over_limit compression outcome, now cross-referenced in the miss-reasons table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cache misses were indistinguishable in metrics: the watermark fence decision in
decodeTrackedRedisFramecollapsed "key absent", "frame unsupported", "watermark missing/malformed", and "fenced by watermark" into onenull. Operators could not separate invalidation churn from cold keys.The miss metric now carries a bounded
reasonlabel (mirroring the existingdisabled{reason}precedent):not_found— key absent/expired (all layers; local layers always use this)frame_unsupported— short frame or unsupported frame versionwatermark_unreadable— tracked read with missing/malformed/non-finite watermarkwatermark_invalidated— tracked frame fenced (createdAt <= watermark)deserialization_failed— payload read butserializer.loadthrewsum by (use_case) (rate(dialcache_miss_counter{reason="watermark_invalidated"}[5m]))now measures invalidation churn directly, including the repeated stale-frame transfer cost during future-buffer windows.frame_unsupported/watermark_unreadableshould be ~zero in steady state and make good anomaly alerts.Design
DialCacheRedisClient.read()return a discriminatedRedisReadOutcome({status:"hit",payload}|{status:"miss",reason}). The decoder remains the single source of truth for classification; bundled adapters needed zero code changes.RedisCacherejects malformed client results (e.g. a stale client still returningpayload | null) into the existing fail-opencache_readerror path, so the label vocabulary cannot go unbounded.remote_shadow; shadow fill behavior is unchanged — a fenced dark read stays fill-eligible because the tracked write script re-fences server-side (pinned by a new test).CacheGetResulttypes unchanged: misses are recorded where discovered, no caller re-records.Breaking changes
DialCacheRedisClient.read()and exporteddecodeRedisFrame/decodeTrackedRedisFramereturnRedisReadOutcomeinstead ofRedisCachePayload | null. Custom clients delegating to the exported decoders need zero logic changes; plain-JS stragglers fail loudly open (typed error →cache_read→ fallback), never mislabel.dialcache_miss_countergains areasonlabel — an old-schema collector in the same Prometheus registry now fails adapter construction (validateExistingCollectors).sum without (reason)totals are unchanged. Datadog gains an additive tag (wire-compatible).miss(labels: CacheMetricLabels)still compile; widen toMissMetricLabelsto consume the reason.Verification
pnpm checkgreen: typecheck, 447 unit tests (coverage 97.5% lines / 94.7% branches, gates pass), build, packed-tarball consumer test (incl. exhaustiveMissReasonrecord and ESM/CJS decoder checks assertingwatermark_invalidated)pnpm test:integrationgreen: 113 tests against real Redis 6.2, Valkey 8, and clusterinvalidateRemote, malformed-outcome guard (5 shapes), fenced shadow-read fill parity, Prometheus/Datadog exhaustive-enum guards