Use Extrema type for Histogram min/max (#3550)

* Use Extrema type for Histogram min/max

* Add case for Extrema in AssertHasAttributes

* Add changes to changelog

* Add NewExtrema

* Add metricdatatest tests

* Use getter for Extrema

* Fix Extrema doc language

* Correct dataset to be one word

* Ensure multiple extrema are tested in a dataset

Author: MrAlias

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
 

exporters/otlp/otlpmetric/internal/transform/metricdata.go

@@ -174,17 +174,22 @@ 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()),
 			Count:             dPt.Count,
 			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
 }

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,
 	}}
 

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

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),
 	}
 }

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,
 						},
 					},

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
+}

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:

sdk/metric/metricdata/metricdatatest/assertion_test.go

@@ -73,14 +73,18 @@ 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,
 		Time:         endA,
 		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))

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) {