diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cc3578201..7299377403 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ * [BUGFIX] Ring: Fix DynamoDB KV CAS not retrying on transactional conditional check failures. `TransactWriteItems` reports condition failures as `TransactionCanceledException` with a `ConditionalCheckFailed` cancellation reason, which was not recognized as retryable, so any concurrent ring update conflict (e.g. many ingesters joining during a rolling update) failed immediately instead of re-reading and retrying. `TransactionConflict` cancellation reasons are also treated as retryable. #7706 * [BUGFIX] Distributor: Return HTTP 499 (Client Closed Request) instead of 500 when a remote-write or OTLP push is canceled by the client, so client-side cancellations are no longer counted as server-side errors. #7717 * [BUGFIX] Querier: Fix gRPC `codes.Canceled` errors being mapped to HTTP 500 instead of 499 when a client cancels a query. #7738 +* [BUGFIX] Ingester: Fix `cortex_ingester_ingestion_delay_seconds` losing the large majority of observations. `NativeHistogramMinResetDuration` was set to an untyped `1` (1 nanosecond) instead of `1 * time.Hour`, causing the native histogram to fully reset instead of gracefully reducing resolution every time it exceeded its bucket limit. #7744 ## 1.21.1 2026-06-04 diff --git a/pkg/ingester/metrics.go b/pkg/ingester/metrics.go index 3ad21faad6..e5e038f68c 100644 --- a/pkg/ingester/metrics.go +++ b/pkg/ingester/metrics.go @@ -156,7 +156,7 @@ func newIngesterMetrics(r prometheus.Registerer, Help: "Delay in seconds between sample ingestion time and sample timestamp.", NativeHistogramBucketFactor: 1.1, NativeHistogramMaxBucketNumber: 100, - NativeHistogramMinResetDuration: 1, + NativeHistogramMinResetDuration: 1 * time.Hour, Buckets: []float64{1, 5, 10, 30, 60, 120, 300, 600}, // 1s, 5s, 10s, 30s, 1m, 2m, 5m, 10m }, []string{"user"}), oooLabelsTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{ diff --git a/pkg/ingester/metrics_test.go b/pkg/ingester/metrics_test.go index af09d07e2a..e9375ae569 100644 --- a/pkg/ingester/metrics_test.go +++ b/pkg/ingester/metrics_test.go @@ -7,6 +7,7 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/testutil" + dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/require" util_math "github.com/cortexproject/cortex/pkg/util/math" @@ -1298,3 +1299,46 @@ func populateTSDBMetrics(base float64) *prometheus.Registry { return r } + +// TestIngestionDelaySecondsHistogram_DoesNotLoseObservationsOnNativeBucketLimit +// is a regression test for a bug where ingestionDelaySeconds was registered +// with NativeHistogramMinResetDuration effectively equal to zero (an untyped +// constant "1", i.e. 1 nanosecond, rather than 1 hour). Once the native +// histogram exceeds NativeHistogramMaxBucketNumber, client_golang's +// limitBuckets() first tries maybeReset(), which fully resets the histogram +// (both native and classic buckets, keeping only the latest observation) if +// at least NativeHistogramMinResetDuration has elapsed since the last reset. +// With a ~0 duration that condition is satisfied on essentially every call, +// so instead of gracefully reducing resolution (bucket width doubling / zero +// bucket widening), the histogram silently drops the vast majority of prior +// observations on every bucket-limit breach. +func TestIngestionDelaySecondsHistogram_DoesNotLoseObservationsOnNativeBucketLimit(t *testing.T) { + ingestionRate := util_math.NewEWMARate(0.2, instanceIngestionRateTickInterval) + inflightPushRequests := util_math.MaxTracker{} + maxInflightQueryRequests := util_math.MaxTracker{} + + reg := prometheus.NewRegistry() + m := newIngesterMetrics(reg, false, false, false, false, + func() *InstanceLimits { return &InstanceLimits{} }, + ingestionRate, &inflightPushRequests, &maxInflightQueryRequests, false, false) + + observer := m.ingestionDelaySeconds.WithLabelValues("user") + + // Observe many widely-spread values so that the native histogram's + // bucket count exceeds NativeHistogramMaxBucketNumber (100) well before + // the loop ends, forcing limitBuckets()/maybeReset() to run repeatedly. + const numObservations = 500 + value := 0.001 + for i := 0; i < numObservations; i++ { + observer.Observe(value) + value *= 1.2 // bucket factor is 1.1, so each step lands in a new native bucket + } + + metric := &dto.Metric{} + require.NoError(t, observer.(prometheus.Metric).Write(metric)) + + // With a correctly configured (non-trivial) minimum reset duration, no + // observations should be lost: the bucket count is reduced by merging + // buckets, not by discarding samples. + require.Equal(t, uint64(numObservations), metric.GetHistogram().GetSampleCount()) +}