From 9979024a30b75eecdc49b4b7aa2af1c5f1b0c182 Mon Sep 17 00:00:00 2001
From: "Xiaochao Dong (@damnever)" <the.xcdong@gmail.com>
Date: Thu, 8 Dec 2022 11:09:43 +0800
Subject: [PATCH] Report error if the series contains invalid metric names or
 labels during scrape

Signed-off-by: Xiaochao Dong (@damnever) <the.xcdong@gmail.com>
---
 model/labels/labels.go      | 14 ++++++++++
 model/labels/labels_test.go | 56 +++++++++++++++++++++++++++++++++++++
 scrape/scrape.go            |  4 +++
 scrape/scrape_test.go       | 45 +++++++++++++++++++++++++++++
 4 files changed, 119 insertions(+)

diff --git a/model/labels/labels.go b/model/labels/labels.go
index 48237bdc0..7652b4e12 100644
--- a/model/labels/labels.go
+++ b/model/labels/labels.go
@@ -20,6 +20,7 @@ import (
 	"strconv"
 
 	"github.com/cespare/xxhash/v2"
+	"github.com/prometheus/common/model"
 )
 
 // Well-known label names used by Prometheus components.
@@ -311,6 +312,19 @@ func (ls Labels) WithoutEmpty() Labels {
 	return ls
 }
 
+// IsValid checks if the metric name or label names are valid.
+func (ls Labels) IsValid() bool {
+	for _, l := range ls {
+		if l.Name == model.MetricNameLabel && !model.IsValidMetricName(model.LabelValue(l.Value)) {
+			return false
+		}
+		if !model.LabelName(l.Name).IsValid() || !model.LabelValue(l.Value).IsValid() {
+			return false
+		}
+	}
+	return true
+}
+
 // Equal returns whether the two label sets are equal.
 func Equal(ls, o Labels) bool {
 	if len(ls) != len(o) {
diff --git a/model/labels/labels_test.go b/model/labels/labels_test.go
index 57ccf1fef..0fd0edacc 100644
--- a/model/labels/labels_test.go
+++ b/model/labels/labels_test.go
@@ -216,6 +216,62 @@ func TestLabels_WithoutEmpty(t *testing.T) {
 	}
 }
 
+func TestLabels_IsValid(t *testing.T) {
+	for _, test := range []struct {
+		input    Labels
+		expected bool
+	}{
+		{
+			input: FromStrings(
+				"__name__", "test",
+				"hostname", "localhost",
+				"job", "check",
+			),
+			expected: true,
+		},
+		{
+			input: FromStrings(
+				"__name__", "test:ms",
+				"hostname_123", "localhost",
+				"_job", "check",
+			),
+			expected: true,
+		},
+		{
+			input:    FromStrings("__name__", "test-ms"),
+			expected: false,
+		},
+		{
+			input:    FromStrings("__name__", "0zz"),
+			expected: false,
+		},
+		{
+			input:    FromStrings("abc:xyz", "invalid"),
+			expected: false,
+		},
+		{
+			input:    FromStrings("123abc", "invalid"),
+			expected: false,
+		},
+		{
+			input:    FromStrings("中文abc", "invalid"),
+			expected: false,
+		},
+		{
+			input:    FromStrings("invalid", "aa\xe2"),
+			expected: false,
+		},
+		{
+			input:    FromStrings("invalid", "\xF7\xBF\xBF\xBF"),
+			expected: false,
+		},
+	} {
+		t.Run("", func(t *testing.T) {
+			require.Equal(t, test.expected, test.input.IsValid())
+		})
+	}
+}
+
 func TestLabels_Equal(t *testing.T) {
 	labels := FromStrings(
 		"aaa", "111",
diff --git a/scrape/scrape.go b/scrape/scrape.go
index 04375ab56..e9d172a3e 100644
--- a/scrape/scrape.go
+++ b/scrape/scrape.go
@@ -1609,6 +1609,10 @@ loop:
 				err = errNameLabelMandatory
 				break loop
 			}
+			if !lset.IsValid() {
+				err = fmt.Errorf("invalid metric name or label names: %s", lset.String())
+				break loop
+			}
 
 			// If any label limits is exceeded the scrape should fail.
 			if err = verifyLabelLimits(lset, sl.labelLimits); err != nil {
diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go
index 27749dbc6..b22f7f095 100644
--- a/scrape/scrape_test.go
+++ b/scrape/scrape_test.go
@@ -1020,6 +1020,51 @@ func TestScrapeLoopSeriesAdded(t *testing.T) {
 	require.Equal(t, 0, seriesAdded)
 }
 
+func TestScrapeLoopFailWithInvalidLabelsAfterRelabel(t *testing.T) {
+	s := teststorage.New(t)
+	defer s.Close()
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	target := &Target{
+		labels: labels.FromStrings("pod_label_invalid_012", "test"),
+	}
+	relabelConfig := []*relabel.Config{{
+		Action:      relabel.LabelMap,
+		Regex:       relabel.MustNewRegexp("pod_label_invalid_(.+)"),
+		Separator:   ";",
+		Replacement: "$1",
+	}}
+	sl := newScrapeLoop(ctx,
+		&testScraper{},
+		nil, nil,
+		func(l labels.Labels) labels.Labels {
+			return mutateSampleLabels(l, target, true, relabelConfig)
+		},
+		nopMutator,
+		s.Appender,
+		nil,
+		0,
+		true,
+		0,
+		nil,
+		0,
+		0,
+		false,
+		false,
+		nil,
+		false,
+	)
+
+	slApp := sl.appender(ctx)
+	total, added, seriesAdded, err := sl.append(slApp, []byte("test_metric 1\n"), "", time.Time{})
+	require.ErrorContains(t, err, "invalid metric name or label names")
+	require.NoError(t, slApp.Rollback())
+	require.Equal(t, 1, total)
+	require.Equal(t, 0, added)
+	require.Equal(t, 0, seriesAdded)
+}
+
 func makeTestMetrics(n int) []byte {
 	// Construct a metrics string to parse
 	sb := bytes.Buffer{}