diff --git a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/PrometheusNaming.java b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/PrometheusNaming.java index ea2653931..63cbb7879 100644 --- a/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/PrometheusNaming.java +++ b/prometheus-metrics-model/src/main/java/io/prometheus/metrics/model/snapshots/PrometheusNaming.java @@ -17,6 +17,17 @@ */ public class PrometheusNaming { + /** + * Reserved metric name suffixes. These suffixes are automatically appended by Prometheus + * exposition format writers for specific metric types: {@code _total} and {@code _created} for + * counters, {@code _info} for info metrics, and {@code _bucket} for histograms. Including these + * in a base metric name via {@link #sanitizeMetricName(String)} would cause confusion or + * double-suffixing, so they are stripped during sanitization. + */ + static final String[] RESERVED_METRIC_NAME_SUFFIXES = { + "_total", "_created", "_bucket", "_info", ".total", ".created", ".bucket", ".info" + }; + /** * Test if a metric name is valid. Any non-empty valid UTF-8 string is accepted. * @@ -35,7 +46,9 @@ public class PrometheusNaming { * format, this will be represented as two values: {@code processing_time_seconds_total} for the * counter value, and the optional {@code processing_time_seconds_created} timestamp. * - *
Use {@link #sanitizeMetricName(String)} to convert arbitrary Strings to valid metric names. + *
Use {@link #sanitizeMetricName(String)} for compatibility-preserving sanitization that + * strips reserved suffixes, or {@link #normalizeMetricName(String)} for permissive normalization + * that keeps the original suffixes intact. */ public static boolean isValidMetricName(String name) { return validateMetricName(name) == null; @@ -153,8 +166,22 @@ public static String prometheusName(String name) { } /** - * Convert an arbitrary string to a valid metric name. Since any non-empty valid UTF-8 string is a - * valid metric name, this simply returns the input unchanged. + * Convert an arbitrary string to a valid metric name. + * + *
Reserved metric name suffixes ({@code _total}, {@code _created}, {@code _bucket}, {@code + * _info} and their dot variants) are stripped. These suffixes are appended automatically by + * Prometheus exposition format writers, so including them in a base metric name would result in + * double-suffixing or unintended type inference. For example, a JMX attribute named {@code + * RequestTotal} would be sanitized from {@code kafka_consumer_request_total} to {@code + * kafka_consumer_request}, and the counter writer would add {@code _total} back at scrape time. + * + *
This behaviour was present in client_java 1.5.x and is restored here to fix a regression + * introduced in 1.6.0 that affected downstream tools (e.g. the JMX Exporter and the simpleclient + * bridge) which relied on {@code sanitizeMetricName} to strip these suffixes before passing names + * to the snapshot builders. + * + *
If you want permissive normalization that keeps reserved suffixes intact, use {@link + * #normalizeMetricName(String)} instead. * * @throws IllegalArgumentException if the input is empty */ @@ -162,7 +189,27 @@ public static String sanitizeMetricName(String metricName) { if (metricName.isEmpty()) { throw new IllegalArgumentException("Cannot convert an empty string to a valid metric name."); } - return metricName; + String sanitizedName = metricName; + boolean stripped = true; + while (stripped) { + stripped = false; + // When the name equals the suffix exactly, drop the leading separator character to avoid + // returning an empty string (e.g. "_total" → "total", ".info" → "info"). + for (String reservedSuffix : RESERVED_METRIC_NAME_SUFFIXES) { + if (sanitizedName.equals(reservedSuffix)) { + return reservedSuffix.substring(1); + } + } + for (String reservedSuffix : RESERVED_METRIC_NAME_SUFFIXES) { + if (sanitizedName.endsWith(reservedSuffix)) { + sanitizedName = + sanitizedName.substring(0, sanitizedName.length() - reservedSuffix.length()); + stripped = true; + break; // restart the outer loop to re-check all suffixes on the shortened name + } + } + } + return sanitizedName; } /** @@ -179,6 +226,37 @@ public static String sanitizeMetricName(String metricName, Unit unit) { return result; } + /** + * Convert an arbitrary string to a valid metric name without stripping reserved suffixes. + * + *
Any non-empty valid UTF-8 string is accepted and returned unchanged. This is the permissive + * normalization behavior introduced in 1.6.0. Use this method for new integrations that want to + * preserve the original metric name and rely on registration-time collision detection instead of + * suffix stripping. + * + * @throws IllegalArgumentException if the input is empty + */ + public static String normalizeMetricName(String metricName) { + if (metricName.isEmpty()) { + throw new IllegalArgumentException("Cannot convert an empty string to a valid metric name."); + } + return metricName; + } + + /** + * Like {@link #normalizeMetricName(String)}, but also makes sure that the unit is appended as a + * suffix if the unit is not {@code null}. + */ + public static String normalizeMetricName(String metricName, Unit unit) { + String result = normalizeMetricName(metricName); + if (unit != null) { + if (!result.endsWith("_" + unit) && !result.endsWith("." + unit)) { + result += "_" + unit; + } + } + return result; + } + /** * Convert an arbitrary string to a name where {@link #isValidLabelName(String) * isValidLabelName(name)} is true. diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricMetadataTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricMetadataTest.java index 8a4731ac8..5781eb146 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricMetadataTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/MetricMetadataTest.java @@ -35,26 +35,30 @@ void testSanitizationIllegalCharacters() { @Test void testNameWithTotalSuffix() { + // sanitizeMetricName strips the reserved _total suffix. MetricMetadata metadata = new MetricMetadata(sanitizeMetricName("my_events_total")); - assertThat(metadata.getName()).isEqualTo("my_events_total"); + assertThat(metadata.getName()).isEqualTo("my_events"); } @Test void testNameWithInfoSuffix() { + // sanitizeMetricName strips the reserved _info suffix. MetricMetadata metadata = new MetricMetadata(sanitizeMetricName("target_info")); - assertThat(metadata.getName()).isEqualTo("target_info"); + assertThat(metadata.getName()).isEqualTo("target"); } @Test void testNameWithCreatedSuffix() { + // sanitizeMetricName strips the reserved _created suffix. MetricMetadata metadata = new MetricMetadata(sanitizeMetricName("my_events_created")); - assertThat(metadata.getName()).isEqualTo("my_events_created"); + assertThat(metadata.getName()).isEqualTo("my_events"); } @Test void testNameWithBucketSuffix() { + // sanitizeMetricName strips the reserved _bucket suffix. MetricMetadata metadata = new MetricMetadata(sanitizeMetricName("my_histogram_bucket")); - assertThat(metadata.getName()).isEqualTo("my_histogram_bucket"); + assertThat(metadata.getName()).isEqualTo("my_histogram"); } @Test diff --git a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/PrometheusNamingTest.java b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/PrometheusNamingTest.java index dcebd14d8..ee6d2d027 100644 --- a/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/PrometheusNamingTest.java +++ b/prometheus-metrics-model/src/test/java/io/prometheus/metrics/model/snapshots/PrometheusNamingTest.java @@ -2,6 +2,7 @@ import static io.prometheus.metrics.model.snapshots.PrometheusNaming.escapeName; import static io.prometheus.metrics.model.snapshots.PrometheusNaming.isValidLabelName; +import static io.prometheus.metrics.model.snapshots.PrometheusNaming.normalizeMetricName; import static io.prometheus.metrics.model.snapshots.PrometheusNaming.prometheusName; import static io.prometheus.metrics.model.snapshots.PrometheusNaming.sanitizeLabelName; import static io.prometheus.metrics.model.snapshots.PrometheusNaming.sanitizeMetricName; @@ -22,27 +23,79 @@ class PrometheusNamingTest { @Test void testSanitizeMetricName() { - assertThat(sanitizeMetricName("my_counter_total")).isEqualTo("my_counter_total"); - assertThat(sanitizeMetricName("jvm.info")).isEqualTo("jvm.info"); - assertThat(sanitizeMetricName("jvm_info")).isEqualTo("jvm_info"); + // Reserved suffixes are stripped to avoid confusion with Prometheus type conventions. + assertThat(sanitizeMetricName("my_counter_total")).isEqualTo("my_counter"); + assertThat(sanitizeMetricName("jvm.info")).isEqualTo("jvm"); + assertThat(sanitizeMetricName("jvm_info")).isEqualTo("jvm"); assertThat(sanitizeMetricName("a.b")).isEqualTo("a.b"); - assertThat(sanitizeMetricName("_total")).isEqualTo("_total"); + // "_total" / ".total" corner cases: the suffix is the entire name, so the separator + // character is dropped to avoid returning an empty string. + assertThat(sanitizeMetricName("_total")).isEqualTo("total"); + assertThat(sanitizeMetricName(".total")).isEqualTo("total"); assertThat(sanitizeMetricName("total")).isEqualTo("total"); - assertThat(sanitizeMetricName("my_events_created")).isEqualTo("my_events_created"); - assertThat(sanitizeMetricName("my_histogram_bucket")).isEqualTo("my_histogram_bucket"); + assertThat(sanitizeMetricName("my_events_created")).isEqualTo("my_events"); + assertThat(sanitizeMetricName("my_histogram_bucket")).isEqualTo("my_histogram"); + } + + /** + * Regression test: reserved suffixes must be stripped even when the raw name comes from an + * external system (e.g. JMX Exporter converting a JMX attribute named {@code "Total"} into a + * Prometheus name {@code kafka_consumer_request_total}). + * + *
Without stripping, an UNKNOWN metric would be stored under {@code + * kafka_consumer_request_total} instead of {@code kafka_consumer_request}, breaking registry + * lookups by the expected base name and potentially triggering unintended counter-type inference + * in tools that check for the {@code _total} suffix. + */ + @Test + void testSanitizeMetricNameStripsReservedSuffixForDownstreamTools() { + // A JMX attribute "Total" produces "kafka_consumer_request_total" as the raw name. + // sanitizeMetricName must strip "_total" so that the metric is stored and looked up under + // "kafka_consumer_request", not "kafka_consumer_request_total". + assertThat(sanitizeMetricName("kafka_consumer_request_total")) + .isEqualTo("kafka_consumer_request"); + // Dot variant is stripped too. + assertThat(sanitizeMetricName("kafka_consumer_request.total")) + .isEqualTo("kafka_consumer_request"); + // Multiple chained reserved suffixes are stripped iteratively. + assertThat(sanitizeMetricName("events_total_created")).isEqualTo("events"); } @Test void testSanitizeMetricNameWithUnit() { assertThat(prometheusName(sanitizeMetricName("def", Unit.RATIO))) .isEqualTo("def_" + Unit.RATIO); + // _total is stripped first, then the unit is appended. assertThat(prometheusName(sanitizeMetricName("my_counter_total", Unit.RATIO))) - .isEqualTo("my_counter_total_" + Unit.RATIO); - assertThat(sanitizeMetricName("jvm.info", Unit.RATIO)).isEqualTo("jvm.info_" + Unit.RATIO); - assertThat(sanitizeMetricName("_total", Unit.RATIO)).isEqualTo("_total_" + Unit.RATIO); + .isEqualTo("my_counter_" + Unit.RATIO); + assertThat(sanitizeMetricName("jvm.info", Unit.RATIO)).isEqualTo("jvm_" + Unit.RATIO); + assertThat(sanitizeMetricName("_total", Unit.RATIO)).isEqualTo("total_" + Unit.RATIO); assertThat(sanitizeMetricName("total", Unit.RATIO)).isEqualTo("total_" + Unit.RATIO); } + @Test + void testNormalizeMetricName() { + assertThat(normalizeMetricName("my_counter_total")).isEqualTo("my_counter_total"); + assertThat(normalizeMetricName("jvm.info")).isEqualTo("jvm.info"); + assertThat(normalizeMetricName("jvm_info")).isEqualTo("jvm_info"); + assertThat(normalizeMetricName("a.b")).isEqualTo("a.b"); + assertThat(normalizeMetricName("_total")).isEqualTo("_total"); + assertThat(normalizeMetricName(".total")).isEqualTo(".total"); + assertThat(normalizeMetricName("my_events_created")).isEqualTo("my_events_created"); + assertThat(normalizeMetricName("my_histogram_bucket")).isEqualTo("my_histogram_bucket"); + } + + @Test + void testNormalizeMetricNameWithUnit() { + assertThat(prometheusName(normalizeMetricName("def", Unit.RATIO))) + .isEqualTo("def_" + Unit.RATIO); + assertThat(prometheusName(normalizeMetricName("my_counter_total", Unit.RATIO))) + .isEqualTo("my_counter_total_" + Unit.RATIO); + assertThat(normalizeMetricName("jvm.info", Unit.RATIO)).isEqualTo("jvm.info_" + Unit.RATIO); + assertThat(normalizeMetricName("_total", Unit.RATIO)).isEqualTo("_total_" + Unit.RATIO); + assertThat(normalizeMetricName("total", Unit.RATIO)).isEqualTo("total_" + Unit.RATIO); + } + @Test void testSanitizeLabelName() { assertThat(prometheusName(sanitizeLabelName("0abc.def"))).isEqualTo("_abc_def");