fix: restore reserved suffix stripping in PrometheusNaming.sanitizeMetricName() (#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>

Author: zeitlinger

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.
    *
-   * <p>Use {@link #sanitizeMetricName(String)} to convert arbitrary Strings to valid metric names.
+   * <p>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,16 +166,50 @@ 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.
+   *
+   * <p>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.
+   *
+   * <p>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.
+   *
+   * <p>If you want permissive normalization that keeps reserved suffixes intact, use {@link
+   * #normalizeMetricName(String)} instead.
    *
    * @throws IllegalArgumentException if the input is empty
    */
   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.
+   *
+   * <p>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.

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

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}).
+   *
+   * <p>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");