From df94ecebab21a58107938d35d67cc624486c1f83 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gy=C3=B6rgy=20Krajcsovits?= <gyorgy.krajcsovits@grafana.com>
Date: Tue, 3 Jun 2025 12:17:39 +0200
Subject: [PATCH] fix(promql): histogram_count inconsistent
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The problem is in the counter reset detection. The code that loads the
samples is matrixIterSlice which uses the typed Buffer iterator, which
will preload the integer histogram samples, however the last sample is
always(!) loaded as a float histogram sample in matrixIterSlice and the
optimized iterator fails to detect counter resets in that case.

Also the iterator does not reset lastH, lastFH properly.

Ref: https://github.com/prometheus/prometheus/issues/16681

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
---
 promql/histogram_stats_iterator.go      | 15 +++++++--
 promql/histogram_stats_iterator_test.go | 41 +++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/promql/histogram_stats_iterator.go b/promql/histogram_stats_iterator.go
index 459d5924ae..7b749edd71 100644
--- a/promql/histogram_stats_iterator.go
+++ b/promql/histogram_stats_iterator.go
@@ -105,6 +105,7 @@ func (f *histogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram)
 }
 
 func (f *histogramStatsIterator) setLastH(h *histogram.Histogram) {
+	f.lastFH = nil
 	if f.lastH == nil {
 		f.lastH = h.Copy()
 	} else {
@@ -113,6 +114,7 @@ func (f *histogramStatsIterator) setLastH(h *histogram.Histogram) {
 }
 
 func (f *histogramStatsIterator) setLastFH(fh *histogram.FloatHistogram) {
+	f.lastH = nil
 	if f.lastFH == nil {
 		f.lastFH = fh.Copy()
 	} else {
@@ -125,7 +127,13 @@ func (f *histogramStatsIterator) getFloatResetHint(hint histogram.CounterResetHi
 		return hint
 	}
 	if f.lastFH == nil {
-		return histogram.NotCounterReset
+		// If there was no previous histogram, this will not be used,
+		// and if there was a previous integer histogram, this will
+		// force a reset detection. The later can happen if we used the
+		// iterator to read integer histograms before, but switched to
+		// float histograms for whatever reason.
+		// https://github.com/prometheus/prometheus/issues/16681
+		return histogram.UnknownCounterReset
 	}
 
 	if f.currentFH.DetectReset(f.lastFH) {
@@ -139,7 +147,10 @@ func (f *histogramStatsIterator) getResetHint(h *histogram.Histogram) histogram.
 		return h.CounterResetHint
 	}
 	if f.lastH == nil {
-		return histogram.NotCounterReset
+		// If there was no previous histogram, this will not be used,
+		// and if there was a previous float histogram, this will
+		// force a reset detection.
+		return histogram.UnknownCounterReset
 	}
 
 	fh, prevFH := h.ToFloat(nil), f.lastH.ToFloat(nil)
diff --git a/promql/histogram_stats_iterator_test.go b/promql/histogram_stats_iterator_test.go
index 3b99f6ea6f..1460da60f9 100644
--- a/promql/histogram_stats_iterator_test.go
+++ b/promql/histogram_stats_iterator_test.go
@@ -68,7 +68,7 @@ func TestHistogramStatsDecoding(t *testing.T) {
 				tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
 			},
 			expectedHints: []histogram.CounterResetHint{
-				histogram.NotCounterReset,
+				histogram.UnknownCounterReset,
 			},
 		},
 		{
@@ -78,7 +78,7 @@ func TestHistogramStatsDecoding(t *testing.T) {
 				tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
 			},
 			expectedHints: []histogram.CounterResetHint{
-				histogram.NotCounterReset,
+				histogram.UnknownCounterReset,
 				histogram.CounterReset,
 			},
 		},
@@ -90,7 +90,7 @@ func TestHistogramStatsDecoding(t *testing.T) {
 				tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
 			},
 			expectedHints: []histogram.CounterResetHint{
-				histogram.NotCounterReset,
+				histogram.UnknownCounterReset,
 				histogram.UnknownCounterReset,
 				histogram.CounterReset,
 			},
@@ -141,6 +141,41 @@ func TestHistogramStatsDecoding(t *testing.T) {
 	}
 }
 
+func TestHistogramStatsMixedUse(t *testing.T) {
+	histograms := []*histogram.Histogram{
+		tsdbutil.GenerateTestHistogramWithHint(2, histogram.UnknownCounterReset),
+		tsdbutil.GenerateTestHistogramWithHint(4, histogram.UnknownCounterReset),
+		tsdbutil.GenerateTestHistogramWithHint(0, histogram.UnknownCounterReset),
+	}
+
+	series := newHistogramSeries(histograms)
+	it := series.Iterator(nil)
+
+	statsIterator := NewHistogramStatsIterator(it)
+
+	expectedHints := []histogram.CounterResetHint{
+		histogram.UnknownCounterReset,
+		histogram.NotCounterReset,
+		histogram.UnknownCounterReset,
+	}
+	actualHints := make([]histogram.CounterResetHint, 3)
+	typ := statsIterator.Next()
+	require.Equal(t, chunkenc.ValHistogram, typ)
+	_, h := statsIterator.AtHistogram(nil)
+	actualHints[0] = h.CounterResetHint
+	typ = statsIterator.Next()
+	require.Equal(t, chunkenc.ValHistogram, typ)
+	_, h = statsIterator.AtHistogram(nil)
+	actualHints[1] = h.CounterResetHint
+	typ = statsIterator.Next()
+	require.Equal(t, chunkenc.ValHistogram, typ)
+	_, fh := statsIterator.AtFloatHistogram(nil)
+	actualHints[2] = fh.CounterResetHint
+
+	require.Equal(t, chunkenc.ValNone, statsIterator.Next())
+	require.Equal(t, expectedHints, actualHints)
+}
+
 type histogramSeries struct {
 	histograms []*histogram.Histogram
 }