prometheus exporter: add units to metric names in TYPE and HELP comme…

…nts (#5718)
open-telemetry · Sep 1, 2023 · 2e2c308 · 2e2c308
1 parent 14f16b0
commit 2e2c308
Show file tree

Hide file tree

Showing 6 changed files with 120 additions and 111 deletions.
diff --git a/...etheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusMetricNameMapper.java b/...etheus/src/main/java/io/opentelemetry/exporter/prometheus/PrometheusMetricNameMapper.java
@@ -15,7 +15,7 @@
 
 /** A class that maps a raw metric name to Prometheus equivalent name. */
 class PrometheusMetricNameMapper implements BiFunction<MetricData, PrometheusType, String> {
-
+  private static final String TOTAL_SUFFIX = "_total";
   static final PrometheusMetricNameMapper INSTANCE = new PrometheusMetricNameMapper();
 
   private final Map<ImmutableMappingKey, String> cache = new ConcurrentHashMap<>();
@@ -42,15 +42,21 @@ private static String mapToPrometheusName(MetricData rawMetric, PrometheusType p
     String name = NameSanitizer.INSTANCE.apply(rawMetric.getName());
     String prometheusEquivalentUnit =
         PrometheusUnitsHelper.getEquivalentPrometheusUnit(rawMetric.getUnit());
+    boolean shouldAppendUnit =
+        !StringUtils.isNullOrEmpty(prometheusEquivalentUnit)
+            && !name.contains(prometheusEquivalentUnit);
+    // trim counter's _total suffix so the unit is placed before it.
+    if (prometheusType == PrometheusType.COUNTER && name.endsWith(TOTAL_SUFFIX)) {
+      name = name.substring(0, name.length() - TOTAL_SUFFIX.length());
+    }
     // append prometheus unit if not null or empty.
-    if (!StringUtils.isNullOrEmpty(prometheusEquivalentUnit)
-        && !name.contains(prometheusEquivalentUnit)) {
+    if (shouldAppendUnit) {
       name = name + "_" + prometheusEquivalentUnit;
     }
 
-    // special case - counter
-    if (prometheusType == PrometheusType.COUNTER && !name.contains("total")) {
-      name = name + "_total";
+    // replace _total suffix, or add if it wasn't already present.
+    if (prometheusType == PrometheusType.COUNTER) {
+      name = name + TOTAL_SUFFIX;
     }
     // special case - gauge
     if (rawMetric.getUnit().equals("1")

diff --git a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Serializer.java b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Serializer.java
@@ -86,7 +86,7 @@ static Serializer create(@Nullable String acceptHeader, Predicate<String> filter
 
   abstract String contentType();
 
-  abstract String headerName(String name, PrometheusType type);
+  abstract String headerName(String name, MetricData rawMetric, PrometheusType type);
 
   abstract void writeHelp(Writer writer, String description) throws IOException;
 
@@ -158,7 +158,7 @@ private void write(List<MetricData> metrics, String metricName, Writer writer)
     // Write header based on first metric
     MetricData first = metrics.get(0);
     PrometheusType type = PrometheusType.forMetric(first);
-    String headerName = headerName(NameSanitizer.INSTANCE.apply(first.getName()), type);
+    String headerName = headerName(metricName, first, type);
     String description = metrics.get(0).getDescription();
 
     writer.write("# TYPE ");
@@ -512,10 +512,7 @@ String contentType() {
     }
 
     @Override
-    String headerName(String name, PrometheusType type) {
-      if (type == PrometheusType.COUNTER && !name.endsWith("_total")) {
-        return name + "_total";
-      }
+    String headerName(String name, MetricData rawMetric, PrometheusType type) {
       return name;
     }
 
@@ -568,7 +565,13 @@ String contentType() {
     }
 
     @Override
-    String headerName(String name, PrometheusType type) {
+    String headerName(String name, MetricData rawMetric, PrometheusType type) {
+      // If the name didn't originally have a _total suffix, and we added it later, omit it from the
+      // header.
+      String sanitizedOriginalName = NameSanitizer.INSTANCE.apply(rawMetric.getName());
+      if (!sanitizedOriginalName.endsWith("_total") && (type == PrometheusType.COUNTER)) {
+        return name.substring(0, name.length() - "_total".length());
+      }
       return name;
     }
 

diff --git a/...ometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java b/...ometheus/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusHttpServerTest.java
@@ -287,8 +287,8 @@ void fetch_DuplicateMetrics() {
                 + "# TYPE otel_scope_info info\n"
                 + "# HELP otel_scope_info Scope metadata\n"
                 + "otel_scope_info{otel_scope_name=\"scope2\"} 1\n"
-                + "# TYPE foo_total counter\n"
-                + "# HELP foo_total description1\n"
+                + "# TYPE foo_unit_total counter\n"
+                + "# HELP foo_unit_total description1\n"
                 + "foo_unit_total{otel_scope_name=\"scope1\"} 1.0 0\n"
                 + "foo_unit_total{otel_scope_name=\"scope2\"} 2.0 0\n");
 

diff --git a/...us/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusMetricNameMapperTest.java b/...us/src/test/java/io/opentelemetry/exporter/prometheus/PrometheusMetricNameMapperTest.java
@@ -115,7 +115,7 @@ private static Stream<Arguments> provideRawMetricDataForTest() {
         // metric unit is not appended if the name already contains the unit - order matters
         Arguments.of(
             createSampleMetricData("metric_total_hertz", "hertz_total", PrometheusType.COUNTER),
-            "metric_total_hertz_hertz_total"),
+            "metric_total_hertz_hertz_total_total"),
         // metric name cannot start with a number
         Arguments.of(
             createSampleMetricData("2_metric_name", "By", PrometheusType.SUMMARY),