From be5673c39d9a545a4b075cd39923094c424e8d20 Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Thu, 17 Aug 2023 14:35:26 +0200
Subject: [PATCH] app/vmagent/remotewrite: follow-up after
 a27c2f37731986f4bf6738404bb6388b1f42ffde

- Fix Prometheus-compatible naming after applying the relabeling if -usePromCompatibleNaming command-line flag is set.
  This should prevent from possible Prometheus-incompatible metric names and label names generated by the relabeling.
- Do not return anything from relabelCtx.appendExtraLabels() function, since it cannot change the number of time series
  passed to it. Append labels for the passed time series in-place.
- Remove promrelabel.FinalizeLabels() call after adding extra labels to time series, since this call has been already
  made at relabelCtx.applyRelabeling(). It is user's responsibility if he passes labels with double underscore prefixes
  to -remoteWrite.label.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4247
---
 app/vmagent/remotewrite/relabel.go      | 39 ++++++++++++-------------
 app/vmagent/remotewrite/relabel_test.go |  6 ++--
 app/vmagent/remotewrite/remotewrite.go  |  4 +--
 3 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/app/vmagent/remotewrite/relabel.go b/app/vmagent/remotewrite/relabel.go
index b79021c64f..93b4caee75 100644
--- a/app/vmagent/remotewrite/relabel.go
+++ b/app/vmagent/remotewrite/relabel.go
@@ -98,24 +98,15 @@ func (rctx *relabelCtx) applyRelabeling(tss []prompbmarshal.TimeSeries, pcs *pro
 		ts := &tss[i]
 		labelsLen := len(labels)
 		labels = append(labels, ts.Labels...)
-		if *usePromCompatibleNaming {
-			// Replace unsupported Prometheus chars in label names and metric names with underscores.
-			tmpLabels := labels[labelsLen:]
-			for j := range tmpLabels {
-				label := &tmpLabels[j]
-				if label.Name == "__name__" {
-					label.Value = promrelabel.SanitizeMetricName(label.Value)
-				} else {
-					label.Name = promrelabel.SanitizeLabelName(label.Name)
-				}
-			}
-		}
 		labels = pcs.Apply(labels, labelsLen)
 		labels = promrelabel.FinalizeLabels(labels[:labelsLen], labels[labelsLen:])
 		if len(labels) == labelsLen {
 			// Drop the current time series, since relabeling removed all the labels.
 			continue
 		}
+		if *usePromCompatibleNaming {
+			fixPromCompatibleNaming(labels[labelsLen:])
+		}
 		tssDst = append(tssDst, prompbmarshal.TimeSeries{
 			Labels:  labels[labelsLen:],
 			Samples: ts.Samples,
@@ -125,11 +116,10 @@ func (rctx *relabelCtx) applyRelabeling(tss []prompbmarshal.TimeSeries, pcs *pro
 	return tssDst
 }
 
-func (rctx *relabelCtx) appendExtraLabels(tss []prompbmarshal.TimeSeries, extraLabels []prompbmarshal.Label) []prompbmarshal.TimeSeries {
+func (rctx *relabelCtx) appendExtraLabels(tss []prompbmarshal.TimeSeries, extraLabels []prompbmarshal.Label) {
 	if len(extraLabels) == 0 {
-		return tss
+		return
 	}
-	tssDst := tss[:0]
 	labels := rctx.labels[:0]
 	for i := range tss {
 		ts := &tss[i]
@@ -147,14 +137,9 @@ func (rctx *relabelCtx) appendExtraLabels(tss []prompbmarshal.TimeSeries, extraL
 				labels = append(labels, extraLabel)
 			}
 		}
-		labels = promrelabel.FinalizeLabels(labels[:labelsLen], labels[labelsLen:])
-		tssDst = append(tssDst, prompbmarshal.TimeSeries{
-			Labels:  labels[labelsLen:],
-			Samples: ts.Samples,
-		})
+		ts.Labels = labels[labelsLen:]
 	}
 	rctx.labels = labels
-	return tssDst
 }
 
 type relabelCtx struct {
@@ -181,3 +166,15 @@ func putRelabelCtx(rctx *relabelCtx) {
 	rctx.labels = rctx.labels[:0]
 	relabelCtxPool.Put(rctx)
 }
+
+func fixPromCompatibleNaming(labels []prompbmarshal.Label) {
+	// Replace unsupported Prometheus chars in label names and metric names with underscores.
+	for i := range labels {
+		label := &labels[i]
+		if label.Name == "__name__" {
+			label.Value = promrelabel.SanitizeMetricName(label.Value)
+		} else {
+			label.Name = promrelabel.SanitizeLabelName(label.Name)
+		}
+	}
+}
diff --git a/app/vmagent/remotewrite/relabel_test.go b/app/vmagent/remotewrite/relabel_test.go
index 441283f23b..c84a316101 100644
--- a/app/vmagent/remotewrite/relabel_test.go
+++ b/app/vmagent/remotewrite/relabel_test.go
@@ -42,9 +42,9 @@ func TestAppendExtraLabels(t *testing.T) {
 	f := func(extraLabels []prompbmarshal.Label, sTss, sExpTss string) {
 		rctx := &relabelCtx{}
 		tss, expTss := parseSeries(sTss), parseSeries(sExpTss)
-		gotTss := rctx.appendExtraLabels(tss, extraLabels)
-		if !reflect.DeepEqual(gotTss, expTss) {
-			t.Fatalf("expected to have: \n%v;\ngot: \n%v", expTss, gotTss)
+		rctx.appendExtraLabels(tss, extraLabels)
+		if !reflect.DeepEqual(tss, expTss) {
+			t.Fatalf("expected to have: \n%v;\ngot: \n%v", expTss, tss)
 		}
 	}
 
diff --git a/app/vmagent/remotewrite/remotewrite.go b/app/vmagent/remotewrite/remotewrite.go
index b4f16ad3ef..62dd1a9089 100644
--- a/app/vmagent/remotewrite/remotewrite.go
+++ b/app/vmagent/remotewrite/remotewrite.go
@@ -355,7 +355,7 @@ func Push(at *auth.Token, wr *prompbmarshal.WriteRequest) {
 	var rctx *relabelCtx
 	rcs := allRelabelConfigs.Load()
 	pcsGlobal := rcs.global
-	if pcsGlobal.Len() > 0 || len(labelsGlobal) > 0 {
+	if pcsGlobal.Len() > 0 {
 		rctx = getRelabelCtx()
 	}
 	tss := wr.Timeseries
@@ -722,7 +722,7 @@ func (rwctx *remoteWriteCtx) pushInternal(tss []prompbmarshal.TimeSeries) {
 	if len(labelsGlobal) > 0 {
 		rctx := getRelabelCtx()
 		defer putRelabelCtx(rctx)
-		tss = rctx.appendExtraLabels(tss, labelsGlobal)
+		rctx.appendExtraLabels(tss, labelsGlobal)
 	}
 
 	pss := rwctx.pss