From 48ab570bcbbe0c7f39882f20811588a3c67a6924 Mon Sep 17 00:00:00 2001 From: Daniel Blando Date: Tue, 20 Oct 2026 18:01:49 +0000 Subject: [PATCH] Ingester: do not count ruler staleness markers as discarded tenant samples Signed-off-by: Daniel Blando --- CHANGELOG.md | 1 + pkg/ingester/ingester.go | 42 ++++++++-- pkg/ingester/ingester_test.go | 153 ++++++++++++++++++++++++++++++++++ 3 files changed, 187 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe8e1f25f4..b6e687fbb47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ * [BUGFIX] Update prometheus alertmanager version to v0.34.1 to apply upstream inhibit rule bug fixes. #7877 * [BUGFIX] Tenant Federation: Fix regex tenant federation not working in single binary mode, and failing with `multiple org IDs present` when a matched tenant ID contains regex metacharacters and also matches other tenants as a regex. #7869 * [BUGFIX] Query Frontend: Fix the data selection time window computed for an instant query sent without a `time` parameter. The default evaluation time was passed to `util.ParseTimeParam` in milliseconds, but that helper formats the default and parses it as seconds, so the value overflowed `UnixNano()` and wrapped to a timestamp decades away from `now`. This made the `data_select_min_time` and `data_select_max_time` query stats wrong, and stopped the `time_window` query attribute of query priority and query rejection from matching such queries correctly. #7880 +* [BUGFIX] Ingester: Staleness markers written by the Ruler are no longer reported as discarded samples for the tenant. #7887 ## 1.22.0 in progress * [CHANGE] Ruler: Remove the deprecated `-ruler.evaluation-delay-duration` flag and its `ruler_evaluation_delay_duration` per-tenant limit. Use `-ruler.query-offset` / `ruler_query_offset`, which no longer takes the higher of the two values. Cortex decodes the runtime config strictly, so a leftover `ruler_evaluation_delay_duration` override makes the runtime config fail to load: Cortex **exits at startup** (`module failed`, `module=runtime-config`), and on an already-running process every reload fails, pinning the last good overrides and dropping `cortex_runtime_config_last_reload_successful` to 0. Run `grep -r ruler_evaluation_delay_duration` over your runtime configs before upgrading. #7792 diff --git a/pkg/ingester/ingester.go b/pkg/ingester/ingester.go index 82dc8fa9894..d877b90a12f 100644 --- a/pkg/ingester/ingester.go +++ b/pkg/ingester/ingester.go @@ -31,6 +31,7 @@ import ( "github.com/prometheus/prometheus/model/exemplar" "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/model/value" "github.com/prometheus/prometheus/storage" "github.com/prometheus/prometheus/tsdb" "github.com/prometheus/prometheus/tsdb/chunkenc" @@ -1436,6 +1437,9 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte perMetricSeriesLimitCount = 0 discardedNativeHistogramCount = 0 + // Only the ruler sets the write request source to RULE. + isRuleSource = req.Source == cortexpb.RULE + // headMaxTime is snapshotted right after the appender is created. The appender // derives its lower time bound checks (e.g. out of bounds) from the head max // time at creation time. We cannot access the appender's internal bound @@ -1449,7 +1453,21 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte } } - handleAppendFailure = func(err error, timestampMs int64, lbls []cortexpb.LabelAdapter, copiedLabels labels.Labels, matchedLabelSetLimits []validation.LimitsPerLabelSet) (rollback bool) { + // isStaleNaN reports whether the failed sample carried the reserved stale NaN + // payload. Callers appending native histograms always pass false, because a + // staleness marker is only ever appended as a float sample. + handleAppendFailure = func(err error, timestampMs int64, lbls []cortexpb.LabelAdapter, copiedLabels labels.Labels, matchedLabelSetLimits []validation.LimitsPerLabelSet, isStaleNaN bool) (rollback bool) { + // A staleness marker from the ruler is internal bookkeeping rather than + // tenant data. Prometheus' rules.Group.cleanupStaleSeries appends one for + // every series a rule stopped producing, and treats out-of-order, too-old + // and duplicate-timestamp rejections as expected when the series is now + // produced by a different rule group, so it does not account for them. + // Do the same here, rather than reporting samples the tenant never sent. + // The rejection is still visible to operators: the TSDB counters exported + // per user are unaffected, and the ruler logs a warning for every failed + // commit. + ruleStaleMarker := isRuleSource && isStaleNaN + // Check if the error is a soft error we can proceed on. If so, we keep track // of it, so that we can return it back to the distributor, which will return a // 400 error to the client. The client (Prometheus) will not retry on 400, and @@ -1463,18 +1481,24 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte }) case errors.Is(cause, storage.ErrOutOfOrderSample): - sampleOutOfOrderCount++ - i.validateMetrics.DiscardedSeriesTracker.Track(sampleOutOfOrder, userID, copiedLabels.Hash()) + if !ruleStaleMarker { + sampleOutOfOrderCount++ + i.validateMetrics.DiscardedSeriesTracker.Track(sampleOutOfOrder, userID, copiedLabels.Hash()) + } updateFirstPartial(func() error { return wrappedTSDBIngestErr(err, model.Time(timestampMs), lbls) }) case errors.Is(cause, storage.ErrDuplicateSampleForTimestamp): - newValueForTimestampCount++ - i.validateMetrics.DiscardedSeriesTracker.Track(newValueForTimestamp, userID, copiedLabels.Hash()) + if !ruleStaleMarker { + newValueForTimestampCount++ + i.validateMetrics.DiscardedSeriesTracker.Track(newValueForTimestamp, userID, copiedLabels.Hash()) + } updateFirstPartial(func() error { return wrappedTSDBIngestErr(err, model.Time(timestampMs), lbls) }) case errors.Is(cause, storage.ErrTooOldSample): - sampleTooOldCount++ - i.validateMetrics.DiscardedSeriesTracker.Track(sampleTooOld, userID, copiedLabels.Hash()) + if !ruleStaleMarker { + sampleTooOldCount++ + i.validateMetrics.DiscardedSeriesTracker.Track(sampleTooOld, userID, copiedLabels.Hash()) + } updateFirstPartial(func() error { return wrappedTSDBIngestErrWithHeadMaxTime(err, model.Time(timestampMs), model.Time(headMaxTime), lbls) }) @@ -1610,7 +1634,7 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte failedSamplesCount++ - if rollback := handleAppendFailure(err, s.TimestampMs, ts.Labels, copiedLabels, matchedLabelSetLimits); !rollback { + if rollback := handleAppendFailure(err, s.TimestampMs, ts.Labels, copiedLabels, matchedLabelSetLimits, value.IsStaleNaN(s.Value)); !rollback { continue } // The error looks an issue on our side, so we should rollback. @@ -1668,7 +1692,7 @@ func (i *Ingester) Push(ctx context.Context, req *cortexpb.WriteRequest) (*corte failedHistogramsCount++ - if rollback := handleAppendFailure(err, hp.TimestampMs, ts.Labels, copiedLabels, matchedLabelSetLimits); !rollback { + if rollback := handleAppendFailure(err, hp.TimestampMs, ts.Labels, copiedLabels, matchedLabelSetLimits, false); !rollback { continue } // The error looks an issue on our side, so we should rollback. diff --git a/pkg/ingester/ingester_test.go b/pkg/ingester/ingester_test.go index 68ca3d23577..d9a5299e0aa 100644 --- a/pkg/ingester/ingester_test.go +++ b/pkg/ingester/ingester_test.go @@ -1414,6 +1414,159 @@ func TestIngester_Push(t *testing.T) { cortex_ingester_memory_series_removed_total{user="test"} 0 `, }, + "ooo disabled, rule staleness marker out of order is not counted as a discarded sample": { + // A staleness marker appended by the ruler is internal bookkeeping, so it is not reported as a discarded tenant sample. The TSDB counter still records the rejection and the request still fails with 400. + reqs: []*cortexpb.WriteRequest{ + cortexpb.ToWriteRequest( + []labels.Labels{metricLabels}, + []cortexpb.Sample{{Value: 2, TimestampMs: 10}}, + nil, + nil, + cortexpb.API), + cortexpb.ToWriteRequest( + []labels.Labels{metricLabels}, + []cortexpb.Sample{{Value: math.Float64frombits(value.StaleNaN), TimestampMs: 9}}, + nil, + nil, + cortexpb.RULE), + }, + expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "%s", wrapWithUser(wrappedTSDBIngestErr(storage.ErrOutOfOrderSample, model.Time(9), cortexpb.FromLabelsToLabelAdapters(metricLabels)), userID).Error()), + expectedIngested: []cortexpb.TimeSeries{ + {Labels: metricLabelAdapters, Samples: []cortexpb.Sample{{Value: 2, TimestampMs: 10}}}, + }, + additionalMetrics: []string{ + "cortex_discarded_samples_total", + "cortex_ingester_tsdb_out_of_order_samples_total", + }, + expectedMetrics: ` + # HELP cortex_ingester_ingested_samples_total The total number of samples ingested. + # TYPE cortex_ingester_ingested_samples_total counter + cortex_ingester_ingested_samples_total 1 + # HELP cortex_ingester_ingested_samples_failures_total The total number of samples that errored on ingestion. + # TYPE cortex_ingester_ingested_samples_failures_total counter + cortex_ingester_ingested_samples_failures_total 1 + # HELP cortex_ingester_memory_users The current number of users in memory. + # TYPE cortex_ingester_memory_users gauge + cortex_ingester_memory_users 1 + # HELP cortex_ingester_memory_series The current number of series in memory. + # TYPE cortex_ingester_memory_series gauge + cortex_ingester_memory_series 1 + # HELP cortex_ingester_memory_series_created_total The total number of series that were created per user. + # TYPE cortex_ingester_memory_series_created_total counter + cortex_ingester_memory_series_created_total{user="test"} 1 + # HELP cortex_ingester_memory_series_removed_total The total number of series that were removed per user. + # TYPE cortex_ingester_memory_series_removed_total counter + cortex_ingester_memory_series_removed_total{user="test"} 0 + # HELP cortex_ingester_tsdb_out_of_order_samples_total Total number of out of order samples ingestion failed attempts due to out of order being disabled. + # TYPE cortex_ingester_tsdb_out_of_order_samples_total counter + cortex_ingester_tsdb_out_of_order_samples_total{type="float",user="test"} 1 + cortex_ingester_tsdb_out_of_order_samples_total{type="histogram",user="test"} 0 + `, + }, + "ooo disabled, rule result out of order is counted as a discarded sample": { + // Rule evaluation results are tenant data even though Source is RULE, so they must still be reported as discarded. + reqs: []*cortexpb.WriteRequest{ + cortexpb.ToWriteRequest( + []labels.Labels{metricLabels}, + []cortexpb.Sample{{Value: 2, TimestampMs: 10}}, + nil, + nil, + cortexpb.API), + cortexpb.ToWriteRequest( + []labels.Labels{metricLabels}, + []cortexpb.Sample{{Value: 1, TimestampMs: 9}}, + nil, + nil, + cortexpb.RULE), + }, + expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "%s", wrapWithUser(wrappedTSDBIngestErr(storage.ErrOutOfOrderSample, model.Time(9), cortexpb.FromLabelsToLabelAdapters(metricLabels)), userID).Error()), + expectedIngested: []cortexpb.TimeSeries{ + {Labels: metricLabelAdapters, Samples: []cortexpb.Sample{{Value: 2, TimestampMs: 10}}}, + }, + additionalMetrics: []string{ + "cortex_discarded_samples_total", + "cortex_ingester_tsdb_out_of_order_samples_total", + }, + expectedMetrics: ` + # HELP cortex_ingester_ingested_samples_total The total number of samples ingested. + # TYPE cortex_ingester_ingested_samples_total counter + cortex_ingester_ingested_samples_total 1 + # HELP cortex_ingester_ingested_samples_failures_total The total number of samples that errored on ingestion. + # TYPE cortex_ingester_ingested_samples_failures_total counter + cortex_ingester_ingested_samples_failures_total 1 + # HELP cortex_ingester_memory_users The current number of users in memory. + # TYPE cortex_ingester_memory_users gauge + cortex_ingester_memory_users 1 + # HELP cortex_ingester_memory_series The current number of series in memory. + # TYPE cortex_ingester_memory_series gauge + cortex_ingester_memory_series 1 + # HELP cortex_ingester_memory_series_created_total The total number of series that were created per user. + # TYPE cortex_ingester_memory_series_created_total counter + cortex_ingester_memory_series_created_total{user="test"} 1 + # HELP cortex_ingester_memory_series_removed_total The total number of series that were removed per user. + # TYPE cortex_ingester_memory_series_removed_total counter + cortex_ingester_memory_series_removed_total{user="test"} 0 + # HELP cortex_ingester_tsdb_out_of_order_samples_total Total number of out of order samples ingestion failed attempts due to out of order being disabled. + # TYPE cortex_ingester_tsdb_out_of_order_samples_total counter + cortex_ingester_tsdb_out_of_order_samples_total{type="float",user="test"} 1 + cortex_ingester_tsdb_out_of_order_samples_total{type="histogram",user="test"} 0 + # HELP cortex_discarded_samples_total The total number of samples that were discarded. + # TYPE cortex_discarded_samples_total counter + cortex_discarded_samples_total{reason="sample-out-of-order",user="test"} 1 + `, + }, + "ooo disabled, tenant staleness marker out of order is counted as a discarded sample": { + // Tenants legitimately send staleness markers over remote write, so a stale NaN from a non-ruler source must still be reported as discarded. + reqs: []*cortexpb.WriteRequest{ + cortexpb.ToWriteRequest( + []labels.Labels{metricLabels}, + []cortexpb.Sample{{Value: 2, TimestampMs: 10}}, + nil, + nil, + cortexpb.API), + cortexpb.ToWriteRequest( + []labels.Labels{metricLabels}, + []cortexpb.Sample{{Value: math.Float64frombits(value.StaleNaN), TimestampMs: 9}}, + nil, + nil, + cortexpb.API), + }, + expectedErr: httpgrpc.Errorf(http.StatusBadRequest, "%s", wrapWithUser(wrappedTSDBIngestErr(storage.ErrOutOfOrderSample, model.Time(9), cortexpb.FromLabelsToLabelAdapters(metricLabels)), userID).Error()), + expectedIngested: []cortexpb.TimeSeries{ + {Labels: metricLabelAdapters, Samples: []cortexpb.Sample{{Value: 2, TimestampMs: 10}}}, + }, + additionalMetrics: []string{ + "cortex_discarded_samples_total", + "cortex_ingester_tsdb_out_of_order_samples_total", + }, + expectedMetrics: ` + # HELP cortex_ingester_ingested_samples_total The total number of samples ingested. + # TYPE cortex_ingester_ingested_samples_total counter + cortex_ingester_ingested_samples_total 1 + # HELP cortex_ingester_ingested_samples_failures_total The total number of samples that errored on ingestion. + # TYPE cortex_ingester_ingested_samples_failures_total counter + cortex_ingester_ingested_samples_failures_total 1 + # HELP cortex_ingester_memory_users The current number of users in memory. + # TYPE cortex_ingester_memory_users gauge + cortex_ingester_memory_users 1 + # HELP cortex_ingester_memory_series The current number of series in memory. + # TYPE cortex_ingester_memory_series gauge + cortex_ingester_memory_series 1 + # HELP cortex_ingester_memory_series_created_total The total number of series that were created per user. + # TYPE cortex_ingester_memory_series_created_total counter + cortex_ingester_memory_series_created_total{user="test"} 1 + # HELP cortex_ingester_memory_series_removed_total The total number of series that were removed per user. + # TYPE cortex_ingester_memory_series_removed_total counter + cortex_ingester_memory_series_removed_total{user="test"} 0 + # HELP cortex_ingester_tsdb_out_of_order_samples_total Total number of out of order samples ingestion failed attempts due to out of order being disabled. + # TYPE cortex_ingester_tsdb_out_of_order_samples_total counter + cortex_ingester_tsdb_out_of_order_samples_total{type="float",user="test"} 1 + cortex_ingester_tsdb_out_of_order_samples_total{type="histogram",user="test"} 0 + # HELP cortex_discarded_samples_total The total number of samples that were discarded. + # TYPE cortex_discarded_samples_total counter + cortex_discarded_samples_total{reason="sample-out-of-order",user="test"} 1 + `, + }, "ooo disabled, should soft fail on sample out of order": { reqs: []*cortexpb.WriteRequest{ cortexpb.ToWriteRequest(