Skip to content

Add a test for histogram conversion out-of-range#17

Open
ralongit wants to merge 2 commits intomasterfrom
tests/histogram-conversion
Open

Add a test for histogram conversion out-of-range#17
ralongit wants to merge 2 commits intomasterfrom
tests/histogram-conversion

Conversation

@ralongit
Copy link
Contributor

@ralongit ralongit commented Mar 16, 2025

Description

Test for index out of range [N] with length N panic that used to occur when converting histogram metric where bucket count and bounds are in the same length.

What type of PR is this?

(check all applicable)

  • 🍕 Feature
  • 🐛 Bug Fix
  • 🧑‍💻 Code Refactor
  • 🔥 Performance Improvements
  • ✅ Test
  • 🤖 Build / CI
  • ⏩ Revert

Added tests?

  • 👍 yes
  • 🙅 no, because they aren't needed
  • 🙋 no, because I need help from somebody

@ralongit ralongit requested review from 8naama and bardabun March 16, 2025 09:19
Comment on lines +342 to +379
func TestConvertFromHistogramIndexOutOfRange(t *testing.T) {
exporter := Exporter{
config: validConfig,
}

// Create a histogram metric with an invalid bucket count to simulate the out-of-range index
histogramMetric := &metricdata.ResourceMetrics{
ScopeMetrics: []metricdata.ScopeMetrics{
{
Metrics: []metricdata.Metrics{
{
Name: "test_histogram",
Data: metricdata.Histogram[int64]{
DataPoints: []metricdata.HistogramDataPoint[int64]{
{
Count: 3600,
Sum: 15,
BucketCounts: []uint64{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, // 15 buckets // 16 buckets
Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, // 15 bounds
},
},
},
},
},
},
},
}

err := func() (err error) {
defer func() {
if r := recover(); r != nil {
err = fmt.Errorf("panic occurred: %v", r)
}
}()
_, err = exporter.ConvertToTimeSeries(histogramMetric)
return err
}()

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be a test case of TestConvertToTimeSeries and not a standalone test function

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants