From 0a925d70cb272d9b00e99629b737b40ce585d2e7 Mon Sep 17 00:00:00 2001
From: Pranav Sharma <sharmapranav@google.com>
Date: Sat, 29 Apr 2023 16:17:26 +0000
Subject: [PATCH] Appends units if not present in metric name

---
 .../exporter/prometheus/Serializer.java       | 13 ++++---
 .../exporter/prometheus/SerializerTest.java   | 36 +++++++++++++++++--
 2 files changed, 42 insertions(+), 7 deletions(-)

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
index fde8cdbd597..dca13da5f52 100644
--- a/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Serializer.java
+++ b/exporters/prometheus/src/main/java/io/opentelemetry/exporter/prometheus/Serializer.java
@@ -653,24 +653,27 @@ static Collection<? extends PointData> getPoints(MetricData metricData) {
 
   // Visible for testing
   static String metricName(MetricData rawMetric, PrometheusType type) {
-    String name = rawMetric.getName();
+    String name = NameSanitizer.INSTANCE.apply(rawMetric.getName());
     String prometheusEquivalentUnit =
         PrometheusUnitsHelper.getEquivalentPrometheusUnit(rawMetric.getUnit());
     // append prometheus unit if not null or empty.
-    if (!StringUtils.isNullOrEmpty(prometheusEquivalentUnit)) {
+    if (!StringUtils.isNullOrEmpty(prometheusEquivalentUnit)
+        && !name.contains(prometheusEquivalentUnit)) {
       name = name + "_" + prometheusEquivalentUnit;
     }
 
     // special case - counter
-    if (type == PrometheusType.COUNTER) {
+    if (type == PrometheusType.COUNTER && !name.contains("total")) {
       name = name + "_total";
     }
     // special case - gauge
-    if (rawMetric.getUnit().equals("1") && type == PrometheusType.GAUGE) {
+    if (rawMetric.getUnit().equals("1")
+        && type == PrometheusType.GAUGE
+        && !name.contains("ratio")) {
       name = name + "_ratio";
     }
 
-    return NameSanitizer.INSTANCE.apply(name);
+    return name;
   }
 
   private static double getExemplarValue(ExemplarData exemplar) {
diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/SerializerTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/SerializerTest.java
index 15e004735ed..89dd69b424d 100644
--- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/SerializerTest.java
+++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/SerializerTest.java
@@ -486,7 +486,7 @@ private static Stream<Arguments> provideRawMetricDataForTest() {
         // special case for gauge
         Arguments.of(
             sampleMetricDataGenerator("sample", "1", PrometheusType.GAUGE), "sample_ratio"),
-        // special case for gauge with drop eligible unit
+        // special case for gauge with drop - metric unit should match "1"
         Arguments.of(
             sampleMetricDataGenerator("sample", "1{dropped}", PrometheusType.GAUGE), "sample"),
         // Gauge without "1" as unit
@@ -496,9 +496,12 @@ private static Stream<Arguments> provideRawMetricDataForTest() {
         Arguments.of(
             sampleMetricDataGenerator("sample", "unit", PrometheusType.COUNTER),
             "sample_unit_total"),
-        // special case unit "1", but no gauge
+        // special case unit "1", but no gauge - "1" is dropped
         Arguments.of(
             sampleMetricDataGenerator("sample", "1", PrometheusType.COUNTER), "sample_total"),
+        // units expressed as numbers other than 1 are retained
+        Arguments.of(
+            sampleMetricDataGenerator("sample", "2", PrometheusType.COUNTER), "sample_2_total"),
         // metric name with unsupported characters
         Arguments.of(
             sampleMetricDataGenerator("s%%ple", "%/m", PrometheusType.SUMMARY),
@@ -510,6 +513,35 @@ private static Stream<Arguments> provideRawMetricDataForTest() {
         // metric unit as a number other than 1 is not treated specially
         Arguments.of(
             sampleMetricDataGenerator("metric_name", "2", PrometheusType.SUMMARY), "metric_name_2"),
+        // metric unit is not appended if the name already contains the unit
+        Arguments.of(
+            sampleMetricDataGenerator("metric_name_total", "total", PrometheusType.COUNTER),
+            "metric_name_total"),
+        // metric unit is not appended if the name already contains the unit - special case for
+        // total with non-counter type
+        Arguments.of(
+            sampleMetricDataGenerator("metric_name_total", "total", PrometheusType.SUMMARY),
+            "metric_name_total"),
+        // metric unit not appended if present in metric name - special case for ratio
+        Arguments.of(
+            sampleMetricDataGenerator("metric_name_ratio", "1", PrometheusType.GAUGE),
+            "metric_name_ratio"),
+        // metric unit not appended if present in metric name - special case for ratio - unit not gauge
+        Arguments.of(
+            sampleMetricDataGenerator("metric_name_ratio", "1", PrometheusType.SUMMARY),
+            "metric_name_ratio"),
+        // metric unit is not appended if the name already contains the unit - unit can be anywhere
+        Arguments.of(
+            sampleMetricDataGenerator("metric_hertz", "hertz", PrometheusType.GAUGE),
+            "metric_hertz"),
+        // metric unit is not appended if the name already contains the unit - applies to every unit
+        Arguments.of(
+            sampleMetricDataGenerator("metric_hertz_total", "hertz_total", PrometheusType.COUNTER),
+            "metric_hertz_total"),
+        // metric unit is not appended if the name already contains the unit - order matters
+        Arguments.of(
+            sampleMetricDataGenerator("metric_total_hertz", "hertz_total", PrometheusType.COUNTER),
+            "metric_total_hertz_hertz_total"),
         // metric name cannot start with a number
         Arguments.of(
             sampleMetricDataGenerator("2_metric_name", "By", PrometheusType.SUMMARY),