diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 92f084bdf6..5a4b065794 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -294,6 +294,9 @@ func (h *FloatHistogram) Mul(factor float64) *FloatHistogram { for i := range h.NegativeBuckets { h.NegativeBuckets[i] *= factor } + if factor < 0 { + h.CounterResetHint = GaugeType + } return h } @@ -317,6 +320,9 @@ func (h *FloatHistogram) Div(scalar float64) *FloatHistogram { for i := range h.NegativeBuckets { h.NegativeBuckets[i] /= scalar } + if scalar < 0 { + h.CounterResetHint = GaugeType + } return h } @@ -417,6 +423,9 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) (*FloatHistogram, error) { return nil, ErrHistogramsIncompatibleBounds } + // Prevent counter resets when subtracting as this might decrease counters. + h.CounterResetHint = GaugeType + if !h.UsesCustomBuckets() { otherZeroCount := h.reconcileZeroBuckets(other) h.ZeroCount -= otherZeroCount diff --git a/model/histogram/float_histogram_test.go b/model/histogram/float_histogram_test.go index 34988e9d39..8772955d05 100644 --- a/model/histogram/float_histogram_test.go +++ b/model/histogram/float_histogram_test.go @@ -145,14 +145,15 @@ func TestFloatHistogramMul(t *testing.T) { }, -1, &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: -11, - Count: -30, - Sum: -23, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{-1, 0, -3, -4, -7}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{-3, -1, -5, -6}, + ZeroThreshold: 0.01, + ZeroCount: -11, + Count: -30, + Sum: -23, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{-1, 0, -3, -4, -7}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{-3, -1, -5, -6}, + CounterResetHint: GaugeType, }, }, { @@ -169,14 +170,15 @@ func TestFloatHistogramMul(t *testing.T) { }, -2, &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: -22, - Count: -60, - Sum: -46, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{-2, 0, -6, -8, -14}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{-6, -2, -10, -12}, + ZeroThreshold: 0.01, + ZeroCount: -22, + Count: -60, + Sum: -46, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{-2, 0, -6, -8, -14}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{-6, -2, -10, -12}, + CounterResetHint: GaugeType, }, }, { @@ -467,14 +469,15 @@ func TestFloatHistogramDiv(t *testing.T) { }, -1, &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: -5.5, - Count: -3493.3, - Sum: -2349209.324, - PositiveSpans: []Span{{-2, 1}, {2, 3}}, - PositiveBuckets: []float64{-1, -3.3, -4.2, -0.1}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{-3.1, -3, -1.234e5, -1000}, + ZeroThreshold: 0.01, + ZeroCount: -5.5, + Count: -3493.3, + Sum: -2349209.324, + PositiveSpans: []Span{{-2, 1}, {2, 3}}, + PositiveBuckets: []float64{-1, -3.3, -4.2, -0.1}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{-3.1, -3, -1.234e5, -1000}, + CounterResetHint: GaugeType, }, }, { @@ -491,14 +494,15 @@ func TestFloatHistogramDiv(t *testing.T) { }, -2, &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: -5.5, - Count: -15, - Sum: -11.5, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{-0.5, 0, -1.5, -2, -3.5}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{-1.5, -0.5, -2.5, -3}, + ZeroThreshold: 0.01, + ZeroCount: -5.5, + Count: -15, + Sum: -11.5, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{-0.5, 0, -1.5, -2, -3.5}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{-1.5, -0.5, -2.5, -3}, + CounterResetHint: GaugeType, }, }, { @@ -2379,14 +2383,15 @@ func TestFloatHistogramSub(t *testing.T) { NegativeBuckets: []float64{1, 1, 4, 4}, }, &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 3, - Count: 9, - Sum: 11, - PositiveSpans: []Span{{-2, 2}, {1, 3}}, - PositiveBuckets: []float64{1, 0, 1, 1, 1}, - NegativeSpans: []Span{{3, 2}, {3, 2}}, - NegativeBuckets: []float64{2, 0, 1, 2}, + ZeroThreshold: 0.01, + ZeroCount: 3, + Count: 9, + Sum: 11, + PositiveSpans: []Span{{-2, 2}, {1, 3}}, + PositiveBuckets: []float64{1, 0, 1, 1, 1}, + NegativeSpans: []Span{{3, 2}, {3, 2}}, + NegativeBuckets: []float64{2, 0, 1, 2}, + CounterResetHint: GaugeType, }, "", }, @@ -2415,14 +2420,15 @@ func TestFloatHistogramSub(t *testing.T) { NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, }, &FloatHistogram{ - ZeroThreshold: 0.01, - ZeroCount: 6, - Count: 40, - Sum: 0.889, - PositiveSpans: []Span{{-2, 5}, {0, 3}}, - PositiveBuckets: []float64{1, 5, 4, 2, 2, 2, 0, 5}, - NegativeSpans: []Span{{3, 3}, {1, 3}}, - NegativeBuckets: []float64{1, 9, 1, 4, 9, 1}, + ZeroThreshold: 0.01, + ZeroCount: 6, + Count: 40, + Sum: 0.889, + PositiveSpans: []Span{{-2, 5}, {0, 3}}, + PositiveBuckets: []float64{1, 5, 4, 2, 2, 2, 0, 5}, + NegativeSpans: []Span{{3, 3}, {1, 3}}, + NegativeBuckets: []float64{1, 9, 1, 4, 9, 1}, + CounterResetHint: GaugeType, }, "", }, @@ -2445,12 +2451,13 @@ func TestFloatHistogramSub(t *testing.T) { CustomValues: []float64{1, 2, 3, 4}, }, &FloatHistogram{ - Schema: CustomBucketsSchema, - Count: 4, - Sum: 11, - PositiveSpans: []Span{{0, 2}, {1, 3}}, - PositiveBuckets: []float64{1, 0, 1, 1, 1}, - CustomValues: []float64{1, 2, 3, 4}, + Schema: CustomBucketsSchema, + Count: 4, + Sum: 11, + PositiveSpans: []Span{{0, 2}, {1, 3}}, + PositiveBuckets: []float64{1, 0, 1, 1, 1}, + CustomValues: []float64{1, 2, 3, 4}, + CounterResetHint: GaugeType, }, "", }, diff --git a/promql/engine_test.go b/promql/engine_test.go index 18671c76aa..67d5fbb5f0 100644 --- a/promql/engine_test.go +++ b/promql/engine_test.go @@ -4051,3 +4051,106 @@ func TestSubQueryHistogramsCopy(t *testing.T) { queryable.Close() } } + +func TestHistogram_CounterResetHint(t *testing.T) { + baseT := timestamp.Time(0) + load := ` + load 2m + test_histogram {{count:0}}+{{count:1}}x4 + ` + testCases := []struct { + name string + query string + want histogram.CounterResetHint + }{ + { + name: "adding histograms", + query: `test_histogram + test_histogram`, + want: histogram.NotCounterReset, + }, + { + name: "subtraction", + query: `test_histogram - test_histogram`, + want: histogram.GaugeType, + }, + { + name: "negated addition", + query: `test_histogram + (-test_histogram)`, + want: histogram.GaugeType, + }, + { + name: "negated subtraction", + query: `test_histogram - (-test_histogram)`, + want: histogram.GaugeType, + }, + { + name: "unary negation", + query: `-test_histogram`, + want: histogram.GaugeType, + }, + { + name: "repeated unary negation", + query: `-(-test_histogram)`, + want: histogram.GaugeType, + }, + { + name: "multiplication", + query: `2 * test_histogram`, + want: histogram.NotCounterReset, + }, + { + name: "negative multiplication", + query: `-1 * test_histogram`, + want: histogram.GaugeType, + }, + { + name: "division", + query: `test_histogram / 2`, + want: histogram.NotCounterReset, + }, + { + name: "negative division", + query: `test_histogram / -1`, + want: histogram.GaugeType, + }, + // Equality and unequality should always return the LHS: + { + name: "equality (not counter reset)", + query: `test_histogram == test_histogram`, + want: histogram.NotCounterReset, + }, + { + name: "equality (gauge)", + query: `-test_histogram == -test_histogram`, + want: histogram.GaugeType, + }, + { + name: "unequality (not counter reset)", + query: `test_histogram != -test_histogram`, + want: histogram.NotCounterReset, + }, + { + name: "unequality (gauge)", + query: `-test_histogram != test_histogram`, + want: histogram.GaugeType, + }, + } + ctx := context.Background() + queryable := promqltest.LoadedStorage(t, load) + defer queryable.Close() + engine := promqltest.NewTestEngine(t, false, 0, promqltest.DefaultMaxSamplesPerQuery) + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + q, err := engine.NewInstantQuery(ctx, queryable, nil, tc.query, baseT.Add(2*time.Minute)) + require.NoError(t, err) + defer q.Close() + res := q.Exec(ctx) + require.NoError(t, res.Err) + v, err := res.Vector() + require.NoError(t, err) + require.Len(t, v, 1) + require.NotNil(t, v[0].H) + require.Equal(t, tc.want, v[0].H.CounterResetHint) + }) + } +} diff --git a/promql/functions.go b/promql/functions.go index 82cfdd65b4..34a43e3ef5 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -283,7 +283,6 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra annos.Add(annotations.NewNativeHistogramNotGaugeWarning(metricName, pos)) } - h.CounterResetHint = histogram.GaugeType return h.Compact(0), annos } diff --git a/promql/parser/parse_test.go b/promql/parser/parse_test.go index d239581ece..1a45b896de 100644 --- a/promql/parser/parse_test.go +++ b/promql/parser/parse_test.go @@ -5310,6 +5310,7 @@ func TestParseHistogramSeries(t *testing.T) { Offset: 0, Length: 3, }}, + CounterResetHint: histogram.GaugeType, }, { Schema: 1, @@ -5318,6 +5319,7 @@ func TestParseHistogramSeries(t *testing.T) { Offset: 0, Length: 3, }}, + CounterResetHint: histogram.GaugeType, }, }, }, diff --git a/promql/promqltest/testdata/native_histograms.test b/promql/promqltest/testdata/native_histograms.test index e38e003b3f..c7933afc48 100644 --- a/promql/promqltest/testdata/native_histograms.test +++ b/promql/promqltest/testdata/native_histograms.test @@ -1523,3 +1523,26 @@ eval instant at 55m histogram_count(increase(metric[55m15s])) eval instant at 54m30s histogram_count(increase(metric[54m45s])) {type="histogram"} 2509.375 + +clear + +# Test that `rate` adds warning when applied to an average of native histograms. +load 5m + metric{id="1"} {{schema:0 sum:4 count:4 buckets:[1 2 1]}}x10 + metric{id="2"} {{schema:0 sum:4 count:4 buckets:[1 2 1]}}x10 + +# Don't warn if there is only a single histogram. +eval instant at 5m rate(avg(metric{id="1"})[10m:]) + expect no_warn + {} {{counter_reset_hint:gauge}} + +# Rate warns if histogram is not a counter and avg sets gauge hint. +eval instant at 5m rate(avg(metric)[10m:]) + expect warn regex: this native histogram metric is not a counter: "" + {} {{counter_reset_hint:gauge}} + +# Rate warns if histogram is not a counter and avg_over_time sets gauge hint. +eval instant at 5m rate(avg_over_time(metric{id="1"}[10m])[5m:]) + expect warn regex: this native histogram metric is not a counter: "metric" + {id="1"} {{counter_reset_hint:gauge}} +