fix: correct prev-tracking in V2 histogram encoders for interleaved custom-bucket histograms

Co-authored-by: ywwg <888940+ywwg@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2026-03-03 18:18:16 +00:00
parent 52d2f2af56
commit f1616df782
2 changed files with 138 additions and 40 deletions

View file

@ -1119,27 +1119,39 @@ func (*Encoder) histogramSamplesV2(histograms []RefHistogramSample, b []byte) ([
var customBucketHistograms []RefHistogramSample
first := histograms[0]
// First sample: full varint values, no deltas, no ST marker.
if first.H.UsesCustomBuckets() {
customBucketHistograms = append(customBucketHistograms, first)
} else {
buf.PutVarint64(int64(first.Ref))
buf.PutVarint64(first.T)
buf.PutVarint64(first.ST)
EncodeHistogram(&buf, first.H)
}
// Subsequent samples: ref delta to prev, T delta to first, ST marker.
for i := 1; i < len(histograms); i++ {
h := histograms[i]
// Find the first non-custom-bucket histogram to use as anchor for deltas.
firstIdx := -1
var first RefHistogramSample
for i, h := range histograms {
if h.H.UsesCustomBuckets() {
customBucketHistograms = append(customBucketHistograms, h)
continue
}
prev := histograms[i-1]
firstIdx = i
first = h
break
}
if firstIdx == -1 {
// All histograms use custom buckets.
buf.Reset()
return buf.Get(), customBucketHistograms
}
// First non-custom-bucket sample: full varint values, no deltas, no ST marker.
buf.PutVarint64(int64(first.Ref))
buf.PutVarint64(first.T)
buf.PutVarint64(first.ST)
EncodeHistogram(&buf, first.H)
// Remaining samples: ref delta to last encoded, T delta to first, ST marker.
// prev tracks the last encoded non-custom-bucket histogram.
prev := first
for _, h := range histograms[firstIdx+1:] {
if h.H.UsesCustomBuckets() {
customBucketHistograms = append(customBucketHistograms, h)
continue
}
buf.PutVarint64(int64(h.Ref) - int64(prev.Ref))
buf.PutVarint64(h.T - first.T)
@ -1153,11 +1165,7 @@ func (*Encoder) histogramSamplesV2(histograms []RefHistogramSample, b []byte) ([
buf.PutVarint64(h.ST - first.ST)
}
EncodeHistogram(&buf, h.H)
}
// Reset buffer if only custom bucket histograms existed in list of histogram samples.
if len(histograms) == len(customBucketHistograms) {
buf.Reset()
prev = h
}
return buf.Get(), customBucketHistograms
@ -1325,25 +1333,39 @@ func (*Encoder) floatHistogramSamplesV2(histograms []RefFloatHistogramSample, b
var customBucketsFloatHistograms []RefFloatHistogramSample
first := histograms[0]
if first.FH.UsesCustomBuckets() {
customBucketsFloatHistograms = append(customBucketsFloatHistograms, first)
} else {
buf.PutVarint64(int64(first.Ref))
buf.PutVarint64(first.T)
buf.PutVarint64(first.ST)
EncodeFloatHistogram(&buf, first.FH)
}
for i := 1; i < len(histograms); i++ {
h := histograms[i]
// Find the first non-custom-bucket histogram to use as anchor for deltas.
firstIdx := -1
var first RefFloatHistogramSample
for i, h := range histograms {
if h.FH.UsesCustomBuckets() {
customBucketsFloatHistograms = append(customBucketsFloatHistograms, h)
continue
}
prev := histograms[i-1]
firstIdx = i
first = h
break
}
if firstIdx == -1 {
// All histograms use custom buckets.
buf.Reset()
return buf.Get(), customBucketsFloatHistograms
}
// First non-custom-bucket sample: full varint values, no deltas, no ST marker.
buf.PutVarint64(int64(first.Ref))
buf.PutVarint64(first.T)
buf.PutVarint64(first.ST)
EncodeFloatHistogram(&buf, first.FH)
// Remaining samples: ref delta to last encoded, T delta to first, ST marker.
// prev tracks the last encoded non-custom-bucket histogram.
prev := first
for _, h := range histograms[firstIdx+1:] {
if h.FH.UsesCustomBuckets() {
customBucketsFloatHistograms = append(customBucketsFloatHistograms, h)
continue
}
buf.PutVarint64(int64(h.Ref) - int64(prev.Ref))
buf.PutVarint64(h.T - first.T)
@ -1357,11 +1379,7 @@ func (*Encoder) floatHistogramSamplesV2(histograms []RefFloatHistogramSample, b
buf.PutVarint64(h.ST - first.ST)
}
EncodeFloatHistogram(&buf, h.FH)
}
// Reset buffer if only custom bucket histograms existed in list of histogram samples
if len(histograms) == len(customBucketsFloatHistograms) {
buf.Reset()
prev = h
}
return buf.Get(), customBucketsFloatHistograms

View file

@ -600,6 +600,86 @@ func TestRecord_EncodeDecode(t *testing.T) {
require.Equal(t, int64(0), h.ST, "V1 float histogram records must decode with ST=0")
}
})
enc = Encoder{EnableSTStorage: true}
// V2 int-histogram with custom bucket as first element.
t.Run("V2 int-histogram leading custom bucket", func(t *testing.T) {
// histograms[2] uses custom buckets; placing it first exercises the
// code path where the anchor must be the first non-custom-bucket sample.
input := []RefHistogramSample{
{Ref: 67, T: 1000, ST: 500, H: histograms[2].H},
{Ref: 56, T: 2000, ST: 500, H: histograms[0].H},
{Ref: 42, T: 3000, ST: 600, H: histograms[1].H},
}
histBuf, customBuckets := enc.HistogramSamples(input, nil)
customBuf := enc.CustomBucketsHistogramSamples(customBuckets, nil)
decHists, err := dec.HistogramSamples(histBuf, nil)
require.NoError(t, err)
decCustom, err := dec.HistogramSamples(customBuf, nil)
require.NoError(t, err)
require.Equal(t, input[1:], decHists)
require.Equal(t, input[:1], decCustom)
})
// V2 int-histogram with custom bucket in the middle.
t.Run("V2 int-histogram interleaved custom bucket", func(t *testing.T) {
// histograms[2] uses custom buckets; placing it between regular histograms
// exercises the prev-tracking fix so ref deltas are computed correctly.
input := []RefHistogramSample{
{Ref: 56, T: 1000, ST: 500, H: histograms[0].H},
{Ref: 67, T: 2000, ST: 500, H: histograms[2].H},
{Ref: 42, T: 3000, ST: 600, H: histograms[1].H},
}
histBuf, customBuckets := enc.HistogramSamples(input, nil)
customBuf := enc.CustomBucketsHistogramSamples(customBuckets, nil)
decHists, err := dec.HistogramSamples(histBuf, nil)
require.NoError(t, err)
decCustom, err := dec.HistogramSamples(customBuf, nil)
require.NoError(t, err)
require.Equal(t, []RefHistogramSample{input[0], input[2]}, decHists)
require.Equal(t, input[1:2], decCustom)
})
// V2 float-histogram with custom bucket as first element.
t.Run("V2 float-histogram leading custom bucket", func(t *testing.T) {
customFH := histograms[2].H.ToFloat(nil)
regularFH0 := histograms[0].H.ToFloat(nil)
regularFH1 := histograms[1].H.ToFloat(nil)
input := []RefFloatHistogramSample{
{Ref: 67, T: 1000, ST: 500, FH: customFH},
{Ref: 56, T: 2000, ST: 500, FH: regularFH0},
{Ref: 42, T: 3000, ST: 600, FH: regularFH1},
}
floatBuf, customBuckets := enc.FloatHistogramSamples(input, nil)
customBuf := enc.CustomBucketsFloatHistogramSamples(customBuckets, nil)
decFloats, err := dec.FloatHistogramSamples(floatBuf, nil)
require.NoError(t, err)
decCustom, err := dec.FloatHistogramSamples(customBuf, nil)
require.NoError(t, err)
require.Equal(t, input[1:], decFloats)
require.Equal(t, input[:1], decCustom)
})
// V2 float-histogram with custom bucket in the middle.
t.Run("V2 float-histogram interleaved custom bucket", func(t *testing.T) {
customFH := histograms[2].H.ToFloat(nil)
regularFH0 := histograms[0].H.ToFloat(nil)
regularFH1 := histograms[1].H.ToFloat(nil)
input := []RefFloatHistogramSample{
{Ref: 56, T: 1000, ST: 500, FH: regularFH0},
{Ref: 67, T: 2000, ST: 500, FH: customFH},
{Ref: 42, T: 3000, ST: 600, FH: regularFH1},
}
floatBuf, customBuckets := enc.FloatHistogramSamples(input, nil)
customBuf := enc.CustomBucketsFloatHistogramSamples(customBuckets, nil)
decFloats, err := dec.FloatHistogramSamples(floatBuf, nil)
require.NoError(t, err)
decCustom, err := dec.FloatHistogramSamples(customBuf, nil)
require.NoError(t, err)
require.Equal(t, []RefFloatHistogramSample{input[0], input[2]}, decFloats)
require.Equal(t, input[1:2], decCustom)
})
}
func TestRecord_DecodeInvalidHistogramSchema(t *testing.T) {