Handle prometheus names and labels consistently (#6049)

Author: jonatan-ivanov

Diff for: implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistry.java

@@ -20,8 +20,8 @@
 import io.micrometer.core.instrument.*;
 import io.micrometer.core.instrument.cumulative.CumulativeFunctionCounter;
 import io.micrometer.core.instrument.cumulative.CumulativeFunctionTimer;
-import io.micrometer.core.instrument.distribution.HistogramSnapshot;
 import io.micrometer.core.instrument.distribution.*;
+import io.micrometer.core.instrument.distribution.HistogramSnapshot;
 import io.micrometer.core.instrument.distribution.pause.PauseDetector;
 import io.micrometer.core.instrument.internal.DefaultGauge;
 import io.micrometer.core.instrument.internal.DefaultLongTaskTimer;
@@ -54,7 +54,6 @@
 
 import static java.util.concurrent.TimeUnit.NANOSECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.StreamSupport.stream;
 
@@ -591,11 +590,11 @@ private void applyToCollector(Meter.Id id, Consumer<MicrometerCollector> consume
 
             meterRegistrationFailed(id,
                     "Prometheus requires that all meters with the same name have the same"
-                            + " set of tag keys. There is already an existing meter named '" + id.getName()
+                            + " set of tag keys. There is already an existing meter named '" + getConventionName(id)
                             + "' containing tag keys ["
                             + String.join(", ", collectorMap.get(getConventionName(id)).getTagKeys())
-                            + "]. The meter you are attempting to register" + " has keys ["
-                            + getConventionTags(id).stream().map(Tag::getKey).collect(joining(", ")) + "].");
+                            + "]. The meter you are attempting to register" + " has keys [" + String.join(", ", tagKeys)
+                            + "].");
             return existingCollector;
         });
     }

Diff for: implementations/micrometer-registry-prometheus/src/main/java/io/micrometer/prometheusmetrics/PrometheusNamingConvention.java

@@ -45,7 +45,7 @@ public PrometheusNamingConvention(String timerSuffix) {
      */
     @Override
     public String name(String name, Meter.Type type, @Nullable String baseUnit) {
-        String conventionName = NamingConvention.snakeCase.name(name, type, baseUnit);
+        String conventionName = PrometheusNaming.prometheusName(name);
 
         switch (type) {
             case COUNTER:
@@ -85,7 +85,7 @@ else if (!conventionName.endsWith("_seconds")) {
      */
     @Override
     public String tagKey(String key) {
-        return PrometheusNaming.sanitizeLabelName(key);
+        return PrometheusNaming.sanitizeLabelName(PrometheusNaming.prometheusName(key));
     }
 
 }

Diff for: implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusMeterRegistryTest.java

@@ -70,23 +70,42 @@ void metersWithSameNameAndDifferentTagsContinueSilently() {
     }
 
     @Test
-    void meterRegistrationFailedListenerCalledOnSameNameDifferentTags() throws InterruptedException {
+    void meterRegistrationFailedListenerCalledOnSameNameDifferentTagsWithEmptyTags() throws InterruptedException {
         CountDownLatch failedLatch = new CountDownLatch(1);
         registry.config().onMeterRegistrationFailed((id, reason) -> failedLatch.countDown());
         registry.counter("my.counter");
-        registry.counter("my.counter", "k", "v").increment();
+        registry.counter("my.counter", "test.k1", "v1").increment();
 
         assertThat(failedLatch.await(1, TimeUnit.SECONDS)).isTrue();
 
-        assertThatThrownBy(() -> registry.throwExceptionOnRegistrationFailure().counter("my.counter", "k1", "v1"))
+        assertThatThrownBy(() -> registry.throwExceptionOnRegistrationFailure().counter("my.counter", "test.k2", "v2"))
+            .isInstanceOf(IllegalArgumentException.class)
+            .hasMessage(
+                    "Prometheus requires that all meters with the same name have the same set of tag keys. There is already an existing meter named 'my_counter' containing tag keys []. The meter you are attempting to register has keys [test_k2].");
+
+        assertThatThrownBy(() -> registry.counter("my.counter", "test.k3", "v3"))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageStartingWith(
-                    "Prometheus requires that all meters with the same name have the same set of tag keys.");
+            .hasMessage(
+                    "Prometheus requires that all meters with the same name have the same set of tag keys. There is already an existing meter named 'my_counter' containing tag keys []. The meter you are attempting to register has keys [test_k3].");
+    }
+
+    @Test
+    void meterRegistrationFailedListenerCalledOnSameNameDifferentTagsWithNonEmptyTags() throws InterruptedException {
+        CountDownLatch failedLatch = new CountDownLatch(1);
+        registry.config().onMeterRegistrationFailed((id, reason) -> failedLatch.countDown());
+        registry.counter("my.counter", "test.k1", "v1");
+        registry.counter("my.counter", "k", "v").increment();
 
-        assertThatThrownBy(() -> registry.counter("my.counter", "k2", "v2"))
+        assertThat(failedLatch.await(1, TimeUnit.SECONDS)).isTrue();
+
+        assertThatThrownBy(() -> registry.throwExceptionOnRegistrationFailure().counter("my.counter", "test.k2", "v2"))
             .isInstanceOf(IllegalArgumentException.class)
-            .hasMessageStartingWith(
-                    "Prometheus requires that all meters with the same name have the same set of tag keys.");
+            .hasMessage(
+                    "Prometheus requires that all meters with the same name have the same set of tag keys. There is already an existing meter named 'my_counter' containing tag keys [test_k1]. The meter you are attempting to register has keys [test_k2].");
+
+        assertThatThrownBy(() -> registry.counter("my.counter")).isInstanceOf(IllegalArgumentException.class)
+            .hasMessage(
+                    "Prometheus requires that all meters with the same name have the same set of tag keys. There is already an existing meter named 'my_counter' containing tag keys [test_k1]. The meter you are attempting to register has keys [].");
     }
 
     @Test

Diff for: implementations/micrometer-registry-prometheus/src/test/java/io/micrometer/prometheusmetrics/PrometheusNamingConventionTest.java

@@ -30,12 +30,12 @@ class PrometheusNamingConventionTest {
 
     @Test
     void formatName() {
-        assertThat(convention.name("123abc/{:id}水", Meter.Type.GAUGE)).startsWith("_23abc___id__");
+        assertThat(convention.name("123.abc/{:id}水", Meter.Type.GAUGE)).startsWith("_23_abc___id__");
     }
 
     @Test
     void formatTagKey() {
-        assertThat(convention.tagKey("123abc/{:id}水")).startsWith("_23abc___id__");
+        assertThat(convention.tagKey("123.abc/{:id}水")).startsWith("_23_abc___id__");
     }
 
     @Test