From e2b895b04c55ab3bd37898f6f52e917973e6a37d Mon Sep 17 00:00:00 2001 From: Anas Khan <83116240+anxkhn@users.noreply.github.com> Date: Sun, 28 Jun 2026 23:22:03 +0530 Subject: [PATCH] Include offending series labels in missing-metric-name push errors When a pushed series has no __name__ label, the error returned to the client only said "no metric name label" / "sample missing metric name" with no indication of which series caused it. Operators had to bisect by stopping Prometheus instances one by one to find the culprit. Enrich both user-facing push paths so the offending series labels are always included: - noMetricNameError now carries the series and renders it via formatLabelSet, matching every sibling validation error in the same file. This covers the validation (400) path used when metric name enforcement is enabled. - The fatal error from tokenForLabels (the shard_by_all_labels=false path that produced the original 500 with no context) is wrapped with the series labels. Fixes #5802 Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com> --- CHANGELOG.md | 1 + pkg/distributor/distributor_test.go | 2 +- pkg/util/validation/errors.go | 12 ++++++++---- pkg/util/validation/validate.go | 2 +- pkg/util/validation/validate_test.go | 14 +++++++++++++- 5 files changed, 24 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c9fd986ee..cbaaf4f1ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ * [ENHANCEMENT] Ring: Cache `ShuffleShardWithLookback` subrings. The cached entry is invalidated on topology change or once `now` reaches the earliest `RegisteredTimestamp + lookbackPeriod` of any included instance. #7628 * [ENHANCEMENT] Query Frontend: Rename `time_taken` field to `time_taken_ms` and make it return millisecond count. #7649 * [ENHANCEMENT] Update prometheus alertmanager version to v0.33.0. #7647 +* [ENHANCEMENT] Distributor: Include the offending series labels in "sample missing metric name" push errors so operators can identify which series is missing a metric name. #7657 * [ENHANCEMENT] Querier/Ingester: Detach ingester series from gRPC buffers to reduce heap. #7670 * [BUGFIX] Querier: Fix queryWithRetry and labelsWithRetry returning (nil, nil) on cancelled context by propagating ctx.Err(). #7370 * [BUGFIX] Metrics Helper: Fix non-deterministic bucket order in merged histograms by sorting buckets after map iteration, matching Prometheus client library behavior. #7380 diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index 2cb0b1522f..97f09a176b 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -2186,7 +2186,7 @@ func TestDistributor_Push_LabelRemoval_RemovingNameLabelWillError(t *testing.T) req := mockWriteRequest([]labels.Labels{tc.inputSeries}, 1, 1, false) _, err = ds[0].Push(ctx, req) require.Error(t, err) - assert.Equal(t, "rpc error: code = Code(400) desc = sample missing metric name", err.Error()) + assert.Equal(t, `rpc error: code = Code(400) desc = sample missing metric name metric: "{cluster=\"one\"}"`, err.Error()) } func TestDistributor_Push_ShouldGuaranteeShardingTokenConsistencyOverTheTime(t *testing.T) { diff --git a/pkg/util/validation/errors.go b/pkg/util/validation/errors.go index 2efc077655..dbab802633 100644 --- a/pkg/util/validation/errors.go +++ b/pkg/util/validation/errors.go @@ -134,14 +134,18 @@ func (e *tooManyLabelsError) Error() string { len(e.series), e.limit, cortexpb.FromLabelAdaptersToMetric(e.series).String()) } -type noMetricNameError struct{} +type noMetricNameError struct { + series string +} -func newNoMetricNameError() ValidationError { - return &noMetricNameError{} +func newNoMetricNameError(series []cortexpb.LabelAdapter) ValidationError { + return &noMetricNameError{ + series: formatLabelSet(series), + } } func (e *noMetricNameError) Error() string { - return "sample missing metric name" + return fmt.Sprintf("sample missing metric name metric: %.200q", e.series) } type invalidMetricNameError struct { diff --git a/pkg/util/validation/validate.go b/pkg/util/validation/validate.go index 31c4b1f34c..5f971a5c41 100644 --- a/pkg/util/validation/validate.go +++ b/pkg/util/validation/validate.go @@ -285,7 +285,7 @@ func ValidateMetricName(limits *Limits, ls []cortexpb.LabelAdapter, nameValidati } unsafeMetricName, err := extract.UnsafeMetricNameFromLabelAdapters(ls) if err != nil { - return newNoMetricNameError(), missingMetricName + return newNoMetricNameError(ls), missingMetricName } if !nameValidationScheme.IsValidMetricName(unsafeMetricName) { return newInvalidMetricNameError(unsafeMetricName), invalidMetricName diff --git a/pkg/util/validation/validate_test.go b/pkg/util/validation/validate_test.go index 6a55e735a9..d6aa1f7efe 100644 --- a/pkg/util/validation/validate_test.go +++ b/pkg/util/validation/validate_test.go @@ -81,7 +81,8 @@ func TestValidateMetricName(t *testing.T) { expectErr error reason string }{ - {map[model.LabelName]model.LabelValue{}, newNoMetricNameError(), missingMetricName}, + {map[model.LabelName]model.LabelValue{}, newNoMetricNameError(cortexpb.FromMetricsToLabelAdapters(model.Metric{})), missingMetricName}, + {map[model.LabelName]model.LabelValue{"foo": "bar"}, newNoMetricNameError(cortexpb.FromMetricsToLabelAdapters(model.Metric{"foo": "bar"})), 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}, @@ -102,6 +103,17 @@ func TestValidateMetricName(t *testing.T) { assert.Empty(t, reason) } +func TestNoMetricNameError_ReportsOffendingSeries(t *testing.T) { + cfg := new(Limits) + cfg.EnforceMetricName = true + + err, reason := ValidateMetricName(cfg, cortexpb.FromMetricsToLabelAdapters(model.Metric{"foo": "bar"}), model.LegacyValidation) + require.Error(t, err) + assert.Equal(t, missingMetricName, reason) + // The error must surface the offending series so operators can identify it (see issue #5802). + assert.Equal(t, `sample missing metric name metric: "{foo=\"bar\"}"`, err.Error()) +} + func TestValidateLabels(t *testing.T) { cfg := new(Limits) userID := "testUser"