Closed
fix: restore reserved suffix stripping in PrometheusNaming.sanitizeMetricName()#2094
PrometheusNaming.sanitizeMetricName()#2094Conversation
In client_java 1.6.0, `PrometheusNaming.sanitizeMetricName()` was changed
to return the input string unchanged (since all non-empty valid UTF-8
strings are now valid metric names). This was a regression for downstream
tools like the JMX Exporter and the simpleclient bridge, which relied on
`sanitizeMetricName` to strip reserved Prometheus metric name suffixes
(`_total`, `_created`, `_info`, `_bucket` and their dot variants) before
passing names to snapshot builders.
Without the stripping:
- A JMX attribute "Total" could produce a metric named
`kafka_consumer_request_total` instead of `kafka_consumer_request`
- With `inferCounterTypeFromName: true`, this triggers unintended
counter-type inference because the name ends with `_total`
- With `inferCounterTypeFromName: false`, the metric is still discoverable
under the wrong name, breaking downstream registry lookups
The fix restores the stripping behaviour. Note that users who create
metrics directly via `Counter.builder().name("events_total")` are not
affected: the builder API bypasses `sanitizeMetricName` entirely.
Agent-Logs-Url: https://github.com/prometheus/client_java/sessions/7c842dd0-c793-4e16-815f-d3b5e03c3ba8
Co-authored-by: jaydeluca <7630696+jaydeluca@users.noreply.github.com>
- Separate the equals-only check (where name IS the suffix) into its own
loop before the endsWith stripping loop, to avoid re-checking it on
every outer iteration
- Add break after stripping a suffix so the inner loop restarts
immediately rather than completing all remaining suffix checks
- Add test for dot-variant corner case: sanitizeMetricName(".total")
returns "total"
Agent-Logs-Url: https://github.com/prometheus/client_java/sessions/7c842dd0-c793-4e16-815f-d3b5e03c3ba8
Co-authored-by: jaydeluca <7630696+jaydeluca@users.noreply.github.com>
Member
|
I added #2098 to give user the choice |
Member
|
@jaydeluca DCO fails |
This was referenced May 18, 2026
Member
|
Superseded by #2124, which carries the same fix on a clean signed-off commit so the DCO check can pass. |
zeitlinger
added a commit
that referenced
this pull request
May 20, 2026
…etricName()` (#2124) Fixes #2087. Supersedes #2094 with the same fix on a clean, signed-off commit so the DCO check can pass. `sanitizeMetricName()` was simplified in 1.6.0 to return its input unchanged because any non-empty UTF-8 string is now a valid metric name. That silently broke downstream tools — notably the JMX Exporter and the simpleclient bridge — that call `sanitizeMetricName()` to normalize external names before passing them to snapshot builders. The missing stripping means a JMX attribute that produces `kafka_consumer_request_total` as a raw name is no longer sanitized to `kafka_consumer_request`. With `inferCounterTypeFromName: true` this triggers unintended counter-type inference; with it `false` the metric is stored under the wrong name, breaking exact-name registry lookups. ## Changes - Restore `RESERVED_METRIC_NAME_SUFFIXES` and the iterative suffix-stripping loop in `PrometheusNaming.sanitizeMetricName()`. - Keep the exact-match case, for example `"_total"` -> `"total"`, in a dedicated pre-pass before the stripping loop. - Update `PrometheusNamingTest` and `MetricMetadataTest` expectations. - Add regression coverage for the JMX Exporter scenario and dot-variant corner case, for example `.total`. ```java // Before fix: suffix preserved -> metric stored as "kafka_consumer_request_total" PrometheusNaming.sanitizeMetricName("kafka_consumer_request_total"); // After fix: suffix stripped -> metric stored as "kafka_consumer_request" PrometheusNaming.sanitizeMetricName("kafka_consumer_request_total"); ``` `Counter.builder().name("events_total")` is unaffected because the builder API does not go through `sanitizeMetricName()`. ## Validation - `mise run build` Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com>
jaydeluca
pushed a commit
that referenced
this pull request
Jun 4, 2026
…etricName()` (#2124) Fixes #2087. Supersedes #2094 with the same fix on a clean, signed-off commit so the DCO check can pass. `sanitizeMetricName()` was simplified in 1.6.0 to return its input unchanged because any non-empty UTF-8 string is now a valid metric name. That silently broke downstream tools — notably the JMX Exporter and the simpleclient bridge — that call `sanitizeMetricName()` to normalize external names before passing them to snapshot builders. The missing stripping means a JMX attribute that produces `kafka_consumer_request_total` as a raw name is no longer sanitized to `kafka_consumer_request`. With `inferCounterTypeFromName: true` this triggers unintended counter-type inference; with it `false` the metric is stored under the wrong name, breaking exact-name registry lookups. ## Changes - Restore `RESERVED_METRIC_NAME_SUFFIXES` and the iterative suffix-stripping loop in `PrometheusNaming.sanitizeMetricName()`. - Keep the exact-match case, for example `"_total"` -> `"total"`, in a dedicated pre-pass before the stripping loop. - Update `PrometheusNamingTest` and `MetricMetadataTest` expectations. - Add regression coverage for the JMX Exporter scenario and dot-variant corner case, for example `.total`. ```java // Before fix: suffix preserved -> metric stored as "kafka_consumer_request_total" PrometheusNaming.sanitizeMetricName("kafka_consumer_request_total"); // After fix: suffix stripped -> metric stored as "kafka_consumer_request" PrometheusNaming.sanitizeMetricName("kafka_consumer_request_total"); ``` `Counter.builder().name("events_total")` is unaffected because the builder API does not go through `sanitizeMetricName()`. ## Validation - `mise run build` Signed-off-by: Gregor Zeitlinger <gregor.zeitlinger@grafana.com> Signed-off-by: Jay DeLuca <jaydeluca4@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.
Fixes #2087
sanitizeMetricName()was simplified in 1.6.0 to return its input unchanged (since any non-empty UTF-8 string is now a valid metric name). This silently broke downstream tools — notably the JMX Exporter and the simpleclient bridge — that callsanitizeMetricNameto normalize external names before passing them to snapshot builders.The missing stripping means a JMX attribute that produces
kafka_consumer_request_totalas a raw name is no longer sanitized tokafka_consumer_request. WithinferCounterTypeFromName: truethis triggers unintended counter-type inference; with itfalsethe metric is stored under the wrong name, breaking exact-name registry lookups.Changes
PrometheusNaming.java— restoreRESERVED_METRIC_NAME_SUFFIXESconstant and the iterative suffix-stripping loop insanitizeMetricName(). The exact-match case (where the entire input is a reserved suffix, e.g."_total"→"total") is handled in a dedicated pre-pass before the stripping loop, and the inner loop nowbreaks immediately after a match to restart cleanly.PrometheusNamingTest.java— update expectations to match restored behaviour; add a regression test for the JMX Exporter scenario and the dot-variant corner case (.total).MetricMetadataTest.java— update expectations accordingly.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
checkstyle.org/opt/hostedtoolcache/CodeQL/2.25.1/x64/codeql/tools/linux64/java/bin/java /opt/hostedtoolcache/CodeQL/2.25.1/x64/codeql/tools/linux64/java/bin/java -jar /opt/hostedtoolcache/CodeQL/2.25.1/x64/codeql/xml/tools/xml-extractor.jar --fileList=/tmp/codeql-scratch-e3b4dfaf923d334a/dbs/java/working/files-to-index15700371373528140634.list --sourceArchiveDir=/tmp/codeql-scratch-e3b4dfaf923d334a/dbs/java/src --outputDir=/tmp/codeql-scratch-e3b4dfaf923d334a/dbs/java/trap/java --local sh =fetch(dns block)If you need me to access, download, or install something from one of these locations, you can either: