From 4df571e02d2c938f28fc478e7bd7d06512541f80 Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 10:40:53 +0200
Subject: [PATCH 1/9] sanitize metric names in the prometheus exporter
---
exporters/prometheus/exporter.go | 22 ++++++++++++++---
exporters/prometheus/exporter_test.go | 10 ++++----
.../prometheus/testdata/sanitized_names.txt | 24 +++++++++++++++++++
3 files changed, 48 insertions(+), 8 deletions(-)
create mode 100644 exporters/prometheus/testdata/sanitized_names.txt
diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go
index a548de3523c..5a32d1c6096 100644
--- a/exporters/prometheus/exporter.go
+++ b/exporters/prometheus/exporter.go
@@ -145,7 +145,7 @@ func getHistogramMetricData(histogram metricdata.Histogram, m metricdata.Metrics
dataPoints := make([]*metricData, 0, len(histogram.DataPoints))
for _, dp := range histogram.DataPoints {
keys, values := getAttrs(dp.Attributes)
- desc := prometheus.NewDesc(m.Name, m.Description, keys, nil)
+ desc := prometheus.NewDesc(sanitizeName(m.Name), m.Description, keys, nil)
buckets := make(map[float64]uint64, len(dp.Bounds))
for i, bound := range dp.Bounds {
buckets[bound] = dp.BucketCounts[i]
@@ -168,7 +168,7 @@ func getSumMetricData[N int64 | float64](sum metricdata.Sum[N], m metricdata.Met
dataPoints := make([]*metricData, 0, len(sum.DataPoints))
for _, dp := range sum.DataPoints {
keys, values := getAttrs(dp.Attributes)
- desc := prometheus.NewDesc(m.Name, m.Description, keys, nil)
+ desc := prometheus.NewDesc(sanitizeName(m.Name), m.Description, keys, nil)
md := &metricData{
name: m.Name,
description: desc,
@@ -185,7 +185,7 @@ func getGaugeMetricData[N int64 | float64](gauge metricdata.Gauge[N], m metricda
dataPoints := make([]*metricData, 0, len(gauge.DataPoints))
for _, dp := range gauge.DataPoints {
keys, values := getAttrs(dp.Attributes)
- desc := prometheus.NewDesc(m.Name, m.Description, keys, nil)
+ desc := prometheus.NewDesc(sanitizeName(m.Name), m.Description, keys, nil)
md := &metricData{
name: m.Name,
description: desc,
@@ -233,3 +233,19 @@ func sanitizeRune(r rune) rune {
}
return '_'
}
+
+func sanitizeName(n string) string {
+ if len(n) == 0 {
+ return n
+ }
+ var sn = make([]string, len(n))
+ for i, b := range n {
+ if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || b == ':' || (b >= '0' && b <= '9' && i > 0)) {
+ sn[i] = "_"
+ } else {
+ sn[i] = string(b)
+ }
+ }
+
+ return strings.Join(sn, "")
+}
diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go
index 32733a7a81c..b88bf937bc1 100644
--- a/exporters/prometheus/exporter_test.go
+++ b/exporters/prometheus/exporter_test.go
@@ -106,8 +106,8 @@ func TestPrometheusExporter(t *testing.T) {
},
},
{
- name: "invalid instruments are dropped",
- expectedFile: "testdata/gauge.txt",
+ name: "invalid instruments are renamed",
+ expectedFile: "testdata/sanitized_names.txt",
recordMetrics: func(ctx context.Context, meter otelmetric.Meter) {
attrs := []attribute.KeyValue{
attribute.Key("A").String("B"),
@@ -120,15 +120,15 @@ func TestPrometheusExporter(t *testing.T) {
gauge.Add(ctx, -25, attrs...)
// Invalid, should be dropped.
- gauge, err = meter.SyncFloat64().UpDownCounter("invalid.gauge.name")
+ gauge, err = meter.SyncFloat64().UpDownCounter("invalid.gauge.name", instrument.WithDescription("a gauge with an invalid name"))
require.NoError(t, err)
gauge.Add(ctx, 100, attrs...)
- counter, err := meter.SyncFloat64().Counter("invalid.counter.name")
+ counter, err := meter.SyncFloat64().Counter("invalid.counter.name", instrument.WithDescription("a counter with an invalid name"))
require.NoError(t, err)
counter.Add(ctx, 100, attrs...)
- histogram, err := meter.SyncFloat64().Histogram("invalid.hist.name")
+ histogram, err := meter.SyncFloat64().Histogram("invalid.hist.name", instrument.WithDescription("a histogram with an invalid name"))
require.NoError(t, err)
histogram.Record(ctx, 23, attrs...)
},
diff --git a/exporters/prometheus/testdata/sanitized_names.txt b/exporters/prometheus/testdata/sanitized_names.txt
new file mode 100644
index 00000000000..03bc9268f62
--- /dev/null
+++ b/exporters/prometheus/testdata/sanitized_names.txt
@@ -0,0 +1,24 @@
+# HELP bar a fun little gauge
+# TYPE bar counter
+bar{A="B",C="D"} 75
+# HELP invalid_counter_name a counter with an invalid name
+# TYPE invalid_counter_name counter
+invalid_counter_name{A="B",C="D"} 100
+# HELP invalid_gauge_name a gauge with an invalid name
+# TYPE invalid_gauge_name counter
+invalid_gauge_name{A="B",C="D"} 100
+# HELP invalid_hist_name a histogram with an invalid name
+# TYPE invalid_hist_name histogram
+invalid_hist_name_bucket{A="B",C="D",le="0"} 0
+invalid_hist_name_bucket{A="B",C="D",le="5"} 0
+invalid_hist_name_bucket{A="B",C="D",le="10"} 0
+invalid_hist_name_bucket{A="B",C="D",le="25"} 1
+invalid_hist_name_bucket{A="B",C="D",le="50"} 0
+invalid_hist_name_bucket{A="B",C="D",le="75"} 0
+invalid_hist_name_bucket{A="B",C="D",le="100"} 0
+invalid_hist_name_bucket{A="B",C="D",le="250"} 0
+invalid_hist_name_bucket{A="B",C="D",le="500"} 0
+invalid_hist_name_bucket{A="B",C="D",le="1000"} 0
+invalid_hist_name_bucket{A="B",C="D",le="+Inf"} 1
+invalid_hist_name_sum{A="B",C="D"} 23
+invalid_hist_name_count{A="B",C="D"} 1
From 3adba391e5942d574d69c8f8186ba7eab00745d3 Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 10:46:20 +0200
Subject: [PATCH 2/9] add changelog entry
---
CHANGELOG.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 40953d25784..4a7c6b59a2a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -19,6 +19,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The metric SDK in `go.opentelemetry.io/otel/sdk/metric` is completely refactored to comply with the OpenTelemetry specification.
Please see the package documentation for how the new SDK is initialized and configured. (#3175)
- Update the minimum supported go version to go1.18. Removes support for go1.17 (#3179)
+- Instead of dropping metric, the Prometheus exporter will replace any invalid character in metric names with `_`. (#3212)
### Removed
From ddd7cc6e2f6d2b4e4208d5195c70c0afbd463c33 Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 11:09:24 +0200
Subject: [PATCH 3/9] fix now invalid comment
---
exporters/prometheus/exporter_test.go | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go
index b88bf937bc1..1d3426fa9b3 100644
--- a/exporters/prometheus/exporter_test.go
+++ b/exporters/prometheus/exporter_test.go
@@ -119,7 +119,7 @@ func TestPrometheusExporter(t *testing.T) {
gauge.Add(ctx, 100, attrs...)
gauge.Add(ctx, -25, attrs...)
- // Invalid, should be dropped.
+ // Invalid, will be renamed.
gauge, err = meter.SyncFloat64().UpDownCounter("invalid.gauge.name", instrument.WithDescription("a gauge with an invalid name"))
require.NoError(t, err)
gauge.Add(ctx, 100, attrs...)
From f0fa2f1bd258959ec6f8bc911a3f96bc4fce4036 Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 17:37:29 +0200
Subject: [PATCH 4/9] replace sanitizeName algorithm with one based on
strings.Map
---
exporters/prometheus/exporter.go | 46 +++++++++++++++++++++++++++-----
1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go
index 5a32d1c6096..bc46df5bc5c 100644
--- a/exporters/prometheus/exporter.go
+++ b/exporters/prometheus/exporter.go
@@ -235,17 +235,49 @@ func sanitizeRune(r rune) rune {
}
func sanitizeName(n string) string {
- if len(n) == 0 {
+ // This algorithm is based on strings.Map from Go 1.19.
+ const (
+ replacement = '_'
+ width = 1
+ )
+
+ valid := func(i int, r rune) bool {
+ // Taken from
+ // https://github.com/prometheus/common/blob/dfbc25bd00225c70aca0d94c3c4bb7744f28ace0/model/metric.go#L92-L102
+ if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || r == '_' || r == ':' || (r >= '0' && r <= '9' && i > 0) {
+ return true
+ }
+ return false
+ }
+
+ // This output buffer b is initialized on demand, the first time a character
+ // needs to be replaced.
+ var b strings.Builder
+ for i, c := range n {
+ if valid(i, c) {
+ continue
+ }
+ b.Grow(len(n))
+ b.WriteString(n[:i])
+ b.WriteRune(replacement)
+ n = n[i+width:]
+ break
+ }
+
+ // Fast path for unchanged input.
+ if b.Cap() == 0 { // b.Grow was not called above.
return n
}
- var sn = make([]string, len(n))
- for i, b := range n {
- if !((b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || b == '_' || b == ':' || (b >= '0' && b <= '9' && i > 0)) {
- sn[i] = "_"
+
+ for _, c := range n {
+ // Due to inlining, it is more performant to invoke WriteByte rather then
+ // WriteRune.
+ if valid(1, c) { // We are guaranteed to not be at the start.
+ b.WriteByte(byte(c))
} else {
- sn[i] = string(b)
+ b.WriteByte(byte(replacement))
}
}
- return strings.Join(sn, "")
+ return b.String()
}
From bc12d2e9010a0215453de64d634aefbe147df675 Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 17:46:01 +0200
Subject: [PATCH 5/9] check that digits as first characters are replaced
---
exporters/prometheus/exporter_test.go | 2 +-
exporters/prometheus/testdata/sanitized_names.txt | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go
index 1d3426fa9b3..7b9986319b9 100644
--- a/exporters/prometheus/exporter_test.go
+++ b/exporters/prometheus/exporter_test.go
@@ -124,7 +124,7 @@ func TestPrometheusExporter(t *testing.T) {
require.NoError(t, err)
gauge.Add(ctx, 100, attrs...)
- counter, err := meter.SyncFloat64().Counter("invalid.counter.name", instrument.WithDescription("a counter with an invalid name"))
+ counter, err := meter.SyncFloat64().Counter("0invalid.counter.name", instrument.WithDescription("a counter with an invalid name"))
require.NoError(t, err)
counter.Add(ctx, 100, attrs...)
diff --git a/exporters/prometheus/testdata/sanitized_names.txt b/exporters/prometheus/testdata/sanitized_names.txt
index 03bc9268f62..1bf66fca8b2 100644
--- a/exporters/prometheus/testdata/sanitized_names.txt
+++ b/exporters/prometheus/testdata/sanitized_names.txt
@@ -1,9 +1,9 @@
# HELP bar a fun little gauge
# TYPE bar counter
bar{A="B",C="D"} 75
-# HELP invalid_counter_name a counter with an invalid name
-# TYPE invalid_counter_name counter
-invalid_counter_name{A="B",C="D"} 100
+# HELP _invalid_counter_name a counter with an invalid name
+# TYPE _invalid_counter_name counter
+_invalid_counter_name{A="B",C="D"} 100
# HELP invalid_gauge_name a gauge with an invalid name
# TYPE invalid_gauge_name counter
invalid_gauge_name{A="B",C="D"} 100
From 30abe6786a1127c8543361105660f530d519ae1c Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 19:24:46 +0200
Subject: [PATCH 6/9] Update exporters/prometheus/exporter.go
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
---
exporters/prometheus/exporter.go | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go
index bc46df5bc5c..a3055c319d0 100644
--- a/exporters/prometheus/exporter.go
+++ b/exporters/prometheus/exporter.go
@@ -236,10 +236,7 @@ func sanitizeRune(r rune) rune {
func sanitizeName(n string) string {
// This algorithm is based on strings.Map from Go 1.19.
- const (
- replacement = '_'
- width = 1
- )
+ const replacement = '_'
valid := func(i int, r rune) bool {
// Taken from
@@ -250,16 +247,24 @@ func sanitizeName(n string) string {
return false
}
- // This output buffer b is initialized on demand, the first time a character
- // needs to be replaced.
+ // This output buffer b is initialized on demand, the first time a
+ // character needs to be replaced.
var b strings.Builder
for i, c := range n {
if valid(i, c) {
continue
}
+
+ if i == 0 && c >= '0' && c <= '9' {
+ // Prefix leading number with replacement character.
+ b.Grow(len(n) + 1)
+ b.WriteByte(byte(replacement))
+ break
+ }
b.Grow(len(n))
b.WriteString(n[:i])
- b.WriteRune(replacement)
+ b.WriteByte(byte(replacement))
+ width := utf8.RuneLen(c)
n = n[i+width:]
break
}
From 0ad5975d701e358ce70fffd2714f5496b82b132e Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 19:26:42 +0200
Subject: [PATCH 7/9] add missing utf8 import
---
exporters/prometheus/exporter.go | 1 +
1 file changed, 1 insertion(+)
diff --git a/exporters/prometheus/exporter.go b/exporters/prometheus/exporter.go
index a3055c319d0..ab51c8f9631 100644
--- a/exporters/prometheus/exporter.go
+++ b/exporters/prometheus/exporter.go
@@ -22,6 +22,7 @@ import (
"sort"
"strings"
"unicode"
+ "unicode/utf8"
"github.com/prometheus/client_golang/prometheus"
From a38727c0156fe067f7ea6a3bedc353db8f82e123 Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 19:26:50 +0200
Subject: [PATCH 8/9] add explicit tests for sanitizeName
---
exporters/prometheus/exporter_test.go | 30 +++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/exporters/prometheus/exporter_test.go b/exporters/prometheus/exporter_test.go
index 7b9986319b9..2fe3faf8816 100644
--- a/exporters/prometheus/exporter_test.go
+++ b/exporters/prometheus/exporter_test.go
@@ -158,3 +158,33 @@ func TestPrometheusExporter(t *testing.T) {
})
}
}
+
+func TestSantitizeName(t *testing.T) {
+ tests := []struct {
+ input string
+ want string
+ }{
+ {"nam€_with_3_width_rune", "nam__with_3_width_rune"},
+ {"`", "_"},
+ {
+ `! "#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMNOPQRSTUVWKYZ[]\^_abcdefghijklmnopqrstuvwkyz{|}~`,
+ `________________0123456789:______ABCDEFGHIJKLMNOPQRSTUVWKYZ_____abcdefghijklmnopqrstuvwkyz____`,
+ },
+
+ // Test cases taken from
+ // https://github.com/prometheus/common/blob/dfbc25bd00225c70aca0d94c3c4bb7744f28ace0/model/metric_test.go#L85-L136
+ {"Avalid_23name", "Avalid_23name"},
+ {"_Avalid_23name", "_Avalid_23name"},
+ {"1valid_23name", "_1valid_23name"},
+ {"avalid_23name", "avalid_23name"},
+ {"Ava:lid_23name", "Ava:lid_23name"},
+ {"a lid_23name", "a_lid_23name"},
+ {":leading_colon", ":leading_colon"},
+ {"colon:in:the:middle", "colon:in:the:middle"},
+ {"", ""},
+ }
+
+ for _, test := range tests {
+ require.Equalf(t, test.want, sanitizeName(test.input), "input: %q", test.input)
+ }
+}
From 8e8fdd87ab67803de9f7de5d947a78423a4b76ab Mon Sep 17 00:00:00 2001
From: Damien Mathieu <42@dmathieu.com>
Date: Wed, 21 Sep 2022 19:31:42 +0200
Subject: [PATCH 9/9] fix testdata with digit prepend
---
exporters/prometheus/testdata/sanitized_names.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/exporters/prometheus/testdata/sanitized_names.txt b/exporters/prometheus/testdata/sanitized_names.txt
index 1bf66fca8b2..5e9516716ae 100644
--- a/exporters/prometheus/testdata/sanitized_names.txt
+++ b/exporters/prometheus/testdata/sanitized_names.txt
@@ -1,9 +1,9 @@
# HELP bar a fun little gauge
# TYPE bar counter
bar{A="B",C="D"} 75
-# HELP _invalid_counter_name a counter with an invalid name
-# TYPE _invalid_counter_name counter
-_invalid_counter_name{A="B",C="D"} 100
+# HELP _0invalid_counter_name a counter with an invalid name
+# TYPE _0invalid_counter_name counter
+_0invalid_counter_name{A="B",C="D"} 100
# HELP invalid_gauge_name a gauge with an invalid name
# TYPE invalid_gauge_name counter
invalid_gauge_name{A="B",C="D"} 100