fix prometheus metricsname err when reuse memory (#1201)

* fix prometheus metricsname err when reuse memory * fix lint --------- Co-authored-by: Tom Yu <[email protected]>
alibaba · Nov 8, 2023 · 622b9aa · 622b9aa
1 parent b6beeb6
commit 622b9aa
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,7 @@ your changes, such as:
 - [public] [both] [updated] add a new feature
 
 ## [Unreleased]
+- [public] [both] [fixed] fix prometheus send wrong sls logs when reuse string memory
 - [public] [both] [fixed] fix dropping jvm metrics when collecting multiple skywalking instances's data
 - [public] [both] [fixed] fix elasticsearch flusher authentication tls config and http config
 - [public] [both] [fixed] fix profiling wrong type when the different profiling type having same stack. 

diff --git a/core/common/LogtailCommonFlags.cpp b/core/common/LogtailCommonFlags.cpp
@@ -95,4 +95,4 @@ DEFINE_FLAG_BOOL(enable_root_path_collection, "", false);
 DEFINE_FLAG_BOOL(enable_containerd_upper_dir_detect,
                  "if enable containerd upper dir detect when locating rootfs",
                  false);
-DEFINE_FLAG_BOOL(enable_sls_metrics_format, "if enable format metrics in SLS metricstore log pattern", false);
+DEFINE_FLAG_BOOL(enable_sls_metrics_format, "if enable format metrics in SLS metricstore log pattern", true);
diff --git a/go.mod b/go.mod
@@ -263,29 +263,26 @@ require (
 )
 
 require (
+	cloud.google.com/go/compute v1.18.0 // indirect
+	cloud.google.com/go/compute/metadata v0.2.3 // indirect
+	github.com/VictoriaMetrics/fasthttp v1.1.0 // indirect
+	github.com/VictoriaMetrics/metrics v1.23.0 // indirect
+	github.com/VictoriaMetrics/metricsql v0.45.0 // indirect
 	github.com/gorilla/websocket v1.4.2 // indirect
 	github.com/hashicorp/errwrap v1.1.0 // indirect
 	github.com/hashicorp/go-multierror v1.1.1 // indirect
 	github.com/influxdata/telegraf v1.20.0 // indirect
 	github.com/openkruise/kruise-api v1.4.0
 	github.com/richardartoul/molecule v1.0.0 // indirect
-	golang.org/x/mod v0.8.0 // indirect
-	gopkg.in/ini.v1 v1.66.2 // indirect
-	sigs.k8s.io/gateway-api v0.6.2 // indirect
-)
-
-require (
-	cloud.google.com/go/compute v1.18.0 // indirect
-	cloud.google.com/go/compute/metadata v0.2.3 // indirect
-	github.com/VictoriaMetrics/fasthttp v1.1.0 // indirect
-	github.com/VictoriaMetrics/metrics v1.23.0 // indirect
-	github.com/VictoriaMetrics/metricsql v0.45.0 // indirect
 	github.com/valyala/fastjson v1.6.3 // indirect
 	github.com/valyala/fastrand v1.1.0 // indirect
 	github.com/valyala/fasttemplate v1.2.2 // indirect
 	github.com/valyala/gozstd v1.17.0 // indirect
 	github.com/valyala/histogram v1.2.0 // indirect
 	github.com/valyala/quicktemplate v1.7.0 // indirect
+	golang.org/x/mod v0.8.0 // indirect
+	gopkg.in/ini.v1 v1.66.2 // indirect
+	sigs.k8s.io/gateway-api v0.6.2 // indirect
 )
 
 replace (

diff --git a/pkg/helper/log_helper.go b/pkg/helper/log_helper.go
@@ -245,6 +245,7 @@ func (hd *HistogramData) ToMetricLogs(name string, timeMs int64, labels *MetricL
 }
 
 // NewMetricLog create a metric log, time support unix milliseconds and unix nanoseconds.
+// Note: must pass safe string
 func NewMetricLog(name string, t int64, value float64, labels *MetricLabels) *protocol.Log {
 	var valStr string
 	if math.Float64bits(value) == StaleNaN {
@@ -256,6 +257,7 @@ func NewMetricLog(name string, t int64, value float64, labels *MetricLabels) *pr
 }
 
 // NewMetricLogStringVal create a metric log with val string, time support unix milliseconds and unix nanoseconds.
+// Note: must pass safe string
 func NewMetricLogStringVal(name string, t int64, value string, labels *MetricLabels) *protocol.Log {
 	strTime := strconv.FormatInt(t, 10)
 	metric := &protocol.Log{}
@@ -329,18 +331,25 @@ func formatNewMetricName(name string) string {
 	if !config.LogtailGlobalConfig.EnableSlsMetricsFormat {
 		return name
 	}
-	newName := []byte(name)
-	for i, b := range newName {
+	var newName []byte
+	for i := 0; i < len(name); i++ {
+		b := name[i]
 		if (b >= 'a' && b <= 'z') ||
 			(b >= 'A' && b <= 'Z') ||
 			(b >= '0' && b <= '9') ||
 			b == '_' ||
 			b == ':' {
 			continue
 		} else {
+			if newName == nil {
+				newName = []byte(name)
+			}
 			newName[i] = SlsMetricstoreInvalidReplaceCharacter
 		}
 	}
+	if newName == nil {
+		return name
+	}
 	return util.ZeroCopyBytesToString(newName)
 }
 

diff --git a/pkg/helper/log_helper_test.go b/pkg/helper/log_helper_test.go
@@ -1,6 +1,8 @@
 package helper
 
 import (
+	"github.com/alibaba/ilogtail/pkg/config"
+
 	"github.com/stretchr/testify/require"
 
 	"testing"
@@ -11,26 +13,36 @@ func TestMetricLabels_Append(t *testing.T) {
 	ml.Append("key2", "val")
 	ml.Append("key", "val")
 	log := NewMetricLog("name", 1691646109945, 1, &ml)
-	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val|key2#$#val" > Contents:<Key:"__value__" Value:"1" > `, log.String())
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val|key2#$#val" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
 
 	ml.Replace("key", "val2")
 
 	log = NewMetricLog("name", 1691646109945, 1, &ml)
-	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val" > Contents:<Key:"__value__" Value:"1" > `, log.String())
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
 
 	ml.Replace("key3", "val3")
 	log = NewMetricLog("name", 1691646109945, 1, &ml)
-	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val|key3#$#val3" > Contents:<Key:"__value__" Value:"1" > `, log.String())
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val|key3#$#val3" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
 
 	cloneLabel := ml.Clone()
 	cloneLabel.Replace("key3", "val4")
 	log = NewMetricLog("name", 1691646109945, 1, cloneLabel)
-	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val|key3#$#val4" > Contents:<Key:"__value__" Value:"1" > `, log.String())
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val|key3#$#val4" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
 
 	log = NewMetricLog("name", 1691646109945, 1, &ml)
-	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val|key3#$#val3" > Contents:<Key:"__value__" Value:"1" > `, log.String())
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key#$#val2|key2#$#val|key3#$#val3" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
+
+	log = NewMetricLog("name", 1691646109945, 1, nil)
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
 
 	log = NewMetricLog("name", 1691646109945, 1, nil)
-	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"" > Contents:<Key:"__value__" Value:"1" > `, log.String())
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
+
+	var ml2 MetricLabels
+	config.LogtailGlobalConfig.EnableSlsMetricsFormat = true
+	ml2.Append("key@", "val|")
+
+	log = NewMetricLog("name@", 1691646109945, 1, &ml2)
+	require.Equal(t, `Time:1691646109 Contents:<Key:"__name__" Value:"name_" > Contents:<Key:"__time_nano__" Value:"1691646109945000000" > Contents:<Key:"__labels__" Value:"key_#$#val_" > Contents:<Key:"__value__" Value:"1" > Time_ns:945000000 `, log.String())
 
 }
diff --git a/plugins/input/prometheus/sls_writer.go b/plugins/input/prometheus/sls_writer.go
@@ -16,8 +16,11 @@ package prometheus
 
 import (
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal"
+
 	"github.com/alibaba/ilogtail/pkg/helper"
 	"github.com/alibaba/ilogtail/pkg/pipeline"
+
+	"strings"
 )
 
 func appendTSDataToSlsLog(c pipeline.Collector, wr *prompbmarshal.WriteRequest) {
@@ -27,9 +30,11 @@ func appendTSDataToSlsLog(c pipeline.Collector, wr *prompbmarshal.WriteRequest)
 		var labels helper.MetricLabels
 		for _, label := range ts.Labels {
 			if label.Name == "__name__" {
-				name = label.Value
+				// Prometheus scrape operation reuse bytes to enhance performance, must clone the unsafe string.
+				name = strings.Clone(label.Value)
 				continue
 			}
+			// Because AddRawLog operation would join labels to a new string, pass unsafe string would be safe.
 			labels.Append(label.Name, label.Value)
 		}
 		for _, sample := range ts.Samples {