From 5ad1de8610292f650e19a96588b2361aacce470a Mon Sep 17 00:00:00 2001 From: yeya24 Date: Mon, 17 Feb 2025 13:42:34 -0800 Subject: [PATCH 1/2] fix native histogram empty bucket marshal panic Signed-off-by: yeya24 --- pkg/querier/codec/protobuf_codec.go | 15 ++-- pkg/querier/codec/protobuf_codec_test.go | 92 ++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/pkg/querier/codec/protobuf_codec.go b/pkg/querier/codec/protobuf_codec.go index 783adcd183e..64bfa2e3945 100644 --- a/pkg/querier/codec/protobuf_codec.go +++ b/pkg/querier/codec/protobuf_codec.go @@ -134,15 +134,13 @@ func getMatrixSampleStreams(data *v1.QueryData) *[]tripperware.SampleStream { if sampleStream.Histograms[j].H.ZeroCount > 0 { bucketsLen = len(sampleStream.Histograms[j].H.NegativeBuckets) + len(sampleStream.Histograms[j].H.PositiveBuckets) + 1 } - buckets := make([]*tripperware.HistogramBucket, bucketsLen) it := sampleStream.Histograms[j].H.AllBucketIterator() - getBuckets(buckets, it) histograms[j] = tripperware.SampleHistogramPair{ TimestampMs: sampleStream.Histograms[j].T, Histogram: tripperware.SampleHistogram{ Count: sampleStream.Histograms[j].H.Count, Sum: sampleStream.Histograms[j].H.Sum, - Buckets: buckets, + Buckets: getBuckets(bucketsLen, it), }, } } @@ -192,22 +190,21 @@ func getVectorSamples(data *v1.QueryData, cortexInternal bool) *[]tripperware.Sa if sample.H.ZeroCount > 0 { bucketsLen = len(sample.H.NegativeBuckets) + len(sample.H.PositiveBuckets) + 1 } - buckets := make([]*tripperware.HistogramBucket, bucketsLen) it := sample.H.AllBucketIterator() - getBuckets(buckets, it) vectorSamples[i].Histogram = &tripperware.SampleHistogramPair{ TimestampMs: sample.T, Histogram: tripperware.SampleHistogram{ Count: sample.H.Count, Sum: sample.H.Sum, - Buckets: buckets, + Buckets: getBuckets(bucketsLen, it), }, } } return &vectorSamples } -func getBuckets(bucketsList []*tripperware.HistogramBucket, it histogram.BucketIterator[float64]) { +func getBuckets(bucketsLen int, it histogram.BucketIterator[float64]) []*tripperware.HistogramBucket { + buckets := make([]*tripperware.HistogramBucket, bucketsLen) bucketIdx := 0 for it.Next() { bucket := it.At() @@ -226,7 +223,7 @@ func getBuckets(bucketsList []*tripperware.HistogramBucket, it histogram.BucketI boundaries = 0 // Inclusive only on upper end AKA left open. } } - bucketsList[bucketIdx] = &tripperware.HistogramBucket{ + buckets[bucketIdx] = &tripperware.HistogramBucket{ Boundaries: int32(boundaries), Lower: bucket.Lower, Upper: bucket.Upper, @@ -234,6 +231,8 @@ func getBuckets(bucketsList []*tripperware.HistogramBucket, it histogram.BucketI } bucketIdx += 1 } + buckets = buckets[:bucketIdx] + return buckets } func getStats(builtin *stats.BuiltinStats) *tripperware.PrometheusResponseSamplesStats { diff --git a/pkg/querier/codec/protobuf_codec_test.go b/pkg/querier/codec/protobuf_codec_test.go index cef4e5649d1..c7fee0ecba5 100644 --- a/pkg/querier/codec/protobuf_codec_test.go +++ b/pkg/querier/codec/protobuf_codec_test.go @@ -39,6 +39,17 @@ func TestProtobufCodec_Encode(t *testing.T) { } testProtoHistogram := cortexpb.FloatHistogramToHistogramProto(1000, testFloatHistogram) + floatHistogramWithEmptyBucket := &histogram.FloatHistogram{ + CounterResetHint: 0, + Schema: 1, + ZeroThreshold: 0.01, + ZeroCount: 0, + Count: 0, + Sum: 1, + PositiveSpans: []histogram.Span{{Offset: 0, Length: 1}}, + PositiveBuckets: []float64{0}, + } + tests := []struct { name string data *v1.QueryData @@ -287,6 +298,47 @@ func TestProtobufCodec_Encode(t *testing.T) { }, }, }, + { + name: "matrix with histogram and not cortex internal, empty bucket", + data: &v1.QueryData{ + ResultType: parser.ValueTypeMatrix, + Result: promql.Matrix{ + promql.Series{ + Histograms: []promql.HPoint{{H: floatHistogramWithEmptyBucket, T: 1000}}, + Metric: labels.FromStrings("__name__", "foo"), + }, + }, + }, + expected: &tripperware.PrometheusResponse{ + Status: tripperware.StatusSuccess, + Data: tripperware.PrometheusData{ + ResultType: model.ValMatrix.String(), + Result: tripperware.PrometheusQueryResult{ + Result: &tripperware.PrometheusQueryResult_Matrix{ + Matrix: &tripperware.Matrix{ + SampleStreams: []tripperware.SampleStream{ + { + Labels: []cortexpb.LabelAdapter{ + {Name: "__name__", Value: "foo"}, + }, + Histograms: []tripperware.SampleHistogramPair{ + { + TimestampMs: 1000, + Histogram: tripperware.SampleHistogram{ + Count: 0, + Sum: 1, + Buckets: []*tripperware.HistogramBucket{}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, { name: "vector with histogram and not cortex internal", data: &v1.QueryData{ @@ -376,6 +428,46 @@ func TestProtobufCodec_Encode(t *testing.T) { }, }, }, + { + name: "vector with histogram with and not cortex internal, empty bucket", + data: &v1.QueryData{ + ResultType: parser.ValueTypeVector, + Result: promql.Vector{ + promql.Sample{ + Metric: labels.FromStrings("__name__", "foo"), + T: 1000, + H: floatHistogramWithEmptyBucket, + }, + }, + }, + expected: &tripperware.PrometheusResponse{ + Status: tripperware.StatusSuccess, + Data: tripperware.PrometheusData{ + ResultType: model.ValVector.String(), + Result: tripperware.PrometheusQueryResult{ + Result: &tripperware.PrometheusQueryResult_Vector{ + Vector: &tripperware.Vector{ + Samples: []tripperware.Sample{ + { + Labels: []cortexpb.LabelAdapter{ + {Name: "__name__", Value: "foo"}, + }, + Histogram: &tripperware.SampleHistogramPair{ + TimestampMs: 1000, + Histogram: tripperware.SampleHistogram{ + Count: 0, + Sum: 1, + Buckets: []*tripperware.HistogramBucket{}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, { name: "vector with histogram and cortex internal", cortexInternal: true, From c38126c4dc4bc42025b8709e6e7beefea75e47d3 Mon Sep 17 00:00:00 2001 From: yeya24 Date: Mon, 17 Feb 2025 13:59:27 -0800 Subject: [PATCH 2/2] changelog Signed-off-by: yeya24 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8899b9108b3..42bc4b5d52c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * [BUGFIX] Ingester: Fix labelset data race condition. #6573 * [BUGFIX] Compactor: Cleaner should not put deletion marker for blocks with no-compact marker. #6576 * [BUGFIX] Compactor: Cleaner would delete bucket index when there is no block in bucket store. #6577 +* [BUGFIX] Querier: Fix marshal native histogram with empty bucket when protobuf codec is enabled. #6595 ## 1.19.0 in progress