From 44df626620f93c02485a03134735e6bb3c8b6a10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linas=20Med=C5=BEi=C5=ABnas?= Date: Sat, 18 Oct 2025 02:03:52 +0300 Subject: [PATCH] promql (histograms): reconcile mismatched NHCB bounds (#17278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #17255. The implementation happens mostly in the Add and Sub method, but the reconciliation works for all relevant operations. For example, you can now `rate` over a range wherein the custom bucket boundaries are changing. Any custom bucket reconciliation is flagged with an info-level annotation. --------- Signed-off-by: Linas Medziunas Signed-off-by: Linas Medžiūnas --- model/histogram/float_histogram.go | 269 ++++++++++++- model/histogram/float_histogram_test.go | 368 ++++++++++++++---- model/histogram/generic.go | 1 - promql/engine.go | 93 +++-- promql/engine_internal_test.go | 27 +- promql/functions.go | 52 ++- promql/parser/parse.go | 4 +- promql/promqltest/testdata/functions.test | 8 +- .../testdata/native_histograms.test | 117 ++++-- rules/manager_test.go | 2 +- util/annotations/annotations.go | 54 +-- 11 files changed, 777 insertions(+), 218 deletions(-) diff --git a/model/histogram/float_histogram.go b/model/histogram/float_histogram.go index 08469ad124..c607448f38 100644 --- a/model/histogram/float_histogram.go +++ b/model/histogram/float_histogram.go @@ -343,10 +343,13 @@ func (h *FloatHistogram) Div(scalar float64) *FloatHistogram { // 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). // +// In case of mismatched NHCB bounds, they will be reconciled to the intersection of +// both histograms, and nhcbBoundsReconciled will be returned as true. +// // This method returns a pointer to the receiving histogram for convenience. -func (h *FloatHistogram) Add(other *FloatHistogram) (res *FloatHistogram, counterResetCollision bool, err error) { +func (h *FloatHistogram) Add(other *FloatHistogram) (res *FloatHistogram, counterResetCollision, nhcbBoundsReconciled bool, err error) { if err := h.checkSchemaAndBounds(other); err != nil { - return nil, false, err + return nil, false, false, err } counterResetCollision = h.adjustCounterReset(other) if !h.UsesCustomBuckets() { @@ -364,8 +367,21 @@ func (h *FloatHistogram) Add(other *FloatHistogram) (res *FloatHistogram, counte ) if h.UsesCustomBuckets() { - h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) - return h, counterResetCollision, nil + if CustomBucketBoundsMatch(h.CustomValues, other.CustomValues) { + h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, false, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) + } else { + nhcbBoundsReconciled = true + intersectedBounds := intersectCustomBucketBounds(h.CustomValues, other.CustomValues) + + // Add with mapping - maps both histograms to intersected layout. + h.PositiveSpans, h.PositiveBuckets = addCustomBucketsWithMismatches( + false, + hPositiveSpans, hPositiveBuckets, h.CustomValues, + otherPositiveSpans, otherPositiveBuckets, other.CustomValues, + intersectedBounds) + h.CustomValues = intersectedBounds + } + return h, counterResetCollision, nhcbBoundsReconciled, nil } var ( @@ -389,7 +405,7 @@ func (h *FloatHistogram) Add(other *FloatHistogram) (res *FloatHistogram, counte 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, counterResetCollision, nil + return h, counterResetCollision, nhcbBoundsReconciled, nil } // Sub works like Add but subtracts the other histogram. It uses the same logic @@ -397,9 +413,9 @@ func (h *FloatHistogram) Add(other *FloatHistogram) (res *FloatHistogram, counte // for incremental mean calculation. However, if it is used for the actual "-" // operator in PromQL, the counter reset needs to be set to GaugeType after // calling this method. -func (h *FloatHistogram) Sub(other *FloatHistogram) (res *FloatHistogram, counterResetCollision bool, err error) { +func (h *FloatHistogram) Sub(other *FloatHistogram) (res *FloatHistogram, counterResetCollision, nhcbBoundsReconciled bool, err error) { if err := h.checkSchemaAndBounds(other); err != nil { - return nil, false, err + return nil, false, false, err } counterResetCollision = h.adjustCounterReset(other) if !h.UsesCustomBuckets() { @@ -417,8 +433,21 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) (res *FloatHistogram, counte ) if h.UsesCustomBuckets() { - h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) - return h, counterResetCollision, nil + if CustomBucketBoundsMatch(h.CustomValues, other.CustomValues) { + h.PositiveSpans, h.PositiveBuckets = addBuckets(h.Schema, h.ZeroThreshold, true, hPositiveSpans, hPositiveBuckets, otherPositiveSpans, otherPositiveBuckets) + } else { + nhcbBoundsReconciled = true + intersectedBounds := intersectCustomBucketBounds(h.CustomValues, other.CustomValues) + + // Subtract with mapping - maps both histograms to intersected layout. + h.PositiveSpans, h.PositiveBuckets = addCustomBucketsWithMismatches( + true, + hPositiveSpans, hPositiveBuckets, h.CustomValues, + otherPositiveSpans, otherPositiveBuckets, other.CustomValues, + intersectedBounds) + h.CustomValues = intersectedBounds + } + return h, counterResetCollision, nhcbBoundsReconciled, nil } var ( @@ -441,7 +470,7 @@ func (h *FloatHistogram) Sub(other *FloatHistogram) (res *FloatHistogram, counte 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, counterResetCollision, nil + return h, counterResetCollision, nhcbBoundsReconciled, nil } // Equals returns true if the given float histogram matches exactly. @@ -604,11 +633,17 @@ func (h *FloatHistogram) DetectReset(previous *FloatHistogram) bool { if h.Count < previous.Count { return true } - if h.UsesCustomBuckets() != previous.UsesCustomBuckets() || (h.UsesCustomBuckets() && !CustomBucketBoundsMatch(h.CustomValues, previous.CustomValues)) { - // Mark that something has changed or that the application has been restarted. However, this does - // not matter so much since the change in schema will be handled directly in the chunks and PromQL - // functions. - return true + if h.UsesCustomBuckets() { + if !previous.UsesCustomBuckets() { + // Mark that something has changed or that the application has been restarted. However, this does + // not matter so much since the change in schema will be handled directly in the chunks and PromQL + // functions. + return true + } + if !CustomBucketBoundsMatch(h.CustomValues, previous.CustomValues) { + // Custom bounds don't match - check if any reconciled bucket value has decreased. + return h.detectResetWithMismatchedCustomBounds(previous, h.CustomValues, previous.CustomValues) + } } if h.Schema > previous.Schema { return true @@ -1331,6 +1366,204 @@ func floatBucketsMatch(b1, b2 []float64) bool { return true } +// detectResetWithMismatchedCustomBounds checks if any bucket count has decreased when +// comparing NHCBs with mismatched custom bounds. It maps both histograms +// to the intersected bounds on-the-fly and compares values without allocating +// arrays for all mapped buckets. +// Will panic if called with histograms that are not NHCB. +func (h *FloatHistogram) detectResetWithMismatchedCustomBounds( + previous *FloatHistogram, currBounds, prevBounds []float64, +) bool { + if h.Schema != CustomBucketsSchema || previous.Schema != CustomBucketsSchema { + panic("detectResetWithMismatchedCustomBounds called with non-NHCB schema") + } + currIt := h.floatBucketIterator(true, 0, CustomBucketsSchema) + prevIt := previous.floatBucketIterator(true, 0, CustomBucketsSchema) + + rollupSumForBound := func(iter *floatBucketIterator, iterStarted bool, iterBucket Bucket[float64], bound float64) (float64, Bucket[float64], bool) { + if !iterStarted { + if !iter.Next() { + return 0, Bucket[float64]{}, false + } + iterBucket = iter.At() + } + var sum float64 + for iterBucket.Upper <= bound { + sum += iterBucket.Count + if !iter.Next() { + return sum, Bucket[float64]{}, false + } + iterBucket = iter.At() + } + return sum, iterBucket, true + } + + var ( + currBoundIdx, prevBoundIdx = 0, 0 + currBucket, prevBucket Bucket[float64] + currIterStarted, currHasMore bool + prevIterStarted, prevHasMore bool + ) + + for currBoundIdx <= len(currBounds) && prevBoundIdx <= len(prevBounds) { + currBound := math.Inf(1) + if currBoundIdx < len(currBounds) { + currBound = currBounds[currBoundIdx] + } + prevBound := math.Inf(1) + if prevBoundIdx < len(prevBounds) { + prevBound = prevBounds[prevBoundIdx] + } + + switch { + case currBound == prevBound: + // Check matching bound, rolling up lesser buckets that have not been accounter for yet. + currRollupSum := 0.0 + if !currIterStarted || currHasMore { + currRollupSum, currBucket, currHasMore = rollupSumForBound(&currIt, currIterStarted, currBucket, currBound) + currIterStarted = true + } + + prevRollupSum := 0.0 + if !prevIterStarted || prevHasMore { + prevRollupSum, prevBucket, prevHasMore = rollupSumForBound(&prevIt, prevIterStarted, prevBucket, currBound) + prevIterStarted = true + } + + if currRollupSum < prevRollupSum { + return true + } + + currBoundIdx++ + prevBoundIdx++ + case currBound < prevBound: + currBoundIdx++ + default: + prevBoundIdx++ + } + } + + return false +} + +// intersectCustomBucketBounds returns the intersection of two custom bucket boundary sets. +func intersectCustomBucketBounds(boundsA, boundsB []float64) []float64 { + if len(boundsA) == 0 || len(boundsB) == 0 { + return nil + } + + var ( + result []float64 + i, j = 0, 0 + ) + + for i < len(boundsA) && j < len(boundsB) { + switch { + case boundsA[i] == boundsB[j]: + if result == nil { + // Allocate a new slice because FloatHistogram.CustomValues has to be immutable. + result = make([]float64, 0, min(len(boundsA), len(boundsB))) + } + result = append(result, boundsA[i]) + i++ + j++ + case boundsA[i] < boundsB[j]: + i++ + default: + j++ + } + } + + return result +} + +// addCustomBucketsWithMismatches handles adding/subtracting custom bucket histograms +// with mismatched bucket layouts by mapping both to an intersected layout. +func addCustomBucketsWithMismatches( + negative bool, + spansA []Span, bucketsA, boundsA []float64, + spansB []Span, bucketsB, boundsB []float64, + intersectedBounds []float64, +) ([]Span, []float64) { + targetBuckets := make([]float64, len(intersectedBounds)+1) + + mapBuckets := func(spans []Span, buckets, bounds []float64, negative bool) { + srcIdx := 0 + bucketIdx := 0 + intersectIdx := 0 + + for _, span := range spans { + srcIdx += int(span.Offset) + for range span.Length { + if bucketIdx < len(buckets) { + value := buckets[bucketIdx] + + // Find target bucket index. + targetIdx := len(targetBuckets) - 1 // Default to +Inf bucket. + if srcIdx < len(bounds) { + srcBound := bounds[srcIdx] + // Since both arrays are sorted, we can continue from where we left off. + for intersectIdx < len(intersectedBounds) { + if intersectedBounds[intersectIdx] >= srcBound { + targetIdx = intersectIdx + break + } + intersectIdx++ + } + } + + if negative { + targetBuckets[targetIdx] -= value + } else { + targetBuckets[targetIdx] += value + } + } + srcIdx++ + bucketIdx++ + } + } + } + + // Map both histograms to the intersected layout. + mapBuckets(spansA, bucketsA, boundsA, false) + mapBuckets(spansB, bucketsB, boundsB, negative) + + // Build spans and buckets, excluding zero-valued buckets from the final result. + destSpans := spansA[:0] // Reuse spansA capacity for destSpans since we don't need it anymore. + destBuckets := targetBuckets[:0] // Reuse targetBuckets capacity for destBuckets since it's guaranteed to be large enough. + lastIdx := int32(-1) + + for i, count := range targetBuckets { + if count == 0 { + continue + } + + destBuckets = append(destBuckets, count) + idx := int32(i) + + if len(destSpans) > 0 && idx == lastIdx+1 { + // Consecutive bucket, extend the last span. + destSpans[len(destSpans)-1].Length++ + } else { + // New span needed. + // TODO: optimize away small gaps. + offset := idx + if len(destSpans) > 0 { + // Convert to relative offset from the end of the last span. + prevEnd := lastIdx + offset = idx - prevEnd - 1 + } + destSpans = append(destSpans, Span{ + Offset: offset, + Length: 1, + }) + } + lastIdx = idx + } + + return destSpans, destBuckets +} + // ReduceResolution reduces the float histogram's spans, buckets into target schema. // The target schema must be smaller than the current float histogram's schema. // This will panic if the histogram has custom buckets or if the target schema is @@ -1354,15 +1587,11 @@ func (h *FloatHistogram) ReduceResolution(targetSchema int32) *FloatHistogram { } // checkSchemaAndBounds checks if two histograms are compatible because they -// both use a standard exponential schema or because they both are NHCBs. In the -// latter case, they also have to use the same custom bounds. +// both use a standard exponential schema or because they both are NHCBs. func (h *FloatHistogram) checkSchemaAndBounds(other *FloatHistogram) error { if h.UsesCustomBuckets() != other.UsesCustomBuckets() { return ErrHistogramsIncompatibleSchema } - if h.UsesCustomBuckets() && !CustomBucketBoundsMatch(h.CustomValues, other.CustomValues) { - return ErrHistogramsIncompatibleBounds - } return nil } diff --git a/model/histogram/float_histogram_test.go b/model/histogram/float_histogram_test.go index be92c87f62..3e63a14300 100644 --- a/model/histogram/float_histogram_test.go +++ b/model/histogram/float_histogram_test.go @@ -1275,6 +1275,146 @@ func TestFloatHistogramDetectReset(t *testing.T) { }, true, }, + { + "mismatched custom bounds - no reset when all buckets increase", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 200, + Sum: 1000, + PositiveSpans: []Span{{0, 4}}, + PositiveBuckets: []float64{20, 35, 40, 50}, // Previous: buckets for: (-Inf,0], (0,1], (1,2], (2,3], then (3,+Inf] + CustomValues: []float64{0, 1, 2, 3}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 200, + Sum: 1000, + PositiveSpans: []Span{{0, 5}}, + PositiveBuckets: []float64{25, 15, 40, 50, 70}, // Current: buckets for: (-Inf,0], (0,0.5], (0.5,1], (1,2], (2,3], then (3,+Inf] + CustomValues: []float64{0, 0.5, 1, 2, 3}, + }, + false, // No reset: (-Inf,0] increases from 20 to 25, (0,1] increases from 35 to 55, (1,2] increases from 40 to 50, (2,3] increases from 50 to 70 + }, + { + "mismatched custom bounds - reset in middle bucket", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 100, + Sum: 500, + PositiveSpans: []Span{{1, 4}}, + PositiveBuckets: []float64{10, 15, 20, 25}, // Buckets for: [0,1], [1,3], [3,5], [5,+Inf] + CustomValues: []float64{0, 1, 3, 5}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 100, + Sum: 500, + PositiveSpans: []Span{{1, 5}}, + PositiveBuckets: []float64{10, 16, 10, 5, 25}, // Buckets for: [0,1], [1,2], [2,3], [3,5], [5,+Inf] + CustomValues: []float64{0, 1, 2, 3, 5}, + }, + true, // Reset detected: [1,3] bucket decreased from 20 to 15 + }, + { + "mismatched custom bounds - reset in last bucket", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 100, + Sum: 500, + PositiveSpans: []Span{{1, 3}}, + PositiveBuckets: []float64{10, 20, 20}, // Buckets for: [0,1], [1,2], [2,+Inf] + CustomValues: []float64{0, 1, 2}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 100, + Sum: 500, + PositiveSpans: []Span{{1, 4}}, + PositiveBuckets: []float64{100, 200, 8, 7}, // Buckets for: [0,1], [1,2], [2,3], [3,+Inf] + CustomValues: []float64{0, 1, 2, 3}, + }, + true, // Reset detected: [2,+Inf] bucket decreased from 20 to 15 + }, + { + "mismatched custom bounds - no common bounds", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 100, + Sum: 500, + PositiveSpans: []Span{{1, 3}}, + PositiveBuckets: []float64{10, 20, 30}, // Buckets for: [1,2], [2,3], [3,+Inf] + CustomValues: []float64{4, 5, 6}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 100, + Sum: 500, + PositiveSpans: []Span{{1, 3}}, + PositiveBuckets: []float64{15, 25, 35}, // Buckets for: [4,5], [5,6], [6,+Inf] + CustomValues: []float64{1, 2, 3}, + }, + false, // no decrease in aggregated single +Inf bounded bucket + }, + { + "mismatched custom bounds - sparse common bounds", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 200, + Sum: 1000, + PositiveSpans: []Span{{0, 4}}, + PositiveBuckets: []float64{10, 20, 30, 40}, // Previous: buckets for: (-Inf,0], (0,1], (1,3], (3,5], then (5,+Inf] + CustomValues: []float64{0, 1, 3, 5}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 200, + Sum: 1000, + PositiveSpans: []Span{{0, 5}}, + PositiveBuckets: []float64{15, 25, 70, 50, 100}, // Current: buckets for: (-Inf,0], (0,2], (2,3], (3,4], (4,5], then (5,+Inf] + CustomValues: []float64{0, 2, 3, 4, 5}, + }, + false, // No reset: common bounds [0,3,5] all increase when mapped + }, + { + "reset detected with mismatched custom bounds and split bucket spans", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 200, + Sum: 1000, + PositiveSpans: []Span{{0, 2}, {1, 3}}, // Split spans: buckets at indices 0,1 and 3,4,5 + PositiveBuckets: []float64{10, 20, 30, 40, 50}, // Buckets for: (-Inf,0], (0,1], skip (1,2], then (2,3], (3,4], (4,5] + CustomValues: []float64{0, 1, 2, 3, 4, 5}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 200, + Sum: 1000, + PositiveSpans: []Span{{0, 3}, {1, 2}}, // Split spans: buckets at indices 0,1,2 and 4,5 + PositiveBuckets: []float64{15, 25, 35, 25, 60}, // Buckets for: (-Inf,0], (0,1], (1,3], skip (3,4], then (4,5], (5,7] + CustomValues: []float64{0, 1, 3, 4, 5, 7}, + }, + true, // Reset detected: bucket (3,4] goes from 40 to 0 (missing in current histogram) + }, + { + "no reset with mismatched custom bounds and split bucket spans", + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 300, + Sum: 1500, + PositiveSpans: []Span{{0, 2}, {2, 3}}, // Split spans: buckets at indices 0,1 and 4,5,6 + PositiveBuckets: []float64{10, 20, 30, 40, 50}, // Buckets for: (-Inf,0], (0,1], skip (1,2], (2,3], then (3,4], (4,5], (5,6] + CustomValues: []float64{0, 1, 2, 3, 4, 5, 6}, + }, + &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 300, + Sum: 1500, + PositiveSpans: []Span{{0, 3}, {1, 2}}, // Split spans: buckets at indices 0,1,2 and 4,5 + PositiveBuckets: []float64{12, 25, 45, 75, 95}, // Buckets for: (-Inf,0], (0,0.5], (0.5,1], skip (1,3], then (3,5], (5,7] + CustomValues: []float64{0, 0.5, 1, 3, 5, 7}, + }, + false, // No reset: all mapped buckets increase + }, } for _, c := range cases { @@ -1649,6 +1789,7 @@ func TestFloatHistogramAdd(t *testing.T) { in1, in2, expected *FloatHistogram expErrMsg string expCounterResetCollision bool + expNHCBBoundsReconciled bool }{ { name: "same bucket layout", @@ -2093,7 +2234,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 2.345, PositiveSpans: []Span{{0, 2}, {1, 3}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2101,7 +2242,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 1.234, PositiveSpans: []Span{{0, 2}, {1, 3}}, PositiveBuckets: []float64{0, 0, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, expected: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2109,7 +2250,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 3.579, PositiveSpans: []Span{{0, 2}, {1, 3}}, PositiveBuckets: []float64{1, 0, 5, 7, 13}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, }, { @@ -2120,7 +2261,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 2.345, PositiveSpans: []Span{{0, 2}, {1, 1}, {0, 2}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2128,7 +2269,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 1.234, PositiveSpans: []Span{{0, 2}, {1, 2}, {0, 1}}, PositiveBuckets: []float64{0, 0, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, expected: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2136,7 +2277,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 3.579, PositiveSpans: []Span{{0, 2}, {1, 1}, {0, 2}}, PositiveBuckets: []float64{1, 0, 5, 7, 13}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, }, { @@ -2147,7 +2288,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 2.345, PositiveSpans: []Span{{0, 2}, {2, 3}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2155,7 +2296,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 1.234, PositiveSpans: []Span{{2, 2}, {3, 3}}, PositiveBuckets: []float64{5, 4, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9}, }, expected: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2163,7 +2304,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 3.579, PositiveSpans: []Span{{0, 4}, {0, 6}}, PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9}, }, }, { @@ -2174,7 +2315,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 1.234, PositiveSpans: []Span{{2, 2}, {3, 3}}, PositiveBuckets: []float64{5, 4, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2182,7 +2323,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 2.345, PositiveSpans: []Span{{0, 2}, {2, 3}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9}, }, expected: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2190,7 +2331,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 3.579, PositiveSpans: []Span{{0, 4}, {0, 6}}, PositiveBuckets: []float64{1, 0, 5, 4, 3, 4, 7, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9}, }, }, { @@ -2201,7 +2342,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 2.345, PositiveSpans: []Span{{0, 2}, {2, 3}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2209,7 +2350,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 1.234, PositiveSpans: []Span{{1, 4}, {0, 3}}, PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7}, }, expected: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2217,7 +2358,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 3.579, PositiveSpans: []Span{{0, 4}, {0, 4}}, PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7}, }, }, { @@ -2228,7 +2369,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 1.234, PositiveSpans: []Span{{1, 4}, {0, 3}}, PositiveBuckets: []float64{5, 4, 2, 3, 6, 2, 5}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2236,7 +2377,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 2.345, PositiveSpans: []Span{{0, 2}, {2, 3}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7}, }, expected: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2244,28 +2385,92 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 3.579, PositiveSpans: []Span{{0, 4}, {0, 4}}, PositiveBuckets: []float64{1, 5, 4, 2, 6, 10, 9, 5}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7}, }, }, { - name: "different custom bucket layout", + name: "custom buckets with partial intersection", in1: &FloatHistogram{ Schema: CustomBucketsSchema, - Count: 15, - Sum: 2.345, - PositiveSpans: []Span{{0, 2}, {1, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + Count: 10, + Sum: 100, + PositiveSpans: []Span{{0, 3}}, + PositiveBuckets: []float64{2, 3, 5}, + CustomValues: []float64{1, 2.5}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, - Count: 11, + Count: 8, + Sum: 80, + PositiveSpans: []Span{{0, 4}}, + PositiveBuckets: []float64{1, 2, 3, 2}, + CustomValues: []float64{1, 2, 3}, + }, + expected: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 18, + Sum: 180, + PositiveSpans: []Span{{0, 2}}, + PositiveBuckets: []float64{3, 15}, + CustomValues: []float64{1}, + }, + expNHCBBoundsReconciled: true, + }, + { + name: "different custom bucket layout - intersection and rollup", + in1: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 6, + Sum: 2.345, + PositiveSpans: []Span{{0, 1}, {1, 1}}, + PositiveBuckets: []float64{1, 5}, + CustomValues: []float64{2, 4}, + }, + in2: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 220, Sum: 1.234, - PositiveSpans: []Span{{0, 2}, {1, 3}}, - PositiveBuckets: []float64{0, 0, 2, 3, 6}, + PositiveSpans: []Span{{0, 2}, {1, 1}, {0, 2}}, + PositiveBuckets: []float64{10, 20, 40, 50, 100}, CustomValues: []float64{1, 2, 3, 4, 5}, }, - expErrMsg: "cannot apply this operation on custom buckets histograms with different custom bounds", + expected: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 226, + Sum: 3.579, + PositiveSpans: []Span{{0, 3}}, + PositiveBuckets: []float64{1 + 10 + 20, 40, 5 + 50 + 100}, + CustomValues: []float64{2, 4}, + }, + expNHCBBoundsReconciled: true, + }, + { + name: "custom buckets with no common boundaries except +Inf", + in1: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 3, + Sum: 50, + PositiveSpans: []Span{{0, 2}}, + PositiveBuckets: []float64{1, 2}, + CustomValues: []float64{1.5}, + }, + in2: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 30, + Sum: 40, + PositiveSpans: []Span{{0, 2}}, + PositiveBuckets: []float64{10, 20}, + CustomValues: []float64{2.5}, + }, + expected: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 33, + Sum: 90, + PositiveSpans: []Span{{0, 1}}, + PositiveBuckets: []float64{1 + 2 + 10 + 20}, + CustomValues: nil, + }, + expNHCBBoundsReconciled: true, }, { name: "mix exponential and custom buckets histograms", @@ -2286,7 +2491,7 @@ func TestFloatHistogramAdd(t *testing.T) { Sum: 12, PositiveSpans: []Span{{0, 2}, {1, 3}}, PositiveBuckets: []float64{0, 0, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, expErrMsg: "cannot apply this operation on histograms with a mix of exponential and custom bucket schemas", }, @@ -2307,13 +2512,16 @@ func TestFloatHistogramAdd(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - testHistogramAdd(t, c.in1, c.in2, c.expected, c.expErrMsg, c.expCounterResetCollision) - testHistogramAdd(t, c.in2, c.in1, c.expected, c.expErrMsg, c.expCounterResetCollision) + testHistogramAdd(t, c.in1, c.in2, c.expected, c.expErrMsg, c.expCounterResetCollision, c.expNHCBBoundsReconciled) + testHistogramAdd(t, c.in2, c.in1, c.expected, c.expErrMsg, c.expCounterResetCollision, c.expNHCBBoundsReconciled) }) } } -func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string, expCounterResetCollision bool) { +func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string, expCounterResetCollision, expNHCBBoundsReconciled bool) { + require.NoError(t, a.Validate(), "a") + require.NoError(t, b.Validate(), "b") + var ( aCopy = a.Copy() bCopy = b.Copy() @@ -2324,7 +2532,7 @@ func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg st expectedCopy = expected.Copy() } - res, warn, err := aCopy.Add(bCopy) + res, counterResetCollision, nhcbBoundsReconciled, err := aCopy.Add(bCopy) if expErrMsg != "" { require.EqualError(t, err, expErrMsg) } else { @@ -2332,7 +2540,8 @@ func testHistogramAdd(t *testing.T, a, b, expected *FloatHistogram, expErrMsg st } // Check that the warnings are correct. - require.Equal(t, expCounterResetCollision, warn) + require.Equal(t, expCounterResetCollision, counterResetCollision) + require.Equal(t, expNHCBBoundsReconciled, nhcbBoundsReconciled) if expected != nil { res.Compact(0) @@ -2356,6 +2565,7 @@ func TestFloatHistogramSub(t *testing.T) { in1, in2, expected *FloatHistogram expErrMsg string expCounterResetCollision bool + expNHCBBoundsReconciled bool }{ { name: "same bucket layout", @@ -2433,34 +2643,7 @@ func TestFloatHistogramSub(t *testing.T) { Sum: 23, PositiveSpans: []Span{{0, 2}, {1, 3}}, PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, - }, - in2: &FloatHistogram{ - Schema: CustomBucketsSchema, - Count: 11, - Sum: 12, - PositiveSpans: []Span{{0, 2}, {1, 3}}, - PositiveBuckets: []float64{0, 0, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, - }, - expected: &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}, - }, - }, - { - name: "different custom bucket layout", - in1: &FloatHistogram{ - Schema: CustomBucketsSchema, - Count: 15, - Sum: 23, - PositiveSpans: []Span{{0, 2}, {1, 3}}, - PositiveBuckets: []float64{1, 0, 3, 4, 7}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5}, }, in2: &FloatHistogram{ Schema: CustomBucketsSchema, @@ -2470,7 +2653,45 @@ func TestFloatHistogramSub(t *testing.T) { PositiveBuckets: []float64{0, 0, 2, 3, 6}, CustomValues: []float64{1, 2, 3, 4, 5}, }, - expErrMsg: "cannot apply this operation on custom buckets histograms with different custom bounds", + expected: &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, 5}, + }, + }, + { + name: "different custom bucket layout - with intersection and rollup", + in1: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 220, + Sum: 9.9, + PositiveSpans: []Span{{0, 2}, {1, 1}, {0, 2}}, + PositiveBuckets: []float64{10, 20, 40, 50, 100}, + CustomValues: []float64{1, 2, 3, 4, 5}, + CounterResetHint: GaugeType, + }, + in2: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 6, + Sum: 4.4, + PositiveSpans: []Span{{0, 1}, {1, 1}}, + PositiveBuckets: []float64{1, 5}, + CustomValues: []float64{2, 4}, + CounterResetHint: GaugeType, + }, + expected: &FloatHistogram{ + Schema: CustomBucketsSchema, + Count: 214, + Sum: 5.5, + PositiveSpans: []Span{{0, 3}}, + PositiveBuckets: []float64{10 + 20 - 1, 40, 50 + 100 - 5}, + CustomValues: []float64{2, 4}, + CounterResetHint: GaugeType, + }, + expNHCBBoundsReconciled: true, }, { name: "mix exponential and custom buckets histograms", @@ -2491,7 +2712,7 @@ func TestFloatHistogramSub(t *testing.T) { Sum: 12, PositiveSpans: []Span{{0, 2}, {1, 3}}, PositiveBuckets: []float64{0, 0, 2, 3, 6}, - CustomValues: []float64{1, 2, 3, 4}, + CustomValues: []float64{1, 2, 3, 4, 5, 6}, }, expErrMsg: "cannot apply this operation on histograms with a mix of exponential and custom bucket schemas", }, @@ -2512,7 +2733,7 @@ func TestFloatHistogramSub(t *testing.T) { for _, c := range cases { t.Run(c.name, func(t *testing.T) { - testFloatHistogramSub(t, c.in1, c.in2, c.expected, c.expErrMsg, c.expCounterResetCollision) + testFloatHistogramSub(t, c.in1, c.in2, c.expected, c.expErrMsg, c.expCounterResetCollision, c.expNHCBBoundsReconciled) var expectedNegative *FloatHistogram if c.expected != nil { @@ -2522,12 +2743,15 @@ func TestFloatHistogramSub(t *testing.T) { // counter reset hint for this test. expectedNegative.CounterResetHint = c.expected.CounterResetHint } - testFloatHistogramSub(t, c.in2, c.in1, expectedNegative, c.expErrMsg, c.expCounterResetCollision) + testFloatHistogramSub(t, c.in2, c.in1, expectedNegative, c.expErrMsg, c.expCounterResetCollision, c.expNHCBBoundsReconciled) }) } } -func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string, expCounterResetCollision bool) { +func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrMsg string, expCounterResetCollision, expNHCBBoundsReconciled bool) { + require.NoError(t, a.Validate(), "a") + require.NoError(t, b.Validate(), "b") + var ( aCopy = a.Copy() bCopy = b.Copy() @@ -2538,7 +2762,7 @@ func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrM expectedCopy = expected.Copy() } - res, warn, err := aCopy.Sub(bCopy) + res, counterResetCollision, nhcbBoundsReconciled, err := aCopy.Sub(bCopy) if expErrMsg != "" { require.EqualError(t, err, expErrMsg) } else { @@ -2558,7 +2782,8 @@ func testFloatHistogramSub(t *testing.T, a, b, expected *FloatHistogram, expErrM require.Equal(t, b, bCopy) // Check that the warnings are correct. - require.Equal(t, expCounterResetCollision, warn) + require.Equal(t, expCounterResetCollision, counterResetCollision) + require.Equal(t, expNHCBBoundsReconciled, nhcbBoundsReconciled) } } @@ -3440,8 +3665,9 @@ func TestFloatHistogramSize(t *testing.T) { }, PositiveBuckets: []float64{1, 3.3, 4.2, 0.1}, // 24 bytes + 4 * 8 bytes. NegativeSpans: []Span{ // 24 bytes. - {3, 2}, // 2 * 4 bytes. - {3, 2}}, // 2 * 4 bytes. + {3, 2}, // 2 * 4 bytes. + {3, 2}, // 2 * 4 bytes. + }, NegativeBuckets: []float64{3.1, 3, 1.234e5, 1000}, // 24 bytes + 4 * 8 bytes. CustomValues: nil, // 24 bytes. }, diff --git a/model/histogram/generic.go b/model/histogram/generic.go index 386eb21815..93ac768449 100644 --- a/model/histogram/generic.go +++ b/model/histogram/generic.go @@ -40,7 +40,6 @@ var ( ErrHistogramCustomBucketsInfinite = errors.New("histogram custom bounds must be finite") ErrHistogramCustomBucketsNaN = errors.New("histogram custom bounds must not be NaN") ErrHistogramsIncompatibleSchema = errors.New("cannot apply this operation on histograms with a mix of exponential and custom bucket schemas") - ErrHistogramsIncompatibleBounds = errors.New("cannot apply this operation on custom buckets histograms with different custom bounds") ErrHistogramCustomBucketsZeroCount = errors.New("custom buckets: must have zero count of 0") ErrHistogramCustomBucketsZeroThresh = errors.New("custom buckets: must have zero threshold of 0") ErrHistogramCustomBucketsNegSpans = errors.New("custom buckets: must not have negative spans") diff --git a/promql/engine.go b/promql/engine.go index 6603972c2a..63ad4aed51 100644 --- a/promql/engine.go +++ b/promql/engine.go @@ -2862,11 +2862,14 @@ func (ev *evaluator) VectorBinop(op parser.ItemType, lhs, rhs Vector, matching * fl, fr = fr, fl hl, hr = hr, hl } - floatValue, histogramValue, keep, err := vectorElemBinop(op, fl, fr, hl, hr, pos) + floatValue, histogramValue, keep, info, err := vectorElemBinop(op, fl, fr, hl, hr, pos) if err != nil { lastErr = err continue } + if info != nil { + lastErr = info + } switch { case returnBool: histogramValue = nil @@ -2986,7 +2989,7 @@ func (ev *evaluator) VectorscalarBinop(op parser.ItemType, lhs Vector, rhs Scala lf, rf = rf, lf lh, rh = rh, lh } - float, histogram, keep, err := vectorElemBinop(op, lf, rf, lh, rh, pos) + float, histogram, keep, _, err := vectorElemBinop(op, lf, rf, lh, rh, pos) if err != nil && !errors.Is(err, annotations.PromQLWarning) { lastErr = err continue @@ -3054,91 +3057,98 @@ func scalarBinop(op parser.ItemType, lhs, rhs float64) float64 { } // vectorElemBinop evaluates a binary operation between two Vector elements. -func vectorElemBinop(op parser.ItemType, lhs, rhs float64, hlhs, hrhs *histogram.FloatHistogram, pos posrange.PositionRange) (float64, *histogram.FloatHistogram, bool, error) { +func vectorElemBinop(op parser.ItemType, lhs, rhs float64, hlhs, hrhs *histogram.FloatHistogram, pos posrange.PositionRange) (res float64, resH *histogram.FloatHistogram, keep bool, info, err error) { opName := parser.ItemTypeStr[op] switch { case hlhs == nil && hrhs == nil: { switch op { case parser.ADD: - return lhs + rhs, nil, true, nil + return lhs + rhs, nil, true, nil, nil case parser.SUB: - return lhs - rhs, nil, true, nil + return lhs - rhs, nil, true, nil, nil case parser.MUL: - return lhs * rhs, nil, true, nil + return lhs * rhs, nil, true, nil, nil case parser.DIV: - return lhs / rhs, nil, true, nil + return lhs / rhs, nil, true, nil, nil case parser.POW: - return math.Pow(lhs, rhs), nil, true, nil + return math.Pow(lhs, rhs), nil, true, nil, nil case parser.MOD: - return math.Mod(lhs, rhs), nil, true, nil + return math.Mod(lhs, rhs), nil, true, nil, nil case parser.EQLC: - return lhs, nil, lhs == rhs, nil + return lhs, nil, lhs == rhs, nil, nil case parser.NEQ: - return lhs, nil, lhs != rhs, nil + return lhs, nil, lhs != rhs, nil, nil case parser.GTR: - return lhs, nil, lhs > rhs, nil + return lhs, nil, lhs > rhs, nil, nil case parser.LSS: - return lhs, nil, lhs < rhs, nil + return lhs, nil, lhs < rhs, nil, nil case parser.GTE: - return lhs, nil, lhs >= rhs, nil + return lhs, nil, lhs >= rhs, nil, nil case parser.LTE: - return lhs, nil, lhs <= rhs, nil + return lhs, nil, lhs <= rhs, nil, nil case parser.ATAN2: - return math.Atan2(lhs, rhs), nil, true, nil + return math.Atan2(lhs, rhs), nil, true, nil, nil } } case hlhs == nil && hrhs != nil: { switch op { case parser.MUL: - return 0, hrhs.Copy().Mul(lhs).Compact(0), true, nil + return 0, hrhs.Copy().Mul(lhs).Compact(0), true, nil, nil case parser.ADD, parser.SUB, parser.DIV, parser.POW, parser.MOD, parser.EQLC, parser.NEQ, parser.GTR, parser.LSS, parser.GTE, parser.LTE, parser.ATAN2: - return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("float", opName, "histogram", pos) + return 0, nil, false, nil, annotations.NewIncompatibleTypesInBinOpInfo("float", opName, "histogram", pos) } } case hlhs != nil && hrhs == nil: { switch op { case parser.MUL: - return 0, hlhs.Copy().Mul(rhs).Compact(0), true, nil + return 0, hlhs.Copy().Mul(rhs).Compact(0), true, nil, nil case parser.DIV: - return 0, hlhs.Copy().Div(rhs).Compact(0), true, nil + return 0, hlhs.Copy().Div(rhs).Compact(0), true, nil, nil case parser.ADD, parser.SUB, parser.POW, parser.MOD, parser.EQLC, parser.NEQ, parser.GTR, parser.LSS, parser.GTE, parser.LTE, parser.ATAN2: - return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", opName, "float", pos) + return 0, nil, false, nil, annotations.NewIncompatibleTypesInBinOpInfo("histogram", opName, "float", pos) } } case hlhs != nil && hrhs != nil: { switch op { case parser.ADD: - res, counterResetCollision, err := hlhs.Copy().Add(hrhs) + res, counterResetCollision, nhcbBoundsReconciled, err := hlhs.Copy().Add(hrhs) if err != nil { - return 0, nil, false, err + return 0, nil, false, nil, err } if counterResetCollision { err = annotations.NewHistogramCounterResetCollisionWarning(pos, annotations.HistogramAdd) } - return 0, res.Compact(0), true, err + if nhcbBoundsReconciled { + info = annotations.NewMismatchedCustomBucketsHistogramsInfo(pos, annotations.HistogramAdd) + } + return 0, res.Compact(0), true, info, err case parser.SUB: - res, counterResetCollision, err := hlhs.Copy().Sub(hrhs) + res, counterResetCollision, nhcbBoundsReconciled, err := hlhs.Copy().Sub(hrhs) if err != nil { - return 0, nil, false, err + return 0, nil, false, nil, err } // The result must be marked as gauge. res.CounterResetHint = histogram.GaugeType if counterResetCollision { err = annotations.NewHistogramCounterResetCollisionWarning(pos, annotations.HistogramSub) } - return 0, res.Compact(0), true, err + if nhcbBoundsReconciled { + info = annotations.NewMismatchedCustomBucketsHistogramsInfo(pos, annotations.HistogramSub) + } + + return 0, res.Compact(0), true, info, err case parser.EQLC: // This operation expects that both histograms are compacted. - return 0, hlhs, hlhs.Equals(hrhs), nil + return 0, hlhs, hlhs.Equals(hrhs), nil, nil case parser.NEQ: // This operation expects that both histograms are compacted. - return 0, hlhs, !hlhs.Equals(hrhs), nil + return 0, hlhs, !hlhs.Equals(hrhs), nil, nil case parser.MUL, parser.DIV, parser.POW, parser.MOD, parser.GTR, parser.LSS, parser.GTE, parser.LTE, parser.ATAN2: - return 0, nil, false, annotations.NewIncompatibleTypesInBinOpInfo("histogram", opName, "histogram", pos) + return 0, nil, false, nil, annotations.NewIncompatibleTypesInBinOpInfo("histogram", opName, "histogram", pos) } } } @@ -3157,7 +3167,7 @@ type groupedAggregation struct { seen bool // Was this output groups seen in the input at this timestamp. hasFloat bool // Has at least 1 float64 sample aggregated. hasHistogram bool // Has at least 1 histogram sample aggregated. - incompatibleHistograms bool // If true, group has seen mixed exponential and custom buckets, or incompatible custom buckets. + incompatibleHistograms bool // If true, group has seen mixed exponential and custom buckets. groupAggrComplete bool // Used by LIMITK to short-cut series loop when we've reached K elem on every group. incrementalMean bool // True after reverting to incremental calculation of the mean value. counterResetSeen bool // Counter reset hint CounterReset seen. Currently only used for histogram samples. @@ -3257,11 +3267,14 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix case histogram.NotCounterReset: group.notCounterResetSeen = true } - _, _, err := group.histogramValue.Add(h) + _, _, nhcbBoundsReconciled, err := group.histogramValue.Add(h) if err != nil { handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos) group.incompatibleHistograms = true } + if nhcbBoundsReconciled { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(e.Expr.PositionRange(), annotations.HistogramAgg)) + } } // Otherwise the aggregation contained floats // previously and will be invalid anyway. No @@ -3316,18 +3329,26 @@ func (ev *evaluator) aggregation(e *parser.AggregateExpr, q float64, inputMatrix } left := h.Copy().Div(group.groupCount) right := group.histogramValue.Copy().Div(group.groupCount) - toAdd, _, err := left.Sub(right) + + toAdd, _, nhcbBoundsReconciled, 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) + if nhcbBoundsReconciled { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(e.Expr.PositionRange(), annotations.HistogramAgg)) + } + + _, _, nhcbBoundsReconciled, err = group.histogramValue.Add(toAdd) if err != nil { handleAggregationError(err, e, inputMatrix[si].Metric.Get(model.MetricNameLabel), &annos) group.incompatibleHistograms = true continue } + if nhcbBoundsReconciled { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(e.Expr.PositionRange(), annotations.HistogramAgg)) + } } // Otherwise the aggregation contained floats // previously and will be invalid anyway. No @@ -3809,8 +3830,6 @@ func handleAggregationError(err error, e *parser.AggregateExpr, metricName strin pos := e.Expr.PositionRange() if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { annos.Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) - } else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { - annos.Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, pos)) } } @@ -3825,7 +3844,7 @@ func handleVectorBinopError(err error, e *parser.BinaryExpr) annotations.Annotat return annotations.New().Add(err) } // TODO(NeerajGartia21): Test the exact annotation output once the testing framework can do so. - if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) || errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { + if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return annotations.New().Add(annotations.NewIncompatibleBucketLayoutInBinOpWarning(op, pos)) } return nil diff --git a/promql/engine_internal_test.go b/promql/engine_internal_test.go index 405f0527bf..4c5d532cbc 100644 --- a/promql/engine_internal_test.go +++ b/promql/engine_internal_test.go @@ -87,6 +87,7 @@ func TestVectorElemBinop_Histograms(t *testing.T) { hlhs, hrhs *histogram.FloatHistogram want *histogram.FloatHistogram wantErr error + wantInfo error }{ { name: "ADD with unknown counter reset hints", @@ -128,15 +129,39 @@ func TestVectorElemBinop_Histograms(t *testing.T) { want: &histogram.FloatHistogram{}, wantErr: annotations.HistogramCounterResetCollisionWarning, }, + { + name: "ADD with mismatched NHCB bounds", + op: parser.ADD, + hlhs: &histogram.FloatHistogram{Schema: histogram.CustomBucketsSchema, PositiveBuckets: []float64{}, CustomValues: []float64{1}}, + hrhs: &histogram.FloatHistogram{Schema: histogram.CustomBucketsSchema, PositiveBuckets: []float64{}, CustomValues: []float64{2}}, + want: &histogram.FloatHistogram{Schema: histogram.CustomBucketsSchema, PositiveBuckets: []float64{}}, + wantInfo: annotations.MismatchedCustomBucketsHistogramsInfo, + }, + { + name: "SUB with mismatched NHCB bounds", + op: parser.SUB, + hlhs: &histogram.FloatHistogram{Schema: histogram.CustomBucketsSchema, PositiveBuckets: []float64{}, CustomValues: []float64{1}}, + hrhs: &histogram.FloatHistogram{Schema: histogram.CustomBucketsSchema, PositiveBuckets: []float64{}, CustomValues: []float64{2}}, + want: &histogram.FloatHistogram{Schema: histogram.CustomBucketsSchema, PositiveBuckets: []float64{}, CounterResetHint: histogram.GaugeType}, + wantInfo: annotations.MismatchedCustomBucketsHistogramsInfo, + }, } 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{}) + _, got, _, info, 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) + + if tc.wantInfo != nil { + require.ErrorIs(t, info, tc.wantInfo) + } else { + require.NoError(t, info) + } + require.Equal(t, tc.want, got) }) } diff --git a/promql/functions.go b/promql/functions.go index 35420a8098..2ff25c3acf 100644 --- a/promql/functions.go +++ b/promql/functions.go @@ -368,14 +368,15 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra // This subtraction may deliberately include conflicting counter resets. // Counter resets are treated explicitly in this function, so the // information about conflicting counter resets is ignored here. - _, _, err := h.Sub(prev) + _, _, nhcbBoundsReconciled, err := h.Sub(prev) if err != nil { if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) - } else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { - return nil, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, pos)) } } + if nhcbBoundsReconciled { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(pos, annotations.HistogramSub)) + } if isCounter { // Second iteration to deal with counter resets. @@ -383,14 +384,15 @@ func histogramRate(points []HPoint, isCounter bool, metricName string, pos posra curr := currPoint.H if curr.DetectReset(prev) { // Counter reset conflict ignored here for the same reason as above. - _, _, err := h.Add(prev) + _, _, nhcbBoundsReconciled, err := h.Add(prev) if err != nil { if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return nil, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, pos)) - } else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { - return nil, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, pos)) } } + if nhcbBoundsReconciled { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(pos, annotations.HistogramAdd)) + } } prev = curr } @@ -508,11 +510,12 @@ func instantValue(vals Matrix, args parser.Expressions, out Vector, isRate bool) // counter resets. Counter resets are treated explicitly // in this function, so the information about // conflicting counter resets is ignored here. - _, _, err := resultSample.H.Sub(ss[0].H) + _, _, nhcbBoundsReconciled, 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 nhcbBoundsReconciled { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(args.PositionRange(), annotations.HistogramSub)) } } resultSample.H.CounterResetHint = histogram.GaugeType @@ -820,7 +823,7 @@ func funcAvgOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh // The passed values only contain histograms. var annos annotations.Annotations vec, err := aggrHistOverTime(matrixVal, enh, func(s Series) (*histogram.FloatHistogram, error) { - var counterResetSeen, notCounterResetSeen bool + var counterResetSeen, notCounterResetSeen, nhcbBoundsReconciledSeen bool trackCounterReset := func(h *histogram.FloatHistogram) { switch h.CounterResetHint { @@ -835,6 +838,9 @@ func funcAvgOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh if counterResetSeen && notCounterResetSeen { annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args[0].PositionRange(), annotations.HistogramAgg)) } + if nhcbBoundsReconciledSeen { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(args[0].PositionRange(), annotations.HistogramAgg)) + } }() mean := s.Histograms[0].H.Copy() @@ -844,14 +850,22 @@ func funcAvgOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh count := float64(i + 2) left := h.H.Copy().Div(count) right := mean.Copy().Div(count) - toAdd, _, err := left.Sub(right) + + toAdd, _, nhcbBoundsReconciled, err := left.Sub(right) if err != nil { return mean, err } - _, _, err = mean.Add(toAdd) + if nhcbBoundsReconciled { + nhcbBoundsReconciledSeen = true + } + + _, _, nhcbBoundsReconciled, err = mean.Add(toAdd) if err != nil { return mean, err } + if nhcbBoundsReconciled { + nhcbBoundsReconciledSeen = true + } } return mean, nil }) @@ -859,8 +873,6 @@ func funcAvgOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh metricName := firstSeries.Metric.Get(labels.MetricName) if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return enh.Out, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, args[0].PositionRange())) - } else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { - return enh.Out, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, args[0].PositionRange())) } } return vec, annos @@ -1090,7 +1102,7 @@ func funcSumOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh // The passed values only contain histograms. var annos annotations.Annotations vec, err := aggrHistOverTime(matrixVal, enh, func(s Series) (*histogram.FloatHistogram, error) { - var counterResetSeen, notCounterResetSeen bool + var counterResetSeen, notCounterResetSeen, nhcbBoundsReconciledSeen bool trackCounterReset := func(h *histogram.FloatHistogram) { switch h.CounterResetHint { @@ -1105,16 +1117,22 @@ func funcSumOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh if counterResetSeen && notCounterResetSeen { annos.Add(annotations.NewHistogramCounterResetCollisionWarning(args[0].PositionRange(), annotations.HistogramAgg)) } + if nhcbBoundsReconciledSeen { + annos.Add(annotations.NewMismatchedCustomBucketsHistogramsInfo(args[0].PositionRange(), annotations.HistogramAgg)) + } }() sum := s.Histograms[0].H.Copy() trackCounterReset(sum) for _, h := range s.Histograms[1:] { trackCounterReset(h.H) - _, _, err := sum.Add(h.H) + _, _, nhcbBoundsReconciled, err := sum.Add(h.H) if err != nil { return sum, err } + if nhcbBoundsReconciled { + nhcbBoundsReconciledSeen = true + } } return sum, nil }) @@ -1122,8 +1140,6 @@ func funcSumOverTime(_ []Vector, matrixVal Matrix, args parser.Expressions, enh metricName := firstSeries.Metric.Get(labels.MetricName) if errors.Is(err, histogram.ErrHistogramsIncompatibleSchema) { return enh.Out, annotations.New().Add(annotations.NewMixedExponentialCustomHistogramsWarning(metricName, args[0].PositionRange())) - } else if errors.Is(err, histogram.ErrHistogramsIncompatibleBounds) { - return enh.Out, annotations.New().Add(annotations.NewIncompatibleCustomBucketsHistogramsWarning(metricName, args[0].PositionRange())) } } return vec, annos diff --git a/promql/parser/parse.go b/promql/parser/parse.go index d7f3b93994..2a02fd4d53 100644 --- a/promql/parser/parse.go +++ b/promql/parser/parse.go @@ -497,14 +497,14 @@ func (p *parser) mergeMaps(left, right *map[string]any) (ret *map[string]any) { 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) { - res, _, err := 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) { - res, _, err := a.Sub(b) + res, _, _, err := a.Sub(b) return res, err }) } diff --git a/promql/promqltest/testdata/functions.test b/promql/promqltest/testdata/functions.test index 8271806a47..ba3df76ff6 100644 --- a/promql/promqltest/testdata/functions.test +++ b/promql/promqltest/testdata/functions.test @@ -270,7 +270,7 @@ eval instant at 20m irate(http_requests_histogram{path="/f"}[20m]) eval instant at 20m irate(http_requests_histogram{path="/g"}[20m]) expect no_warn - {path="/g"} {{schema:-53 sum:0.01 count:0.01 custom_values:[5 10] buckets:[0.01]}} + {path="/g"} {{schema:-53 counter_reset_hint:gauge}} clear @@ -314,7 +314,7 @@ load 5m http_requests_histogram{path="/d"} 0 0 {{sum:1 count:1 counter_reset_hint:gauge}} {{sum:2 count:2}} http_requests_histogram{path="/e"} 0 1 2 {{sum:1 count:2 counter_reset_hint:gauge}} http_requests_histogram{path="/f"} 0 0 {{sum:1 count:1 counter_reset_hint:gauge}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1] counter_reset_hint:gauge}} - http_requests_histogram{path="/g"} 0 0 {{schema:-53 sum:1 count:1 custom_values:[1] buckets:[2] counter_reset_hint:gauge}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1] counter_reset_hint:gauge}} + http_requests_histogram{path="/g"} 0 0 {{schema:-53 sum:1 count:1 custom_values:[1] buckets:[2] counter_reset_hint:gauge}} {{schema:-53 sum:1 count:1 custom_values:[1 10] buckets:[1] counter_reset_hint:gauge}} eval instant at 20m idelta(http_requests[20m]) expect no_warn @@ -351,7 +351,9 @@ eval instant at 20m idelta(http_requests_histogram{path="/f"}[20m]) expect warn msg: PromQL warning: vector contains a mix of histograms with exponential and custom buckets schemas for metric name "http_requests_histogram" eval instant at 20m idelta(http_requests_histogram{path="/g"}[20m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "http_requests_histogram" + expect no_warn + expect info msg: PromQL info: mismatched custom buckets were reconciled during subtraction + {path="/g"} {{schema:-53 custom_values:[1] counter_reset_hint:gauge buckets:[-1]}} clear diff --git a/promql/promqltest/testdata/native_histograms.test b/promql/promqltest/testdata/native_histograms.test index 986b8de04f..a0268a90f0 100644 --- a/promql/promqltest/testdata/native_histograms.test +++ b/promql/promqltest/testdata/native_histograms.test @@ -1198,24 +1198,21 @@ eval range from 0 to 12m step 6m avg(metric) clear -# Test incompatible custom bucket boundaries. +# Test mismatched custom bucket boundaries. load 6m metric{series="1"} _ {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} - metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} _ {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} - metric{series="3"} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} + metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[10] buckets:[1]}} _ {{schema:-53 sum:1 count:1 custom_values:[5] buckets:[1]}} + metric{series="3"} {{schema:-53 sum:1 count:1 custom_values:[2 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} -# T=0: incompatible, should be ignored and emit a warning -# T=6: compatible -# T=12: incompatible followed by compatible, should be ignored and emit a warning eval range from 0 to 12m step 6m sum(metric) - expect warn - {} _ {{schema:-53 sum:2 count:2 custom_values:[5 10] buckets:[2]}} _ + expect no_warn + {} {{schema:-53 count:2 sum:2 custom_values:[10] buckets:[2]}} {{schema:-53 sum:2 count:2 custom_values:[5 10] buckets:[2]}} {{schema:-53 count:3 sum:3 custom_values:[5] buckets:[3]}} eval range from 0 to 12m step 6m avg(metric) - expect warn - {} _ {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} _ + expect no_warn + {} {{schema:-53 count:1 sum:1 custom_values:[10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 count:1 sum:1 custom_values:[5] buckets:[1]}} -# Test incompatible boundaries with additional aggregation operators +# Test mismatched boundaries with additional aggregation operators eval range from 0 to 12m step 6m count(metric) {} 2 2 3 @@ -1227,32 +1224,34 @@ eval range from 0 to 12m step 6m count(limitk(1, metric)) eval range from 0 to 12m step 6m limitk(3, metric) metric{series="1"} _ {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} - metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} _ {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} - metric{series="3"} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} + metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[10] buckets:[1]}} _ {{schema:-53 sum:1 count:1 custom_values:[5] buckets:[1]}} + metric{series="3"} {{schema:-53 sum:1 count:1 custom_values:[2 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} eval range from 0 to 12m step 6m limit_ratio(1, metric) metric{series="1"} _ {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} - metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} _ {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} - metric{series="3"} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} + metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[10] buckets:[1]}} _ {{schema:-53 sum:1 count:1 custom_values:[5] buckets:[1]}} + metric{series="3"} {{schema:-53 sum:1 count:1 custom_values:[2 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} -# Test incompatible boundaries with and/or +# Test mismatched schemas with and/or eval range from 0 to 12m step 6m metric{series="1"} and ignoring(series) metric{series="2"} metric{series="1"} _ _ {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} eval range from 0 to 12m step 6m metric{series="1"} or ignoring(series) metric{series="2"} metric{series="1"} _ {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} - metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} _ _ + metric{series="2"} {{schema:-53 sum:1 count:1 custom_values:[10] buckets:[1]}} _ _ -# Test incompatible boundaries with arithmetic binary operators +# Test mismatched boundaries with arithmetic binary operators eval range from 0 to 12m step 6m metric{series="2"} + ignoring (series) metric{series="3"} - expect warn + expect info msg:PromQL info: mismatched custom buckets were reconciled during addition + {} {{schema:-53 count:2 sum:2 custom_values:[10] buckets:[2]}} _ {{schema:-53 count:2 sum:2 custom_values:[5] buckets:[2]}} eval range from 0 to 12m step 6m metric{series="2"} - ignoring (series) metric{series="3"} - expect warn + expect info msg:PromQL info: mismatched custom buckets were reconciled during subtraction + {} {{schema:-53 custom_values:[10] counter_reset_hint:gauge}} _ {{schema:-53 custom_values:[5] counter_reset_hint:gauge}} clear -# Test incompatible boundaries with comparison binary operators +# Test mismatched boundaries with comparison binary operators load 6m metric1 {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} metric2 {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} @@ -1271,16 +1270,20 @@ eval range from 0 to 6m step 6m metric2 > metric2 clear load 6m - nhcb_metric {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} + nhcb_metric {{schema:-53 sum:1 count:1 custom_values:[5] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} # If evaluating at 12m, the first two NHCBs have the same custom values # while the 3rd one has different ones. eval instant at 12m sum_over_time(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect no_warn + expect info msg: PromQL info: mismatched custom buckets were reconciled during aggregation + {} {{schema:-53 count:3 sum:3 custom_values:[5] buckets:[3]}} eval instant at 12m avg_over_time(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect no_warn + expect info msg: PromQL info: mismatched custom buckets were reconciled during aggregation + {} {{schema:-53 count:1 sum:1 custom_values:[5] counter_reset_hint:gauge buckets:[1]}} eval instant at 12m last_over_time(nhcb_metric[13m]) expect no_warn @@ -1299,28 +1302,31 @@ eval instant at 12m changes(nhcb_metric[13m]) {} 1 eval instant at 12m delta(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect warn msg: PromQL warning: this native histogram metric is not a gauge: "nhcb_metric" + {} {{schema:-53 custom_values:[5]}} eval instant at 12m increase(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect no_warn + {} {{schema:-53 custom_values:[5]}} eval instant at 12m rate(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect no_warn + {} {{schema:-53 custom_values:[5] }} eval instant at 12m resets(nhcb_metric[13m]) expect no_warn - {} 1 + {} 0 # Now doing the same again, but at 18m, where the first NHCB has -# different custom_values compared to the other two. This now -# works with no warning for increase() and rate(). No change -# otherwise. +# different custom_values compared to the other two. eval instant at 18m sum_over_time(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect no_warn + {} {{schema:-53 count:3 sum:3 custom_values:[5] buckets:[3]}} eval instant at 18m avg_over_time(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect no_warn + {} {{schema:-53 count:1 sum:1 custom_values:[5] buckets:[1]}} eval instant at 18m last_over_time(nhcb_metric[13m]) expect no_warn @@ -1339,19 +1345,21 @@ eval instant at 18m changes(nhcb_metric[13m]) {} 1 eval instant at 18m delta(nhcb_metric[13m]) - expect warn msg: PromQL warning: vector contains histograms with incompatible custom buckets for metric name "nhcb_metric" + expect warn msg: PromQL warning: this native histogram metric is not a gauge: "nhcb_metric" + expect info msg: PromQL info: mismatched custom buckets were reconciled during subtraction + {} {{schema:-53 custom_values:[5]}} eval instant at 18m increase(nhcb_metric[13m]) expect no_warn - {} {{schema:-53 count:1.0833333333333333 sum:1.0833333333333333 custom_values:[5 10] buckets:[1.0833333333333333]}} + {} {{schema:-53 custom_values:[5]}} eval instant at 18m rate(nhcb_metric[13m]) expect no_warn - {} {{schema:-53 count:0.0013888888888888887 sum:0.0013888888888888887 custom_values:[5 10] buckets:[0.0013888888888888887]}} + {} {{schema:-53 custom_values:[5]}} eval instant at 18m resets(nhcb_metric[13m]) expect no_warn - {} 1 + {} 0 clear @@ -1366,14 +1374,15 @@ load 1m metric{group="floats-and-histograms", series="2"} {{sum:2 count:3 buckets:[1 1 1]}} metric{group="exponential-and-custom-histograms", series="1"} {{sum:2 count:3 buckets:[1 1 1]}} metric{group="exponential-and-custom-histograms", series="2"} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} - metric{group="incompatible-custom-histograms", series="1"} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} - metric{group="incompatible-custom-histograms", series="2"} {{schema:-53 sum:1 count:1 custom_values:[2] buckets:[1]}} + metric{group="mismatched-custom-histograms", series="1"} {{schema:-53 sum:1 count:1 custom_values:[5 10] buckets:[1]}} + metric{group="mismatched-custom-histograms", series="2"} {{schema:-53 sum:1 count:1 custom_values:[10] buckets:[1]}} eval instant at 0 sum by (group) (metric) expect warn {group="just-floats"} 5 {group="just-exponential-histograms"} {{sum:5 count:7 buckets:[2 3 2]}} {group="just-custom-histograms"} {{schema:-53 sum:4 count:5 custom_values:[2] buckets:[8]}} + {group="mismatched-custom-histograms"} {{schema:-53 count:2 sum:2 custom_values:[10] buckets:[2]}} clear @@ -1833,3 +1842,33 @@ eval range from 0 to 8m step 1m mixed_metric1 eval range from 0 to 5m step 1m mixed_metric2 mixed_metric2 1 2 3 {{count:4 sum:5 buckets:[1 2 1]}} {{count:6 sum:8 buckets:[1 4 1]}} {{count:6 sum:8 buckets:[1 4 1]}} + +clear + +# Test native histograms with custom buckets, reconciling mismatched bounds. +load 1m + nhcb_add_buckets {{schema:-53 sum:55 count:15 custom_values:[2 4 6] buckets:[1 2 5 7]}} {{schema:-53 sum:555 count:450 custom_values:[1 2 3 4 5 6 7 8] buckets:[10 20 30 40 50 60 70 80 90]}} + +eval instant at 1m irate(nhcb_add_buckets[2m]) * 60 + expect no_warn + expect info msg: PromQL info: mismatched custom buckets were reconciled during subtraction + {} {{schema:-53 sum:500 count:435 custom_values:[2 4 6] buckets:[29 68 105 233]}} + +load 1m + nhcb_remove_buckets {{schema:-53 sum:55 count:45 custom_values:[1 2 3 4 5 6 7 8] buckets:[1 2 3 4 5 6 7 8 9]}} {{schema:-53 sum:5560 count:1000 custom_values:[3 5 7] buckets:[100 200 300 400]}} + +eval instant at 1m irate(nhcb_remove_buckets[2m]) * 60 + expect no_warn + expect info msg: PromQL info: mismatched custom buckets were reconciled during subtraction + {} {{schema:-53 sum:5505 count:955 custom_values:[3 5 7] buckets:[94 191 287 383]}} + +clear + +# Test native histograms with custom buckets, reconciling mismatched bounds, with counter reset in one bucket. +load 1m + nhcb_add_bucket {{schema:-53 sum:55 count:15 custom_values:[2 4 6] buckets:[1 2 5 7]}} {{schema:-53 sum:56 count:15 custom_values:[2 3 4 6] buckets:[1 0 1 5 8]}} + +eval instant at 1m irate(nhcb_add_bucket[2m]) * 60 + expect no_warn + expect no_info + {} {{schema:-53 sum:56 count:15 custom_values:[2 3 4 6] buckets:[1 0 1 5 8] counter_reset_hint:gauge}} diff --git a/rules/manager_test.go b/rules/manager_test.go index 76d802f336..856c4a6c17 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -1508,7 +1508,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 bbd3b0927f..6d6427e371 100644 --- a/util/annotations/annotations.go +++ b/util/annotations/annotations.go @@ -134,16 +134,15 @@ var ( PromQLInfo = errors.New("PromQL info") PromQLWarning = errors.New("PromQL warning") - InvalidRatioWarning = fmt.Errorf("%w: ratio value should be between -1 and 1", PromQLWarning) - InvalidQuantileWarning = fmt.Errorf("%w: quantile value should be between 0 and 1", PromQLWarning) - BadBucketLabelWarning = fmt.Errorf("%w: bucket label %q is missing or has a malformed value", PromQLWarning, model.BucketLabel) - MixedFloatsHistogramsWarning = fmt.Errorf("%w: encountered a mix of histograms and floats for", PromQLWarning) - MixedClassicNativeHistogramsWarning = fmt.Errorf("%w: vector contains a mix of classic and native histograms", PromQLWarning) - NativeHistogramNotCounterWarning = fmt.Errorf("%w: this native histogram metric is not a counter:", PromQLWarning) - NativeHistogramNotGaugeWarning = fmt.Errorf("%w: this native histogram metric is not a gauge:", PromQLWarning) - MixedExponentialCustomHistogramsWarning = fmt.Errorf("%w: vector contains a mix of histograms with exponential and custom buckets schemas for metric name", PromQLWarning) - IncompatibleCustomBucketsHistogramsWarning = fmt.Errorf("%w: vector contains histograms with incompatible custom buckets for metric name", PromQLWarning) - IncompatibleBucketLayoutInBinOpWarning = fmt.Errorf("%w: incompatible bucket layout encountered for binary operator", PromQLWarning) + InvalidRatioWarning = fmt.Errorf("%w: ratio value should be between -1 and 1", PromQLWarning) + InvalidQuantileWarning = fmt.Errorf("%w: quantile value should be between 0 and 1", PromQLWarning) + BadBucketLabelWarning = fmt.Errorf("%w: bucket label %q is missing or has a malformed value", PromQLWarning, model.BucketLabel) + MixedFloatsHistogramsWarning = fmt.Errorf("%w: encountered a mix of histograms and floats for", PromQLWarning) + MixedClassicNativeHistogramsWarning = fmt.Errorf("%w: vector contains a mix of classic and native histograms", PromQLWarning) + NativeHistogramNotCounterWarning = fmt.Errorf("%w: this native histogram metric is not a counter:", PromQLWarning) + NativeHistogramNotGaugeWarning = fmt.Errorf("%w: this native histogram metric is not a gauge:", PromQLWarning) + MixedExponentialCustomHistogramsWarning = fmt.Errorf("%w: vector contains a mix of histograms with exponential and custom buckets schemas for metric name", PromQLWarning) + IncompatibleBucketLayoutInBinOpWarning = fmt.Errorf("%w: incompatible bucket layout encountered for binary operator", PromQLWarning) PossibleNonCounterInfo = fmt.Errorf("%w: metric might not be a counter, name does not end in _total/_sum/_count/_bucket:", PromQLInfo) PossibleNonCounterLabelInfo = fmt.Errorf("%w: metric might not be a counter, __type__ label is not set to %q or %q", PromQLInfo, model.MetricTypeCounter, model.MetricTypeHistogram) @@ -155,6 +154,7 @@ var ( 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 counter resets during histogram", PromQLWarning) + MismatchedCustomBucketsHistogramsInfo = fmt.Errorf("%w: mismatched custom buckets were reconciled during", PromQLInfo) ) type annoErr struct { @@ -264,15 +264,6 @@ func NewMixedExponentialCustomHistogramsWarning(metricName string, pos posrange. } } -// NewIncompatibleCustomBucketsHistogramsWarning is used when the queried series includes -// custom buckets histograms with incompatible custom bounds. -func NewIncompatibleCustomBucketsHistogramsWarning(metricName string, pos posrange.PositionRange) error { - return annoErr{ - PositionRange: pos, - Err: fmt.Errorf("%w %q", IncompatibleCustomBucketsHistogramsWarning, metricName), - } -} - // NewPossibleNonCounterInfo is used when a named counter metric with only float samples does not // have the suffixes _total, _sum, _count, or _bucket. func NewPossibleNonCounterInfo(metricName string, pos posrange.PositionRange) error { @@ -365,16 +356,29 @@ const ( HistogramAgg HistogramOperation = "aggregation" ) +func (op HistogramOperation) String() string { + switch op { + case HistogramAdd, HistogramSub, HistogramAgg: + return string(op) + default: + return "unknown operation" + } +} + // 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, operation HistogramOperation) error { - switch operation { - case HistogramAdd, HistogramSub, HistogramAgg: - default: - operation = "unknown operation" - } return annoErr{ PositionRange: pos, - Err: fmt.Errorf("%w %s", HistogramCounterResetCollisionWarning, operation), + Err: fmt.Errorf("%w %s", HistogramCounterResetCollisionWarning, operation.String()), + } +} + +// NewMismatchedCustomBucketsHistogramsInfo is used when the queried series includes +// custom buckets histograms with mismatched custom bounds that cause reconciling. +func NewMismatchedCustomBucketsHistogramsInfo(pos posrange.PositionRange, operation HistogramOperation) error { + return annoErr{ + PositionRange: pos, + Err: fmt.Errorf("%w %s", MismatchedCustomBucketsHistogramsInfo, operation.String()), } }