openmetrics: escape sample metric names consistently with HELP/TYPE - #1204
Open
amdadulbari wants to merge 1 commit into
Open
openmetrics: escape sample metric names consistently with HELP/TYPE#1204amdadulbari wants to merge 1 commit into
amdadulbari wants to merge 1 commit into
Conversation
In the OpenMetrics exposition, the # HELP/# TYPE/# UNIT lines escape the metric name with escape_metric_name() (which treats ':' as a valid metric name character), but the sample line escaped it with the label-name rune set, turning ':' into '_'. A metric such as 'sglang:token_usage' was then emitted as 'sglang:token_usage' in the metadata lines but 'sglang_token_usage' in the sample line, producing output that strict OpenMetrics parsers reject as an orphaned metadata set plus an untyped metric. Use escape_metric_name() for the sample name too, so the same helper drives HELP, TYPE, UNIT and the sample line for every escaping scheme. Fixes prometheus#1177 Signed-off-by: Md. Amdadul Bari Imad <amdadulbari@gmail.com>
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
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
Fixes #1177.
In the OpenMetrics exposition, the
# HELP/# TYPE/# UNITlines escape the metric name withescape_metric_name()(where:is a valid metric-name character and is kept), but the sample line escaped the name with the label-name rune set (_is_legacy_labelname_rune), which turns:into_.So a metric named
sglang:token_usagewas emitted as:The metadata name and the sample name disagree, which strict OpenMetrics parsers (e.g.
promtool check metrics) reject as an orphaned metadata set plus an untyped metric.Change
Use
escape_metric_name()for the sample name as well, so a single helper drivesHELP,TYPE,UNITand the sample line for every escaping scheme (underscores,dots,values,allowutf8). This matches the existingescape_metric_namesemantics already asserted bytest_escape_metric_name(a colon is preserved in metric names).Tests
Added
test_metric_name_with_colon_consistent_across_metadata_and_samples, which checks the exact underscores-escaped output and asserts the metric-name identifier is identical acrossHELP,TYPEand the sample line for every escaping scheme. It fails onmasterand passes with this change. Fulltests/openmetrics,tests/test_exposition.pyandtests/test_parser.pysuites pass;flake8andisortare clean.