Add histogram conversion test cases and test data #374
Conversation
| } | ||
|
|
||
| if dp.HasMax() { | ||
| if math.IsNaN(dp.Max()) { |
There was a problem hiding this comment.
consider making this a helper function to minimize duplication
| "go.opentelemetry.io/collector/pdata/pmetric" | ||
| ) | ||
|
|
||
| func TestCheckValidity(t *testing.T) { |
| var datasets []GeneratedDataset | ||
| for _, config := range configs { | ||
| // each dataset gets a copy of the same rand so that they don't interfere with each other | ||
| rng := rand.New(rand.NewPCG(seed, seed)) |
There was a problem hiding this comment.
How much randomness do we need and how significant is it? what happens if each config shares the same rand?
There was a problem hiding this comment.
What we want is to be able to generate data that reasonably resembles some data distribution. We want that generated data to be reliably repeatable so that we can write integration tests around it. We could hard code all of the values, but we decided to use a set seed on a random number generator instead. If the configs share the same rand, then they will interfere with each other making the generated data not reliably repeatable.
There was a problem hiding this comment.
Oh yeah, I'm not questioning why use seed but why each config would require a new rand. They are diff distributions already so having the same rand is fine?
| } | ||
|
|
||
| func gammaRandom(shape float64, rng *rand.Rand) float64 { | ||
| if shape < 1 { |
There was a problem hiding this comment.
is this written from scratch? some comments or a source will be helpful.
|
|
||
| assertOptionalFloat(t, "min", tc.Expected.Min, tc.Input.Min) | ||
| assertOptionalFloat(t, "max", tc.Expected.Max, tc.Input.Max) | ||
| assert.InDelta(t, tc.Expected.Average, tc.Input.Sum/float64(tc.Input.Count), 0.01) |
There was a problem hiding this comment.
is delta calc conservative or more releaxed? just asking since we have seen some flakiness with tight delta calcs before
There was a problem hiding this comment.
It's more relaxed than direct floating point comparisons, but its more conservative than a % diff check (e.g. (expected-actual)/actual).
| } | ||
|
|
||
| // Rest of checks only apply if we have boundaries/counts | ||
| if len(hi.Boundaries) > 0 || len(hi.Counts) > 0 { |
There was a problem hiding this comment.
maybe create local vars with lens of these since they are used a quite few in thie block
|
|
||
| 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. |
| // Copyright The OpenTelemetry Authors | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| package histograms // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/aws/cloudwatch/histograms" |
4 participants
Description
Creating test cases for testing the classic histogram mapping algorithm. This includes both raw datasets (simple values) for testing accuracy and synthetic histograms for testing correctness.
I put this in a new Go package under
pkg/awsas we want to use the mapping algorithm in both opentelemetry-collector-contrib and amazon-cloudwatch-agent.Testing
New unit tests
Unit tests for
receiver/elasticsearchreceiverare failing on GitHub runner (they pass locally):These changes should have no impact on that receiver as this PR introduces and entirely new, unreferenced module (outside of version.yaml).
The same test is failing for aws-cwa-dev as well: https://github.com/amazon-contributing/opentelemetry-collector-contrib/actions/runs/18651721544