diff --git a/.chloggen/42163.yaml b/.chloggen/42163.yaml new file mode 100644 index 0000000000000..2029fee4662b1 --- /dev/null +++ b/.chloggen/42163.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: "bug_fix" + +# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog) +component: "processor/deltatocumulative" + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: "Fix panic when processing exponential histograms with empty bucket counts" + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [42163] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [user] diff --git a/processor/deltatocumulativeprocessor/internal/data/expo/scale.go b/processor/deltatocumulativeprocessor/internal/data/expo/scale.go index 50fdef75c9f65..693860b55c5ab 100644 --- a/processor/deltatocumulativeprocessor/internal/data/expo/scale.go +++ b/processor/deltatocumulativeprocessor/internal/data/expo/scale.go @@ -73,20 +73,27 @@ func Downscale(bs Buckets, from, to Scale) { // area at a later time. func Collapse(bs Buckets) { counts := bs.BucketCounts() + + offsetWasOdd := bs.Offset()%2 != 0 + shift := 0 + if offsetWasOdd { + shift-- + } + size := counts.Len() / 2 - if counts.Len()%2 != 0 || bs.Offset()%2 != 0 { + if counts.Len()%2 != 0 || offsetWasOdd { size++ } - // merging needs to happen in pairs aligned to i=0. if offset is non-even, - // we need to shift the whole merging by one to make above condition true. - shift := 0 - if bs.Offset()%2 != 0 { + if offsetWasOdd { bs.SetOffset(bs.Offset() - 1) - shift-- } bs.SetOffset(bs.Offset() / 2) + if counts.Len() == 0 { + return + } + for i := 0; i < size; i++ { // size is ~half of len. we add two buckets per iteration. // k jumps in steps of 2, shifted if offset makes this necessary. diff --git a/processor/deltatocumulativeprocessor/internal/data/expo/scale_test.go b/processor/deltatocumulativeprocessor/internal/data/expo/scale_test.go index ff8863a199603..ea8e03a5f7877 100644 --- a/processor/deltatocumulativeprocessor/internal/data/expo/scale_test.go +++ b/processor/deltatocumulativeprocessor/internal/data/expo/scale_test.go @@ -101,4 +101,78 @@ func TestDownscale(t *testing.T) { expo.Downscale(bins{}.Into(), 8, 12) }) }) + + t.Run("empty-buckets", func(t *testing.T) { + t.Run("odd-offset", func(t *testing.T) { + buckets := pmetric.NewExponentialHistogramDataPointBuckets() + buckets.SetOffset(1) + + assert.NotPanics(t, func() { + expo.Downscale(buckets, 2, 1) + }) + + assert.Equal(t, int32(0), buckets.Offset()) + assert.Equal(t, 0, buckets.BucketCounts().Len()) + }) + + t.Run("even-offset", func(t *testing.T) { + buckets := pmetric.NewExponentialHistogramDataPointBuckets() + buckets.SetOffset(2) + + assert.NotPanics(t, func() { + expo.Downscale(buckets, 2, 1) + }) + + assert.Equal(t, int32(1), buckets.Offset()) + assert.Equal(t, 0, buckets.BucketCounts().Len()) + }) + + t.Run("zero-offset", func(t *testing.T) { + buckets := pmetric.NewExponentialHistogramDataPointBuckets() + buckets.SetOffset(0) + + assert.NotPanics(t, func() { + expo.Downscale(buckets, 2, 1) + }) + + assert.Equal(t, int32(0), buckets.Offset()) + assert.Equal(t, 0, buckets.BucketCounts().Len()) + }) + + t.Run("negative-offset-odd", func(t *testing.T) { + buckets := pmetric.NewExponentialHistogramDataPointBuckets() + buckets.SetOffset(-3) + + assert.NotPanics(t, func() { + expo.Downscale(buckets, 2, 1) + }) + + assert.Equal(t, int32(-2), buckets.Offset()) + assert.Equal(t, 0, buckets.BucketCounts().Len()) + }) + + t.Run("negative-offset-even", func(t *testing.T) { + buckets := pmetric.NewExponentialHistogramDataPointBuckets() + buckets.SetOffset(-4) + + assert.NotPanics(t, func() { + expo.Downscale(buckets, 2, 1) + }) + + assert.Equal(t, int32(-2), buckets.Offset()) + assert.Equal(t, 0, buckets.BucketCounts().Len()) + }) + + t.Run("multiple-scales", func(t *testing.T) { + buckets := pmetric.NewExponentialHistogramDataPointBuckets() + buckets.SetOffset(7) + + assert.NotPanics(t, func() { + expo.Downscale(buckets, 5, 0) + }) + + assert.Equal(t, int32(0), buckets.Offset()) + assert.Equal(t, 0, buckets.BucketCounts().Len()) + }) + }) } diff --git a/processor/deltatocumulativeprocessor/internal/data/expo_test.go b/processor/deltatocumulativeprocessor/internal/data/expo_test.go index bcbb77d43fd99..9a32615bd468a 100644 --- a/processor/deltatocumulativeprocessor/internal/data/expo_test.go +++ b/processor/deltatocumulativeprocessor/internal/data/expo_test.go @@ -212,6 +212,85 @@ func TestExpoAdd(t *testing.T) { } t.Run(cs.name, run(cs.dp, cs.in, cs.want)) } + + t.Run("empty-buckets", func(t *testing.T) { + var add Adder + + t.Run("both-empty", func(t *testing.T) { + state := expdp{ + Scale: 2, + PosNeg: pmetric.NewExponentialHistogramDataPointBuckets(), + }.Into() + state.Positive().SetOffset(1) + + in := expdp{ + Scale: 2, + PosNeg: pmetric.NewExponentialHistogramDataPointBuckets(), + }.Into() + + assert.NotPanics(t, func() { + err := add.Exponential(state, in) + assert.NoError(t, err) + }) + }) + + t.Run("state-empty-in-has-data", func(t *testing.T) { + state := expdp{ + Scale: 1, + PosNeg: pmetric.NewExponentialHistogramDataPointBuckets(), + }.Into() + state.Positive().SetOffset(3) + + in := expdp{ + Scale: 1, + PosNeg: bins{ø, ø, ø, 1, 2, 3, ø, ø}.Into(), + Count: 2 * (1 + 2 + 3), + }.Into() + + assert.NotPanics(t, func() { + err := add.Exponential(state, in) + assert.NoError(t, err) + }) + }) + + t.Run("in-empty-state-has-data", func(t *testing.T) { + state := expdp{ + Scale: 1, + PosNeg: bins{ø, ø, ø, 1, 2, 3, ø, ø}.Into(), + Count: 2 * (1 + 2 + 3), + }.Into() + + in := expdp{ + Scale: 1, + PosNeg: pmetric.NewExponentialHistogramDataPointBuckets(), + }.Into() + in.Positive().SetOffset(3) + + assert.NotPanics(t, func() { + err := add.Exponential(state, in) + assert.NoError(t, err) + }) + }) + + t.Run("different-scales-with-empty", func(t *testing.T) { + state := expdp{ + Scale: 3, + PosNeg: pmetric.NewExponentialHistogramDataPointBuckets(), + }.Into() + state.Positive().SetOffset(5) + + in := expdp{ + Scale: 1, + PosNeg: pmetric.NewExponentialHistogramDataPointBuckets(), + }.Into() + in.Positive().SetOffset(7) + + assert.NotPanics(t, func() { + err := add.Exponential(state, in) + assert.NoError(t, err) + }) + }) + }) } func cloneNegExpdp(dp expotest.Histogram) expotest.Histogram {