Skip to content

Commit

Permalink
Cast directly from new PromQL FPoint and HPoint to mimirpb equivalents (
Browse files Browse the repository at this point in the history 
#4775)

* Allow the option of ignoring name when comparing shape

* Update the ignored name when comparing shape according to code review

* Improve tests for conversion between histogram and proto

* Add benchmarks for histogram conversion methods in mimirpb compat

* Fix benchmarks according to code review

* Direct cast from FPoints to Samples

* Direct cast between BucketSpans and histogram.Spans

* Direct cast from HPoints to FloatHistogramPairs

* Fix field numbers changing for FloatHistogramPair

* Do reflect.TypeOf() in helper function instead

* Update tests according to code review

* Fix `TestQuerierTenantFederation` integration test that started failing after FloatHistogramPair was changed to use pointers

* Revise according to code review
  • Loading branch information
@zenador
zenador authored May 18, 2023
1 parent d01bc84 commit b6e3fa6
Show file tree
Hide file tree
Showing 14 changed files with 318 additions and 227 deletions.
2 changes: 1 addition & 1 deletion pkg/api/protobuf_codec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (c protobufCodec) encodeMatrixSeries(s promql.Series) mimirpb.MatrixSeries
for _, p := range s.Histograms {
histograms = append(histograms, mimirpb.FloatHistogramPair{
TimestampMs: p.T,
Histogram: *mimirpb.FloatHistogramFromPrometheusModel(p.H),
Histogram: mimirpb.FloatHistogramFromPrometheusModel(p.H),
})
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/api/protobuf_codec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ var protobufCodecScenarios = map[string]struct {
Histograms: []mimirpb.FloatHistogramPair{
{
TimestampMs: 1234,
Histogram: mimirpb.FloatHistogram{
Histogram: &mimirpb.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 3,
ZeroThreshold: 1.23,
Expand Down Expand Up @@ -827,7 +827,7 @@ var protobufCodecScenarios = map[string]struct {
Histograms: []mimirpb.FloatHistogramPair{
{
TimestampMs: 1234,
Histogram: mimirpb.FloatHistogram{
Histogram: &mimirpb.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 3,
ZeroThreshold: 1.23,
Expand All @@ -848,7 +848,7 @@ var protobufCodecScenarios = map[string]struct {
},
{
TimestampMs: 12340,
Histogram: mimirpb.FloatHistogram{
Histogram: &mimirpb.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 4,
ZeroThreshold: 1.203,
Expand Down
4 changes: 2 additions & 2 deletions pkg/frontend/querymiddleware/codec_json_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ func TestPrometheusCodec_JSONEncoding(t *testing.T) {
Data: &PrometheusData{
ResultType: model.ValMatrix.String(),
Result: []SampleStream{
{Labels: []mimirpb.LabelAdapter{{Name: "foo", Value: "bar"}}, Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1_234, Histogram: responseHistogram}}},
{Labels: []mimirpb.LabelAdapter{{Name: "foo", Value: "bar"}}, Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1_234, Histogram: &responseHistogram}}},
},
},
},
Expand Down Expand Up @@ -316,7 +316,7 @@ func TestPrometheusCodec_JSONEncoding(t *testing.T) {
{
Labels: []mimirpb.LabelAdapter{{Name: "foo", Value: "bar"}},
Samples: []mimirpb.Sample{{TimestampMs: 1_000, Value: 101}, {TimestampMs: 2_000, Value: 201}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 3_000, Histogram: responseHistogram}}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 3_000, Histogram: &responseHistogram}}},
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/frontend/querymiddleware/codec_protobuf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (protobufFormatter) encodeVectorData(data []SampleStream) (mimirpb.VectorDa

histograms = append(histograms, mimirpb.VectorHistogram{
Metric: metric,
Histogram: sample.Histogram,
Histogram: *sample.Histogram,
TimestampMs: sample.TimestampMs,
})
}
Expand Down Expand Up @@ -288,7 +288,7 @@ func (f protobufFormatter) decodeVectorData(data *mimirpb.VectorData) (*Promethe
Histograms: []mimirpb.FloatHistogramPair{
{
TimestampMs: sample.TimestampMs,
Histogram: sample.Histogram,
Histogram: &data.Histograms[i].Histogram,
},
},
}
Expand Down
40 changes: 34 additions & 6 deletions pkg/frontend/querymiddleware/codec_protobuf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,25 @@ var protobufResponseHistogram = mimirpb.FloatHistogram{
NegativeBuckets: []float64{400, 500, 600, 700},
}

var protobufResponseHistogram2 = mimirpb.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 3,
ZeroThreshold: 1.23,
ZeroCount: 556,
Count: 9101,
Sum: 789.1,
PositiveSpans: []mimirpb.BucketSpan{
{Offset: 4, Length: 1},
{Offset: 3, Length: 2},
},
NegativeSpans: []mimirpb.BucketSpan{
{Offset: 7, Length: 3},
{Offset: 9, Length: 1},
},
PositiveBuckets: []float64{100, 200, 300},
NegativeBuckets: []float64{400, 500, 600, 700},
}

var protobufCodecScenarios = []struct {
name string
payload mimirpb.QueryResponse
Expand Down Expand Up @@ -236,7 +255,7 @@ var protobufCodecScenarios = []struct {
Result: []SampleStream{
{
Labels: []mimirpb.LabelAdapter{{Name: "name-1", Value: "value-1"}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: protobufResponseHistogram}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram}},
},
},
},
Expand All @@ -258,6 +277,11 @@ var protobufCodecScenarios = []struct {
TimestampMs: 1234,
Histogram: protobufResponseHistogram,
},
{
Metric: []string{"baz2", "blah2"},
TimestampMs: 1234,
Histogram: protobufResponseHistogram2,
},
},
},
},
Expand All @@ -273,7 +297,11 @@ var protobufCodecScenarios = []struct {
},
{
Labels: []mimirpb.LabelAdapter{{Name: "baz", Value: "blah"}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: protobufResponseHistogram}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram}},
},
{
Labels: []mimirpb.LabelAdapter{{Name: "baz2", Value: "blah2"}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram2}},
},
},
},
Expand Down Expand Up @@ -499,7 +527,7 @@ var protobufCodecScenarios = []struct {
Series: []mimirpb.MatrixSeries{
{
Metric: []string{"name-1", "value-1", "name-2", "value-2"},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: protobufResponseHistogram}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram}},
},
},
},
Expand All @@ -512,7 +540,7 @@ var protobufCodecScenarios = []struct {
Result: []SampleStream{
{
Labels: []mimirpb.LabelAdapter{{Name: "name-1", Value: "value-1"}, {Name: "name-2", Value: "value-2"}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: protobufResponseHistogram}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram}},
},
},
},
Expand All @@ -529,7 +557,7 @@ var protobufCodecScenarios = []struct {
{
Metric: []string{"name-1", "value-1", "name-2", "value-2"},
Samples: []mimirpb.Sample{{TimestampMs: 1000, Value: 200}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: protobufResponseHistogram}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram}},
},
},
},
Expand All @@ -543,7 +571,7 @@ var protobufCodecScenarios = []struct {
{
Labels: []mimirpb.LabelAdapter{{Name: "name-1", Value: "value-1"}, {Name: "name-2", Value: "value-2"}},
Samples: []mimirpb.Sample{{TimestampMs: 1000, Value: 200}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: protobufResponseHistogram}},
Histograms: []mimirpb.FloatHistogramPair{{TimestampMs: 1234, Histogram: &protobufResponseHistogram}},
},
},
},
Expand Down
12 changes: 6 additions & 6 deletions pkg/frontend/querymiddleware/codec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,7 @@ func TestMergeAPIResponses(t *testing.T) {
{
Labels: []mimirpb.LabelAdapter{{Name: "a", Value: "b"}},
Histograms: []mimirpb.FloatHistogramPair{
{TimestampMs: 1000, Histogram: histogram1},
{TimestampMs: 1000, Histogram: &histogram1},
},
},
},
Expand All @@ -581,7 +581,7 @@ func TestMergeAPIResponses(t *testing.T) {
Histograms: []mimirpb.FloatHistogramPair{
{
TimestampMs: 1000,
Histogram: histogram1,
Histogram: &histogram1,
},
},
},
Expand All @@ -601,7 +601,7 @@ func TestMergeAPIResponses(t *testing.T) {
{
Labels: []mimirpb.LabelAdapter{{Name: "a", Value: "b"}},
Histograms: []mimirpb.FloatHistogramPair{
{TimestampMs: 1000, Histogram: histogram1},
{TimestampMs: 1000, Histogram: &histogram1},
},
},
},
Expand All @@ -615,7 +615,7 @@ func TestMergeAPIResponses(t *testing.T) {
{
Labels: []mimirpb.LabelAdapter{{Name: "a", Value: "b"}},
Histograms: []mimirpb.FloatHistogramPair{
{TimestampMs: 2000, Histogram: histogram2},
{TimestampMs: 2000, Histogram: &histogram2},
},
},
},
Expand All @@ -630,8 +630,8 @@ func TestMergeAPIResponses(t *testing.T) {
{
Labels: []mimirpb.LabelAdapter{{Name: "a", Value: "b"}},
Histograms: []mimirpb.FloatHistogramPair{
{TimestampMs: 1000, Histogram: histogram1},
{TimestampMs: 2000, Histogram: histogram2},
{TimestampMs: 1000, Histogram: &histogram1},
{TimestampMs: 2000, Histogram: &histogram2},
},
},
},
Expand Down
4 changes: 2 additions & 2 deletions pkg/frontend/querymiddleware/sharded_queryable_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,13 @@ func seriesSetToSampleStreams(set storage.SeriesSet) ([]SampleStream, error) {
case chunkenc.ValHistogram:
t, v := it.AtHistogram()
stream.Histograms = append(stream.Histograms, mimirpb.FloatHistogramPair{
Histogram: *mimirpb.FloatHistogramFromPrometheusModel(v.ToFloat()),
Histogram: mimirpb.FloatHistogramFromPrometheusModel(v.ToFloat()),
TimestampMs: t,
})
case chunkenc.ValFloatHistogram:
t, v := it.AtFloatHistogram()
stream.Histograms = append(stream.Histograms, mimirpb.FloatHistogramPair{
Histogram: *mimirpb.FloatHistogramFromPrometheusModel(v),
Histogram: mimirpb.FloatHistogramFromPrometheusModel(v),
TimestampMs: t,
})
default:
Expand Down
12 changes: 6 additions & 6 deletions pkg/frontend/querymiddleware/split_and_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestSplitAndCacheMiddleware_SplitByInterval(t *testing.T) {
mockProtobufResponseWithSamplesAndHistograms(seriesLabels, nil, []mimirpb.FloatHistogramPair{
{
TimestampMs: dayThreeStartTime.Unix() * 1000,
Histogram: thirdDayHistogram,
Histogram: &thirdDayHistogram,
},
}))

Expand All @@ -108,7 +108,7 @@ func TestSplitAndCacheMiddleware_SplitByInterval(t *testing.T) {
mockProtobufResponseWithSamplesAndHistograms(seriesLabels, nil, []mimirpb.FloatHistogramPair{
{
TimestampMs: dayFourEndTime.Unix() * 1000,
Histogram: fourthDayHistogram,
Histogram: &fourthDayHistogram,
},
}))

Expand All @@ -121,11 +121,11 @@ func TestSplitAndCacheMiddleware_SplitByInterval(t *testing.T) {
[]mimirpb.FloatHistogramPair{
{
TimestampMs: dayThreeStartTime.Unix() * 1000,
Histogram: thirdDayHistogram,
Histogram: &thirdDayHistogram,
},
{
TimestampMs: dayFourEndTime.Unix() * 1000,
Histogram: fourthDayHistogram,
Histogram: &fourthDayHistogram,
},
},
))
Expand Down Expand Up @@ -262,7 +262,7 @@ func TestSplitAndCacheMiddleware_ResultsCache(t *testing.T) {
Histograms: []mimirpb.FloatHistogramPair{
{
TimestampMs: 1634292000000,
Histogram: mimirpb.FloatHistogram{
Histogram: &mimirpb.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 3,
ZeroThreshold: 1.23,
Expand Down Expand Up @@ -370,7 +370,7 @@ func TestSplitAndCacheMiddleware_ResultsCache_ShouldNotLookupCacheIfStepIsNotAli
Histograms: []mimirpb.FloatHistogramPair{
{
TimestampMs: 1634292000000,
Histogram: mimirpb.FloatHistogram{
Histogram: &mimirpb.FloatHistogram{
CounterResetHint: histogram.GaugeType,
Schema: 3,
ZeroThreshold: 1.23,
Expand Down
40 changes: 6 additions & 34 deletions pkg/mimirpb/compat.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -279,12 +279,7 @@ func fromSpansProtoToSpans(s []BucketSpan) []histogram.Span {
if len(s) == 0 {
return nil
}
spans := make([]histogram.Span, len(s))
for i := 0; i < len(s); i++ {
spans[i] = histogram.Span{Offset: s[i].Offset, Length: s[i].Length}
}

return spans
return *(*[]histogram.Span)(unsafe.Pointer(&s))
}

func FromHistogramToHistogramProto(timestamp int64, h *histogram.Histogram) Histogram {
Expand Down Expand Up @@ -333,40 +328,17 @@ func fromSpansToSpansProto(s []histogram.Span) []BucketSpan {
if len(s) == 0 {
return nil
}
spans := make([]BucketSpan, len(s))
for i := 0; i < len(s); i++ {
spans[i] = BucketSpan{Offset: s[i].Offset, Length: s[i].Length}
}

return spans
return *(*[]BucketSpan)(unsafe.Pointer(&s))
}

// FromFPointsToSamples converts []promql.FPoint to []Sample.
// FromFPointsToSamples casts []promql.FPoint to []Sample. It uses unsafe.
func FromFPointsToSamples(points []promql.FPoint) []Sample {
samples := make([]Sample, 0, len(points))
for _, point := range points {
samples = append(samples, Sample{
TimestampMs: point.T,
Value: point.F,
})
}
return samples
return *(*[]Sample)(unsafe.Pointer(&points))
}

// FromHPointsToHistograms converts []promql.HPoint to []FloatHistogramPair.
// FromHPointsToHistograms converts []promql.HPoint to []FloatHistogramPair. It uses unsafe.
func FromHPointsToHistograms(points []promql.HPoint) []FloatHistogramPair {
samples := make([]FloatHistogramPair, 0, len(points))
for _, point := range points {
h := point.H
if h == nil {
continue
}
samples = append(samples, FloatHistogramPair{
TimestampMs: point.T,
Histogram: *FloatHistogramFromPrometheusModel(point.H),
})
}
return samples
return *(*[]FloatHistogramPair)(unsafe.Pointer(&points))
}

// FromFloatHistogramToPromHistogram converts histogram.FloatHistogram to model.SampleHistogram.
Expand Down
Loading

0 comments on commit b6e3fa6

Please sign in to comment.