From 4b1f01e45d044233c51538096b1d6f99650ef522 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Mon, 14 Aug 2023 16:14:40 +0200
Subject: [PATCH] lib/promrelabel: properly replace `:` char with `_` in metric
 names when -usePromCompatibleNaming command-line flag is set

This addresses https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3113#issuecomment-1275077071 comment from @johnseekins
---
 app/vmagent/remotewrite/relabel.go     |  4 ++--
 app/vmctl/opentsdb/parser.go           |  4 ++--
 app/vminsert/relabel/relabel.go        |  4 ++--
 docs/CHANGELOG.md                      |  1 +
 lib/promrelabel/relabel.go             | 25 ++++++++++++++++++------
 lib/promrelabel/relabel_test.go        | 22 ++++++++++++++++++---
 lib/promrelabel/relabel_timing_test.go | 27 ++++++++++++++++++++++----
 7 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/app/vmagent/remotewrite/relabel.go b/app/vmagent/remotewrite/relabel.go
index b1cfc16e6..146930f8d 100644
--- a/app/vmagent/remotewrite/relabel.go
+++ b/app/vmagent/remotewrite/relabel.go
@@ -114,9 +114,9 @@ func (rctx *relabelCtx) applyRelabeling(tss []prompbmarshal.TimeSeries, extraLab
 			for j := range tmpLabels {
 				label := &tmpLabels[j]
 				if label.Name == "__name__" {
-					label.Value = promrelabel.SanitizeName(label.Value)
+					label.Value = promrelabel.SanitizeMetricName(label.Value)
 				} else {
-					label.Name = promrelabel.SanitizeName(label.Name)
+					label.Name = promrelabel.SanitizeLabelName(label.Name)
 				}
 			}
 		}
diff --git a/app/vmctl/opentsdb/parser.go b/app/vmctl/opentsdb/parser.go
index 6993e8d13..6c2f209e7 100644
--- a/app/vmctl/opentsdb/parser.go
+++ b/app/vmctl/opentsdb/parser.go
@@ -180,7 +180,7 @@ func modifyData(msg Metric, normalize bool) (Metric, error) {
 	/*
 		replace bad characters in metric name with _ per the data model
 	*/
-	finalMsg.Metric = promrelabel.SanitizeName(name)
+	finalMsg.Metric = promrelabel.SanitizeMetricName(name)
 	// replace bad characters in tag keys with _ per the data model
 	for key, value := range msg.Tags {
 		// if normalization requested, lowercase the key and value
@@ -191,7 +191,7 @@ func modifyData(msg Metric, normalize bool) (Metric, error) {
 		/*
 			replace all explicitly bad characters with _
 		*/
-		key = promrelabel.SanitizeName(key)
+		key = promrelabel.SanitizeLabelName(key)
 		// tags that start with __ are considered custom stats for internal prometheus stuff, we should drop them
 		if !strings.HasPrefix(key, "__") {
 			finalMsg.Tags[key] = value
diff --git a/app/vminsert/relabel/relabel.go b/app/vminsert/relabel/relabel.go
index fcb107c24..3409fb4c0 100644
--- a/app/vminsert/relabel/relabel.go
+++ b/app/vminsert/relabel/relabel.go
@@ -134,9 +134,9 @@ func (ctx *Ctx) ApplyRelabeling(labels []prompb.Label) []prompb.Label {
 		for i := range tmpLabels {
 			label := &tmpLabels[i]
 			if label.Name == "__name__" {
-				label.Value = promrelabel.SanitizeName(label.Value)
+				label.Value = promrelabel.SanitizeMetricName(label.Value)
 			} else {
-				label.Name = promrelabel.SanitizeName(label.Name)
+				label.Name = promrelabel.SanitizeLabelName(label.Name)
 			}
 		}
 	}
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
index 0b213cc7b..4c85cc009 100644
--- a/docs/CHANGELOG.md
+++ b/docs/CHANGELOG.md
@@ -27,6 +27,7 @@ The following `tip` changes can be tested by building VictoriaMetrics components
 * FEATURE: [vmbackup](https://docs.victoriametrics.com/vmbackup.html): add support for server-side copy of existing backups. See [these docs](https://docs.victoriametrics.com/vmbackup.html#server-side-copy-of-the-existing-backup) for details.
 
 * BUGFIX: remove `DEBUG` logging when parsing `if` filters inside [relabeling rules](https://docs.victoriametrics.com/vmagent.html#relabeling-enhancements) and when parsing `match` filters inside [stream aggregation rules](https://docs.victoriametrics.com/stream-aggregation.html).
+* BUGFIX: properly replace `:` chars in label names with `_` when `-usePromCompatibleNaming` command-line flag is passed to `vmagent`, `vminsert` or single-node VictoriaMetrics. This addresses [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3113#issuecomment-1275077071).
 
 ## [v1.93.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.93.0)
 
diff --git a/lib/promrelabel/relabel.go b/lib/promrelabel/relabel.go
index c6c17b4c9..e2d5d68da 100644
--- a/lib/promrelabel/relabel.go
+++ b/lib/promrelabel/relabel.go
@@ -619,15 +619,28 @@ func fillLabelReferences(dst []byte, replacement string, labels []prompbmarshal.
 	return dst
 }
 
-// SanitizeName replaces unsupported by Prometheus chars in metric names and label names with _.
+// SanitizeLabelName replaces unsupported by Prometheus chars in label names with _.
 //
 // See https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
-func SanitizeName(name string) string {
-	return promSanitizer.Transform(name)
+func SanitizeLabelName(name string) string {
+	return labelNameSanitizer.Transform(name)
 }
 
-var promSanitizer = bytesutil.NewFastStringTransformer(func(s string) string {
-	return unsupportedPromChars.ReplaceAllString(s, "_")
+var labelNameSanitizer = bytesutil.NewFastStringTransformer(func(s string) string {
+	return unsupportedLabelNameChars.ReplaceAllString(s, "_")
 })
 
-var unsupportedPromChars = regexp.MustCompile(`[^a-zA-Z0-9_:]`)
+var unsupportedLabelNameChars = regexp.MustCompile(`[^a-zA-Z0-9_]`)
+
+// SanitizeMetricName replaces unsupported by Prometheus chars in metric names with _.
+//
+// See https://prometheus.io/docs/concepts/data_model/#metric-names-and-labels
+func SanitizeMetricName(value string) string {
+	return metricNameSanitizer.Transform(value)
+}
+
+var metricNameSanitizer = bytesutil.NewFastStringTransformer(func(s string) string {
+	return unsupportedMetricNameChars.ReplaceAllString(s, "_")
+})
+
+var unsupportedMetricNameChars = regexp.MustCompile(`[^a-zA-Z0-9_:]`)
diff --git a/lib/promrelabel/relabel_test.go b/lib/promrelabel/relabel_test.go
index 987374252..df8080af4 100644
--- a/lib/promrelabel/relabel_test.go
+++ b/lib/promrelabel/relabel_test.go
@@ -9,13 +9,13 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils"
 )
 
-func TestSanitizeName(t *testing.T) {
+func TestSanitizeMetricName(t *testing.T) {
 	f := func(s, resultExpected string) {
 		t.Helper()
 		for i := 0; i < 5; i++ {
-			result := SanitizeName(s)
+			result := SanitizeMetricName(s)
 			if result != resultExpected {
-				t.Fatalf("unexpected result for SanitizeName(%q) at iteration %d; got %q; want %q", s, i, result, resultExpected)
+				t.Fatalf("unexpected result for SanitizeMetricName(%q) at iteration %d; got %q; want %q", s, i, result, resultExpected)
 			}
 		}
 	}
@@ -25,6 +25,22 @@ func TestSanitizeName(t *testing.T) {
 	f("foo...bar", "foo___bar")
 }
 
+func TestSanitizeLabelName(t *testing.T) {
+	f := func(s, resultExpected string) {
+		t.Helper()
+		for i := 0; i < 5; i++ {
+			result := SanitizeLabelName(s)
+			if result != resultExpected {
+				t.Fatalf("unexpected result for SanitizeLabelName(%q) at iteration %d; got %q; want %q", s, i, result, resultExpected)
+			}
+		}
+	}
+	f("", "")
+	f("a", "a")
+	f("foo.bar/baz:a", "foo_bar_baz_a")
+	f("foo...bar", "foo___bar")
+}
+
 func TestLabelsToString(t *testing.T) {
 	f := func(labels []prompbmarshal.Label, sExpected string) {
 		t.Helper()
diff --git a/lib/promrelabel/relabel_timing_test.go b/lib/promrelabel/relabel_timing_test.go
index 31682e787..485002020 100644
--- a/lib/promrelabel/relabel_timing_test.go
+++ b/lib/promrelabel/relabel_timing_test.go
@@ -8,20 +8,39 @@ import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal"
 )
 
-func BenchmarkSanitizeName(b *testing.B) {
+func BenchmarkSanitizeMetricName(b *testing.B) {
 	for _, name := range []string{"", "foo", "foo-bar-baz", "http_requests_total"} {
 		b.Run(name, func(b *testing.B) {
-			benchmarkSanitizeName(b, name)
+			benchmarkSanitizeMetricName(b, name)
 		})
 	}
 }
 
-func benchmarkSanitizeName(b *testing.B, name string) {
+func benchmarkSanitizeMetricName(b *testing.B, name string) {
 	b.ReportAllocs()
 	b.SetBytes(1)
 	b.RunParallel(func(pb *testing.PB) {
 		for pb.Next() {
-			sanitizedName := SanitizeName(name)
+			sanitizedName := SanitizeMetricName(name)
+			GlobalSink += len(sanitizedName)
+		}
+	})
+}
+
+func BenchmarkSanitizeLabelName(b *testing.B) {
+	for _, name := range []string{"", "foo", "foo-bar-baz", "http_requests_total"} {
+		b.Run(name, func(b *testing.B) {
+			benchmarkSanitizeLabelName(b, name)
+		})
+	}
+}
+
+func benchmarkSanitizeLabelName(b *testing.B, name string) {
+	b.ReportAllocs()
+	b.SetBytes(1)
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			sanitizedName := SanitizeLabelName(name)
 			GlobalSink += len(sanitizedName)
 		}
 	})