[prometheusremotewrite] fix: counter name check (#2613)

* [prometheusremotewrite] fix: counter name check fixes #2608 Ensure that the metric name is greater than the length of the counter suffix ("_total") before checking if it contains the counter suffix to prevent crashes for short (smaller or eq to the length of the suffix) metric names. The edge case where the metric name is "_total" is not handled (considered not to have the suffix), but should not occur IRL. Also fix the spelling of "delimiter". Signed-off-by: naseemkullah <[email protected]> * use strings.HasSuffix() Signed-off-by: naseemkullah <[email protected]> * add already suffixed counter test Signed-off-by: naseemkullah <[email protected]>
open-telemetry · Mar 8, 2021 · 35cf41e · 35cf41e
1 parent 65c4c4a
commit 35cf41e
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 19 deletions.
diff --git a/exporter/prometheusremotewriteexporter/helper.go b/exporter/prometheusremotewriteexporter/helper.go
@@ -31,16 +31,16 @@ import (
 )
 
 const (
-	nameStr     = "__name__"
-	sumStr      = "_sum"
-	countStr    = "_count"
-	bucketStr   = "_bucket"
-	leStr       = "le"
-	quantileStr = "quantile"
-	pInfStr     = "+Inf"
-	totalStr    = "total"
-	delimeter   = "_"
-	keyStr      = "key"
+	nameStr       = "__name__"
+	sumStr        = "_sum"
+	countStr      = "_count"
+	bucketStr     = "_bucket"
+	leStr         = "le"
+	quantileStr   = "quantile"
+	pInfStr       = "+Inf"
+	counterSuffix = "_total"
+	delimiter     = "_"
+	keyStr        = "key"
 )
 
 // ByLabelName enables the usage of sort.Sort() with a slice of labels
@@ -188,14 +188,11 @@ func getPromMetricName(metric *otlp.Metric, ns string) string {
 	b.WriteString(ns)
 
 	if b.Len() > 0 {
-		b.WriteString(delimeter)
+		b.WriteString(delimiter)
 	}
 	name := metric.GetName()
 	b.WriteString(name)
 
-	// do not add the total suffix if the metric name already ends in "total"
-	isCounter = isCounter && name[len(name)-len(totalStr):] != totalStr
-
 	// Including units makes two metrics with the same name and label set belong to two different TimeSeries if the
 	// units are different.
 	/*
@@ -205,9 +202,8 @@ func getPromMetricName(metric *otlp.Metric, ns string) string {
 		}
 	*/
 
-	if b.Len() > 0 && isCounter {
-		b.WriteString(delimeter)
-		b.WriteString(totalStr)
+	if b.Len() > 0 && isCounter && !strings.HasSuffix(name, counterSuffix) {
+		b.WriteString(counterSuffix)
 	}
 	return sanitize(b.String())
 }
@@ -262,7 +258,7 @@ func sanitize(s string) string {
 	// See https://github.com/orijtech/prometheus-go-metrics-exporter/issues/4.
 	s = strings.Map(sanitizeRune, s)
 	if unicode.IsDigit(rune(s[0])) {
-		s = keyStr + delimeter + s
+		s = keyStr + delimiter + s
 	}
 	if s[0] == '_' {
 		s = keyStr + s

diff --git a/exporter/prometheusremotewriteexporter/helper_test.go b/exporter/prometheusremotewriteexporter/helper_test.go
@@ -287,7 +287,13 @@ func Test_getPromMetricName(t *testing.T) {
 			"total_suffix",
 			validMetrics1[validIntSum],
 			ns1,
-			"test_ns_" + validIntSum + delimeter + totalStr,
+			"test_ns_" + validIntSum + counterSuffix,
+		},
+		{
+			"already_has_total_suffix",
+			validMetrics1[suffixedCounter],
+			ns1,
+			"test_ns_" + suffixedCounter,
 		},
 		{
 			"dirty_string",

diff --git a/exporter/prometheusremotewriteexporter/testutil_test.go b/exporter/prometheusremotewriteexporter/testutil_test.go
@@ -94,6 +94,7 @@ var (
 	validIntHistogram    = "valid_IntHistogram"
 	validDoubleHistogram = "valid_DoubleHistogram"
 	validDoubleSummary   = "valid_DoubleSummary"
+	suffixedCounter      = "valid_IntSum_total"
 
 	validIntGaugeDirty = "*valid_IntGauge$"
 
@@ -136,6 +137,18 @@ var (
 				},
 			},
 		},
+		suffixedCounter: {
+			Name: suffixedCounter,
+			Data: &otlp.Metric_IntSum{
+				IntSum: &otlp.IntSum{
+					DataPoints: []*otlp.IntDataPoint{
+						getIntDataPoint(lbs1, intVal1, time1),
+						nil,
+					},
+					AggregationTemporality: otlp.AggregationTemporality_AGGREGATION_TEMPORALITY_CUMULATIVE,
+				},
+			},
+		},
 		validDoubleSum: {
 			Name: validDoubleSum,
 			Data: &otlp.Metric_DoubleSum{