Skip to content

feat(metrics): export KV counters and populate the tiered-log family (#44) - #69

Merged
oreofeolurin merged 2 commits into
devfrom
fix/44-kv-tiered-metrics
Aug 30, 2026
Merged

feat(metrics): export KV counters and populate the tiered-log family (#44)#69
oreofeolurin merged 2 commits into
devfrom
fix/44-kv-tiered-metrics

Conversation

@oreofeolurin

@oreofeolurin oreofeolurin commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

The last two families from #44. Closes it.

The decision the issue asked for

Export, not delete. Per-namespace KV operation rates and hot/warm hit rates are exactly what you need to size a hot buffer or notice a workload shift — deleting them would remove the observability, not the dead code problem.

KV — recorded nowhere, and never emitted

Two separate gaps. recordGet/recordSet/recordDelete had no callers, and exportPrometheus had no KV branch at all — so unlike the other families, the series didn't exist rather than reading 0.

Added writeKVMetrics plus the export loop, and wired the three record calls into the KV dispatch paths using the per-namespace metrics that registerKVNamespace already returned and the code discarded.

Tiered log — written but never registered

The opposite shape: writeTieredLogMetrics was already called by exportPrometheus, but registerTieredLog had zero callers, so no entry ever existed to iterate over.

The shard now registers its log once at wire-up and hands the pointer to the stream handler. getPayloadAndTier already decides hot vs warm per record, so recording there costs nothing extra — and resolving the pointer at wire-up keeps a registry lookup off the read path.

if (self.partition.ual.read(ual_index)) |ual_entry| { ... tm.recordHotHit(); }
if (self.partition.readPayloadWarm(ual_index)) |raw| { ... tm.recordWarmHit(); }
tm.recordMiss();

cold_hits stays 0 by design — that path consults the hot ring and warm store only, never cold. Left visible rather than faked, since a fabricated number is the exact problem this issue is about.

Tests

Two more value-asserting e2e tests: two sets, a get and a delete produce 2/1/1 on the KV counters; an append plus read produces a non-zero hot-hit count. Both fail with the instrumentation stashed.

test-unit, test-integration, e2e/kv and e2e/stream all pass — the 4 kv/cluster failures are the pre-existing set from #62.

…44)

The last two families from #44. Both are worth keeping rather than deleting:
per-namespace KV operation rates and hot/warm hit rates are the numbers an
operator needs to size a hot buffer or spot a workload change.

KV was recorded nowhere and, separately, never emitted — `exportPrometheus`
had no KV branch at all, so the series did not exist rather than reading zero.
Added `writeKVMetrics` and wired get/set/delete in the KV dispatch paths, using
the per-namespace metrics `registerKVNamespace` already returned and discarded.

The tiered-log family was already written by `exportPrometheus`, but
`registerTieredLog` had no callers, so nothing ever created an entry to
export. The shard now registers its log once at wire-up and hands the pointer
to the stream handler, so `getPayloadAndTier` — which already decides hot vs
warm per record — records the hit without a registry lookup on the read path.

`cold_hits` stays at 0 by design: that path consults the hot ring and the warm
store only, and never cold. Left visible rather than faked.

## Tests

Two more value-asserting e2e tests: two sets, one get and one delete produce
2/1/1 on the KV counters; an append and read produce a non-zero hot-hit count.
Both fail with the instrumentation stashed.

test-unit, test-integration and the kv/stream e2e filters pass — the 4
kv/cluster failures are the pre-existing set from #62.
Found reviewing this PR's own diff. decrementKeyCount was an unguarded
fetchSub, and key_count is not rebuilt on recovery — the apply path never goes
through recordSet. So after a restart the counter is 0 while keys exist on
disk, and deleting any recovered key wrapped the gauge to u64 max.

That is the exact failure this issue is about: a number that looks real and is
not. Saturating at zero via CAS, with a regression test that restarts the
server, deletes a recovered key, and asserts the gauge stays sane. The test
fails against the unguarded version.
@oreofeolurin
oreofeolurin merged commit 57c453c into dev Aug 30, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant