Merge pull request #14514 from fpetkovski/counter-reset-nan

Ignore stale histograms for counter reset detection
This commit is contained in:
George Krajcsovits 2024-07-30 08:43:06 +02:00 committed by GitHub
commit 395f7088c3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 117 additions and 51 deletions

View file

@ -48,7 +48,6 @@ func (f *histogramStatsIterator) AtHistogram(h *histogram.Histogram) (int64, *hi
var t int64 var t int64
t, f.currentH = f.Iterator.AtHistogram(f.currentH) t, f.currentH = f.Iterator.AtHistogram(f.currentH)
if value.IsStaleNaN(f.currentH.Sum) { if value.IsStaleNaN(f.currentH.Sum) {
f.setLastH(f.currentH)
h = &histogram.Histogram{Sum: f.currentH.Sum} h = &histogram.Histogram{Sum: f.currentH.Sum}
return t, h return t, h
} }
@ -77,7 +76,6 @@ func (f *histogramStatsIterator) AtFloatHistogram(fh *histogram.FloatHistogram)
var t int64 var t int64
t, f.currentFH = f.Iterator.AtFloatHistogram(f.currentFH) t, f.currentFH = f.Iterator.AtFloatHistogram(f.currentFH)
if value.IsStaleNaN(f.currentFH.Sum) { if value.IsStaleNaN(f.currentFH.Sum) {
f.setLastFH(f.currentFH)
return t, &histogram.FloatHistogram{Sum: f.currentFH.Sum} return t, &histogram.FloatHistogram{Sum: f.currentFH.Sum}
} }

View file

@ -14,62 +14,132 @@
package promql package promql
import ( import (
"fmt"
"math"
"testing" "testing"
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
"github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/model/value"
"github.com/prometheus/prometheus/tsdb/chunkenc" "github.com/prometheus/prometheus/tsdb/chunkenc"
"github.com/prometheus/prometheus/tsdb/tsdbutil" "github.com/prometheus/prometheus/tsdb/tsdbutil"
) )
func TestHistogramStatsDecoding(t *testing.T) { func TestHistogramStatsDecoding(t *testing.T) {
histograms := []*histogram.Histogram{ cases := []struct {
tsdbutil.GenerateTestHistogram(0), name string
tsdbutil.GenerateTestHistogram(1), histograms []*histogram.Histogram
tsdbutil.GenerateTestHistogram(2), expectedHints []histogram.CounterResetHint
tsdbutil.GenerateTestHistogram(2), }{
} {
histograms[0].CounterResetHint = histogram.NotCounterReset name: "unknown counter reset triggers detection",
histograms[1].CounterResetHint = histogram.UnknownCounterReset histograms: []*histogram.Histogram{
histograms[2].CounterResetHint = histogram.CounterReset tsdbutil.GenerateTestHistogramWithHint(0, histogram.NotCounterReset),
histograms[3].CounterResetHint = histogram.UnknownCounterReset tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
tsdbutil.GenerateTestHistogramWithHint(2, histogram.CounterReset),
expectedHints := []histogram.CounterResetHint{ tsdbutil.GenerateTestHistogramWithHint(2, histogram.UnknownCounterReset),
},
expectedHints: []histogram.CounterResetHint{
histogram.NotCounterReset, histogram.NotCounterReset,
histogram.NotCounterReset, histogram.NotCounterReset,
histogram.CounterReset, histogram.CounterReset,
histogram.NotCounterReset, histogram.NotCounterReset,
},
},
{
name: "stale sample before unknown reset hint",
histograms: []*histogram.Histogram{
tsdbutil.GenerateTestHistogramWithHint(0, histogram.NotCounterReset),
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
{Sum: math.Float64frombits(value.StaleNaN)},
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
},
expectedHints: []histogram.CounterResetHint{
histogram.NotCounterReset,
histogram.NotCounterReset,
histogram.UnknownCounterReset,
histogram.NotCounterReset,
},
},
{
name: "unknown counter reset at the beginning",
histograms: []*histogram.Histogram{
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
},
expectedHints: []histogram.CounterResetHint{
histogram.NotCounterReset,
},
},
{
name: "detect real counter reset",
histograms: []*histogram.Histogram{
tsdbutil.GenerateTestHistogramWithHint(2, histogram.UnknownCounterReset),
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
},
expectedHints: []histogram.CounterResetHint{
histogram.NotCounterReset,
histogram.CounterReset,
},
},
{
name: "detect real counter reset after stale NaN",
histograms: []*histogram.Histogram{
tsdbutil.GenerateTestHistogramWithHint(2, histogram.UnknownCounterReset),
{Sum: math.Float64frombits(value.StaleNaN)},
tsdbutil.GenerateTestHistogramWithHint(1, histogram.UnknownCounterReset),
},
expectedHints: []histogram.CounterResetHint{
histogram.NotCounterReset,
histogram.UnknownCounterReset,
histogram.CounterReset,
},
},
} }
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Run("histogram_stats", func(t *testing.T) { t.Run("histogram_stats", func(t *testing.T) {
decodedStats := make([]*histogram.Histogram, 0) decodedStats := make([]*histogram.Histogram, 0)
statsIterator := NewHistogramStatsIterator(newHistogramSeries(histograms).Iterator(nil)) statsIterator := NewHistogramStatsIterator(newHistogramSeries(tc.histograms).Iterator(nil))
for statsIterator.Next() != chunkenc.ValNone { for statsIterator.Next() != chunkenc.ValNone {
_, h := statsIterator.AtHistogram(nil) _, h := statsIterator.AtHistogram(nil)
decodedStats = append(decodedStats, h) decodedStats = append(decodedStats, h)
} }
for i := 0; i < len(histograms); i++ { for i := 0; i < len(tc.histograms); i++ {
require.Equal(t, expectedHints[i], decodedStats[i].CounterResetHint) require.Equal(t, tc.expectedHints[i], decodedStats[i].CounterResetHint, fmt.Sprintf("mismatch in counter reset hint for histogram %d", i))
require.Equal(t, histograms[i].Count, decodedStats[i].Count) h := tc.histograms[i]
require.Equal(t, histograms[i].Sum, decodedStats[i].Sum) if value.IsStaleNaN(h.Sum) {
require.True(t, value.IsStaleNaN(decodedStats[i].Sum))
require.Equal(t, uint64(0), decodedStats[i].Count)
} else {
require.Equal(t, tc.histograms[i].Count, decodedStats[i].Count)
require.Equal(t, tc.histograms[i].Sum, decodedStats[i].Sum)
}
} }
}) })
t.Run("float_histogram_stats", func(t *testing.T) { t.Run("float_histogram_stats", func(t *testing.T) {
decodedStats := make([]*histogram.FloatHistogram, 0) decodedStats := make([]*histogram.FloatHistogram, 0)
statsIterator := NewHistogramStatsIterator(newHistogramSeries(histograms).Iterator(nil)) statsIterator := NewHistogramStatsIterator(newHistogramSeries(tc.histograms).Iterator(nil))
for statsIterator.Next() != chunkenc.ValNone { for statsIterator.Next() != chunkenc.ValNone {
_, h := statsIterator.AtFloatHistogram(nil) _, h := statsIterator.AtFloatHistogram(nil)
decodedStats = append(decodedStats, h) decodedStats = append(decodedStats, h)
} }
for i := 0; i < len(histograms); i++ { for i := 0; i < len(tc.histograms); i++ {
fh := histograms[i].ToFloat(nil) require.Equal(t, tc.expectedHints[i], decodedStats[i].CounterResetHint)
require.Equal(t, expectedHints[i], decodedStats[i].CounterResetHint) fh := tc.histograms[i].ToFloat(nil)
if value.IsStaleNaN(fh.Sum) {
require.True(t, value.IsStaleNaN(decodedStats[i].Sum))
require.Equal(t, float64(0), decodedStats[i].Count)
} else {
require.Equal(t, fh.Count, decodedStats[i].Count) require.Equal(t, fh.Count, decodedStats[i].Count)
require.Equal(t, fh.Sum, decodedStats[i].Sum) require.Equal(t, fh.Sum, decodedStats[i].Sum)
} }
}
}) })
})
}
} }
type histogramSeries struct { type histogramSeries struct {

View file

@ -30,12 +30,10 @@ func GenerateTestHistograms(n int) (r []*histogram.Histogram) {
return r return r
} }
func GenerateTestHistogramsWithUnknownResetHint(n int) []*histogram.Histogram { func GenerateTestHistogramWithHint(n int, hint histogram.CounterResetHint) *histogram.Histogram {
hs := GenerateTestHistograms(n) h := GenerateTestHistogram(n)
for i := range hs { h.CounterResetHint = hint
hs[i].CounterResetHint = histogram.UnknownCounterReset return h
}
return hs
} }
// GenerateTestHistogram but it is up to the user to set any known counter reset hint. // GenerateTestHistogram but it is up to the user to set any known counter reset hint.