ingester: fix ingestion delay histogram reset duration

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## master / unreleased
+* [BUGFIX] Ingester: Fix `cortex_ingester_ingestion_delay_seconds` native histogram losing ~86% of observations by setting `NativeHistogramMinResetDuration` to 1h instead of the bare integer `1` (interpreted as 1ns). #7731
 * [FEATURE] Engine: Add `-querier.selector-batch-size` and `-ruler.selector-batch-size` flags to configure series batching in the Thanos promQL engine. 0 disables batching. #7763
 * [CHANGE] Querier: Make query time range configurations per-tenant: `query_ingesters_within`, `query_store_after`, and `shuffle_sharding_ingesters_lookback_period`. Uses `model.Duration` instead of `time.Duration` to support serialization but has minimum unit of 1ms (nanoseconds/microseconds not supported). #7160
 * [CHANGE] Cache: Setting `-blocks-storage.bucket-store.metadata-cache.bucket-index-content-ttl` to 0 will disable the bucket-index cache. #7446

pkg/ingester/metrics.go

@@ -88,6 +88,20 @@ type ingesterMetrics struct {
 	unoptimizedRegexRejectedTotal    *prometheus.CounterVec
 }
 
+// ingestionDelaySecondsHistogramOpts defines the options for the
+// cortex_ingester_ingestion_delay_seconds native histogram. NativeHistogramMinResetDuration
+// must be a real duration (time.Hour): a bare integer literal is interpreted as
+// nanoseconds, which resets the native histogram on essentially every scrape and
+// discards the vast majority of observations (see cortexproject/cortex#7731).
+var ingestionDelaySecondsHistogramOpts = 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 * time.Hour,
+	Buckets:                         []float64{1, 5, 10, 30, 60, 120, 300, 600}, // 1s, 5s, 10s, 30s, 1m, 2m, 5m, 10m
+}
+
 func newIngesterMetrics(r prometheus.Registerer,
 	createMetricsConflictingWithTSDB bool,
 	activeSeriesEnabled bool,
@@ -151,14 +165,7 @@ func newIngesterMetrics(r prometheus.Registerer,
 			NativeHistogramMinResetDuration: 1 * time.Hour,
 			Buckets:                         prometheus.ExponentialBuckets(1, 2, 10), // 1 to 512 buckets
 		}, []string{"user"}),
-		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}, // 1s, 5s, 10s, 30s, 1m, 2m, 5m, 10m
-		}, []string{"user"}),
+		ingestionDelaySeconds: promauto.With(r).NewHistogramVec(ingestionDelaySecondsHistogramOpts, []string{"user"}),
 		oooLabelsTotal: promauto.With(r).NewCounterVec(prometheus.CounterOpts{
 			Name: "cortex_ingester_out_of_order_labels_total",
 			Help: "The total number of out of order label found per user.",

pkg/ingester/metrics_test.go

@@ -3,6 +3,7 @@ package ingester
 import (
 	"bytes"
 	"testing"
+	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
@@ -1298,3 +1299,14 @@ func populateTSDBMetrics(base float64) *prometheus.Registry {
 
 	return r
 }
+
+// TestIngestionDelaySecondsHistogramResetDuration is a regression test for
+// cortexproject/cortex#7731: ingestionDelaySeconds was registered with
+// NativeHistogramMinResetDuration: 1, where 1 is interpreted as 1 nanosecond
+// (the field is a time.Duration). That reset the native histogram on essentially
+// every scrape and discarded ~86% of observations. The reset duration must be a
+// real hour-long window.
+func TestIngestionDelaySecondsHistogramResetDuration(t *testing.T) {
+	require.Equal(t, time.Hour, ingestionDelaySecondsHistogramOpts.NativeHistogramMinResetDuration,
+		"NativeHistogramMinResetDuration must be time.Hour, not a bare integer (interpreted as nanoseconds)")
+}

pkg/util/request_tracker/request_extractor.go

@@ -83,7 +83,10 @@ func trimStringByBytes(str string, size int) string {
 	bytesStr := []byte(str)
 	trimIndex := len(bytesStr)
 	if size < len(bytesStr) {
-		for !utf8.RuneStart(bytesStr[size]) {
+		// Scan backwards to a rune boundary. Bound the scan at size > 0: if the
+		// string has no rune start (e.g. only UTF-8 continuation bytes) the loop
+		// must not underflow past zero, which would panic on bytesStr[-1].
+		for size > 0 && !utf8.RuneStart(bytesStr[size]) {
 			size--
 		}
 		trimIndex = size

pkg/util/request_tracker/request_tracker_test.go

@@ -191,3 +191,20 @@ func TestRangedQueryExtractorMultiByteTruncation(t *testing.T) {
 		assert.True(t, utf8.Valid(entry), "entry should be valid UTF-8")
 	})
 }
+
+// TestTrimForJsonMarshalContinuationBytes reproduces cortexproject/cortex#7729:
+// when the string consists only of UTF-8 continuation bytes (0x80-0xBF) there is
+// no rune start to scan back to, so the backwards scan in trimStringByBytes
+// underflows past zero and panics with index out of range [-1]. The fix bounds
+// the scan at size > 0.
+func TestTrimForJsonMarshalContinuationBytes(t *testing.T) {
+	// 1200 continuation bytes (0x80) with a truncation size below the length.
+	continuation := strings.Repeat("\x80", 1200)
+
+	require.NotPanics(t, func() {
+		out := trimForJsonMarshal(continuation, 900)
+		// Result must always be valid UTF-8 (no partial runes).
+		assert.True(t, utf8.ValidString(out), "result should be valid UTF-8")
+		assert.LessOrEqual(t, len(out), len(continuation))
+	})
+}