Prometheus: append postfix only if missing (#773)

* add postfix only if missinig * move normalizing counter name to its own function * add test without unts Co-authored-by: Eugene Miretsky <eugene@paytm.com>
kamon-io · May 18, 2020 · 1cbdc61 · 1cbdc61
1 parent 3d28f40
commit 1cbdc61
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 3 deletions.
diff --git a/reporters/kamon-prometheus/src/main/scala/kamon/prometheus/ScrapeDataBuilder.scala b/reporters/kamon-prometheus/src/main/scala/kamon/prometheus/ScrapeDataBuilder.scala
@@ -51,7 +51,7 @@ class ScrapeDataBuilder(prometheusConfig: PrometheusSettings.Generic, environmen
 
   private def appendCounterMetric(metric: MetricSnapshot.Values[Long]): Unit = {
     val unit = metric.settings.unit
-    val normalizedMetricName = normalizeMetricName(metric.name, unit) + "_total"
+    val normalizedMetricName = normalizeCounterMetricName(metric.name, unit)
 
     if(metric.description.nonEmpty)
       append("# HELP ").append(normalizedMetricName).append(" ").append(metric.description).append("\n")
@@ -190,15 +190,27 @@ class ScrapeDataBuilder(prometheusConfig: PrometheusSettings.Generic, environmen
     if(allTags.nonEmpty) buffer.append("}")
   }
 
+  private def normalizeCounterMetricName(metricName: String, unit: MeasurementUnit): String = {
+    val normalizedMetricName = metricName.map(validNameChar(_))
+
+    unit.dimension match  {
+      case Time         => addPostfixOnlyIfMissing(normalizedMetricName, "_seconds_total")
+      case Information  => addPostfixOnlyIfMissing(normalizedMetricName, "_bytes_total")
+      case _            => addPostfixOnlyIfMissing(normalizedMetricName, "_total")
+    }
+  }
   private def normalizeMetricName(metricName: String, unit: MeasurementUnit): String = {
     val normalizedMetricName = metricName.map(validNameChar(_))
 
     unit.dimension match  {
-      case Time         => normalizedMetricName + "_seconds"
-      case Information  => normalizedMetricName + "_bytes"
+      case Time         => addPostfixOnlyIfMissing(normalizedMetricName, "_seconds")
+      case Information  => addPostfixOnlyIfMissing(normalizedMetricName, "_bytes")
       case _            => normalizedMetricName
     }
   }
+  private def addPostfixOnlyIfMissing(metricName: String, postfix: String) =
+    if (metricName.endsWith(postfix)) metricName
+    else metricName + postfix
 
   private def normalizeLabelName(label: String): String =
     label.map(validLabelChar)

diff --git a/reporters/kamon-prometheus/src/test/scala/kamon/prometheus/ScrapeDataBuilderSpec.scala b/reporters/kamon-prometheus/src/test/scala/kamon/prometheus/ScrapeDataBuilderSpec.scala
@@ -60,6 +60,38 @@ class ScrapeDataBuilderSpec extends WordSpec with Matchers {
         """.stripMargin.trim()
       }
     }
+    "not add extra '_total' postfix if metric name already ends with it when using units" in {
+      val counterOne = MetricSnapshotBuilder.counter("app:counter-one-seconds-total", "", TagSet.of("tag.with.dots", "value"), time.seconds, 10)
+      val gaugeOne = MetricSnapshotBuilder.gauge("gauge-one-seconds", "", TagSet.of("tag-with-dashes", "value"), time.seconds, 20)
+
+      builder()
+        .appendCounters(Seq(counterOne))
+        .appendGauges(Seq(gaugeOne))
+        .build() should include {
+        """
+          |# TYPE app:counter_one_seconds_total counter
+          |app:counter_one_seconds_total{tag_with_dots="value"} 10.0
+          |# TYPE gauge_one_seconds gauge
+          |gauge_one_seconds{tag_with_dashes="value"} 20.0
+        """.stripMargin.trim()
+      }
+    }
+    "not add extra '_total' postfix if metric name already ends with it when not using units" in {
+      val counterOne = MetricSnapshotBuilder.counter("app:counter-one-total", "", TagSet.of("tag.with.dots", "value"), 10)
+      val gaugeOne = MetricSnapshotBuilder.gauge("gauge-one", "", TagSet.of("tag-with-dashes", "value"), 20)
+
+      builder()
+        .appendCounters(Seq(counterOne))
+        .appendGauges(Seq(gaugeOne))
+        .build() should include {
+        """
+          |# TYPE app:counter_one_total counter
+          |app:counter_one_total{tag_with_dots="value"} 10.0
+          |# TYPE gauge_one gauge
+          |gauge_one{tag_with_dashes="value"} 20.0
+        """.stripMargin.trim()
+      }
+    }
 
 //    "append counters and group them together by metric name" in {
 //      val counterOne = MetricValue("counter-one", Map.empty, none, 10)