Repository: prometheus/prometheus
File: promql/histogram_stats_iterator.go (Go)

Code before:
		Iterator:  it,
		currentFH: &histogram.FloatHistogram{},
	}
}

// Reset resets this iterator for use with a new underlying iterator, reusing
// objects already allocated where possible.
func (f *HistogramStatsIterator) Reset(it chunkenc.Iterator) {
	f.Iterator = it
	f.lastFH = nil
}

// Next mostly relays to the underlying iterator, but changes a ValHistogram
// return into a ValFloatHistogram return.
func (f *HistogramStatsIterator) Next() chunkenc.ValueType {
	vt := f.Iterator.Next()
	if vt == chunkenc.ValHistogram {
		return chunkenc.ValFloatHistogram
	}
	return vt
}

// Seek mostly relays to the underlying iterator, but changes a ValHistogram
// return into a ValFloatHistogram return.
func (f *HistogramStatsIterator) Seek(t int64) chunkenc.ValueType {
	vt := f.Iterator.Seek(t)
	if vt == chunkenc.ValHistogram {
		return chunkenc.ValFloatHistogram
	}
	return vt
}

func (f *HistogramStatsIterator) At() (int64, float64) {
	return f.Iterator.At()
}

// AtHistogram must never be called.
func (*HistogramStatsIterator) AtHistogram(*histogram.Histogram) (int64, *histogram.Histogram) {
	panic("HistogramStatsIterator.AtHistogram must never be called")
}

// AtFloatHistogram returns the next timestamp/float histogram pair. The method
// performs a counter reset detection on the fly. It will return an explicit
// hint (not UnknownCounterReset) if the previous sample has been accessed with
// the same iterator.
func (f *HistogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int64, *histogram.FloatHistogram) {
	var t int64
	t, f.currentFH = f.Iterator.AtFloatHistogram(f.currentFH)
	if value.IsStaleNaN(f.currentFH.Sum) {
		return t, &histogram.FloatHistogram{Sum: f.currentFH.Sum}
	}

Review comment (bug):
I was wondering if we should reset `lastFH` here, because if we seek past a reset to a sample that has "NotReset" hint set, we'll miss the reset , even if we could have detected it.

This happens in this case for example:
```
load 1m
    h{} {{schema:0 count:0 sum:0}}+{{schema:0 count:1 sum:10 buckets:[1]}}x20 {{schema:0 count:0 sum:0}}+{{schema:0 count:1 sum:10 buckets:[1]}}x20

eval instant at 41m increase(histogram_count(h)[40m:9m])
    h{} 35.55555555555556
```

Debug printout:
```
HistogramStatsIterator.AtFloatHistogram: 18 {count:18, sum:180, (0.5,1]:18} hint:2 retHint:2
vectorSelectorSingle: refTime=1080000, returning t=1080000
HistogramStatsIterator.AtFloatHistogram: 22 {count:1, sum:10, (0.5,1]:1} hint:2 retHint:2
```

It's not a problem in this case actually, since increase will work on the float numbers that result from this, so the result is correct. But it would be wrong if the reset happened in a bucket, e.g. this fails for the first eval:
```
load 1m
     h{} {{schema:0 count:1 sum:10 buckets:[1]}}+{{}}x20 {{schema:0 count:1 sum:10 buckets:[0 1]}}+{{}}x20

# This fails, because the result is 0 instead.
eval instant at 41m increase(histogram_count(h)[40m:9m])
    {} 1.4814814814814814

eval instant at 41m increase(h[40m:9m])
    {} {{count:1.4814814814814814 sum:14.814814814814813 counter_reset_hint:gauge offset:1 buckets:[1.4814814814814814]}}
```

Interestingly, clearing `lastFH` doesn't help: 1) we don't look at it since `f.getFloatResetHint` trusts the `NotReset` hint, and 2) increase will look at the resulting float anyway.

So my conclusion is that we need to document that the order of `histogram_count`, `histogram_sum` and `histogram_avg` vs. increase, irate, rate isn't equivalent in some edge cases. (cc @charleskorn) . Also adding the above test with the wrong number could be useful to manage expectations and document in code how this works.

Makes me wonder if should even bother doing the counter reset detection, but that's a different question.

Code after:
	return &HistogramStatsIterator{
		Iterator: it,
		current:  &histogram.FloatHistogram{},
	}
}

// Reset resets this iterator for use with a new underlying iterator, reusing
// objects already allocated where possible.
func (hsi *HistogramStatsIterator) Reset(it chunkenc.Iterator) {
	hsi.Iterator = it
	hsi.last = nil
	hsi.lastIsCurrent = false
}

// Next mostly relays to the underlying iterator, but changes a ValHistogram
// return into a ValFloatHistogram return.
func (hsi *HistogramStatsIterator) Next() chunkenc.ValueType {
	hsi.lastIsCurrent = false
	vt := hsi.Iterator.Next()
	if vt == chunkenc.ValHistogram {
		return chunkenc.ValFloatHistogram
	}
	return vt
}

// Seek mostly relays to the underlying iterator, but changes a ValHistogram
// return into a ValFloatHistogram return.
func (hsi *HistogramStatsIterator) Seek(t int64) chunkenc.ValueType {
	// If the Seek is going to move the iterator, we have to forget the
	// lastFH and mark the currentFH as not current anymore.
	if t > hsi.AtT() {
		hsi.last = nil
		hsi.lastIsCurrent = false
	}
	vt := hsi.Iterator.Seek(t)
	if vt == chunkenc.ValHistogram {
		return chunkenc.ValFloatHistogram
	}
	return vt
}

// AtHistogram must never be called.
func (*HistogramStatsIterator) AtHistogram(*histogram.Histogram) (int64, *histogram.Histogram) {
	panic("HistogramStatsIterator.AtHistogram must never be called")
}

// AtFloatHistogram returns the next timestamp/float histogram pair. The method
// performs a counter reset detection on the fly. It will return an explicit
// hint (not UnknownCounterReset) if the previous sample has been accessed with
// the same iterator.
func (hsi *HistogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram) (int64, *histogram.FloatHistogram) {