diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 5a4b065794..2b78c6d630 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -336,15 +336,18 @@ func (h *FloatHistogram) Div(scalar float64) *FloatHistogram { // The method reconciles differences in the zero threshold and in the schema, and // changes them if needed. The other histogram will not be modified in any case. // Adding is currently only supported between 2 exponential histograms, or between -// 2 custom buckets histograms with the exact same custom bounds. +// 2 custom buckets histograms with the exact same custom bounds. If CounterResetHint +// values conflict, the receiver's hint is set to unknown, and counterResetCollision +// is returned as true. A counter reset conflict occurs iff one of two histograms indicate +// a counter reset (CounterReset) while the other indicates no reset (NotCounterReset). // // This method returns a pointer to the receiving histogram for convenience. -func (h *FloatHistogram) Add(other *FloatHistogram) (*FloatHistogram, error) { +func (h *FloatHistogram) Add(other *FloatHistogram) (res *FloatHistogram, counterResetCollision bool, err error) { if h.UsesCustomBuckets() != other.UsesCustomBuckets() { - return nil, ErrHistogramsIncompatibleSchema + return nil, false, ErrHistogramsIncompatibleSchema } if h.UsesCustomBuckets() && !FloatBucketsMatch(h.CustomValues, other.CustomValues) { - return nil, ErrHistogramsIncompatibleBounds + return nil, false, ErrHistogramsIncompatibleBounds } switch { @@ -368,7 +371,7 @@ func (h *FloatHistogram) Add(other *FloatHistogram) (*FloatHistogram, error) { // They are a direct collision of CounterReset and NotCounterReset. // Conservatively set the CounterResetHint to "unknown" and issue a warning. h.CounterResetHint = UnknownCounterReset - // TODO(trevorwhitney): Actually issue the warning as soon as the plumbing for it is in place + counterResetCollision = true } if !h.UsesCustomBuckets() { @@ -387,7 +390,7 @@ func (h *FloatHistogram) Add(other *FloatHistogram) (*FloatHistogram, error) { if h.UsesCustomBuckets() { h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) - return h, nil + return h, counterResetCollision, nil } var ( @@ -411,19 +414,20 @@ func (h *FloatHistogram) Add(other *FloatHistogram) (*FloatHistogram, error) { h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, hNegativeSpans, hNegativeBuckets, otherNegativeSpans, otherNegativeBuckets) - return h, nil + return h, counterResetCollision, nil } // Sub works like Add but subtracts the other histogram. -func (h *FloatHistogram) Sub(other *FloatHistogram) (*FloatHistogram, error) { +func (h *FloatHistogram) Sub(other *FloatHistogram) (res *FloatHistogram, counterResetCollision bool, err error) { if h.UsesCustomBuckets() != other.UsesCustomBuckets() { - return nil, ErrHistogramsIncompatibleSchema + return nil, false, ErrHistogramsIncompatibleSchema } if h.UsesCustomBuckets() && !FloatBucketsMatch(h.CustomValues, other.CustomValues) { - return nil, ErrHistogramsIncompatibleBounds + return nil, false, ErrHistogramsIncompatibleBounds } - // Prevent counter resets when subtracting as this might decrease counters. + counterResetCollision = hasCounterResetCollision(h, other) + h.CounterResetHint = GaugeType if !h.UsesCustomBuckets() { @@ -442,7 +446,7 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) (*FloatHistogram, error) { if h.UsesCustomBuckets() { h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) - return h, nil + return h, counterResetCollision, nil } var ( @@ -465,7 +469,14 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) (*FloatHistogram, error) { h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) h.NegativeSpans, h.NegativeBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, hNegativeSpans, hNegativeBuckets, otherNegativeSpans, otherNegativeBuckets) - return h, nil + return h, counterResetCollision, nil +} + +// hasCounterResetCollision returns true iff one of two histogram indicates +// a counter reset (CounterReset) while the other indicates no reset (NotCounterReset). +func hasCounterResetCollision(a, b *FloatHistogram) bool { + return a.CounterResetHint == CounterReset && b.CounterResetHint == NotCounterReset || + a.CounterResetHint == NotCounterReset && b.CounterResetHint == CounterReset } // Equals returns true if the given float histogram matches exactly. diff --git a/model/histogram/float_histogram_test.go b/model/histogram/float_histogram_test.go index 8772955d05..d7720ccc8c 100644 --- a/model/histogram/float_histogram_test.go +++ b/model/histogram/float_histogram_test.go @@ -1645,13 +1645,14 @@ func TestFloatHistogramCompact(t *testing.T) { func TestFloatHistogramAdd(t *testing.T) { cases := []struct { - name string - in1, in2, expected *FloatHistogram - expErrMsg string + name string + in1, in2, expected *FloatHistogram + expErrMsg string + expCounterResetCollision bool }{ { - "same bucket layout", - &FloatHistogram{ + name: "same bucket layout", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1661,7 +1662,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -1671,7 +1672,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 19, Count: 51, @@ -1681,11 +1682,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{4, 2, 9, 10}, }, - "", }, { - "same bucket layout, defined differently", - &FloatHistogram{ + name: "same bucket layout, defined differently", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1695,7 +1695,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -1705,7 +1705,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 7}}, NegativeBuckets: []float64{1, 1, 0, 0, 0, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 19, Count: 51, @@ -1715,11 +1715,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 5}, {0, 2}}, NegativeBuckets: []float64{4, 2, 0, 0, 0, 9, 10}, }, - "", }, { - "non-overlapping spans", - &FloatHistogram{ + name: "non-overlapping spans", + in1: &FloatHistogram{ ZeroThreshold: 0.001, ZeroCount: 11, Count: 30, @@ -1729,7 +1728,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.001, ZeroCount: 8, Count: 21, @@ -1739,7 +1738,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{-9, 2}, {3, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.001, ZeroCount: 19, Count: 51, @@ -1749,11 +1748,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{-9, 2}, {3, 2}, {5, 2}, {3, 2}}, NegativeBuckets: []float64{1, 1, 4, 4, 3, 1, 5, 6}, }, - "", }, { - "non-overlapping spans inverted order", - &FloatHistogram{ + name: "non-overlapping spans inverted order", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -1763,7 +1761,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{-6, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1773,7 +1771,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 19, Count: 51, @@ -1783,11 +1781,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{-6, 2}, {1, 2}, {4, 2}, {3, 2}}, NegativeBuckets: []float64{1, 1, 4, 4, 3, 1, 5, 6}, }, - "", }, { - "overlapping spans", - &FloatHistogram{ + name: "overlapping spans", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1797,7 +1794,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -1807,7 +1804,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 19, Count: 51, @@ -1817,11 +1814,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "overlapping spans inverted order", - &FloatHistogram{ + name: "overlapping spans inverted order", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -1831,7 +1827,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1841,7 +1837,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 19, Count: 51, @@ -1851,11 +1847,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "schema change", - &FloatHistogram{ + name: "schema change", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -1866,7 +1861,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1877,7 +1872,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{6, 3}, {6, 4}}, NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 19, Count: 51, @@ -1887,11 +1882,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "larger zero bucket in first histogram", - &FloatHistogram{ + name: "larger zero bucket in first histogram", + in1: &FloatHistogram{ ZeroThreshold: 1, ZeroCount: 17, Count: 21, @@ -1901,7 +1895,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1911,7 +1905,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 1, ZeroCount: 29, Count: 51, @@ -1921,11 +1915,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "larger zero bucket in second histogram", - &FloatHistogram{ + name: "larger zero bucket in second histogram", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1935,7 +1928,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 1, ZeroCount: 17, Count: 21, @@ -1945,7 +1938,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 1, ZeroCount: 29, Count: 51, @@ -1955,11 +1948,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "larger zero threshold in first histogram ends up inside a populated bucket of second histogram", - &FloatHistogram{ + name: "larger zero threshold in first histogram ends up inside a populated bucket of second histogram", + in1: &FloatHistogram{ ZeroThreshold: 0.2, ZeroCount: 17, Count: 21, @@ -1969,7 +1961,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -1979,7 +1971,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.25, ZeroCount: 29, Count: 51, @@ -1989,11 +1981,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "larger zero threshold in second histogram ends up inside a populated bucket of first histogram", - &FloatHistogram{ + name: "larger zero threshold in second histogram ends up inside a populated bucket of first histogram", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -2003,7 +1994,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.2, ZeroCount: 17, Count: 21, @@ -2013,7 +2004,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.25, ZeroCount: 29, Count: 51, @@ -2023,11 +2014,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "schema change combined with larger zero bucket in second histogram", - &FloatHistogram{ + name: "schema change combined with larger zero bucket in second histogram", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -2038,7 +2028,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.25, ZeroCount: 12, Count: 30, @@ -2049,7 +2039,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{6, 3}, {6, 4}}, NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.25, ZeroCount: 22, Count: 51, @@ -2059,11 +2049,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "schema change combined with larger zero bucket in first histogram", - &FloatHistogram{ + name: "schema change combined with larger zero bucket in first histogram", + in1: &FloatHistogram{ ZeroThreshold: 0.25, ZeroCount: 8, Count: 21, @@ -2074,7 +2063,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{4, 2}, {1, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -2085,7 +2074,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{6, 3}, {6, 4}}, NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.25, ZeroCount: 20, Count: 51, @@ -2095,11 +2084,10 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{3, 2, 1, 4, 9, 6}, }, - "", }, { - "same custom bucket layout", - &FloatHistogram{ + name: "same custom bucket layout", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2107,7 +2095,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 1.234, @@ -2115,7 +2103,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 26, Sum: 3.579, @@ -2123,11 +2111,10 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 5, 7, 13}, CustomValues: []float64{1, 2, 3, 4}, }, - "", }, { - "same custom bucket layout, defined differently", - &FloatHistogram{ + name: "same custom bucket layout, defined differently", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2135,7 +2122,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 1.234, @@ -2143,7 +2130,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 26, Sum: 3.579, @@ -2151,11 +2138,10 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 5, 7, 13}, CustomValues: []float64{1, 2, 3, 4}, }, - "", }, { - "non-overlapping spans with custom buckets", - &FloatHistogram{ + name: "non-overlapping spans with custom buckets", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2163,7 +2149,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 20, Sum: 1.234, @@ -2171,7 +2157,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{5, 4, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 35, Sum: 3.579, @@ -2179,11 +2165,10 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - "", }, { - "non-overlapping spans inverted order with custom buckets", - &FloatHistogram{ + name: "non-overlapping spans inverted order with custom buckets", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 20, Sum: 1.234, @@ -2191,7 +2176,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{5, 4, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2199,7 +2184,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 35, Sum: 3.579, @@ -2207,11 +2192,10 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - "", }, { - "overlapping spans with custom buckets", - &FloatHistogram{ + name: "overlapping spans with custom buckets", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2219,7 +2203,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 27, Sum: 1.234, @@ -2227,7 +2211,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 42, Sum: 3.579, @@ -2235,11 +2219,10 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, CustomValues: []float64{1, 2, 3, 4}, }, - "", }, { - "overlapping spans inverted order with custom buckets", - &FloatHistogram{ + name: "overlapping spans inverted order with custom buckets", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 27, Sum: 1.234, @@ -2247,7 +2230,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2255,7 +2238,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 42, Sum: 3.579, @@ -2263,11 +2246,10 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, CustomValues: []float64{1, 2, 3, 4}, }, - "", }, { - "different custom bucket layout", - &FloatHistogram{ + name: "different custom bucket layout", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 2.345, @@ -2275,7 +2257,7 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 1.234, @@ -2283,12 +2265,11 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4, 5}, }, - nil, - "cannot apply this operation on custom buckets histograms with different custom bounds", + expErrMsg: "cannot apply this operation on custom buckets histograms with different custom bounds", }, { - "mix exponential and custom buckets histograms", - &FloatHistogram{ + name: "mix exponential and custom buckets histograms", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 59, @@ -2299,7 +2280,7 @@ func TestFloatHistogramAdd(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{4, 10, 1, 4, 14, 7}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 12, @@ -2307,20 +2288,32 @@ func TestFloatHistogramAdd(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - nil, - "cannot apply this operation on histograms with a mix of exponential and custom bucket schemas", + expErrMsg: "cannot apply this operation on histograms with a mix of exponential and custom bucket schemas", + }, + { + name: "warn on counter reset hint collision", + in1: &FloatHistogram{ + Schema: CustomBucketsSchema, + CounterResetHint: CounterReset, + }, + in2: &FloatHistogram{ + Schema: CustomBucketsSchema, + CounterResetHint: NotCounterReset, + }, + expErrMsg: "", + expCounterResetCollision: true, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - testHistogramAdd(t, c.in1, c.in2, c.expected, c.expErrMsg) - testHistogramAdd(t, c.in2, c.in1, c.expected, c.expErrMsg) + testHistogramAdd(t, c.in1, c.in2, c.expected, c.expErrMsg, c.expCounterResetCollision) + testHistogramAdd(t, c.in2, c.in1, c.expected, c.expErrMsg, c.expCounterResetCollision) }) } } -func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string) { +func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string, expCounterResetCollision bool) { var ( aCopy = a.Copy() bCopy = b.Copy() @@ -2331,13 +2324,16 @@ func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg st expectedCopy = expected.Copy() } - res, err := aCopy.Add(bCopy) + res, warn, err := aCopy.Add(bCopy) if expErrMsg != "" { require.EqualError(t, err, expErrMsg) } else { require.NoError(t, err) } + // Check that the warnings are correct. + require.Equal(t, expCounterResetCollision, warn) + if expected != nil { res.Compact(0) expectedCopy.Compact(0) @@ -2356,13 +2352,14 @@ func TestFloatHistogramSub(t *testing.T) { // This has fewer test cases than TestFloatHistogramAdd because Add and // Sub share most of the trickier code. cases := []struct { - name string - in1, in2, expected *FloatHistogram - expErrMsg string + name string + in1, in2, expected *FloatHistogram + expErrMsg string + expCounterResetCollision bool }{ { - "same bucket layout", - &FloatHistogram{ + name: "same bucket layout", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 11, Count: 30, @@ -2372,7 +2369,7 @@ func TestFloatHistogramSub(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{3, 1, 5, 6}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 21, @@ -2382,7 +2379,7 @@ func TestFloatHistogramSub(t *testing.T) { NegativeSpans: []Span{{3, 2}, {3, 2}}, NegativeBuckets: []float64{1, 1, 4, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 3, Count: 9, @@ -2393,11 +2390,10 @@ func TestFloatHistogramSub(t *testing.T) { NegativeBuckets: []float64{2, 0, 1, 2}, CounterResetHint: GaugeType, }, - "", }, { - "schema change", - &FloatHistogram{ + name: "schema change", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 59, @@ -2408,7 +2404,7 @@ func TestFloatHistogramSub(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{4, 10, 1, 4, 14, 7}, }, - &FloatHistogram{ + in2: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 2, Count: 19, @@ -2419,7 +2415,7 @@ func TestFloatHistogramSub(t *testing.T) { NegativeSpans: []Span{{6, 3}, {6, 4}}, NegativeBuckets: []float64{3, 0.5, 0.5, 2, 3, 2, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 6, Count: 40, @@ -2430,11 +2426,10 @@ func TestFloatHistogramSub(t *testing.T) { NegativeBuckets: []float64{1, 9, 1, 4, 9, 1}, CounterResetHint: GaugeType, }, - "", }, { - "same custom bucket layout", - &FloatHistogram{ + name: "same custom bucket layout", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 23, @@ -2442,7 +2437,7 @@ func TestFloatHistogramSub(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 12, @@ -2450,7 +2445,7 @@ func TestFloatHistogramSub(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + expected: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 4, Sum: 11, @@ -2459,11 +2454,10 @@ func TestFloatHistogramSub(t *testing.T) { CustomValues: []float64{1, 2, 3, 4}, CounterResetHint: GaugeType, }, - "", }, { - "different custom bucket layout", - &FloatHistogram{ + name: "different custom bucket layout", + in1: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 15, Sum: 23, @@ -2471,7 +2465,7 @@ func TestFloatHistogramSub(t *testing.T) { PositiveBuckets: []float64{1, 0, 3, 4, 7}, CustomValues: []float64{1, 2, 3, 4}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 12, @@ -2479,12 +2473,11 @@ func TestFloatHistogramSub(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4, 5}, }, - nil, - "cannot apply this operation on custom buckets histograms with different custom bounds", + expErrMsg: "cannot apply this operation on custom buckets histograms with different custom bounds", }, { - "mix exponential and custom buckets histograms", - &FloatHistogram{ + name: "mix exponential and custom buckets histograms", + in1: &FloatHistogram{ ZeroThreshold: 0.01, ZeroCount: 8, Count: 59, @@ -2495,7 +2488,7 @@ func TestFloatHistogramSub(t *testing.T) { NegativeSpans: []Span{{3, 3}, {1, 3}}, NegativeBuckets: []float64{4, 10, 1, 4, 14, 7}, }, - &FloatHistogram{ + in2: &FloatHistogram{ Schema: CustomBucketsSchema, Count: 11, Sum: 12, @@ -2503,25 +2496,37 @@ func TestFloatHistogramSub(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4}, }, - nil, - "cannot apply this operation on histograms with a mix of exponential and custom bucket schemas", + expErrMsg: "cannot apply this operation on histograms with a mix of exponential and custom bucket schemas", + }, + { + name: "warn on counter reset hint collision", + in1: &FloatHistogram{ + Schema: CustomBucketsSchema, + CounterResetHint: CounterReset, + }, + in2: &FloatHistogram{ + Schema: CustomBucketsSchema, + CounterResetHint: NotCounterReset, + }, + expErrMsg: "", + expCounterResetCollision: true, }, } for _, c := range cases { t.Run(c.name, func(t *testing.T) { - testFloatHistogramSub(t, c.in1, c.in2, c.expected, c.expErrMsg) + testFloatHistogramSub(t, c.in1, c.in2, c.expected, c.expErrMsg, c.expCounterResetCollision) var expectedNegative *FloatHistogram if c.expected != nil { expectedNegative = c.expected.Copy().Mul(-1) } - testFloatHistogramSub(t, c.in2, c.in1, expectedNegative, c.expErrMsg) + testFloatHistogramSub(t, c.in2, c.in1, expectedNegative, c.expErrMsg, c.expCounterResetCollision) }) } } -func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string) { +func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string, expCounterResetCollision bool) { var ( aCopy = a.Copy() bCopy = b.Copy() @@ -2532,7 +2537,7 @@ func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrM expectedCopy = expected.Copy() } - res, err := aCopy.Sub(bCopy) + res, warn, err := aCopy.Sub(bCopy) if expErrMsg != "" { require.EqualError(t, err, expErrMsg) } else { @@ -2550,6 +2555,9 @@ func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrM // Check that the argument was not mutated. require.Equal(t, b, bCopy) + + // Check that the warnings are correct. + require.Equal(t, expCounterResetCollision, warn) } } diff --git a/promql/engine.go b/promql/engine.go index 645fe28647..8e2074c6ca 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2830,7 +2830,7 @@ func (ev *evaluator) VectorscalarBinop(op parser.ItemType, lhs Vector, rhs Scala lh, rh = rh, lh } float, histogram, keep, err := vectorElemBinop(op, lf, rf, lh, rh, pos) - if err != nil { + if err != nil && !errors.Is(err, annotations.PromQLWarning) { lastErr = err continue } @@ -2955,17 +2955,23 @@ func vectorElemBinop(op parser.ItemType, lhs, rhs float64, hlhs, hrhs *histogram { switch op { case parser.ADD: - res, err := hlhs.Copy().Add(hrhs) + res, counterResetCollision, err := hlhs.Copy().Add(hrhs) if err != nil { return 0, nil, false, err } - return 0, res.Compact(0), true, nil + if counterResetCollision { + err = annotations.NewHistogramCounterResetCollisionWarning(pos) + } + return 0, res.Compact(0), true, err case parser.SUB: - res, err := hlhs.Copy().Sub(hrhs) + res, counterResetCollision, err := hlhs.Copy().Sub(hrhs) if err != nil { return 0, nil, false, err } - return 0, res.Compact(0), true, nil + if counterResetCollision { + err = annotations.NewHistogramCounterResetCollisionWarning(pos) + } + return 0, res.Compact(0), true, err case parser.EQLC: // This operation expects that both histograms are compacted. return 0, hlhs, hlhs.Equals(hrhs), nil @@ -3078,7 +3084,7 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix if h != nil { group.hasHistogram = true if group.histogramValue != nil { - _, err := group.histogramValue.Add(h) + _, _, err := group.histogramValue.Add(h) if err != nil { handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos) group.incompatibleHistograms = true @@ -3131,13 +3137,13 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix if group.histogramValue != nil { left := h.Copy().Div(group.groupCount) right := group.histogramValue.Copy().Div(group.groupCount) - toAdd, err := left.Sub(right) + toAdd, _, err := left.Sub(right) if err != nil { handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos) group.incompatibleHistograms = true continue } - _, err = group.histogramValue.Add(toAdd) + _, _, err = group.histogramValue.Add(toAdd) if err != nil { handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos) group.incompatibleHistograms = true diff --git a/promql/engine_internal_test.go b/promql/engine_internal_test.go index 8662891da6..405f0527bf 100644 --- a/promql/engine_internal_test.go +++ b/promql/engine_internal_test.go @@ -21,7 +21,9 @@ import ( "github.com/prometheus/common/promslog" "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/model/histogram" "github.com/prometheus/prometheus/promql/parser" + "github.com/prometheus/prometheus/promql/parser/posrange" "github.com/prometheus/prometheus/util/annotations" ) @@ -77,3 +79,65 @@ func TestRecoverEvaluatorErrorWithWarnings(t *testing.T) { panic(e) } + +func TestVectorElemBinop_Histograms(t *testing.T) { + testCases := []struct { + name string + op parser.ItemType + hlhs, hrhs *histogram.FloatHistogram + want *histogram.FloatHistogram + wantErr error + }{ + { + name: "ADD with unknown counter reset hints", + op: parser.ADD, + hlhs: &histogram.FloatHistogram{}, + hrhs: &histogram.FloatHistogram{}, + want: &histogram.FloatHistogram{}, + }, + { + name: "ADD with mixed counter reset hints", + op: parser.ADD, + hlhs: &histogram.FloatHistogram{ + CounterResetHint: histogram.CounterReset, + }, + hrhs: &histogram.FloatHistogram{ + CounterResetHint: histogram.NotCounterReset, + }, + want: &histogram.FloatHistogram{}, + wantErr: annotations.HistogramCounterResetCollisionWarning, + }, + { + name: "SUB with unknown counter reset hints", + op: parser.SUB, + hlhs: &histogram.FloatHistogram{}, + hrhs: &histogram.FloatHistogram{}, + want: &histogram.FloatHistogram{ + CounterResetHint: histogram.GaugeType, + }, + }, + { + name: "SUB with mixed counter reset hints", + op: parser.SUB, + hlhs: &histogram.FloatHistogram{ + CounterResetHint: histogram.CounterReset, + }, + hrhs: &histogram.FloatHistogram{ + CounterResetHint: histogram.NotCounterReset, + }, + want: &histogram.FloatHistogram{}, + wantErr: annotations.HistogramCounterResetCollisionWarning, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, got, _, err := vectorElemBinop(tc.op, 0, 0, tc.hlhs, tc.hrhs, posrange.PositionRange{}) + if tc.wantErr != nil { + require.ErrorIs(t, err, tc.wantErr) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } +} diff --git a/promql/functions.go b/promql/functions.go index 189c4c7c2d..0b4716a026 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -254,7 +254,7 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra } h := last.CopyToSchema(minSchema) - _, err := h.Sub(prev) + _, counterResetCollision, err := h.Sub(prev) if err != nil { if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) @@ -263,12 +263,16 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra } } + if counterResetCollision { + annos.Add(annotations.NewHistogramCounterResetCollisionWarning(pos)) + } + if isCounter { // Second iteration to deal with counter resets. for _, currPoint := range points[1:] { curr := currPoint.H if curr.DetectReset(prev) { - _, err := h.Add(prev) + _, counterResetCollision, err := h.Add(prev) if err != nil { if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) @@ -276,6 +280,9 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra return nil, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, pos)) } } + if counterResetCollision { + annos.Add(annotations.NewHistogramCounterResetCollisionWarning(pos)) + } } prev = curr } @@ -389,12 +396,15 @@ func instantValue(vals Matrix, args parser.Expressions, out Vector, isRate bool) annos.Add(annotations.NewNativeHistogramNotGaugeWarning(metricName, args.PositionRange())) } if !isRate || !ss[1].H.DetectReset(ss[0].H) { - _, err := resultSample.H.Sub(ss[0].H) + _, counterResetCollision, err := resultSample.H.Sub(ss[0].H) if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return out, annos.Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, args.PositionRange())) } else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { return out, annos.Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, args.PositionRange())) } + if counterResetCollision { + annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args.PositionRange())) + } } resultSample.H.CounterResetHint = histogram.GaugeType resultSample.H.Compact(0) @@ -699,20 +709,27 @@ func funcAvgOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh // the current implementation is accurate enough for practical purposes. if len(firstSeries.Floats) == 0 { // The passed values only contain histograms. + var annos annotations.Annotations vec, err := aggrHistOverTime(matrixVal, enh, func(s Series) (*histogram.FloatHistogram, error) { mean := s.Histograms[0].H.Copy() for i, h := range s.Histograms[1:] { count := float64(i + 2) left := h.H.Copy().Div(count) right := mean.Copy().Div(count) - toAdd, err := left.Sub(right) + toAdd, counterResetCollision, err := left.Sub(right) if err != nil { return mean, err } - _, err = mean.Add(toAdd) + if counterResetCollision { + annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args[0].PositionRange())) + } + _, counterResetCollision, err = mean.Add(toAdd) if err != nil { return mean, err } + if counterResetCollision { + annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args[0].PositionRange())) + } } return mean, nil }) @@ -724,7 +741,7 @@ func funcAvgOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh return enh.Out, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, args[0].PositionRange())) } } - return vec, nil + return vec, annos } return aggrOverTime(matrixVal, enh, func(s Series) float64 { var ( @@ -901,13 +918,17 @@ func funcSumOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh } if len(firstSeries.Floats) == 0 { // The passed values only contain histograms. + var annos annotations.Annotations vec, err := aggrHistOverTime(matrixVal, enh, func(s Series) (*histogram.FloatHistogram, error) { sum := s.Histograms[0].H.Copy() for _, h := range s.Histograms[1:] { - _, err := sum.Add(h.H) + _, counterResetCollision, err := sum.Add(h.H) if err != nil { return sum, err } + if counterResetCollision { + annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args[0].PositionRange())) + } } return sum, nil }) @@ -919,7 +940,7 @@ func funcSumOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh return enh.Out, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, args[0].PositionRange())) } } - return vec, nil + return vec, annos } return aggrOverTime(matrixVal, enh, func(s Series) float64 { var sum, c float64 diff --git a/promql/functions_internal_test.go b/promql/functions_internal_test.go index 85c6cd7973..5ca4e54483 100644 --- a/promql/functions_internal_test.go +++ b/promql/functions_internal_test.go @@ -19,6 +19,10 @@ import ( "testing" "github.com/stretchr/testify/require" + + "github.com/prometheus/prometheus/model/histogram" + "github.com/prometheus/prometheus/promql/parser/posrange" + "github.com/prometheus/prometheus/util/annotations" ) func TestKahanSumInc(t *testing.T) { @@ -79,3 +83,51 @@ func TestKahanSumInc(t *testing.T) { }) } } + +func newAnnotations(errs ...error) annotations.Annotations { + var annos annotations.Annotations + for _, err := range errs { + annos.Add(err) + } + return annos +} + +// TODO(juliusmh): this test ensures histogramRate sets correct annotations. +// This test can be removed in favor of a user facing promqltest when +// https://github.com/prometheus/prometheus/issues/15346 is resolved. +func TestHistogramRate_Annotations(t *testing.T) { + const metricName = "test" + pos := posrange.PositionRange{} + for _, tc := range []struct { + name string + points []HPoint + wantAnnotations annotations.Annotations + }{ + { + name: "empty histograms", + points: []HPoint{ + {H: &histogram.FloatHistogram{}}, + {H: &histogram.FloatHistogram{}}, + }, + wantAnnotations: newAnnotations( + annotations.NewNativeHistogramNotGaugeWarning(metricName, pos), + ), + }, + { + name: "counter reset hint collision", + points: []HPoint{ + {H: &histogram.FloatHistogram{CounterResetHint: histogram.NotCounterReset}}, + {H: &histogram.FloatHistogram{CounterResetHint: histogram.CounterReset}}, + }, + wantAnnotations: newAnnotations( + annotations.NewNativeHistogramNotGaugeWarning(metricName, pos), + annotations.NewHistogramCounterResetCollisionWarning(pos), + ), + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, annos := histogramRate(tc.points, false, metricName, pos) + require.Equal(t, tc.wantAnnotations, annos) + }) + } +} diff --git a/promql/parser/parse.go b/promql/parser/parse.go index 8f24ee34fe..138054c537 100644 --- a/promql/parser/parse.go +++ b/promql/parser/parse.go @@ -494,13 +494,15 @@ func (p *parser) mergeMaps(left, right *map[string]interface{}) (ret *map[string func (p *parser) histogramsIncreaseSeries(base, inc *histogram.FloatHistogram, times uint64) ([]SequenceValue, error) { return p.histogramsSeries(base, inc, times, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) { - return a.Add(b) + res, _, err := a.Add(b) + return res, err }) } func (p *parser) histogramsDecreaseSeries(base, inc *histogram.FloatHistogram, times uint64) ([]SequenceValue, error) { return p.histogramsSeries(base, inc, times, func(a, b *histogram.FloatHistogram) (*histogram.FloatHistogram, error) { - return a.Sub(b) + res, _, err := a.Sub(b) + return res, err }) } diff --git a/rules/manager_test.go b/rules/manager_test.go index dd24bf19de..1d18d4dd28 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -1505,7 +1505,7 @@ func TestNativeHistogramsInRecordingRules(t *testing.T) { expHist := hists[0].ToFloat(nil) for _, h := range hists[1:] { - expHist, err = expHist.Add(h.ToFloat(nil)) + expHist, _, err = expHist.Add(h.ToFloat(nil)) require.NoError(t, err) } diff --git a/util/annotations/annotations.go b/util/annotations/annotations.go index ec92b06d06..a4d03f5e3c 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -155,6 +155,7 @@ var ( NativeHistogramQuantileNaNResultInfo = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is NaN", PromQLInfo) NativeHistogramQuantileNaNSkewInfo = fmt.Errorf("%w: input to histogram_quantile has NaN observations, result is skewed higher", PromQLInfo) NativeHistogramFractionNaNsInfo = fmt.Errorf("%w: input to histogram_fraction has NaN observations, which are excluded from all fractions", PromQLInfo) + HistogramCounterResetCollisionWarning = fmt.Errorf("%w: conflicting histogram counter resets", PromQLWarning) ) type annoErr struct { @@ -356,3 +357,12 @@ func NewNativeHistogramFractionNaNsInfo(metricName string, pos posrange.Position Err: maybeAddMetricName(NativeHistogramFractionNaNsInfo, metricName), } } + +// NewHistogramCounterResetCollisionWarning is used when two counter histograms are added or subtracted where one has +// a CounterReset hint and the other has NotCounterReset. +func NewHistogramCounterResetCollisionWarning(pos posrange.PositionRange) error { + return annoErr{ + PositionRange: pos, + Err: fmt.Errorf("%w", HistogramCounterResetCollisionWarning), + } +}