runtime: key counter rename rules by the parsed family name - #1703
runtime: key counter rename rules by the parsed family name#1703aeron-gh wants to merge 2 commits into
Conversation
Signed-off-by: aeron-gh <agab0323@gmail.com>
| # The runtime's parser munges counter families into their OpenMetrics | ||
| # form: the FAMILY name loses the _total suffix while samples keep it. | ||
| # Rules are matched by family name, so the counter rule is keyed by the | ||
| # munged family and RenameMetric's suffix arithmetic restores _total on | ||
| # the exposed samples, yielding the documented | ||
| # kthena:generation_tokens_total series. |
There was a problem hiding this comment.
I don't think it's necessary to write such complicated comments
| # The runtime's parser munges counter families into their OpenMetrics | ||
| # form: the FAMILY name loses the _total suffix while samples keep it. | ||
| # Rules are matched by family name, so the counter rule is keyed by the | ||
| # munged family and RenameMetric's suffix arithmetic restores _total on | ||
| # the exposed samples, yielding the documented | ||
| # kthena:generation_tokens_total series. |
Signed-off-by: aeron-gh <agab0323@gmail.com>
|
@FAUST-BENCHOU @kube-gopher |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: kube-gopher The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
What this PR does / why we need it:
The runtime documents a standardized
kthena:generation_tokens_totalseries, but it is never emitted for either engine. The rename rules are keyed by the raw counter name (vllm:generation_tokens_total,sglang:generation_tokens_total) while the lookup uses the PARSED family name, and the pinned parser (prometheus-client 0.19.0) munges counter families into their OpenMetrics form, stripping_totalfrom the family name. The counter family therefore arrives namedvllm:generation_tokensand the rule never matches. Gauge and histogram family names are untouched, which is why every other rename works and exactly this counter is missing.The fix keys the two counter rules by the munged family name and renames to a
kthena:generation_tokensfamily.RenameMetricalready does per sample suffix arithmetic, so the exposed samples come out askthena:generation_tokens_totalexactly as documented (a_createdsample would map the same way).Which issue(s) this PR fixes:
Fixes #1652
Bug evidence (required for bug-related PRs):
The parser level and end to end reproduction are in #1652. The new tests are a flip proof on this branch:
with the tests added but the fix reverted:
The last failure shows the sidecar output containing only the original
vllm:generation_tokens_totaland nokthena:twin. With the fix,9 passed, and the end to end test asserts the documented series in the actualprocess_metricsoutput:Production path: the rules are registered by family name at standard.py#L37-L64, the lookup is
metric_operators_dict.get(origin_metric.name)at standard.py#L121, the parser is pinned at requirements.txt#L5, and the suffix arithmetic that restores_totalon samples is metric.py#L77.ruff checkpasses on both touched files. The full python suite passes apart from the six modules that importfcntland cannot run on a Windows dev box; they are untouched by this change.Special notes for your reviewer:
The exposed TYPE line becomes
# TYPE kthena:generation_tokens counterwith thekthena:generation_tokens_totalsample, which is standard counter exposition and what any Prometheus client scrapes askthena:generation_tokens_total. The docs page (runtime.md#L126) therefore stays accurate as written.Does this PR introduce a user-facing change?: