AI Tool Usage Notice
If you used an AI tool to help draft this issue,
please make sure you have reviewed and validated all content before submitting.
You are responsible for the accuracy and quality of everything in this report.
Low-quality or unreviewed AI-generated submissions may be closed without further investigation.
See our Generative AI Contribution Policy for details.
Describe the bug
cortex_ingester_ingestion_delay_seconds (added in #7443) is registered with NativeHistogramMinResetDuration: 1 (pkg/ingester/metrics.go:159):
ingestionDelaySeconds: promauto.With(r).NewHistogramVec(prometheus.HistogramOpts{
Name: "cortex_ingester_ingestion_delay_seconds",
Help: "Delay in seconds between sample ingestion time and sample timestamp.",
NativeHistogramBucketFactor: 1.1,
NativeHistogramMaxBucketNumber: 100,
NativeHistogramMinResetDuration: 1,
Buckets: []float64{1, 5, 10, 30, 60, 120, 300, 600},
}, []string{"user"}),
NativeHistogramMinResetDuration is a time.Duration, i.e. nanoseconds, so the untyped constant 1 means 1 nanosecond, not 1 hour. Every other histogram in this file and repo-wide uses time.Hour — a grep over pkg/ shows line 159 is the only occurrence of a non-time-unit value.
In client_golang, limitBuckets() calls maybeReset(), which resets whenever now - lastResetTime >= nativeHistogramMinResetDuration. At 1ns that condition is effectively always true, so instead of reducing resolution when the native bucket count exceeds NativeHistogramMaxBucketNumber (100), the histogram performs a full reset. histogram.resetCounts() zeroes sumBits, count and the classic upperBounds buckets, so the classic representation is destroyed too — this is not confined to the native representation.
To Reproduce
Steps to reproduce the behavior:
- Construct two histograms with exactly the shipped
HistogramOpts, differing only in NativeHistogramMinResetDuration (1 vs 1 * time.Hour).
- Observe 100,000 values spread over a realistic delay range (0.001s–600s) into both.
- Compare
_count, _sum and the le="600" classic bucket.
I ran this against master (85a4553):
shipped (MinResetDuration=1ns): count=13589 sum=163583.5 le600_bucket=13589
correct (MinResetDuration=1h): count=100000 sum=220144.6 le600_bucket=100000
OBSERVATION LOSS: 86411 of 100000 observations lost (86.4%)
Note the classic le="600" bucket is equally affected, confirming _count and _bucket are both wrong, not just the native buckets.
Expected behavior
The histogram should retain all observations, as every other Cortex histogram does. NativeHistogramMinResetDuration should be 1 * time.Hour.
Environment:
Additional Context
Operator impact: this is the metric one would alert on for ingestion lag. rate(cortex_ingester_ingestion_delay_seconds_count[5m]) and histogram_quantile(0.99, ...ingestion_delay_seconds_bucket) under-report by a large and erratic margin (with spurious counter resets), so the metric can silently fail to fire during a genuine ingestion-delay incident.
Suggested fix:
NativeHistogramMinResetDuration: 1 * time.Hour,
Two adjacent observations, for maintainers to take or leave:
- The classic bucket set starts at 1s, so it cannot resolve any healthy-range delay; sub-second buckets would make the metric more useful.
observeDelay() is called once per sample on the ingester Push path (pkg/ingester/ingester.go:1578 and :1626) with no config gate. Fixing the reset duration removes the frequent-reset lock acquisition in limitBuckets, but observing once per timeseries rather than once per sample would still be cheaper on a hot write path.
Found while reviewing recent master commits.
AI Tool Usage Notice
If you used an AI tool to help draft this issue,
please make sure you have reviewed and validated all content before submitting.
You are responsible for the accuracy and quality of everything in this report.
Low-quality or unreviewed AI-generated submissions may be closed without further investigation.
See our Generative AI Contribution Policy for details.
Describe the bug
cortex_ingester_ingestion_delay_seconds(added in #7443) is registered withNativeHistogramMinResetDuration: 1(pkg/ingester/metrics.go:159):NativeHistogramMinResetDurationis atime.Duration, i.e. nanoseconds, so the untyped constant1means 1 nanosecond, not 1 hour. Every other histogram in this file and repo-wide usestime.Hour— a grep overpkg/shows line 159 is the only occurrence of a non-time-unit value.In client_golang,
limitBuckets()callsmaybeReset(), which resets whenevernow - lastResetTime >= nativeHistogramMinResetDuration. At 1ns that condition is effectively always true, so instead of reducing resolution when the native bucket count exceedsNativeHistogramMaxBucketNumber(100), the histogram performs a full reset.histogram.resetCounts()zeroessumBits,countand the classicupperBoundsbuckets, so the classic representation is destroyed too — this is not confined to the native representation.To Reproduce
Steps to reproduce the behavior:
HistogramOpts, differing only inNativeHistogramMinResetDuration(1vs1 * time.Hour)._count,_sumand thele="600"classic bucket.I ran this against master (85a4553):
Note the classic
le="600"bucket is equally affected, confirming_countand_bucketare both wrong, not just the native buckets.Expected behavior
The histogram should retain all observations, as every other Cortex histogram does.
NativeHistogramMinResetDurationshould be1 * time.Hour.Environment:
Additional Context
Operator impact: this is the metric one would alert on for ingestion lag.
rate(cortex_ingester_ingestion_delay_seconds_count[5m])andhistogram_quantile(0.99, ...ingestion_delay_seconds_bucket)under-report by a large and erratic margin (with spurious counter resets), so the metric can silently fail to fire during a genuine ingestion-delay incident.Suggested fix:
Two adjacent observations, for maintainers to take or leave:
observeDelay()is called once per sample on the ingesterPushpath (pkg/ingester/ingester.go:1578and:1626) with no config gate. Fixing the reset duration removes the frequent-reset lock acquisition inlimitBuckets, but observing once per timeseries rather than once per sample would still be cheaper on a hot write path.Found while reviewing recent master commits.