Validate metric name before removing empty labels

CHANGELOG.md

@@ -40,6 +40,7 @@
 * [ENHANCEMENT] Ingester: Add feature flag to collect metrics of how expensive an unoptimized regex matcher is and new limits to protect Ingester query path against expensive unoptimized regex matchers. #7194 #7210
 * [ENHANCEMENT] Compactor: Add partition group creation time to visit marker. #7217
 * [ENHANCEMENT] Compactor: Add concurrency for partition cleanup and mark block for deletion #7246
+* [ENHANCEMENT] Distributor: Validate metric name before removing empty labels. #7253
 * [BUGFIX] Distributor: If remote write v2 is disabled, explicitly return HTTP 415 (Unsupported Media Type) for Remote Write V2 requests instead of attempting to parse them as V1. #7238
 * [BUGFIX] Ring: Change DynamoDB KV to retry indefinitely for WatchKey. #7088
 * [BUGFIX] Ruler: Add XFunctions validation support. #7111

pkg/distributor/distributor.go

@@ -1111,6 +1111,18 @@ func (d *Distributor) prepareSeriesKeys(ctx context.Context, req *cortexpb.Write
 			removeLabel(labelName, &ts.Labels)
 		}
 
+		// Reject series with missing or empty metric name before removeEmptyLabels (which would strip __name__="").
+		if validationErr, reason := validation.ValidateMetricName(limits, ts.Labels, d.cfg.NameValidationScheme); reason != "" {
+			samplesCount := float64(len(ts.Samples) + len(ts.Histograms))
+			exemplarsCount := float64(len(ts.Exemplars))
+			if firstPartialErr == nil {
+				firstPartialErr = httpgrpc.Errorf(http.StatusBadRequest, "%s", validationErr.Error())
+			}
+			d.validateMetrics.DiscardedSamples.WithLabelValues(reason, userID).Add(samplesCount)
+			d.validateMetrics.DiscardedExemplars.WithLabelValues(reason, userID).Add(exemplarsCount)
+			continue
+		}
+
 		// Make sure no label with empty value is sent to the Ingester.
 		removeEmptyLabels(&ts.Labels)
 

pkg/distributor/distributor_test.go

@@ -2267,6 +2267,10 @@ func TestDistributor_Push_ExemplarValidation(t *testing.T) {
 			req:    makeWriteRequestExemplar([]string{model.MetricNameLabel, "test", "", "bar"}, 0, nil),
 			errMsg: "invalid label",
 		},
+		"rejects exemplar with empty metric name": {
+			req:    makeWriteRequestExemplar([]string{model.MetricNameLabel, ""}, 1000, []string{"foo", "bar"}),
+			errMsg: "invalid metric name",
+		},
 	}
 
 	for testName, tc := range tests {
@@ -4015,6 +4019,28 @@ func TestDistributorValidation(t *testing.T) {
 			}},
 			err: httpgrpc.Errorf(http.StatusBadRequest, `metadata missing metric name`),
 		},
+		// Test series with empty metric name is rejected
+		{
+			labels: []labels.Labels{
+				labels.FromStrings(labels.MetricName, "", "foo", "bar"),
+			},
+			samples: []cortexpb.Sample{{
+				TimestampMs: int64(now),
+				Value:       1,
+			}},
+			err: httpgrpc.Errorf(http.StatusBadRequest, `sample invalid metric name: ""`),
+		},
+		// Test series with only empty metric name (no other labels) is rejected
+		{
+			labels: []labels.Labels{
+				labels.FromStrings(labels.MetricName, ""),
+			},
+			samples: []cortexpb.Sample{{
+				TimestampMs: int64(now),
+				Value:       1,
+			}},
+			err: httpgrpc.Errorf(http.StatusBadRequest, `sample invalid metric name: ""`),
+		},
 		// Test maximum labels names per series for histogram samples.
 		{
 			labels: []labels.Labels{

pkg/util/validation/validate.go

@@ -276,22 +276,27 @@ func ValidateExemplar(validateMetrics *ValidateMetrics, userID string, ls []cort
 	return nil
 }
 
+// ValidateMetricName checks that ls has a valid non-empty metric name when limits.EnforceMetricName is true.
+// It returns (nil, "") when valid, or (error, discardReason) when invalid.
+// Callers should increment DiscardedSamples/DiscardedExemplars with the returned reason when non-empty.
+func ValidateMetricName(limits *Limits, ls []cortexpb.LabelAdapter, nameValidationScheme model.ValidationScheme) (ValidationError, string) {
+	if !limits.EnforceMetricName {
+		return nil, ""
+	}
+	unsafeMetricName, err := extract.UnsafeMetricNameFromLabelAdapters(ls)
+	if err != nil {
+		return newNoMetricNameError(), missingMetricName
+	}
+	if !nameValidationScheme.IsValidMetricName(unsafeMetricName) {
+		return newInvalidMetricNameError(unsafeMetricName), invalidMetricName
+	}
+	return nil, ""
+}
+
 // ValidateLabels returns an err if the labels are invalid.
 // The returned error may retain the provided series labels.
+// Callers must validate metric name (e.g. via ValidateMetricName) before calling this when EnforceMetricName is true.
 func ValidateLabels(validateMetrics *ValidateMetrics, limits *Limits, userID string, ls []cortexpb.LabelAdapter, skipLabelNameValidation bool, nameValidationScheme model.ValidationScheme) ValidationError {
-	if limits.EnforceMetricName {
-		unsafeMetricName, err := extract.UnsafeMetricNameFromLabelAdapters(ls)
-		if err != nil {
-			validateMetrics.DiscardedSamples.WithLabelValues(missingMetricName, userID).Inc()
-			return newNoMetricNameError()
-		}
-
-		if !nameValidationScheme.IsValidMetricName(unsafeMetricName) {
-			validateMetrics.DiscardedSamples.WithLabelValues(invalidMetricName, userID).Inc()
-			return newInvalidMetricNameError(unsafeMetricName)
-		}
-	}
-
 	numLabelNames := len(ls)
 	if numLabelNames > limits.MaxLabelNamesPerSeries {
 		validateMetrics.DiscardedSamples.WithLabelValues(maxLabelNamesPerSeries, userID).Inc()

pkg/util/validation/validate_test.go

@@ -41,24 +41,12 @@ func TestValidateLabels_UTF8(t *testing.T) {
 		skipLabelNameValidation bool
 		expectedErr             error
 	}{
-		{
-			description:             "empty metric name",
-			metric:                  map[model.LabelName]model.LabelValue{},
-			skipLabelNameValidation: false,
-			expectedErr:             newNoMetricNameError(),
-		},
 		{
 			description:             "utf8 metric name",
 			metric:                  map[model.LabelName]model.LabelValue{model.MetricNameLabel: "test.utf8.metric"},
 			skipLabelNameValidation: false,
 			expectedErr:             nil,
 		},
-		{
-			description:             "invalid utf8 metric name",
-			metric:                  map[model.LabelName]model.LabelValue{model.MetricNameLabel: "test.\xc5.metric"},
-			skipLabelNameValidation: false,
-			expectedErr:             newInvalidMetricNameError("test.\xc5.metric"),
-		},
 		{
 			description:             "invalid utf8 label name, but skipLabelNameValidation is true",
 			metric:                  map[model.LabelName]model.LabelValue{model.MetricNameLabel: "test.utf8.metric", "label1": "test.\xc5.label"},
@@ -84,6 +72,36 @@ func TestValidateLabels_UTF8(t *testing.T) {
 	}
 }
 
+func TestValidateMetricName(t *testing.T) {
+	cfg := new(Limits)
+	cfg.EnforceMetricName = true
+
+	for _, c := range []struct {
+		metric    model.Metric
+		expectErr error
+		reason    string
+	}{
+		{map[model.LabelName]model.LabelValue{}, newNoMetricNameError(), missingMetricName},
+		{map[model.LabelName]model.LabelValue{model.MetricNameLabel: ""}, newInvalidMetricNameError(""), invalidMetricName},
+		{map[model.LabelName]model.LabelValue{model.MetricNameLabel: " "}, newInvalidMetricNameError(" "), invalidMetricName},
+		{map[model.LabelName]model.LabelValue{model.MetricNameLabel: "test.\xc5.metric"}, newInvalidMetricNameError("test.\xc5.metric"), invalidMetricName},
+		{map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid"}, nil, ""},
+	} {
+		err, reason := ValidateMetricName(cfg, cortexpb.FromMetricsToLabelAdapters(c.metric), model.LegacyValidation)
+		assert.Equal(t, c.expectErr, err)
+		if c.reason != "" {
+			assert.Equal(t, c.reason, reason)
+		} else {
+			assert.Empty(t, reason)
+		}
+	}
+
+	cfg.EnforceMetricName = false
+	err, reason := ValidateMetricName(cfg, cortexpb.FromMetricsToLabelAdapters(map[model.LabelName]model.LabelValue{}), model.LegacyValidation)
+	assert.Nil(t, err)
+	assert.Empty(t, reason)
+}
+
 func TestValidateLabels(t *testing.T) {
 	cfg := new(Limits)
 	userID := "testUser"
@@ -117,16 +135,6 @@ func TestValidateLabels(t *testing.T) {
 		skipLabelNameValidation bool
 		err                     error
 	}{
-		{
-			map[model.LabelName]model.LabelValue{},
-			false,
-			newNoMetricNameError(),
-		},
-		{
-			map[model.LabelName]model.LabelValue{model.MetricNameLabel: " "},
-			false,
-			newInvalidMetricNameError(" "),
-		},
 		{
 			map[model.LabelName]model.LabelValue{model.MetricNameLabel: "valid", "foo ": "bar"},
 			false,
@@ -192,8 +200,6 @@ func TestValidateLabels(t *testing.T) {
 			cortex_discarded_samples_total{reason="label_name_too_long",user="testUser"} 1
 			cortex_discarded_samples_total{reason="label_value_too_long",user="testUser"} 1
 			cortex_discarded_samples_total{reason="max_label_names_per_series",user="testUser"} 1
-			cortex_discarded_samples_total{reason="metric_name_invalid",user="testUser"} 1
-			cortex_discarded_samples_total{reason="missing_metric_name",user="testUser"} 1
 			cortex_discarded_samples_total{reason="labels_size_bytes_exceeded",user="testUser"} 1
 
 			cortex_discarded_samples_total{reason="random reason",user="different user"} 1