Skip to content

Commit 1565f0d

Browse files
committed
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>
1 parent c55214c commit 1565f0d

6 files changed

Lines changed: 54 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* [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
4646
* [ENHANCEMENT] Query Frontend: Rename `time_taken` field to `time_taken_ms` and make it return millisecond count. #7649
4747
* [ENHANCEMENT] Update prometheus alertmanager version to v0.33.0. #7647
48+
* [ENHANCEMENT] Distributor: Include the offending series labels in the "sample missing metric name" and "no metric name label" push errors so operators can identify which series is missing a metric name. #7657
4849
* [BUGFIX] Querier: Fix queryWithRetry and labelsWithRetry returning (nil, nil) on cancelled context by propagating ctx.Err(). #7370
4950
* [BUGFIX] Metrics Helper: Fix non-deterministic bucket order in merged histograms by sorting buckets after map iteration, matching Prometheus client library behavior. #7380
5051
* [BUGFIX] Distributor: Return HTTP 401 Unauthorized when tenant ID resolution fails in the Prometheus Remote Write 2.0 path. #7389

pkg/distributor/distributor.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,7 @@ func (d *Distributor) prepareSeriesKeys(ctx context.Context, req *cortexpb.Write
12081208
// label and dropped labels (if any)
12091209
key, err := d.tokenForLabels(userID, ts.Labels)
12101210
if err != nil {
1211-
return nil, nil, nil, nil, 0, 0, 0, 0, nil, err
1211+
return nil, nil, nil, nil, 0, 0, 0, 0, nil, errors.Wrapf(err, "failed to compute sharding token for series %s", cortexpb.FromLabelAdaptersToMetric(ts.Labels).String())
12121212
}
12131213
validatedSeries, validationErr := d.validateSeries(*ts, userID, skipLabelNameValidation, limits)
12141214

pkg/distributor/distributor_test.go

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2186,7 +2186,36 @@ func TestDistributor_Push_LabelRemoval_RemovingNameLabelWillError(t *testing.T)
21862186
req := mockWriteRequest([]labels.Labels{tc.inputSeries}, 1, 1, false)
21872187
_, err = ds[0].Push(ctx, req)
21882188
require.Error(t, err)
2189-
assert.Equal(t, "rpc error: code = Code(400) desc = sample missing metric name", err.Error())
2189+
assert.Equal(t, `rpc error: code = Code(400) desc = sample missing metric name metric: "{cluster=\"one\"}"`, err.Error())
2190+
}
2191+
2192+
// TestDistributor_Push_NoMetricNameShardingErrorReportsSeries covers the path that
2193+
// the original bug report hit (issue #5802): with metric name enforcement disabled
2194+
// and shard_by_all_labels=false, a series without a metric name fails while computing
2195+
// its sharding token. The resulting error must name the offending series so operators
2196+
// can identify it instead of only seeing "no metric name label".
2197+
func TestDistributor_Push_NoMetricNameShardingErrorReportsSeries(t *testing.T) {
2198+
t.Parallel()
2199+
ctx := user.InjectOrgID(context.Background(), "user")
2200+
2201+
var limits validation.Limits
2202+
flagext.DefaultValues(&limits)
2203+
limits.EnforceMetricName = false
2204+
2205+
ds, _, _, _ := prepare(t, prepConfig{
2206+
numIngesters: 2,
2207+
happyIngesters: 2,
2208+
numDistributors: 1,
2209+
shardByAllLabels: false,
2210+
limits: &limits,
2211+
})
2212+
2213+
inputSeries := labels.FromStrings("foo", "bar", "team", "a")
2214+
req := mockWriteRequest([]labels.Labels{inputSeries}, 1, 1, false)
2215+
_, err := ds[0].Push(ctx, req)
2216+
require.Error(t, err)
2217+
assert.Contains(t, err.Error(), "no metric name label")
2218+
assert.Contains(t, err.Error(), `{foo="bar", team="a"}`)
21902219
}
21912220

21922221
func TestDistributor_Push_ShouldGuaranteeShardingTokenConsistencyOverTheTime(t *testing.T) {

pkg/util/validation/errors.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,18 @@ func (e *tooManyLabelsError) Error() string {
134134
len(e.series), e.limit, cortexpb.FromLabelAdaptersToMetric(e.series).String())
135135
}
136136

137-
type noMetricNameError struct{}
137+
type noMetricNameError struct {
138+
series []cortexpb.LabelAdapter
139+
}
138140

139-
func newNoMetricNameError() ValidationError {
140-
return &noMetricNameError{}
141+
func newNoMetricNameError(series []cortexpb.LabelAdapter) ValidationError {
142+
return &noMetricNameError{
143+
series: series,
144+
}
141145
}
142146

143147
func (e *noMetricNameError) Error() string {
144-
return "sample missing metric name"
148+
return fmt.Sprintf("sample missing metric name metric: %.200q", formatLabelSet(e.series))
145149
}
146150

147151
type invalidMetricNameError struct {

pkg/util/validation/validate.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ func ValidateMetricName(limits *Limits, ls []cortexpb.LabelAdapter, nameValidati
285285
}
286286
unsafeMetricName, err := extract.UnsafeMetricNameFromLabelAdapters(ls)
287287
if err != nil {
288-
return newNoMetricNameError(), missingMetricName
288+
return newNoMetricNameError(ls), missingMetricName
289289
}
290290
if !nameValidationScheme.IsValidMetricName(unsafeMetricName) {
291291
return newInvalidMetricNameError(unsafeMetricName), invalidMetricName

pkg/util/validation/validate_test.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ func TestValidateMetricName(t *testing.T) {
8181
expectErr error
8282
reason string
8383
}{
84-
{map[model.LabelName]model.LabelValue{}, newNoMetricNameError(), missingMetricName},
84+
{map[model.LabelName]model.LabelValue{}, newNoMetricNameError(cortexpb.FromMetricsToLabelAdapters(model.Metric{})), missingMetricName},
85+
{map[model.LabelName]model.LabelValue{"foo": "bar"}, newNoMetricNameError(cortexpb.FromMetricsToLabelAdapters(model.Metric{"foo": "bar"})), missingMetricName},
8586
{map[model.LabelName]model.LabelValue{model.MetricNameLabel: ""}, newInvalidMetricNameError(""), invalidMetricName},
8687
{map[model.LabelName]model.LabelValue{model.MetricNameLabel: " "}, newInvalidMetricNameError(" "), invalidMetricName},
8788
{map[model.LabelName]model.LabelValue{model.MetricNameLabel: "test.\xc5.metric"}, newInvalidMetricNameError("test.\xc5.metric"), invalidMetricName},
@@ -102,6 +103,17 @@ func TestValidateMetricName(t *testing.T) {
102103
assert.Empty(t, reason)
103104
}
104105

106+
func TestNoMetricNameError_ReportsOffendingSeries(t *testing.T) {
107+
cfg := new(Limits)
108+
cfg.EnforceMetricName = true
109+
110+
err, reason := ValidateMetricName(cfg, cortexpb.FromMetricsToLabelAdapters(model.Metric{"foo": "bar"}), model.LegacyValidation)
111+
require.Error(t, err)
112+
assert.Equal(t, missingMetricName, reason)
113+
// The error must surface the offending series so operators can identify it (see issue #5802).
114+
assert.Equal(t, `sample missing metric name metric: "{foo=\"bar\"}"`, err.Error())
115+
}
116+
105117
func TestValidateLabels(t *testing.T) {
106118
cfg := new(Limits)
107119
userID := "testUser"

0 commit comments

Comments
 (0)