feat(utils): make Stats event-driven and fully featured#1653
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the Stats class to support unified increment/decrement APIs, computed hit/miss rates, mutation/reset timestamps, and event subscription capabilities for automatic stat tracking from emitters like cacheable, node-cache, and cache-manager. Comprehensive tests have been added to cover these new features. The review feedback highlights two key improvements: updating the cacheManagerStatsEventMap to correctly handle multi-key operations (mget, mset, mdel) using custom handlers, and guarding the applyEvent method to prevent custom event handlers from executing when stats are disabled.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1653 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 27 27
Lines 2820 2939 +119
Branches 623 656 +33
==========================================
+ Hits 2820 2939 +119 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05620d1352
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Enhance the shared Stats class in @cacheable/utils so it can serve as the single statistics primitive across all cacheable libraries. - Event-driven: subscribe()/unsubscribe() attach to any duck-typed emitter (Hookified or Node EventEmitter) and update counters automatically, with built-in event maps for cacheable, node-cache, and cache-manager - Imperative: unified increment(field, amount)/decrement(field, amount) plus the existing named methods for consumers not using events - Computed hitRate/missRate (zero-division guarded) - toJSON()/snapshot() for serialization - lastUpdated/lastReset timestamps - enable()/disable()/clear() ergonomics Fully backward compatible: all existing getters and methods are preserved, so cacheable and node-cache are unaffected. 100% coverage on stats.ts. https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE
…disabled Address PR review feedback on the cache-manager stats event map: - mget/mset/mdel now count by batch size via custom handlers (mset by list length, mdel by keys length, mget by values length with per-value hit/miss derivation) instead of counting a whole batch as one - mget is now tracked (previously ignored entirely) - Guard applyEvent to return early when stats are disabled, so custom event handlers no longer run (and incur cost/side effects) while off https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE
Per PR review, the cacheable and cache-manager event streams cannot be faithfully counted by a simple event map: cache-manager emits no event on a normal miss and double-emits set (per-store + aggregate), and cacheable emits per-store cache:hit/cache:miss so one L1/L2 lookup counts twice. - Remove cacheableStatsEventMap and cacheManagerStatsEventMap; these integrations belong in rollout PRs where the libraries can emit clean per-operation stat events (or be wired imperatively) - Keep the accurate nodeCacheStatsEventMap and add flush_stats -> reset so a subscribed Stats mirrors node-cache's flushStats() lifecycle - subscribe(emitter, eventMap) now requires an explicit eventMap (no universal default exists); the constructor only auto-subscribes when both emitter and eventMap are provided The generic event-driven mechanism, imperative API, computed rates, snapshot, and timestamps are unchanged. 100% coverage on stats.ts maintained. https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE
Replace the outdated "Stats Helpers" section (which referenced a non-existent stats() function) with a complete "Statistics" section covering enabling, imperative increment/decrement, computed rates, snapshot/toJSON, timestamps, and event-driven subscription with the node-cache map and custom maps. https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE
fc6f2d7 to
852dcd5
Compare
Add opt-in per-key tracking to Stats so consumers can identify their hottest and coldest keys: - recordKey(key, field, amount?) keeps a per-key breakdown of hits, misses, gets, sets, and deletes (no-op unless enabled and trackKeys) - topKeys(limit = 100, field?) / bottomKeys(limit = 100, field?) return ranked entries with per-key totals and hit rates; rank by total operations or a single counter - keyStats(key), trackedKeyCount, clearKeys(); reset() clears per-key stats too, and toJSON() now reports trackedKeys - trackKeys / maxTrackedKeys options; when the cap is exceeded the lowest-count keys are pruned in batches (documented: pruning keeps topKeys approximate but makes bottomKeys unreliable) - nodeCacheStatsEventMap now records keys from set/del payloads when key tracking is on Documented in the README Statistics section. 100% coverage on stats.ts. https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE
The new names are self-documenting and align with LRU/LFU terminology; "bottomKeys" in particular was ambiguous. No behavior change. Nothing has shipped yet, so no deprecation alias is needed. https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE
Summary
Several cacheable packages need statistics, but the only implementation today is
Statsin@cacheable/utils— a purely imperative counter class used bycacheable(opt-in) andnode-cache(always on), with no event integration,no computed properties, and no serialization.
This PR upgrades that shared
Statsclass in place so it can become thesingle, fully-featured, event-driven statistics primitive across all libraries.
It is 100% backward compatible — every existing getter and method keeps the
same signature and behavior, so
cacheableandnode-cacheare unaffected(both still build cleanly).
This is the module-only step; wiring the unified stats into each library is
intended as follow-up work.
What's new
subscribe(emitter, eventMap)/unsubscribe(emitter?)attach to any duck-typed emitter (works with
Hookifiedand Node'sEventEmitter, no hard dependency added) and update counters automatically.Counting is gated by
enabled, so you can subscribe first and toggle later;applyEventshort-circuits entirely when disabled."sets"), an array (["hits","gets"]),or a custom handler
(stats, ...args) => void.nodeCacheStatsEventMap(set/del/flush,plus
flush_stats→ reset to mirror node-cache'sflushStats()lifecycle).node-cache emits each event once, so its counts map cleanly — and its
set/delhandlers also record per-key stats when key tracking is on.increment(field, amount?)/decrement(field, amount?)for consumers not using events; existing
incrementHits()etc. are preserved(now thin delegates) and gained an optional
amount.trackKeys:recordKey(key, field, amount?)keeps a per-key breakdown ofhits/misses/gets/sets/deletes.mostUsedKeys(limit = 100, field?)/leastUsedKeys(limit = 100, field?)return ranked
StatsKeyEntry[](with per-key totalcountandhitRate),sorted by total operations or a single counter; deterministic key tie-break.
keyStats(key),trackedKeyCount,clearKeys();reset()clears per-keystats too, and
toJSON()reportstrackedKeys.maxTrackedKeysis an optional safety cap: lowest-count keys are pruned inbatches when exceeded (documented: keeps
mostUsedKeysapproximate but makesleastUsedKeysunreliable — leave unset for exact least-used rankings).hitRate/missRate, guarded against divide-by-zero.toJSON()/snapshot()return a plainStatsSnapshotwith allcounters, rates,
trackedKeys, and timestamps.lastUpdated(last mutation while enabled) andlastReset.enable(),disable(), andclear()(alias ofreset()).non-existent
stats()function) is replaced with a full Statistics sectioncovering usage, available stats, event-driven tracking, and per-key tracking.
Why no
cacheable/cache-managerpresetsInitial drafts shipped
cacheableStatsEventMapandcacheManagerStatsEventMap,but review (confirmed in source) showed those event streams can't be faithfully
counted by a simple map:
(
getreturns early whenresult === undefined), andsetdouble-emits(once per store with
payload.store, then an aggregate). So miss rate wouldbias to zero and set counts would inflate.
cache:hit/cache:miss, so a single L1→L2lookup (primary miss, secondary hit) would count as two gets + one hit + one
miss versus the imperative one get + one hit.
Rather than ship misleading presets, those integrations are deferred to rollout
PRs where the libraries can emit clean per-operation stat events or be wired
imperatively. Consumers can still subscribe today with a custom event map.
New exports from
@cacheable/utilsStatField,KeyStatField,StatsEmitter,StatsEventHandler,StatsEventMap,StatsKeyEntry,StatsSnapshot,nodeCacheStatsEventMap(alongside the existingStats/StatsOptions).Testing
pnpm --filter @cacheable/utils test— Biome lint clean, 205 tests pass,100% coverage on
stats.ts(enabled/disabled gating, computed rates incl.zero-division, timestamps, snapshot, event subscribe/unsubscribe via
offandremoveListener, selective unsubscribe, positional payloads, custom maps,node-cache preset incl.
flush_statsreset and key recording, constructorauto-subscribe and the no-
eventMapguard, per-key breakdowns, most/least-usedranking incl. field ranking, tie-breaks, default-100 limits, and
maxTrackedKeyspruning).pnpm --filter @cacheable/utils build— dual CJS/ESM + type declarations emitcorrectly; new API surfaces in
index.d.mts/index.d.cts.cacheableandnode-cachebuild cleanly against the change (backward-compat check).Notes / follow-ups
cacheable,node-cache,memory, andcache-manageris follow-up work. Forcacheable/cache-managerthat shouldinclude emitting clean, per-user-operation stat events so event-driven counts
match the imperative ones.
instance, counts can double; use one approach per instance.
https://claude.ai/code/session_019LJSwEXR15J1d8PHrNHMrE