diff --git a/CHANGELOG.md b/CHANGELOG.md index 822d55bd053..b7537aa5242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `Int64UpDownCounter` replaces the `syncint64.UpDownCounter` - `Int64Histogram` replaces the `syncint64.Histogram` - Add `NewTracerProvider` to `go.opentelemetry.io/otel/bridge/opentracing` to create `WrapperTracer` instances from a `TracerProvider`. (#3316) +- The `Extrema` type to `go.opentelemetry.io/otel/sdk/metric/metricdata` to represent min/max values and still be able to distinguish unset and zero values. (#3487) - Add the `go.opentelemetry.io/otel/semconv/v1.17.0` package. The package contains semantic conventions from the `v1.17.0` version of the OpenTelemetry specification. (#3599) @@ -106,6 +107,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm The slice of `instrument.Asynchronous` parameter is now passed as a variadic argument. (#3587) - The `Callback` in `go.opentelemetry.io/otel/metric` has the added `Observer` parameter added. This new parameter is used by `Callback` implementations to observe values for asynchronous instruments instead of calling the `Observe` method of the instrument directly. (#3584) +- The `Min` and `Max` fields of the `HistogramDataPoint` in `go.opentelemetry.io/otel/sdk/metric/metricdata` are now defined with the added `Extrema` type instead of a `*float64`. (#3487) ### Fixed diff --git a/exporters/otlp/otlpmetric/internal/transform/metricdata.go b/exporters/otlp/otlpmetric/internal/transform/metricdata.go index 4d6edc90330..d952f8b06be 100644 --- a/exporters/otlp/otlpmetric/internal/transform/metricdata.go +++ b/exporters/otlp/otlpmetric/internal/transform/metricdata.go @@ -174,7 +174,7 @@ func HistogramDataPoints(dPts []metricdata.HistogramDataPoint) []*mpb.HistogramD out := make([]*mpb.HistogramDataPoint, 0, len(dPts)) for _, dPt := range dPts { sum := dPt.Sum - out = append(out, &mpb.HistogramDataPoint{ + hdp := &mpb.HistogramDataPoint{ Attributes: AttrIter(dPt.Attributes.Iter()), StartTimeUnixNano: uint64(dPt.StartTime.UnixNano()), TimeUnixNano: uint64(dPt.Time.UnixNano()), @@ -182,9 +182,14 @@ func HistogramDataPoints(dPts []metricdata.HistogramDataPoint) []*mpb.HistogramD Sum: &sum, BucketCounts: dPt.BucketCounts, ExplicitBounds: dPt.Bounds, - Min: dPt.Min, - Max: dPt.Max, - }) + } + if v, ok := dPt.Min.Value(); ok { + hdp.Min = &v + } + if v, ok := dPt.Max.Value(); ok { + hdp.Max = &v + } + out = append(out, hdp) } return out } diff --git a/exporters/otlp/otlpmetric/internal/transform/metricdata_test.go b/exporters/otlp/otlpmetric/internal/transform/metricdata_test.go index 60773a92eaf..7241a5cf76c 100644 --- a/exporters/otlp/otlpmetric/internal/transform/metricdata_test.go +++ b/exporters/otlp/otlpmetric/internal/transform/metricdata_test.go @@ -60,8 +60,8 @@ var ( Count: 30, Bounds: []float64{1, 5}, BucketCounts: []uint64{0, 30, 0}, - Min: &minA, - Max: &maxA, + Min: metricdata.NewExtrema(minA), + Max: metricdata.NewExtrema(maxA), Sum: sumA, }, { Attributes: bob, @@ -70,8 +70,8 @@ var ( Count: 3, Bounds: []float64{1, 5}, BucketCounts: []uint64{0, 1, 2}, - Min: &minB, - Max: &maxB, + Min: metricdata.NewExtrema(minB), + Max: metricdata.NewExtrema(maxB), Sum: sumB, }} diff --git a/sdk/metric/internal/histogram.go b/sdk/metric/internal/histogram.go index 058af8419b1..fb4f4d47db1 100644 --- a/sdk/metric/internal/histogram.go +++ b/sdk/metric/internal/histogram.go @@ -156,8 +156,8 @@ func (s *deltaHistogram[N]) Aggregation() metricdata.Aggregation { Sum: b.sum, } if !s.noMinMax { - hdp.Min = &b.min - hdp.Max = &b.max + hdp.Min = metricdata.NewExtrema(b.min) + hdp.Max = metricdata.NewExtrema(b.max) } h.DataPoints = append(h.DataPoints, hdp) @@ -227,10 +227,8 @@ func (s *cumulativeHistogram[N]) Aggregation() metricdata.Aggregation { Sum: b.sum, } if !s.noMinMax { - // Similar to counts, make a copy. - min, max := b.min, b.max - hdp.Min = &min - hdp.Max = &max + hdp.Min = metricdata.NewExtrema(b.min) + hdp.Max = metricdata.NewExtrema(b.max) } h.DataPoints = append(h.DataPoints, hdp) // TODO (#3006): This will use an unbounded amount of memory if there diff --git a/sdk/metric/internal/histogram_test.go b/sdk/metric/internal/histogram_test.go index fb2968fc325..57f04560a16 100644 --- a/sdk/metric/internal/histogram_test.go +++ b/sdk/metric/internal/histogram_test.go @@ -92,8 +92,8 @@ func hPoint(a attribute.Set, v float64, multi uint64) metricdata.HistogramDataPo Count: multi, Bounds: bounds, BucketCounts: counts, - Min: &v, - Max: &v, + Min: metricdata.NewExtrema(v), + Max: metricdata.NewExtrema(v), Sum: v * float64(multi), } } diff --git a/sdk/metric/meter_test.go b/sdk/metric/meter_test.go index a5248bedfb0..154ae924fd5 100644 --- a/sdk/metric/meter_test.go +++ b/sdk/metric/meter_test.go @@ -169,8 +169,8 @@ func TestCallbackUnregisterConcurrency(t *testing.T) { // Instruments should produce correct ResourceMetrics. func TestMeterCreatesInstruments(t *testing.T) { + extrema := metricdata.NewExtrema(7.) attrs := []attribute.KeyValue{attribute.String("name", "alice")} - seven := 7.0 testCases := []struct { name string fn func(*testing.T, metric.Meter) @@ -391,8 +391,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Count: 1, Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, - Min: &seven, - Max: &seven, + Min: extrema, + Max: extrema, Sum: 7.0, }, }, @@ -455,8 +455,8 @@ func TestMeterCreatesInstruments(t *testing.T) { Count: 1, Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, BucketCounts: []uint64{0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, - Min: &seven, - Max: &seven, + Min: extrema, + Max: extrema, Sum: 7.0, }, }, @@ -882,8 +882,6 @@ func TestAttributeFilter(t *testing.T) { } func testAttributeFilter(temporality metricdata.Temporality) func(*testing.T) { - one := 1.0 - two := 2.0 testcases := []struct { name string register func(t *testing.T, mtr metric.Meter) error @@ -1130,8 +1128,8 @@ func testAttributeFilter(temporality metricdata.Temporality) func(*testing.T) { Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, BucketCounts: []uint64{0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, Count: 2, - Min: &one, - Max: &two, + Min: metricdata.NewExtrema(1.), + Max: metricdata.NewExtrema(2.), Sum: 3.0, }, }, @@ -1212,8 +1210,8 @@ func testAttributeFilter(temporality metricdata.Temporality) func(*testing.T) { Bounds: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}, BucketCounts: []uint64{0, 2, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}, Count: 2, - Min: &one, - Max: &two, + Min: metricdata.NewExtrema(1.), + Max: metricdata.NewExtrema(2.), Sum: 3.0, }, }, diff --git a/sdk/metric/metricdata/data.go b/sdk/metric/metricdata/data.go index a729879e673..cd882c0bd2d 100644 --- a/sdk/metric/metricdata/data.go +++ b/sdk/metric/metricdata/data.go @@ -122,9 +122,26 @@ type HistogramDataPoint struct { BucketCounts []uint64 // Min is the minimum value recorded. (optional) - Min *float64 `json:",omitempty"` + Min Extrema // Max is the maximum value recorded. (optional) - Max *float64 `json:",omitempty"` + Max Extrema // Sum is the sum of the values recorded. Sum float64 } + +// Extrema is the minimum or maximum value of a dataset. +type Extrema struct { + value float64 + valid bool +} + +// NewExtrema returns an Extrema set to v. +func NewExtrema(v float64) Extrema { + return Extrema{value: v, valid: true} +} + +// Value returns the Extrema value and true if the Extrema is defined. +// Otherwise, if the Extrema is its zero-value, defined will be false. +func (e Extrema) Value() (v float64, defined bool) { + return e.value, e.valid +} diff --git a/sdk/metric/metricdata/metricdatatest/assertion.go b/sdk/metric/metricdata/metricdatatest/assertion.go index 193be1ff708..d1ad1e70195 100644 --- a/sdk/metric/metricdata/metricdatatest/assertion.go +++ b/sdk/metric/metricdata/metricdatatest/assertion.go @@ -32,6 +32,7 @@ type Datatypes interface { metricdata.Gauge[int64] | metricdata.Histogram | metricdata.HistogramDataPoint | + metricdata.Extrema | metricdata.Metrics | metricdata.ResourceMetrics | metricdata.ScopeMetrics | @@ -92,6 +93,8 @@ func AssertEqual[T Datatypes](t *testing.T, expected, actual T, opts ...Option) r = equalHistograms(e, aIface.(metricdata.Histogram), cfg) case metricdata.HistogramDataPoint: r = equalHistogramDataPoints(e, aIface.(metricdata.HistogramDataPoint), cfg) + case metricdata.Extrema: + r = equalExtrema(e, aIface.(metricdata.Extrema), cfg) case metricdata.Metrics: r = equalMetrics(e, aIface.(metricdata.Metrics), cfg) case metricdata.ResourceMetrics: @@ -152,6 +155,8 @@ func AssertHasAttributes[T Datatypes](t *testing.T, actual T, attrs ...attribute reasons = hasAttributesSum(e, attrs...) case metricdata.HistogramDataPoint: reasons = hasAttributesHistogramDataPoints(e, attrs...) + case metricdata.Extrema: + // Nothing to check. case metricdata.Histogram: reasons = hasAttributesHistogram(e, attrs...) case metricdata.Metrics: diff --git a/sdk/metric/metricdata/metricdatatest/assertion_test.go b/sdk/metric/metricdata/metricdatatest/assertion_test.go index b7da4344353..7056ee71293 100644 --- a/sdk/metric/metricdata/metricdatatest/assertion_test.go +++ b/sdk/metric/metricdata/metricdatatest/assertion_test.go @@ -73,7 +73,10 @@ var ( Value: -1.0, } - max, min = 99.0, 3. + minA = metricdata.NewExtrema(-1.) + minB, maxB = metricdata.NewExtrema(3.), metricdata.NewExtrema(99.) + minC = metricdata.NewExtrema(-1.) + histogramDataPointA = metricdata.HistogramDataPoint{ Attributes: attrA, StartTime: startA, @@ -81,6 +84,7 @@ var ( Count: 2, Bounds: []float64{0, 10}, BucketCounts: []uint64{1, 1}, + Min: minA, Sum: 2, } histogramDataPointB = metricdata.HistogramDataPoint{ @@ -90,8 +94,8 @@ var ( Count: 3, Bounds: []float64{0, 10, 100}, BucketCounts: []uint64{1, 1, 1}, - Max: &max, - Min: &min, + Max: maxB, + Min: minB, Sum: 3, } histogramDataPointC = metricdata.HistogramDataPoint{ @@ -101,6 +105,7 @@ var ( Count: 2, Bounds: []float64{0, 10}, BucketCounts: []uint64{1, 1}, + Min: minC, Sum: 2, } @@ -247,6 +252,7 @@ func TestAssertEqual(t *testing.T) { t.Run("HistogramDataPoint", testDatatype(histogramDataPointA, histogramDataPointB, equalHistogramDataPoints)) t.Run("DataPointInt64", testDatatype(dataPointInt64A, dataPointInt64B, equalDataPoints[int64])) t.Run("DataPointFloat64", testDatatype(dataPointFloat64A, dataPointFloat64B, equalDataPoints[float64])) + t.Run("Extrema", testDatatype(minA, minB, equalExtrema)) } func TestAssertEqualIgnoreTime(t *testing.T) { @@ -261,6 +267,7 @@ func TestAssertEqualIgnoreTime(t *testing.T) { t.Run("HistogramDataPoint", testDatatypeIgnoreTime(histogramDataPointA, histogramDataPointC, equalHistogramDataPoints)) t.Run("DataPointInt64", testDatatypeIgnoreTime(dataPointInt64A, dataPointInt64C, equalDataPoints[int64])) t.Run("DataPointFloat64", testDatatypeIgnoreTime(dataPointFloat64A, dataPointFloat64C, equalDataPoints[float64])) + t.Run("Extrema", testDatatypeIgnoreTime(minA, minC, equalExtrema)) } type unknownAggregation struct { @@ -316,6 +323,7 @@ func TestAssertAggregationsEqual(t *testing.T) { } func TestAssertAttributes(t *testing.T) { + AssertHasAttributes(t, minA, attribute.Bool("A", true)) // No-op, always pass. AssertHasAttributes(t, dataPointInt64A, attribute.Bool("A", true)) AssertHasAttributes(t, dataPointFloat64A, attribute.Bool("A", true)) AssertHasAttributes(t, gaugeInt64A, attribute.Bool("A", true)) diff --git a/sdk/metric/metricdata/metricdatatest/comparisons.go b/sdk/metric/metricdata/metricdatatest/comparisons.go index 9879c96f109..a6bc83f6b7d 100644 --- a/sdk/metric/metricdata/metricdatatest/comparisons.go +++ b/sdk/metric/metricdata/metricdatatest/comparisons.go @@ -267,10 +267,10 @@ func equalHistogramDataPoints(a, b metricdata.HistogramDataPoint, cfg config) (r if !equalSlices(a.BucketCounts, b.BucketCounts) { reasons = append(reasons, notEqualStr("BucketCounts", a.BucketCounts, b.BucketCounts)) } - if !equalPtrValues(a.Min, b.Min) { + if !eqExtrema(a.Min, b.Min) { reasons = append(reasons, notEqualStr("Min", a.Min, b.Min)) } - if !equalPtrValues(a.Max, b.Max) { + if !eqExtrema(a.Max, b.Max) { reasons = append(reasons, notEqualStr("Max", a.Max, b.Max)) } if a.Sum != b.Sum { @@ -295,12 +295,21 @@ func equalSlices[T comparable](a, b []T) bool { return true } -func equalPtrValues[T comparable](a, b *T) bool { - if a == nil || b == nil { - return a == b +func equalExtrema(a, b metricdata.Extrema, _ config) (reasons []string) { + if !eqExtrema(a, b) { + reasons = append(reasons, notEqualStr("Extrema", a, b)) } + return reasons +} - return *a == *b +func eqExtrema(a, b metricdata.Extrema) bool { + aV, aOk := a.Value() + bV, bOk := b.Value() + + if !aOk || !bOk { + return aOk == bOk + } + return aV == bV } func diffSlices[T any](a, b []T, equal func(T, T) bool) (extraA, extraB []T) {