Fix metrics update for OOO native histograms

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

tsdb/head_append.go

@@ -321,8 +321,8 @@ type headAppender struct {
 }
 
 func (a *headAppender) Append(ref storage.SeriesRef, lset labels.Labels, t int64, v float64) (storage.SeriesRef, error) {
-	// For OOO inserts, this restriction is irrelevant and will be checked later once we confirm the sample is an in-order append.
+	// Fail fast if OOO is disabled and the sample is out of bounds.
-	// Fail fast if OOO is disabled.
+	// Otherwise a full check will be done later to decide if the sample is in-order or out-of-order.
 	if a.oooTimeWindow == 0 && t < a.minValidTime {
 		a.head.metrics.outOfBoundSamples.WithLabelValues(sampleMetricTypeFloat).Inc()
 		return 0, storage.ErrOutOfBounds
@@ -625,16 +625,12 @@ func (a *headAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels
 		return 0, storage.ErrNativeHistogramsDisabled
 	}
 
-	// For OOO inserts, this restriction is irrelevant and will be checked later once we confirm the histogram sample is an in-order append.
+	// Fail fast if OOO is disabled and the sample is out of bounds.
-	// Fail fast if OOO is disabled.
+	// Otherwise a full check will be done later to decide if the sample is in-order or out-of-order.
-	if a.oooTimeWindow == 0 && t < a.minValidTime {
+	if (a.oooTimeWindow == 0 || !a.head.opts.EnableOOONativeHistograms.Load()) && t < a.minValidTime {
 		a.head.metrics.outOfBoundSamples.WithLabelValues(sampleMetricTypeHistogram).Inc()
 		return 0, storage.ErrOutOfBounds
 	}
-	// Also fail fast if OOO is enabled, but OOO native histogram ingestion is disabled.
-	if a.oooTimeWindow > 0 && t < a.minValidTime && t >= a.headMaxt-a.oooTimeWindow && !a.head.opts.EnableOOONativeHistograms.Load() {
-		return 0, storage.ErrOOONativeHistogramsDisabled
-	}
 
 	if h != nil {
 		if err := h.Validate(); err != nil {
@@ -696,6 +692,8 @@ func (a *headAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels
 		if err != nil {
 			switch {
 			case errors.Is(err, storage.ErrOutOfOrderSample):
+				fallthrough
+			case errors.Is(err, storage.ErrOOONativeHistogramsDisabled):
 				a.head.metrics.outOfOrderSamples.WithLabelValues(sampleMetricTypeHistogram).Inc()
 			case errors.Is(err, storage.ErrTooOldSample):
 				a.head.metrics.tooOldSamples.WithLabelValues(sampleMetricTypeHistogram).Inc()
@@ -723,6 +721,8 @@ func (a *headAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels
 		if err != nil {
 			switch {
 			case errors.Is(err, storage.ErrOutOfOrderSample):
+				fallthrough
+			case errors.Is(err, storage.ErrOOONativeHistogramsDisabled):
 				a.head.metrics.outOfOrderSamples.WithLabelValues(sampleMetricTypeHistogram).Inc()
 			case errors.Is(err, storage.ErrTooOldSample):
 				a.head.metrics.tooOldSamples.WithLabelValues(sampleMetricTypeHistogram).Inc()

tsdb/head_test.go

@@ -2667,22 +2667,37 @@ func TestIsolationWithoutAdd(t *testing.T) {
 func TestOutOfOrderSamplesMetric(t *testing.T) {
 	for name, scenario := range sampleTypeScenarios {
 		t.Run(name, func(t *testing.T) {
-			testOutOfOrderSamplesMetric(t, scenario)
+			options := DefaultOptions()
+			options.EnableNativeHistograms = true
+			options.EnableOOONativeHistograms = true
+			testOutOfOrderSamplesMetric(t, scenario, options, storage.ErrOutOfOrderSample)
 		})
 	}
 }
 
-func testOutOfOrderSamplesMetric(t *testing.T, scenario sampleTypeScenario) {
+func TestOutOfOrderSamplesMetricNativeHistogramOOODisabled(t *testing.T) {
-	dir := t.TempDir()
+	for name, scenario := range sampleTypeScenarios {
+		if scenario.sampleType != "histogram" {
+			continue
+		}
+		t.Run(name, func(t *testing.T) {
+			options := DefaultOptions()
+			options.OutOfOrderTimeWindow = (1000 * time.Minute).Milliseconds()
+			options.EnableNativeHistograms = true
+			options.EnableOOONativeHistograms = false
+			testOutOfOrderSamplesMetric(t, scenario, options, storage.ErrOOONativeHistogramsDisabled)
+		})
+	}
+}
 
-	db, err := Open(dir, nil, nil, DefaultOptions(), nil)
+func testOutOfOrderSamplesMetric(t *testing.T, scenario sampleTypeScenario, options *Options, expectOutOfOrderError error) {
+	dir := t.TempDir()
+	db, err := Open(dir, nil, nil, options, nil)
 	require.NoError(t, err)
 	defer func() {
 		require.NoError(t, db.Close())
 	}()
 	db.DisableCompactions()
-	db.EnableNativeHistograms()
-	db.EnableOOONativeHistograms()
 
 	appendSample := func(appender storage.Appender, ts int64) (storage.SeriesRef, error) {
 		ref, _, err := scenario.appendFunc(appender, labels.FromStrings("a", "b"), ts, 99)
@@ -2701,15 +2716,15 @@ func testOutOfOrderSamplesMetric(t *testing.T, scenario sampleTypeScenario) {
 	require.Equal(t, 0.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 	app = db.Appender(ctx)
 	_, err = appendSample(app, 2)
-	require.Equal(t, storage.ErrOutOfOrderSample, err)
+	require.Equal(t, expectOutOfOrderError, err)
 	require.Equal(t, 1.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 
 	_, err = appendSample(app, 3)
-	require.Equal(t, storage.ErrOutOfOrderSample, err)
+	require.Equal(t, expectOutOfOrderError, err)
 	require.Equal(t, 2.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 
 	_, err = appendSample(app, 4)
-	require.Equal(t, storage.ErrOutOfOrderSample, err)
+	require.Equal(t, expectOutOfOrderError, err)
 	require.Equal(t, 3.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 	require.NoError(t, app.Commit())
 
@@ -2744,15 +2759,15 @@ func testOutOfOrderSamplesMetric(t *testing.T, scenario sampleTypeScenario) {
 	// Test out of order metric.
 	app = db.Appender(ctx)
 	_, err = appendSample(app, db.head.minValidTime.Load()+DefaultBlockDuration+2)
-	require.Equal(t, storage.ErrOutOfOrderSample, err)
+	require.Equal(t, expectOutOfOrderError, err)
 	require.Equal(t, 4.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 
 	_, err = appendSample(app, db.head.minValidTime.Load()+DefaultBlockDuration+3)
-	require.Equal(t, storage.ErrOutOfOrderSample, err)
+	require.Equal(t, expectOutOfOrderError, err)
 	require.Equal(t, 5.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 
 	_, err = appendSample(app, db.head.minValidTime.Load()+DefaultBlockDuration+4)
-	require.Equal(t, storage.ErrOutOfOrderSample, err)
+	require.Equal(t, expectOutOfOrderError, err)
 	require.Equal(t, 6.0, prom_testutil.ToFloat64(db.head.metrics.outOfOrderSamples.WithLabelValues(scenario.sampleType)))
 	require.NoError(t, app.Commit())
 }