-
Notifications
You must be signed in to change notification settings - Fork 34
Add histogram conversion test cases and test data #374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fac3493
0c54bd0
054efd0
6397f24
eb1b071
181850d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| include ../../Makefile.Common |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package cloudwatch // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/aws/cloudwatch" | ||
|
|
||
| // HistogramDataPoint is a single data point that describes the values of a Histogram. | ||
| // The values of a histogram are represented by equal-length series of values and counts. Each value/count pair | ||
| // is a single bucket of the Histogram. | ||
| type HistogramDataPoint interface { | ||
| ValuesAndCounts() ([]float64, []float64) | ||
| Sum() float64 | ||
| SampleCount() float64 | ||
| Minimum() float64 | ||
| Maximum() float64 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package histograms // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/aws/cloudwatch/histograms" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ultra small Nit: excessive comments? |
||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "math" | ||
|
|
||
| "go.opentelemetry.io/collector/pdata/pmetric" | ||
| ) | ||
|
|
||
| func CheckValidity(dp pmetric.HistogramDataPoint) error { | ||
| errs := []error{} | ||
|
|
||
| bounds := dp.ExplicitBounds() | ||
| bucketCounts := dp.BucketCounts() | ||
|
|
||
| // Check counts length matches boundaries + 1 | ||
| // special case: no bucketCounts and no boundaries is still valid | ||
| if bucketCounts.Len() != bounds.Len()+1 && bucketCounts.Len() != 0 && bounds.Len() != 0 { | ||
| errs = append(errs, fmt.Errorf("bucket counts length (%d) doesn't match boundaries length (%d) + 1", | ||
| bucketCounts.Len(), bounds.Len())) | ||
| } | ||
|
|
||
| if dp.HasMax() && dp.HasMin() && dp.Min() > dp.Max() { | ||
| errs = append(errs, fmt.Errorf("min %f is greater than max %f", dp.Min(), dp.Max())) | ||
| } | ||
|
|
||
| if dp.HasMax() { | ||
| errs = append(errs, checkNanInf(dp.Max(), "max")) | ||
| } | ||
| if dp.HasMin() { | ||
| errs = append(errs, checkNanInf(dp.Min(), "min")) | ||
| } | ||
| if dp.HasSum() { | ||
| errs = append(errs, checkNanInf(dp.Sum(), "sum")) | ||
| } | ||
|
|
||
| if bounds.Len() > 0 { | ||
| // Check boundaries are in ascending order | ||
| for i := 1; i < bounds.Len(); i++ { | ||
Paramadon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if bounds.At(i) <= bounds.At(i-1) { | ||
| errs = append(errs, fmt.Errorf("boundaries not in ascending order: bucket index %d (%v) <= bucket index %d %v", | ||
| i, bounds.At(i), i-1, bounds.At(i-1))) | ||
| } | ||
| errs = append(errs, checkNanInf(bounds.At(i), fmt.Sprintf("boundary %d", i))) | ||
| } | ||
| } | ||
|
|
||
| return errors.Join(errs...) | ||
| } | ||
|
|
||
| func checkNanInf(value float64, name string) error { | ||
| errs := []error{} | ||
| if math.IsNaN(value) { | ||
| errs = append(errs, errors.New(name+" is NaN")) | ||
| } | ||
| if math.IsInf(value, 0) { | ||
| errs = append(errs, errors.New(name+" is +/-inf")) | ||
| } | ||
| return errors.Join(errs...) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,196 @@ | ||
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package histograms | ||
|
|
||
| import ( | ||
| "math" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "go.opentelemetry.io/collector/pdata/pmetric" | ||
| ) | ||
|
|
||
| func TestCheckValidity(t *testing.T) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could add test cases for min/max There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, will add. |
||
| tests := []struct { | ||
| name string | ||
| dp pmetric.HistogramDataPoint | ||
| valid bool | ||
| }{ | ||
| { | ||
| name: "Boundaries Not Ascending", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 40, 100, 150}) // 40 < 50 | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "Counts Length Mismatch", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 75, 100}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) // Should be 5 counts for 4 boundaries | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "min greater than max", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(200.0) | ||
| dp.SetMax(10.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 70, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "Inf min", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(math.Inf(-1)) | ||
| dp.SetMax(10.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 70, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "NaN min", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(math.NaN()) | ||
| dp.SetMax(10.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 70, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "Inf max", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(math.Inf(1)) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 70, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "NaN max", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(math.NaN()) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 70, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "NaN Sum", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(math.NaN()) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 75, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "Inf Sum", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(math.Inf(1)) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 75, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "NaN Boundary", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, math.NaN(), 75, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| { | ||
| name: "Inf Boundary", | ||
| dp: func() pmetric.HistogramDataPoint { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, math.Inf(1), 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
| return dp | ||
| }(), | ||
| valid: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| assert.Error(t, CheckValidity(tt.dp)) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func BenchmarkCheckValidity(b *testing.B) { | ||
| dp := pmetric.NewHistogramDataPoint() | ||
| dp.SetCount(100) | ||
| dp.SetSum(5000) | ||
| dp.SetMin(10.0) | ||
| dp.SetMax(200.0) | ||
| dp.ExplicitBounds().FromRaw([]float64{25, 50, 75, 100, 150}) | ||
| dp.BucketCounts().FromRaw([]uint64{20, 30, 25, 15, 8, 2}) | ||
|
|
||
| b.ResetTimer() | ||
| for i := 0; i < b.N; i++ { | ||
| assert.NoError(b, CheckValidity(dp)) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Block quotes are more readable