From c8e3f8c97ac7213ceb6f8385f7c4f117934bdce5 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Wed, 17 Sep 2025 15:32:16 +0530 Subject: [PATCH 01/11] drop(flag): moving feature flag to other pr Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 98 +++++++++++++++++ model/histogram/convert_test.go | 184 ++++++++++++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 model/histogram/convert.go create mode 100644 model/histogram/convert_test.go diff --git a/model/histogram/convert.go b/model/histogram/convert.go new file mode 100644 index 0000000000..deab25db2e --- /dev/null +++ b/model/histogram/convert.go @@ -0,0 +1,98 @@ +// Copyright 2025 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package histogram + +import ( + "errors" + "fmt" + "math" + + "github.com/prometheus/prometheus/model/labels" +) + +type BucketEmitter func(labels labels.Labels, value float64) error + +func ConvertNHCBToClassicHistogram(nhcb interface{}, labels labels.Labels, lblBuilder *labels.Builder, bucketSeries BucketEmitter) error { + baseName := labels.Get("__name__") + if baseName == "" { + return errors.New("metric name label '__name__' is missing") + } + + oldLabels := lblBuilder.Labels() + defer lblBuilder.Reset(oldLabels) + + var ( + customValues []float64 + positiveBuckets []float64 + count, sum float64 + ) + + switch h := nhcb.(type) { + case *Histogram: + customValues = h.CustomValues + positiveBuckets = make([]float64, len(h.PositiveBuckets)) + for i, v := range h.PositiveBuckets { + positiveBuckets[i] = float64(v) + } + count = float64(h.Count) + sum = h.Sum + case *FloatHistogram: + customValues = h.CustomValues + positiveBuckets = h.PositiveBuckets + count = h.Count + sum = h.Sum + default: + return errors.New("unsupported histogram type") + } + + if len(customValues) != len(positiveBuckets) { + return errors.New("mismatched lengths of custom values and positive buckets") + } + + currCount := float64(0) + for i := range customValues { + currCount += positiveBuckets[i] + lblBuilder.Reset(labels) + lblBuilder.Set("__name__", baseName+"_bucket") + lblBuilder.Set("le", fmt.Sprintf("%g", customValues[i])) + bucketLabels := lblBuilder.Labels() + if err := bucketSeries(bucketLabels, currCount); err != nil { + return err + } + } + + lblBuilder.Reset(labels) + lblBuilder.Set("__name__", baseName+"_bucket") + lblBuilder.Set("le", fmt.Sprintf("%g", math.Inf(1))) + infBucketLabels := lblBuilder.Labels() + if err := bucketSeries(infBucketLabels, currCount); err != nil { + return err + } + + lblBuilder.Reset(labels) + lblBuilder.Set("__name__", baseName+"_count") + countLabels := lblBuilder.Labels() + if err := bucketSeries(countLabels, count); err != nil { + return err + } + + lblBuilder.Reset(labels) + lblBuilder.Set("__name__", baseName+"_sum") + sumLabels := lblBuilder.Labels() + if err := bucketSeries(sumLabels, sum); err != nil { + return err + } + + return nil +} diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go new file mode 100644 index 0000000000..4c7787c884 --- /dev/null +++ b/model/histogram/convert_test.go @@ -0,0 +1,184 @@ +// Copyright 2025 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package histogram + +import ( + "testing" + + "github.com/prometheus/prometheus/model/labels" +) + +func TestConvertNHCBToClassicHistogram(t *testing.T) { + tests := []struct { + name string + nhcb interface{} + labels labels.Labels + expectErr bool + expectedLabels []labels.Labels + expectedValues []float64 + }{ + { + name: "Valid Histogram", + nhcb: &Histogram{ + CustomValues: []float64{1, 2, 3}, + PositiveBuckets: []int64{10, 20, 30}, + Count: 60, + Sum: 100.0, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expectedLabels: []labels.Labels{ + labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "2"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "3"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), + labels.FromStrings("__name__", "test_metric_count"), + labels.FromStrings("__name__", "test_metric_sum"), + }, + expectedValues: []float64{10, 30, 60, 60, 60, 100}, + }, + { + name: "Valid FloatHistogram", + nhcb: &FloatHistogram{ + CustomValues: []float64{1, 2, 3}, + PositiveBuckets: []float64{20.0, 40.0, 60.0}, + Count: 120.0, + Sum: 100.0, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expectedLabels: []labels.Labels{ + labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "2"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "3"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), + labels.FromStrings("__name__", "test_metric_count"), + labels.FromStrings("__name__", "test_metric_sum"), + }, + expectedValues: []float64{20, 60, 120, 120, 120, 100}, + }, + { + name: "Empty Histogram", + nhcb: &Histogram{ + CustomValues: []float64{}, + PositiveBuckets: []int64{}, + Count: 0, + Sum: 0.0, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expectedLabels: []labels.Labels{ + labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), + labels.FromStrings("__name__", "test_metric_count"), + labels.FromStrings("__name__", "test_metric_sum"), + }, + expectedValues: []float64{0, 0, 0}, + }, + { + name: "Missing __name__ label", + nhcb: &Histogram{ + CustomValues: []float64{1, 2, 3}, + PositiveBuckets: []int64{10, 20, 30}, + Count: 60, + Sum: 100.0, + }, + labels: labels.FromStrings("job", "test_job"), + expectErr: true, + }, + { + name: "Unsupported histogram type", + nhcb: nil, + labels: labels.FromStrings("__name__", "test_metric"), + expectErr: true, + }, + { + name: "Histogram with zero bucket counts", + nhcb: &Histogram{ + CustomValues: []float64{1, 2, 3}, + PositiveBuckets: []int64{0, 10, 0}, + Count: 10, + Sum: 50.0, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expectedLabels: []labels.Labels{ + labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "2"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "3"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), + labels.FromStrings("__name__", "test_metric_count"), + labels.FromStrings("__name__", "test_metric_sum"), + }, + expectedValues: []float64{0, 10, 10, 10, 10, 50}, + }, + { + name: "Mismatched bucket lengths", + nhcb: &Histogram{ + CustomValues: []float64{1, 2}, + PositiveBuckets: []int64{10, 20, 30}, + Count: 60, + Sum: 100.0, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expectErr: true, + }, + { + name: "single series Histogram", + nhcb: &Histogram{ + CustomValues: []float64{1}, + PositiveBuckets: []int64{10}, + Count: 10, + Sum: 20.0, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expectedLabels: []labels.Labels{ + labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), + labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), + labels.FromStrings("__name__", "test_metric_count"), + labels.FromStrings("__name__", "test_metric_sum"), + }, + expectedValues: []float64{10, 10, 10, 20}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var actualLabels []labels.Labels + var actualValues []float64 + + err := ConvertNHCBToClassicHistogram(tt.nhcb, tt.labels, &labels.Builder{}, func(lbls labels.Labels, value float64) error { + actualLabels = append(actualLabels, lbls) + actualValues = append(actualValues, value) + return nil + }) + + if (err != nil) != tt.expectErr { + t.Errorf("ConvertNHCBToClassicHistogram() error = %v, expectErr %v", err, tt.expectErr) + return + } + + if !tt.expectErr { + if len(actualLabels) != len(tt.expectedLabels) { + t.Errorf("Expected %d emissions, got %d", len(tt.expectedLabels), len(actualLabels)) + return + } + + for i, expectedLabel := range tt.expectedLabels { + if !labels.Equal(actualLabels[i], expectedLabel) { + t.Errorf("Expected label[%d] = %v, got %v", i, expectedLabel, actualLabels[i]) + } + if actualValues[i] != tt.expectedValues[i] { + t.Errorf("Expected value[%d] = %f, got %f", i, tt.expectedValues[i], actualValues[i]) + } + } + } + }) + } +} From 5eeba3638da01b50c1086655853a41141cb24212 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Wed, 17 Sep 2025 15:48:57 +0530 Subject: [PATCH 02/11] adding comment for ConvertNHCBToClassicHistogram Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/model/histogram/convert.go b/model/histogram/convert.go index deab25db2e..6eb50a1de7 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -23,6 +23,9 @@ import ( type BucketEmitter func(labels labels.Labels, value float64) error +// ConvertNHCBToClassicHistogram 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. func ConvertNHCBToClassicHistogram(nhcb interface{}, labels labels.Labels, lblBuilder *labels.Builder, bucketSeries BucketEmitter) error { baseName := labels.Get("__name__") if baseName == "" { From 73904b4c7501a5654d472806ee71d8efd60ff099 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Thu, 25 Sep 2025 03:42:23 +0530 Subject: [PATCH 03/11] refactor(histogram): Converting to Absolute values and fixing the test Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 15 ++- model/histogram/convert_test.go | 157 +++++++++++++++++--------------- 2 files changed, 98 insertions(+), 74 deletions(-) diff --git a/model/histogram/convert.go b/model/histogram/convert.go index 6eb50a1de7..847baea494 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -21,12 +21,14 @@ import ( "github.com/prometheus/prometheus/model/labels" ) +// BucketEmitter is a callback function type for emitting histogram bucket series. +// Used in remote write to append converted bucket time series. type BucketEmitter func(labels labels.Labels, value float64) error // ConvertNHCBToClassicHistogram 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. -func ConvertNHCBToClassicHistogram(nhcb interface{}, labels labels.Labels, lblBuilder *labels.Builder, bucketSeries BucketEmitter) error { +func ConvertNHCBToClassicHistogram(nhcb any, labels labels.Labels, lblBuilder *labels.Builder, bucketSeries BucketEmitter) error { baseName := labels.Get("__name__") if baseName == "" { return errors.New("metric name label '__name__' is missing") @@ -46,7 +48,11 @@ func ConvertNHCBToClassicHistogram(nhcb interface{}, labels labels.Labels, lblBu customValues = h.CustomValues positiveBuckets = make([]float64, len(h.PositiveBuckets)) for i, v := range h.PositiveBuckets { - positiveBuckets[i] = float64(v) + if i == 0 { + positiveBuckets[i] = float64(v) + } else { + positiveBuckets[i] = float64(v) + positiveBuckets[i-1] + } } count = float64(h.Count) sum = h.Sum @@ -59,13 +65,16 @@ func ConvertNHCBToClassicHistogram(nhcb interface{}, labels labels.Labels, lblBu return errors.New("unsupported histogram type") } + // 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 := range customValues { - currCount += positiveBuckets[i] + currCount = positiveBuckets[i] lblBuilder.Reset(labels) lblBuilder.Set("__name__", baseName+"_bucket") lblBuilder.Set("le", fmt.Sprintf("%g", customValues[i])) diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go index 4c7787c884..bbfc6ad193 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -14,57 +14,71 @@ package histogram import ( + "errors" "testing" "github.com/prometheus/prometheus/model/labels" + "github.com/stretchr/testify/require" ) +type BucketExpectation struct { + le string + val float64 +} + +type ExpectedHistogram struct { + buckets []BucketExpectation + count float64 + sum float64 +} + func TestConvertNHCBToClassicHistogram(t *testing.T) { tests := []struct { - name string - nhcb interface{} - labels labels.Labels - expectErr bool - expectedLabels []labels.Labels - expectedValues []float64 + name string + nhcb any + labels labels.Labels + expectErr bool + expected ExpectedHistogram }{ { name: "Valid Histogram", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []int64{10, 20, 30}, + PositiveBuckets: []int64{10, 20, 30}, // Delta format: {10, 20, 30} -> Absolute: {10, 30, 60} Count: 60, Sum: 100.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expectedLabels: []labels.Labels{ - labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "2"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "3"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), - labels.FromStrings("__name__", "test_metric_count"), - labels.FromStrings("__name__", "test_metric_sum"), + expected: ExpectedHistogram{ + buckets: []BucketExpectation{ + {le: "1", val: 10}, + {le: "2", val: 30}, + {le: "3", val: 60}, + {le: "+Inf", val: 60}, + }, + count: 60, + sum: 100, }, - expectedValues: []float64{10, 30, 60, 60, 60, 100}, }, { name: "Valid FloatHistogram", nhcb: &FloatHistogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []float64{20.0, 40.0, 60.0}, - Count: 120.0, + Count: 60.0, Sum: 100.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expectedLabels: []labels.Labels{ - labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "2"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "3"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), - labels.FromStrings("__name__", "test_metric_count"), - labels.FromStrings("__name__", "test_metric_sum"), + expected: ExpectedHistogram{ + buckets: []BucketExpectation{ + {le: "1", val: 20}, + {le: "2", val: 40}, + {le: "3", val: 60}, + {le: "+Inf", val: 60}, + }, + count: 60, + sum: 100, }, - expectedValues: []float64{20, 60, 120, 120, 120, 100}, }, { name: "Empty Histogram", @@ -75,18 +89,19 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { Sum: 0.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expectedLabels: []labels.Labels{ - labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), - labels.FromStrings("__name__", "test_metric_count"), - labels.FromStrings("__name__", "test_metric_sum"), + expected: ExpectedHistogram{ + buckets: []BucketExpectation{ + {le: "+Inf", val: 0}, + }, + count: 0, + sum: 0, }, - expectedValues: []float64{0, 0, 0}, }, { name: "Missing __name__ label", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []int64{10, 20, 30}, + PositiveBuckets: []int64{10, 20, 30}, // Delta format: {10, 20, 30} -> Absolute: {10, 30, 60} Count: 60, Sum: 100.0, }, @@ -103,26 +118,27 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "Histogram with zero bucket counts", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []int64{0, 10, 0}, + PositiveBuckets: []int64{0, 10, 0}, // Delta format: {0, 10, 0} -> Absolute: {0, 10, 10} Count: 10, Sum: 50.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expectedLabels: []labels.Labels{ - labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "2"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "3"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), - labels.FromStrings("__name__", "test_metric_count"), - labels.FromStrings("__name__", "test_metric_sum"), + expected: ExpectedHistogram{ + buckets: []BucketExpectation{ + {le: "1", val: 0}, + {le: "2", val: 10}, + {le: "3", val: 10}, + {le: "+Inf", val: 10}, + }, + count: 10, + sum: 50, }, - expectedValues: []float64{0, 10, 10, 10, 10, 50}, }, { name: "Mismatched bucket lengths", nhcb: &Histogram{ CustomValues: []float64{1, 2}, - PositiveBuckets: []int64{10, 20, 30}, + PositiveBuckets: []int64{10, 20, 30}, // Mismatched lengths: 2 vs 3 Count: 60, Sum: 100.0, }, @@ -133,51 +149,50 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "single series Histogram", nhcb: &Histogram{ CustomValues: []float64{1}, - PositiveBuckets: []int64{10}, + PositiveBuckets: []int64{10}, // Delta format: {10} -> Absolute: {10} Count: 10, Sum: 20.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expectedLabels: []labels.Labels{ - labels.FromStrings("__name__", "test_metric_bucket", "le", "1"), - labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), - labels.FromStrings("__name__", "test_metric_count"), - labels.FromStrings("__name__", "test_metric_sum"), + expected: ExpectedHistogram{ + buckets: []BucketExpectation{ + {le: "1", val: 10}, + {le: "+Inf", val: 10}, + }, + count: 10, + sum: 20, }, - expectedValues: []float64{10, 10, 10, 20}, }, } + labelBuilder := labels.NewBuilder(labels.EmptyLabels()) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var actualLabels []labels.Labels - var actualValues []float64 - - err := ConvertNHCBToClassicHistogram(tt.nhcb, tt.labels, &labels.Builder{}, func(lbls labels.Labels, value float64) error { - actualLabels = append(actualLabels, lbls) - actualValues = append(actualValues, value) + var got ExpectedHistogram + err := ConvertNHCBToClassicHistogram(tt.nhcb, tt.labels, labelBuilder, func(lbls labels.Labels, val float64) error { + switch lbls.Get("__name__") { + case tt.labels.Get("__name__") + "_bucket": + got.buckets = append(got.buckets, BucketExpectation{ + le: lbls.Get("le"), + val: val, + }) + case tt.labels.Get("__name__") + "_count": + got.count = val + case tt.labels.Get("__name__") + "_sum": + got.sum = val + default: + return errors.New("unexpected metric name") + } return nil }) - - if (err != nil) != tt.expectErr { - t.Errorf("ConvertNHCBToClassicHistogram() error = %v, expectErr %v", err, tt.expectErr) - return - } - + require.Equal(t, tt.expectErr, err != nil, "unexpected error: %v", err) if !tt.expectErr { - if len(actualLabels) != len(tt.expectedLabels) { - t.Errorf("Expected %d emissions, got %d", len(tt.expectedLabels), len(actualLabels)) - return - } - - for i, expectedLabel := range tt.expectedLabels { - if !labels.Equal(actualLabels[i], expectedLabel) { - t.Errorf("Expected label[%d] = %v, got %v", i, expectedLabel, actualLabels[i]) - } - if actualValues[i] != tt.expectedValues[i] { - t.Errorf("Expected value[%d] = %f, got %f", i, tt.expectedValues[i], actualValues[i]) - } + require.Equal(t, len(tt.expected.buckets), len(got.buckets)) + for i, expBucket := range tt.expected.buckets { + require.Equal(t, expBucket, got.buckets[i]) } + require.Equal(t, tt.expected.count, got.count) + require.Equal(t, tt.expected.sum, got.sum) } }) } From f71f9110404d0d9c671673e4468102a71201e34c Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Thu, 25 Sep 2025 15:28:25 +0530 Subject: [PATCH 04/11] fix(lint): Changing tests Signed-off-by: Naman-B-Parlecha --- model/histogram/convert_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go index bbfc6ad193..517e6d3d6d 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -17,8 +17,9 @@ import ( "errors" "testing" - "github.com/prometheus/prometheus/model/labels" "github.com/stretchr/testify/require" + + "github.com/prometheus/prometheus/model/labels" ) type BucketExpectation struct { @@ -187,7 +188,7 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { }) require.Equal(t, tt.expectErr, err != nil, "unexpected error: %v", err) if !tt.expectErr { - require.Equal(t, len(tt.expected.buckets), len(got.buckets)) + require.Len(t, got.buckets, len(tt.expected.buckets)) for i, expBucket := range tt.expected.buckets { require.Equal(t, expBucket, got.buckets[i]) } From ed67a0cbf11a5d0278443d85f8bccb16da9e380d Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Thu, 25 Sep 2025 17:40:10 +0530 Subject: [PATCH 05/11] refactor(histogram): rename types for clarity in histogram conversion tests Signed-off-by: Naman-B-Parlecha --- model/histogram/convert_test.go | 42 ++++++++++++++++----------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go index 517e6d3d6d..74d86286dc 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -22,13 +22,13 @@ import ( "github.com/prometheus/prometheus/model/labels" ) -type BucketExpectation struct { +type ExpectedBucket struct { le string val float64 } -type ExpectedHistogram struct { - buckets []BucketExpectation +type ExpectedClassicHistogram struct { + buckets []ExpectedBucket count float64 sum float64 } @@ -39,19 +39,19 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb any labels labels.Labels expectErr bool - expected ExpectedHistogram + expected ExpectedClassicHistogram }{ { name: "Valid Histogram", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []int64{10, 20, 30}, // Delta format: {10, 20, 30} -> Absolute: {10, 30, 60} + PositiveBuckets: []int64{10, 20, 30}, Count: 60, Sum: 100.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedHistogram{ - buckets: []BucketExpectation{ + expected: ExpectedClassicHistogram{ + buckets: []ExpectedBucket{ {le: "1", val: 10}, {le: "2", val: 30}, {le: "3", val: 60}, @@ -70,8 +70,8 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { Sum: 100.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedHistogram{ - buckets: []BucketExpectation{ + expected: ExpectedClassicHistogram{ + buckets: []ExpectedBucket{ {le: "1", val: 20}, {le: "2", val: 40}, {le: "3", val: 60}, @@ -90,8 +90,8 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { Sum: 0.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedHistogram{ - buckets: []BucketExpectation{ + expected: ExpectedClassicHistogram{ + buckets: []ExpectedBucket{ {le: "+Inf", val: 0}, }, count: 0, @@ -102,7 +102,7 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "Missing __name__ label", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []int64{10, 20, 30}, // Delta format: {10, 20, 30} -> Absolute: {10, 30, 60} + PositiveBuckets: []int64{10, 20, 30}, Count: 60, Sum: 100.0, }, @@ -119,13 +119,13 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "Histogram with zero bucket counts", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []int64{0, 10, 0}, // Delta format: {0, 10, 0} -> Absolute: {0, 10, 10} + PositiveBuckets: []int64{0, 10, 0}, Count: 10, Sum: 50.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedHistogram{ - buckets: []BucketExpectation{ + expected: ExpectedClassicHistogram{ + buckets: []ExpectedBucket{ {le: "1", val: 0}, {le: "2", val: 10}, {le: "3", val: 10}, @@ -139,7 +139,7 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "Mismatched bucket lengths", nhcb: &Histogram{ CustomValues: []float64{1, 2}, - PositiveBuckets: []int64{10, 20, 30}, // Mismatched lengths: 2 vs 3 + PositiveBuckets: []int64{10, 20, 30}, Count: 60, Sum: 100.0, }, @@ -150,13 +150,13 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "single series Histogram", nhcb: &Histogram{ CustomValues: []float64{1}, - PositiveBuckets: []int64{10}, // Delta format: {10} -> Absolute: {10} + PositiveBuckets: []int64{10}, Count: 10, Sum: 20.0, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedHistogram{ - buckets: []BucketExpectation{ + expected: ExpectedClassicHistogram{ + buckets: []ExpectedBucket{ {le: "1", val: 10}, {le: "+Inf", val: 10}, }, @@ -169,11 +169,11 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { labelBuilder := labels.NewBuilder(labels.EmptyLabels()) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var got ExpectedHistogram + var got ExpectedClassicHistogram err := ConvertNHCBToClassicHistogram(tt.nhcb, tt.labels, labelBuilder, func(lbls labels.Labels, val float64) error { switch lbls.Get("__name__") { case tt.labels.Get("__name__") + "_bucket": - got.buckets = append(got.buckets, BucketExpectation{ + got.buckets = append(got.buckets, ExpectedBucket{ le: lbls.Get("le"), val: val, }) From 083d0fa8357c787b0f08b87d2200ae8c5ce33d66 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Mon, 6 Oct 2025 22:56:45 +0530 Subject: [PATCH 06/11] refactor(convert): updated tests and moved formatOpenMetricsFloat Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 51 ++++---- model/histogram/convert_test.go | 177 +++++++++++++++------------- model/labels/float.go | 60 ++++++++++ model/textparse/openmetricsparse.go | 2 +- model/textparse/protobufparse.go | 45 +------ 5 files changed, 183 insertions(+), 152 deletions(-) create mode 100644 model/labels/float.go diff --git a/model/histogram/convert.go b/model/histogram/convert.go index 847baea494..2ac23d1d6f 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -15,27 +15,22 @@ package histogram import ( "errors" - "fmt" "math" "github.com/prometheus/prometheus/model/labels" ) -// BucketEmitter is a callback function type for emitting histogram bucket series. -// Used in remote write to append converted bucket time series. -type BucketEmitter func(labels labels.Labels, value float64) error - // ConvertNHCBToClassicHistogram 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. -func ConvertNHCBToClassicHistogram(nhcb any, labels labels.Labels, lblBuilder *labels.Builder, bucketSeries BucketEmitter) error { - baseName := labels.Get("__name__") +func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Builder, emitSeriesFn func(labels labels.Labels, value float64) error) error { + baseName := lset.Get("__name__") if baseName == "" { return errors.New("metric name label '__name__' is missing") } - oldLabels := lblBuilder.Labels() - defer lblBuilder.Reset(oldLabels) + oldLabels := lsetBuilder.Labels() + defer lsetBuilder.Reset(oldLabels) var ( customValues []float64 @@ -45,6 +40,9 @@ func ConvertNHCBToClassicHistogram(nhcb any, labels labels.Labels, lblBuilder *l switch h := nhcb.(type) { case *Histogram: + if h.Schema != -53 { + return errors.New("unsupported histogram schema, only NHCB converstion is supported") + } customValues = h.CustomValues positiveBuckets = make([]float64, len(h.PositiveBuckets)) for i, v := range h.PositiveBuckets { @@ -57,6 +55,9 @@ func ConvertNHCBToClassicHistogram(nhcb any, labels labels.Labels, lblBuilder *l count = float64(h.Count) sum = h.Sum case *FloatHistogram: + if h.Schema != -53 { + return errors.New("unsupported histogram schema, only NHCB converstion is supported") + } customValues = h.CustomValues positiveBuckets = h.PositiveBuckets count = h.Count @@ -75,34 +76,30 @@ func ConvertNHCBToClassicHistogram(nhcb any, labels labels.Labels, lblBuilder *l currCount := float64(0) for i := range customValues { currCount = positiveBuckets[i] - lblBuilder.Reset(labels) - lblBuilder.Set("__name__", baseName+"_bucket") - lblBuilder.Set("le", fmt.Sprintf("%g", customValues[i])) - bucketLabels := lblBuilder.Labels() - if err := bucketSeries(bucketLabels, currCount); err != nil { + lsetBuilder.Reset(lset) + lsetBuilder.Set("__name__", baseName+"_bucket") + lsetBuilder.Set("le", labels.FormatOpenMetricsFloat(customValues[i])) + if err := emitSeriesFn(lsetBuilder.Labels(), currCount); err != nil { return err } } - lblBuilder.Reset(labels) - lblBuilder.Set("__name__", baseName+"_bucket") - lblBuilder.Set("le", fmt.Sprintf("%g", math.Inf(1))) - infBucketLabels := lblBuilder.Labels() - if err := bucketSeries(infBucketLabels, currCount); err != nil { + lsetBuilder.Reset(lset) + lsetBuilder.Set("__name__", baseName+"_bucket") + lsetBuilder.Set("le", labels.FormatOpenMetricsFloat(math.Inf(1))) + if err := emitSeriesFn(lsetBuilder.Labels(), currCount); err != nil { return err } - lblBuilder.Reset(labels) - lblBuilder.Set("__name__", baseName+"_count") - countLabels := lblBuilder.Labels() - if err := bucketSeries(countLabels, count); err != nil { + lsetBuilder.Reset(lset) + lsetBuilder.Set("__name__", baseName+"_count") + if err := emitSeriesFn(lsetBuilder.Labels(), count); err != nil { return err } - lblBuilder.Reset(labels) - lblBuilder.Set("__name__", baseName+"_sum") - sumLabels := lblBuilder.Labels() - if err := bucketSeries(sumLabels, sum); err != nil { + lsetBuilder.Reset(lset) + lsetBuilder.Set("__name__", 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 74d86286dc..e1d7b9ec29 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -1,11 +1,11 @@ // Copyright 2025 The Prometheus Authors // Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. +// you may not use this filset elabels.FromStrings("__name__", "test_metric", "le",)xcept in compliance with the License. // You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // -// Unless required by applicable law or agreed to in writing, software +// Unlsetsslabels.FromStrings("__name__", "test_metric", "le",) required by applicablset llabels.FromStrings("__name__", "test_metric", "le",)aw or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and @@ -14,7 +14,6 @@ package histogram import ( - "errors" "testing" "github.com/stretchr/testify/require" @@ -22,15 +21,9 @@ import ( "github.com/prometheus/prometheus/model/labels" ) -type ExpectedBucket struct { - le string - val float64 -} - -type ExpectedClassicHistogram struct { - buckets []ExpectedBucket - count float64 - sum float64 +type sample struct { + lset labels.Labels + val float64 } func TestConvertNHCBToClassicHistogram(t *testing.T) { @@ -39,111 +32,109 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb any labels labels.Labels expectErr bool - expected ExpectedClassicHistogram + expected []sample }{ { - name: "Valid Histogram", + name: "valid histogram", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []int64{10, 20, 30}, Count: 60, Sum: 100.0, + Schema: -53, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedClassicHistogram{ - buckets: []ExpectedBucket{ - {le: "1", val: 10}, - {le: "2", val: 30}, - {le: "3", val: 60}, - {le: "+Inf", val: 60}, - }, - count: 60, - sum: 100, + 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: 30}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 100}, }, }, { - name: "Valid FloatHistogram", + name: "valid floatHistogram", nhcb: &FloatHistogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []float64{20.0, 40.0, 60.0}, Count: 60.0, Sum: 100.0, + Schema: -53, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedClassicHistogram{ - buckets: []ExpectedBucket{ - {le: "1", val: 20}, - {le: "2", val: 40}, - {le: "3", val: 60}, - {le: "+Inf", val: 60}, - }, - count: 60, - sum: 100, + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 20}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 40}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 100}, }, }, { - name: "Empty Histogram", + name: "empty histogram", nhcb: &Histogram{ CustomValues: []float64{}, PositiveBuckets: []int64{}, Count: 0, Sum: 0.0, + Schema: -53, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedClassicHistogram{ - buckets: []ExpectedBucket{ - {le: "+Inf", val: 0}, - }, - count: 0, - sum: 0, + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 0}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 0}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 0}, }, }, { - name: "Missing __name__ label", + name: "missing __name__ label", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []int64{10, 20, 30}, Count: 60, Sum: 100.0, + Schema: -53, }, labels: labels.FromStrings("job", "test_job"), expectErr: true, }, { - name: "Unsupported histogram type", + name: "unsupported histogram type", nhcb: nil, labels: labels.FromStrings("__name__", "test_metric"), expectErr: true, }, { - name: "Histogram with zero bucket counts", + name: "histogram with zero bucket counts", nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []int64{0, 10, 0}, Count: 10, Sum: 50.0, + Schema: -53, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedClassicHistogram{ - buckets: []ExpectedBucket{ - {le: "1", val: 0}, - {le: "2", val: 10}, - {le: "3", val: 10}, - {le: "+Inf", val: 10}, - }, - count: 10, - sum: 50, + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 0}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 50}, }, }, { - name: "Mismatched bucket lengths", + name: "mismatched bucket lengths", nhcb: &Histogram{ CustomValues: []float64{1, 2}, PositiveBuckets: []int64{10, 20, 30}, Count: 60, Sum: 100.0, + Schema: -53, }, - labels: labels.FromStrings("__name__", "test_metric"), + labels: labels.FromStrings("__name__", "test_metric_bucket"), expectErr: true, }, { @@ -153,47 +144,71 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { PositiveBuckets: []int64{10}, Count: 10, Sum: 20.0, + Schema: -53, }, labels: labels.FromStrings("__name__", "test_metric"), - expected: ExpectedClassicHistogram{ - buckets: []ExpectedBucket{ - {le: "1", val: 10}, - {le: "+Inf", val: 10}, - }, - count: 10, - sum: 20, + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 20}, }, }, + { + name: "multiset label histogram", + nhcb: &Histogram{ + CustomValues: []float64{1}, + PositiveBuckets: []int64{10}, + Count: 10, + Sum: 20.0, + Schema: -53, + }, + labels: labels.FromStrings("__name__", "test_metric", "job", "test_job", "instance", "localhost:9090"), + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "job", "test_job", "instance", "localhost:9090", "le", "1.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "job", "test_job", "instance", "localhost:9090", "le", "+Inf"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_count", "job", "test_job", "instance", "localhost:9090"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_sum", "job", "test_job", "instance", "localhost:9090"), val: 20}, + }, + }, + { + name: "exponential histogram", + nhcb: &FloatHistogram{ + Schema: 1, + ZeroThreshold: 0.01, + ZeroCount: 5.5, + Count: 3493.3, + Sum: 2349209.324, + PositiveSpans: []Span{ + {-2, 1}, + {2, 3}, + }, + PositiveBuckets: []float64{1, 3.3, 4.2, 0.1}, + NegativeSpans: []Span{ + {3, 2}, + {3, 2}, + }, + NegativeBuckets: []float64{3.1, 3, 1.234e5, 1000}, + }, + labels: labels.FromStrings("__name__", "test_metric_bucket"), + expectErr: true, + }, } labelBuilder := labels.NewBuilder(labels.EmptyLabels()) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - var got ExpectedClassicHistogram - err := ConvertNHCBToClassicHistogram(tt.nhcb, tt.labels, labelBuilder, func(lbls labels.Labels, val float64) error { - switch lbls.Get("__name__") { - case tt.labels.Get("__name__") + "_bucket": - got.buckets = append(got.buckets, ExpectedBucket{ - le: lbls.Get("le"), - val: val, - }) - case tt.labels.Get("__name__") + "_count": - got.count = val - case tt.labels.Get("__name__") + "_sum": - got.sum = val - default: - return errors.New("unexpected metric name") - } + var emittedSamples []sample + err := ConvertNHCBToClassic(tt.nhcb, tt.labels, labelBuilder, func(lbls labels.Labels, val float64) error { + emittedSamples = append(emittedSamples, sample{lset: lbls, val: val}) return nil }) require.Equal(t, tt.expectErr, err != nil, "unexpected error: %v", err) if !tt.expectErr { - require.Len(t, got.buckets, len(tt.expected.buckets)) - for i, expBucket := range tt.expected.buckets { - require.Equal(t, expBucket, got.buckets[i]) + require.Len(t, emittedSamples, len(tt.expected)) + for i, expSample := range tt.expected { + require.Equal(t, expSample, emittedSamples[i]) } - require.Equal(t, tt.expected.count, got.count) - require.Equal(t, tt.expected.sum, got.sum) } }) } diff --git a/model/labels/float.go b/model/labels/float.go new file mode 100644 index 0000000000..e052347cd3 --- /dev/null +++ b/model/labels/float.go @@ -0,0 +1,60 @@ +// Copyright 2025 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package labels + +import ( + "bytes" + "math" + "strconv" + "sync" +) + +// floatFormatBufPool is exclusively used in formatOpenMetricsFloat. +var floatFormatBufPool = sync.Pool{ + New: func() any { + // To contain at most 17 digits and additional syntax for a float64. + b := make([]byte, 0, 24) + return &b + }, +} + +// formatOpenMetricsFloat works like the usual Go string formatting of a float +// but appends ".0" if the resulting number would otherwise contain neither a +// "." nor an "e". +func FormatOpenMetricsFloat(f float64) string { + // A few common cases hardcoded. + switch { + case f == 1: + return "1.0" + case f == 0: + return "0.0" + case f == -1: + return "-1.0" + case math.IsNaN(f): + return "NaN" + case math.IsInf(f, +1): + return "+Inf" + case math.IsInf(f, -1): + return "-Inf" + } + bp := floatFormatBufPool.Get().(*[]byte) + defer floatFormatBufPool.Put(bp) + + *bp = strconv.AppendFloat((*bp)[:0], f, 'g', -1, 64) + if bytes.ContainsAny(*bp, "e.") { + return string(*bp) + } + *bp = append(*bp, '.', '0') + return string(*bp) +} diff --git a/model/textparse/openmetricsparse.go b/model/textparse/openmetricsparse.go index 4e592167f3..505e45fc40 100644 --- a/model/textparse/openmetricsparse.go +++ b/model/textparse/openmetricsparse.go @@ -773,7 +773,7 @@ func normalizeFloatsInLabelValues(t model.MetricType, l, v string) string { if (t == model.MetricTypeSummary && l == model.QuantileLabel) || (t == model.MetricTypeHistogram && l == model.BucketLabel) { f, err := strconv.ParseFloat(v, 64) if err == nil { - return formatOpenMetricsFloat(f) + return labels.FormatOpenMetricsFloat(f) } } return v diff --git a/model/textparse/protobufparse.go b/model/textparse/protobufparse.go index e7ce710491..80091bfd85 100644 --- a/model/textparse/protobufparse.go +++ b/model/textparse/protobufparse.go @@ -19,9 +19,7 @@ import ( "fmt" "io" "math" - "strconv" "strings" - "sync" "unicode/utf8" "github.com/gogo/protobuf/types" @@ -34,15 +32,6 @@ import ( "github.com/prometheus/prometheus/schema" ) -// floatFormatBufPool is exclusively used in formatOpenMetricsFloat. -var floatFormatBufPool = sync.Pool{ - New: func() any { - // To contain at most 17 digits and additional syntax for a float64. - b := make([]byte, 0, 24) - return &b - }, -} - // ProtobufParser parses the old Prometheus protobuf format and present it // as the text-style textparse.Parser interface. // @@ -632,7 +621,7 @@ func (p *ProtobufParser) getMagicLabel() (bool, string, string) { qq := p.dec.GetSummary().GetQuantile() q := qq[p.fieldPos] p.fieldsDone = p.fieldPos == len(qq)-1 - return true, model.QuantileLabel, formatOpenMetricsFloat(q.GetQuantile()) + return true, model.QuantileLabel, labels.FormatOpenMetricsFloat(q.GetQuantile()) case dto.MetricType_HISTOGRAM, dto.MetricType_GAUGE_HISTOGRAM: bb := p.dec.GetHistogram().GetBucket() if p.fieldPos >= len(bb) { @@ -641,41 +630,11 @@ func (p *ProtobufParser) getMagicLabel() (bool, string, string) { } b := bb[p.fieldPos] p.fieldsDone = math.IsInf(b.GetUpperBound(), +1) - return true, model.BucketLabel, formatOpenMetricsFloat(b.GetUpperBound()) + return true, model.BucketLabel, labels.FormatOpenMetricsFloat(b.GetUpperBound()) } return false, "", "" } -// formatOpenMetricsFloat works like the usual Go string formatting of a float -// but appends ".0" if the resulting number would otherwise contain neither a -// "." nor an "e". -func formatOpenMetricsFloat(f float64) string { - // A few common cases hardcoded. - switch { - case f == 1: - return "1.0" - case f == 0: - return "0.0" - case f == -1: - return "-1.0" - case math.IsNaN(f): - return "NaN" - case math.IsInf(f, +1): - return "+Inf" - case math.IsInf(f, -1): - return "-Inf" - } - bp := floatFormatBufPool.Get().(*[]byte) - defer floatFormatBufPool.Put(bp) - - *bp = strconv.AppendFloat((*bp)[:0], f, 'g', -1, 64) - if bytes.ContainsAny(*bp, "e.") { - return string(*bp) - } - *bp = append(*bp, '.', '0') - return string(*bp) -} - // isNativeHistogram returns false iff the provided histograms has no spans at // all (neither positive nor negative) and a zero threshold of 0 and a zero // count of 0. In principle, this could still be meant to be a native histogram From c072b0000a1475084438eef56c3e4aa523b19cf7 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Mon, 6 Oct 2025 23:05:01 +0530 Subject: [PATCH 07/11] fix(convert): fix typos in comments Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 6 +++--- model/labels/float.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/model/histogram/convert.go b/model/histogram/convert.go index 2ac23d1d6f..250289c8d2 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -20,7 +20,7 @@ import ( "github.com/prometheus/prometheus/model/labels" ) -// ConvertNHCBToClassicHistogram converts Native Histogram Custom Buckets (NHCB) to classic histogram series. +// 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. func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Builder, emitSeriesFn func(labels labels.Labels, value float64) error) error { @@ -41,7 +41,7 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil switch h := nhcb.(type) { case *Histogram: if h.Schema != -53 { - return errors.New("unsupported histogram schema, only NHCB converstion is supported") + return errors.New("unsupported histogram schema, only NHCB conversion is supported") } customValues = h.CustomValues positiveBuckets = make([]float64, len(h.PositiveBuckets)) @@ -56,7 +56,7 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil sum = h.Sum case *FloatHistogram: if h.Schema != -53 { - return errors.New("unsupported histogram schema, only NHCB converstion is supported") + return errors.New("unsupported histogram schema, only NHCB conversion is supported") } customValues = h.CustomValues positiveBuckets = h.PositiveBuckets diff --git a/model/labels/float.go b/model/labels/float.go index e052347cd3..030ae9c0e0 100644 --- a/model/labels/float.go +++ b/model/labels/float.go @@ -20,7 +20,7 @@ import ( "sync" ) -// floatFormatBufPool is exclusively used in formatOpenMetricsFloat. +// floatFormatBufPool is exclusively used in FormatOpenMetricsFloat. var floatFormatBufPool = sync.Pool{ New: func() any { // To contain at most 17 digits and additional syntax for a float64. @@ -29,7 +29,7 @@ var floatFormatBufPool = sync.Pool{ }, } -// formatOpenMetricsFloat works like the usual Go string formatting of a float +// FormatOpenMetricsFloat works like the usual Go string formatting of a float // but appends ".0" if the resulting number would otherwise contain neither a // "." nor an "e". func FormatOpenMetricsFloat(f float64) string { From 79f3e76d892ebeeda2e755ec4e059561c4c6ee66 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Tue, 7 Oct 2025 00:22:25 +0530 Subject: [PATCH 08/11] fix(test): Comparing the labels correctly Signed-off-by: Naman-B-Parlecha --- model/histogram/convert_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go index e1d7b9ec29..561e963b95 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -207,7 +207,8 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { if !tt.expectErr { require.Len(t, emittedSamples, len(tt.expected)) for i, expSample := range tt.expected { - require.Equal(t, expSample, emittedSamples[i]) + require.True(t, labels.Equal(expSample.lset, emittedSamples[i].lset), "labels mismatch at index %d: expected %v, got %v", i, expSample.lset, emittedSamples[i].lset) + require.Equal(t, expSample.val, emittedSamples[i].val, "value mismatch at index %d", i) } } }) From 7871bcb4653f15bb6f74654ed256dca3548cddc6 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Tue, 7 Oct 2025 14:20:32 +0530 Subject: [PATCH 09/11] fix(convert): error message Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/model/histogram/convert.go b/model/histogram/convert.go index 250289c8d2..19376db93c 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -15,6 +15,7 @@ package histogram import ( "errors" + "fmt" "math" "github.com/prometheus/prometheus/model/labels" @@ -40,8 +41,8 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil switch h := nhcb.(type) { case *Histogram: - if h.Schema != -53 { - return errors.New("unsupported histogram schema, only NHCB conversion is supported") + if !IsCustomBucketsSchema(h.Schema) { + return errors.New("unsupported histogram schema, not a NHCB") } customValues = h.CustomValues positiveBuckets = make([]float64, len(h.PositiveBuckets)) @@ -55,15 +56,15 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil count = float64(h.Count) sum = h.Sum case *FloatHistogram: - if h.Schema != -53 { - return errors.New("unsupported histogram schema, only NHCB conversion is supported") + if !IsCustomBucketsSchema(h.Schema) { + return errors.New("unsupported histogram schema, not a NHCB") } customValues = h.CustomValues positiveBuckets = h.PositiveBuckets count = h.Count sum = h.Sum default: - return errors.New("unsupported histogram type") + return fmt.Errorf("unsupported histogram type: %T", h) } // Each customValue corresponds to a positive bucket (aligned with the "le" label). From 1df1f53ea0d0ca2c2d4f381e97ecab2f1c774a40 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Fri, 10 Oct 2025 19:12:30 +0530 Subject: [PATCH 10/11] fix: Added Unroll support to Sparse NHCBs Signed-off-by: Naman-B-Parlecha --- model/histogram/convert.go | 75 ++++++++++++--- model/histogram/convert_test.go | 164 +++++++++++++++++++++++++------- model/labels/float.go | 2 +- 3 files changed, 193 insertions(+), 48 deletions(-) diff --git a/model/histogram/convert.go b/model/histogram/convert.go index 19376db93c..d43b8182d6 100644 --- a/model/histogram/convert.go +++ b/model/histogram/convert.go @@ -1,4 +1,4 @@ -// Copyright 2025 The Prometheus Authors +// Copyright The Prometheus Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -30,6 +30,9 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil return errors.New("metric name label '__name__' is missing") } + // We preserve original labels and restore them after conversion. + // This is to ensure that no modifications are made to the original labels + // that the queue_manager relies on. oldLabels := lsetBuilder.Labels() defer lsetBuilder.Reset(oldLabels) @@ -37,6 +40,8 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil customValues []float64 positiveBuckets []float64 count, sum float64 + idx int // This index is to track buckets in Classic Histogram + currIdx int // This index is to track buckets in Native Histogram ) switch h := nhcb.(type) { @@ -44,13 +49,35 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil if !IsCustomBucketsSchema(h.Schema) { 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") + } + customValues = h.CustomValues - positiveBuckets = make([]float64, len(h.PositiveBuckets)) - for i, v := range h.PositiveBuckets { - if i == 0 { - positiveBuckets[i] = float64(v) - } else { - positiveBuckets[i] = float64(v) + positiveBuckets[i-1] + positiveBuckets = make([]float64, len(customValues)) + + // Histograms are in delta format so we first bring them to absolute format. + acc := int64(0) + for _, s := range h.PositiveSpans { + for i := 0; i < int(s.Offset); i++ { + positiveBuckets[idx] = float64(acc) + idx++ + } + for i := 0; i < int(s.Length); i++ { + acc += h.PositiveBuckets[currIdx] + positiveBuckets[idx] = float64(acc) + idx++ + currIdx++ } } count = float64(h.Count) @@ -59,8 +86,34 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil if !IsCustomBucketsSchema(h.Schema) { 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) + } + 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 = h.PositiveBuckets + positiveBuckets = make([]float64, len(customValues)) + + for _, span := range h.PositiveSpans { + // Since Float Histogram is already in absolute format we should + // keep the sparse buckets empty so we jump and go to next filled + // bucket index. + idx += int(span.Offset) + for i := 0; i < int(span.Length); i++ { + positiveBuckets[idx] = h.PositiveBuckets[currIdx] + idx++ + currIdx++ + } + } count = h.Count sum = h.Sum default: @@ -75,11 +128,11 @@ func ConvertNHCBToClassic(nhcb any, lset labels.Labels, lsetBuilder *labels.Buil } currCount := float64(0) - for i := range customValues { - currCount = positiveBuckets[i] + for i, val := range customValues { + currCount += positiveBuckets[i] lsetBuilder.Reset(lset) lsetBuilder.Set("__name__", baseName+"_bucket") - lsetBuilder.Set("le", labels.FormatOpenMetricsFloat(customValues[i])) + lsetBuilder.Set("le", labels.FormatOpenMetricsFloat(val)) if err := emitSeriesFn(lsetBuilder.Labels(), currCount); err != nil { return err } diff --git a/model/histogram/convert_test.go b/model/histogram/convert_test.go index 561e963b95..16c50d6263 100644 --- a/model/histogram/convert_test.go +++ b/model/histogram/convert_test.go @@ -1,11 +1,11 @@ -// Copyright 2025 The Prometheus Authors +// Copyright The Prometheus Authors // Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this filset elabels.FromStrings("__name__", "test_metric", "le",)xcept in compliance with the License. +// you may not use this file except in compliance with the License. // You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // -// Unlsetsslabels.FromStrings("__name__", "test_metric", "le",) required by applicablset llabels.FromStrings("__name__", "test_metric", "le",)aw or agreed to in writing, software +// Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and @@ -39,17 +39,20 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []int64{10, 20, 30}, - Count: 60, - Sum: 100.0, - Schema: -53, + PositiveSpans: []Span{ + {Offset: 0, Length: 3}, + }, + Count: 100, + Sum: 100.0, + Schema: CustomBucketsSchema, }, 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: 30}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 60}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 60}, - {lset: labels.FromStrings("__name__", "test_metric_count"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 40}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 100}, + {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}, }, }, @@ -57,18 +60,21 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { name: "valid floatHistogram", nhcb: &FloatHistogram{ CustomValues: []float64{1, 2, 3}, - PositiveBuckets: []float64{20.0, 40.0, 60.0}, - Count: 60.0, - Sum: 100.0, - Schema: -53, + PositiveBuckets: []float64{20.0, 40.0, 60.0}, // 20 -> 60 ->120 + PositiveSpans: []Span{ + {Offset: 0, Length: 3}, + }, + Count: 120.0, + Sum: 100.0, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("__name__", "test_metric"), expected: []sample{ {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 20}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 40}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 60}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 60}, - {lset: labels.FromStrings("__name__", "test_metric_count"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 60}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 120}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 120}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 120}, {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 100}, }, }, @@ -77,9 +83,10 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb: &Histogram{ CustomValues: []float64{}, PositiveBuckets: []int64{}, + PositiveSpans: []Span{}, Count: 0, Sum: 0.0, - Schema: -53, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("__name__", "test_metric"), expected: []sample{ @@ -93,9 +100,9 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []int64{10, 20, 30}, - Count: 60, + Count: 100, Sum: 100.0, - Schema: -53, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("job", "test_job"), expectErr: true, @@ -111,28 +118,45 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb: &Histogram{ CustomValues: []float64{1, 2, 3}, PositiveBuckets: []int64{0, 10, 0}, - Count: 10, - Sum: 50.0, - Schema: -53, + PositiveSpans: []Span{ + {Offset: 0, Length: 3}, + }, + Count: 20, + Sum: 50.0, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("__name__", "test_metric"), expected: []sample{ {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 0}, {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 10}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 10}, - {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 10}, - {lset: labels.FromStrings("__name__", "test_metric_count"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 20}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 20}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 20}, {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 50}, }, }, { - name: "mismatched bucket lengths", + name: "mismatched bucket lengths with more filled bucket count", nhcb: &Histogram{ CustomValues: []float64{1, 2}, PositiveBuckets: []int64{10, 20, 30}, - Count: 60, + PositiveSpans: []Span{{Offset: 0, Length: 3}}, + Count: 100, Sum: 100.0, - Schema: -53, + Schema: CustomBucketsSchema, + }, + labels: labels.FromStrings("__name__", "test_metric_bucket"), + expectErr: true, + }, + { + name: "mismatched bucket lengths with less filled bucket count", + nhcb: &Histogram{ + CustomValues: []float64{1, 2}, + PositiveBuckets: []int64{10}, + PositiveSpans: []Span{{Offset: 0, Length: 2}}, + Count: 100, + Sum: 100.0, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("__name__", "test_metric_bucket"), expectErr: true, @@ -142,9 +166,12 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb: &Histogram{ CustomValues: []float64{1}, PositiveBuckets: []int64{10}, - Count: 10, - Sum: 20.0, - Schema: -53, + PositiveSpans: []Span{ + {Offset: 0, Length: 1}, + }, + Count: 10, + Sum: 20.0, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("__name__", "test_metric"), expected: []sample{ @@ -159,9 +186,12 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { nhcb: &Histogram{ CustomValues: []float64{1}, PositiveBuckets: []int64{10}, - Count: 10, - Sum: 20.0, - Schema: -53, + PositiveSpans: []Span{ + {Offset: 0, Length: 1}, + }, + Count: 10, + Sum: 20.0, + Schema: CustomBucketsSchema, }, labels: labels.FromStrings("__name__", "test_metric", "job", "test_job", "instance", "localhost:9090"), expected: []sample{ @@ -193,6 +223,68 @@ func TestConvertNHCBToClassicHistogram(t *testing.T) { labels: labels.FromStrings("__name__", "test_metric_bucket"), expectErr: true, }, + { + name: "sparse histogram", + nhcb: &Histogram{ + Schema: CustomBucketsSchema, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, + PositiveSpans: []Span{ + {0, 2}, + {4, 1}, + {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 + Sum: 123, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 1}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 4}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 7}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "4.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "5.0"), val: 13}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "6.0"), val: 16}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "7.0"), val: 22}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "8.0"), val: 28}, + {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_sum"), val: 123}, + }, + }, + { + name: "sparse float histogram", + nhcb: &FloatHistogram{ + Schema: CustomBucketsSchema, + CustomValues: []float64{1, 2, 3, 4, 5, 6, 7, 8, 9, 10}, + PositiveSpans: []Span{ + {0, 2}, + {4, 1}, + {1, 2}, + }, + PositiveBuckets: []float64{1, 2, 3, 4, 5}, // 1 -> 2 -> 0 -> 0 -> 0 -> 0 -> 3 -> 0 -> 4 -> 5 + Count: 15, // 1 -> 3 -> 3 -> 3 -> 3 -> 3 -> 6 -> 6 -> 10 -> 15 + Sum: 123, + }, + labels: labels.FromStrings("__name__", "test_metric"), + expected: []sample{ + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "1.0"), val: 1}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "2.0"), val: 3}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "3.0"), val: 3}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "4.0"), val: 3}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "5.0"), val: 3}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "6.0"), val: 3}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "7.0"), val: 6}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "8.0"), val: 6}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "9.0"), val: 10}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "10.0"), val: 15}, + {lset: labels.FromStrings("__name__", "test_metric_bucket", "le", "+Inf"), val: 15}, + {lset: labels.FromStrings("__name__", "test_metric_count"), val: 15}, + {lset: labels.FromStrings("__name__", "test_metric_sum"), val: 123}, + }, + }, } labelBuilder := labels.NewBuilder(labels.EmptyLabels()) diff --git a/model/labels/float.go b/model/labels/float.go index 030ae9c0e0..c526a5b2a6 100644 --- a/model/labels/float.go +++ b/model/labels/float.go @@ -1,4 +1,4 @@ -// Copyright 2025 The Prometheus Authors +// Copyright The Prometheus Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at From f14c515cbee2213f1cc1df83bda4368202ecc2f9 Mon Sep 17 00:00:00 2001 From: Naman-B-Parlecha Date: Tue, 28 Oct 2025 20:29:44 +0530 Subject: [PATCH 11/11] 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}, }, },