From f14c515cbee2213f1cc1df83bda4368202ecc2f9 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Tue, 28 Oct 2025 20:29:44 +0530 Subject: [PATCH] fix(histogram): handling +Inf bucket count and metric label Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 62 ++++++++++++--------------------- model/histogram/convert_test.go | 16 ++++++--- 2 files changed, 34 insertions(+), 44 deletions(-) diff --git a/model/histogram/convert.go b/model/histogram/convert.go index d43b8182d6..218fbe197e 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -18,14 +18,18 @@ import ( "fmt" "math" + "github.com/prometheus/common/model" + "github.com/prometheus/prometheus/model/labels" ) // ConvertNHCBToClassic converts Native Histogram Custom Buckets (NHCB) to classic histogram series. // This conversion is needed in various scenarios where users need to get NHCB back to classic histogram format, // such as Remote Write v1 for external system compatibility and migration use cases. +// +// When calling this function, caller must ensure that provided nhcb is valid NHCB histogram. func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Builder, emitSeriesFn func(labels labels.Labels, value float64) error) error { - baseName := lset.Get("__name__") + baseName := lset.Get(model.MetricNameLabel) if baseName == "" { return errors.New("metric name label '__name__' is missing") } @@ -50,21 +54,14 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil return errors.New("unsupported histogram schema, not a NHCB") } - filledBuckets := 0 - totalBuckets := 0 - for _, span := range h.PositiveSpans { - filledBuckets += int(span.Length) - totalBuckets += int(span.Offset) + int(span.Length) - } - if filledBuckets != len(h.PositiveBuckets) { - return errors.New("mismatched lengths of positive buckets and spans") - } - if totalBuckets > len(h.CustomValues) { - return errors.New("mismatched lengths of custom values and buckets from span calculation") + // Validate the histogram before conversion. + // The caller must ensure that the provided histogram is valid NHCB. + if h.Validate() != nil { + return errors.New(h.Validate().Error()) } customValues = h.CustomValues - positiveBuckets = make([]float64, len(customValues)) + positiveBuckets = make([]float64, len(customValues)+1) // Histograms are in delta format so we first bring them to absolute format. acc := int64(0) @@ -87,21 +84,13 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil return errors.New("unsupported histogram schema, not a NHCB") } - filledBuckets := 0 - totalBuckets := 0 - for _, s := range h.PositiveSpans { - filledBuckets += int(s.Length) - totalBuckets += int(s.Offset) + int(s.Length) + // Validate the histogram before conversion. + // The caller must ensure that the provided histogram is valid NHCB. + if h.Validate() != nil { + return errors.New(h.Validate().Error()) } - if filledBuckets != len(h.PositiveBuckets) { - return errors.New("mismatched lengths of positive buckets and spans") - } - if totalBuckets > len(h.CustomValues) { - return errors.New("mismatched lengths of custom values and buckets from span calculation") - } - customValues = h.CustomValues - positiveBuckets = make([]float64, len(customValues)) + positiveBuckets = make([]float64, len(customValues)+1) for _, span := range h.PositiveSpans { // Since Float Histogram is already in absolute format we should @@ -120,39 +109,34 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil return fmt.Errorf("unsupported histogram type: %T", h) } - // Each customValue corresponds to a positive bucket (aligned with the "le" label). - // The lengths of customValues and positiveBuckets must match to avoid inconsistencies - // while mapping bucket counts to their upper bounds. - if len(customValues) != len(positiveBuckets) { - return errors.New("mismatched lengths of custom values and positive buckets") - } - currCount := float64(0) for i, val := range customValues { currCount += positiveBuckets[i] lsetBuilder.Reset(lset) - lsetBuilder.Set("__name__", baseName+"_bucket") - lsetBuilder.Set("le", labels.FormatOpenMetricsFloat(val)) + lsetBuilder.Set(model.MetricNameLabel, baseName+"_bucket") + lsetBuilder.Set(model.BucketLabel, labels.FormatOpenMetricsFloat(val)) if err := emitSeriesFn(lsetBuilder.Labels(), currCount); err != nil { return err } } + currCount += positiveBuckets[len(positiveBuckets)-1] + lsetBuilder.Reset(lset) - lsetBuilder.Set("__name__", baseName+"_bucket") - lsetBuilder.Set("le", labels.FormatOpenMetricsFloat(math.Inf(1))) + lsetBuilder.Set(model.MetricNameLabel, baseName+"_bucket") + lsetBuilder.Set(model.BucketLabel, labels.FormatOpenMetricsFloat(math.Inf(1))) if err := emitSeriesFn(lsetBuilder.Labels(), currCount); err != nil { return err } lsetBuilder.Reset(lset) - lsetBuilder.Set("__name__", baseName+"_count") + lsetBuilder.Set(model.MetricNameLabel, baseName+"_count") if err := emitSeriesFn(lsetBuilder.Labels(), count); err != nil { return err } lsetBuilder.Reset(lset) - lsetBuilder.Set("__name__", baseName+"_sum") + lsetBuilder.Set(model.MetricNameLabel, baseName+"_sum") if err := emitSeriesFn(lsetBuilder.Labels(), sum); err != nil { return err } diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go index 16c50d6263..af7777e0b4 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -136,7 +136,7 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { }, }, { - name: "mismatched bucket lengths with more filled bucket count", + name: "extra bucket counts than custom values", nhcb: &Histogram{ CustomValues: []float64{1, 2}, PositiveBuckets: []int64{10, 20, 30}, @@ -145,8 +145,14 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { Sum: 100.0, Schema: CustomBucketsSchema, }, - labels: labels.FromStrings("__name__", "test_metric_bucket"), - expectErr: true, + labels: labels.FromStrings("__name__", "test_metric"), + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 40}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 100}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 100}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 100}, + }, }, { name: "mismatched bucket lengths with less filled bucket count", @@ -234,7 +240,7 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { {1, 2}, }, PositiveBuckets: []int64{1, 2, 3, 4, 5}, // 1 -> 3 -> 3 -> 3 -> 3 -> 3 -> 6 ->6 ->10 -> 15 - Count: 53, // 1 -> 4 -> 7 -> 10 -> 13 -> 16 -> 22 -> 28 -> 38 -> 53 + Count: 35, // 1 -> 4 -> 7 -> 10 -> 13 -> 16 -> 22 -> 28 -> 38 -> 53 Sum: 123, }, labels: labels.FromStrings("__name__", "test_metric"), @@ -250,7 +256,7 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "9.0"), val: 38}, {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "10.0"), val: 53}, {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 53}, - {lset: labels.FromStrings("__name__", "test_metric_count"), val: 53}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 35}, {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 123}, }, },