docs(observability): mark the CKF counter as registered but never incremented - #13149
Conversation
…remented Follow-up to #12983, which corrected the CKF counter name on this branch but not the more important fact. That PR renamed dynamo_kvrouter_ckf_mutation_total to dynamo_component_ckf_mutation_total, which is right: the kvrouter-prefixed name comes only from new_registered, the standalone indexer path, while an embedded router registers through from_component and exports the component-scoped name. PromQL copied off this page returned no series on a normal Frontend+KV deployment. The rename was not enough. inc_ckf_mutation has no call sites outside its own definition in lib/kv-router/src/indexer/metrics.rs, and CkfMutationKind is referenced nowhere else in the tree, so the counter is created and exported but never incremented under either name. A reader scraping /metrics finds a counter called ..._mutation_total and reasonably builds a panel on it. Uses the catalog's existing "Registered, always zero" framing rather than deleting the entry, since the series is exported either way and removing the documentation would remove the warning without removing the metric. The metric-labels entry for `outcome` says the same, because no series carries that label in practice. Whether the missing call sites are an oversight or a reserved series is a question out with the KV-router owner. The documentation should describe what ships regardless. Signed-off-by: Dan Gil <dagil@nvidia.com>
github-actions
Bot
added
docs
documentation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| <ParamField path="dynamo_component_ckf_mutation_total" type="counter"> | ||
| CKF block-level mutation outcomes recorded by the router indexer, finer-grained than the event `status` on the counter above. Labeled by `outcome` — see [Metric-specific labels](metric-labels.mdx#metric-specific-labels). | ||
| CKF block-level mutation outcomes recorded by the router indexer, labeled by `outcome` — see [Metric-specific labels](metric-labels.mdx#metric-specific-labels). **Registered, always zero.** The series is created and exported, but no code path currently increments it, so it reads 0 in every deployment. Do not build alerts or dashboard panels on it. The standalone indexer exports the same counter as `dynamo_kvrouter_ckf_mutation_total`, with the same caveat. |
There was a problem hiding this comment.
🔍 "Exported, reads 0" is likely stronger than reality: an empty label vector emits no series at all
I verified the PR's core claim: inc_ckf_mutation (lib/kv-router/src/indexer/metrics.rs:507) has no call sites and CkfMutationKind (lib/kv-router/src/indexer/metrics.rs:63) is referenced nowhere else, so the counter is indeed never incremented under either name (from_component → dynamo_component_ckf_mutation_total, new_registered → dynamo_kvrouter_ckf_mutation_total, used by the standalone indexer at lib/kv-router/src/services/indexer/server.rs:60).
However, the counter is an IntCounterVec with an outcome label, and no code ever calls with_label_values, so the metric family has zero children. The Rust prometheus text encoder skips metric families with no metrics, meaning nothing is emitted at /metrics at all — a PromQL query returns no data, not 0. The wording here ("The series is created and exported ... it reads 0 in every deployment") could mislead a reader into writing an alert like rate(dynamo_component_ckf_mutation_total[5m]) == 0 and being surprised by an empty result. The companion sentence in docs/fern/reference/observability/metric-labels.mdx:115 ("no series carries this label in practice") is precise; consider aligning the catalog wording, e.g. "registered, but with no label values ever set, so no series is exported".
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf4f865db6
ℹ️ 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".
|
|
||
| <ParamField path="outcome" type="string"> | ||
| `[Shared]` On `dynamo_kvrouter_ckf_mutation_total` — the block-level CKF mutation outcome, finer-grained than the event `status` above. | ||
| `[Shared]` On `dynamo_component_ckf_mutation_total`, and on `dynamo_kvrouter_ckf_mutation_total` from the standalone indexer — the block-level CKF mutation outcome. Both counters are registered but never incremented today, so no series carries this label in practice. |
There was a problem hiding this comment.
When a local or standalone indexer is constructed, KvIndexerMetrics::prebind calls with_label_values for both unknown_remove and capacity_exhausted (lib/kv-router/src/indexer/metrics.rs lines 458–465), which materializes two exported zero-valued Prometheus series carrying the outcome label even though inc_ckf_mutation is never called. Therefore an active indexer does expose this label, and this statement also contradicts the catalog's new claim that the series is created and exported; describe the label values as remaining zero rather than saying that no series carries them.
Useful? React with 👍 / 👎.
|
|
||
| <ParamField path="dynamo_component_ckf_mutation_total" type="counter"> | ||
| CKF block-level mutation outcomes recorded by the router indexer, finer-grained than the event `status` on the counter above. Labeled by `outcome` — see [Metric-specific labels](metric-labels.mdx#metric-specific-labels). | ||
| CKF block-level mutation outcomes recorded by the router indexer, labeled by `outcome` — see [Metric-specific labels](metric-labels.mdx#metric-specific-labels). **Registered, always zero.** The series is created and exported, but no code path currently increments it, so it reads 0 in every deployment. Do not build alerts or dashboard panels on it. The standalone indexer exports the same counter as `dynamo_kvrouter_ckf_mutation_total`, with the same caveat. |
There was a problem hiding this comment.
The component series is not created in every deployment: when overlap_score_credit == 0.0, Indexer::new returns Indexer::None before any KvIndexerMetrics::from_component call (lib/llm/src/kv_router/indexer/mod.rs lines 89–90), which includes non-KV configurations already described elsewhere in this catalog as not registering indexer metrics. Consequently /metrics has no dynamo_component_ckf_mutation_total in that configuration, rather than a zero-valued series; qualify this statement to deployments that construct a local KV indexer and reflect that condition in the availability information.
Useful? React with 👍 / 👎.
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
Re-snapshot docs/ from main (46 further commits since the first snapshot), re-apply the release ledger at the new paths - the 1.4.0 image-tag pins, the shipped-behavior reference sync (#12983), the CKF metric caveats (#13149), and the UCX/EFA facts (#13049) - and sync docs CI plus both READMEs from main so no link targets the deleted old layout. Promote v1.4.0 to current release across the docs values: releases.data.ts (CURRENT_* consts, v1.4.0 release entry with shipped pins SGLang 0.5.16 / TRT-LLM 1.3.0rc22 / vLLM 0.26.0 / NIXL 1.3.0-1.3.2 / UCX 1.21.x, CUDA rows, artifact tags) with regenerated reference pages, and bump copy-paste container tags in install guides (EN/zh-CN), READMEs, profiler and guide examples. Recipe-mirroring pages keep their deliberate validation pins. Signed-off-by: Dan Gil <dagil@nvidia.com>
2 participants
Follow-up to #12983, which corrected the CKF counter name on this branch but not the more important fact.
What #12983 got right, and what it missed
It renamed
dynamo_kvrouter_ckf_mutation_totaltodynamo_component_ckf_mutation_total. That was correct: thekvrouter-prefixed name comes only fromnew_registered, the standalone indexer path, while an embedded router registers throughfrom_componentand exports the component-scoped name. PromQL copied off this page returned no series on a normal Frontend+KV deployment.The rename was not sufficient.
inc_ckf_mutationhas no call sites outside its own definition inlib/kv-router/src/indexer/metrics.rs, andCkfMutationKindis referenced nowhere else in the tree. The counter is created and exported, but nothing increments it under either name, so it reads 0 in every deployment.Since #12983 already merged, the release branch currently documents a counter that will never move, and a reader scraping
/metricsfinds something called..._mutation_totaland reasonably builds a panel on it.The fix
Uses the catalog's existing Registered, always zero category rather than deleting the entry. The series is exported whether or not it is documented, so removing the entry would remove the warning without removing the metric. The
metric-labels.mdxentry foroutcomegets the same treatment, since no series carries that label in practice.Notes
Docs only. The equivalent correction for
mainis in #12975. Whether the missing call sites are an oversight or a deliberately reserved series is a question out with the KV-router owner; the documentation should describe what ships either way.