Fix aggregation.Default to properly return the default one (#3967)

* fix aggregation.Default to properly return the default one * add changelog entry * default aggregation does not error anymore * test all instrument kinds
open-telemetry · Jun 16, 2024 · 573a115 · 573a115
1 parent b5cc372
commit 573a115
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 - `TracerProvider` allows calling `Tracer()` while it's shutting down.
   It used to deadlock. (#3924)
 - Use the SDK version for the Telemetry SDK resource detector in `go.opentelemetry.io/otel/sdk/resource`. (#3949)
+- Automatically figure out the default aggregation with `aggregation.Default`. (#3967)
 
 ## [1.15.0-rc.2/0.38.0-rc.2] 2023-03-23
 

diff --git a/sdk/metric/pipeline.go b/sdk/metric/pipeline.go
@@ -390,6 +390,8 @@ func (i *inserter[N]) streamID(kind InstrumentKind, stream Stream) streamID {
 // returned.
 func (i *inserter[N]) aggregator(agg aggregation.Aggregation, kind InstrumentKind, temporality metricdata.Temporality, monotonic bool) (internal.Aggregator[N], error) {
 	switch a := agg.(type) {
+	case aggregation.Default:
+		return i.aggregator(DefaultAggregationSelector(kind), kind, temporality, monotonic)
 	case aggregation.Drop:
 		return nil, nil
 	case aggregation.LastValue:
@@ -444,6 +446,8 @@ func (i *inserter[N]) aggregator(agg aggregation.Aggregation, kind InstrumentKin
 // | Observable Gauge         | X    | X         |     |           |                       |.
 func isAggregatorCompatible(kind InstrumentKind, agg aggregation.Aggregation) error {
 	switch agg.(type) {
+	case aggregation.Default:
+		return nil
 	case aggregation.ExplicitBucketHistogram:
 		if kind == InstrumentKindCounter || kind == InstrumentKindHistogram {
 			return nil

diff --git a/sdk/metric/pipeline_registry_test.go b/sdk/metric/pipeline_registry_test.go
@@ -199,11 +199,52 @@ func testCreateAggregators[N int64 | float64](t *testing.T) {
 			wantLen:  2,
 		},
 		{
-			name:    "reader with invalid aggregation should error",
-			reader:  NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
-			views:   []View{defaultView},
-			inst:    instruments[InstrumentKindCounter],
-			wantErr: errCreatingAggregators,
+			name:     "reader with default aggregation should figure out a Counter",
+			reader:   NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
+			views:    []View{defaultView},
+			inst:     instruments[InstrumentKindCounter],
+			wantKind: internal.NewCumulativeSum[N](true),
+			wantLen:  1,
+		},
+		{
+			name:     "reader with default aggregation should figure out an UpDownCounter",
+			reader:   NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
+			views:    []View{defaultView},
+			inst:     instruments[InstrumentKindUpDownCounter],
+			wantKind: internal.NewCumulativeSum[N](true),
+			wantLen:  1,
+		},
+		{
+			name:     "reader with default aggregation should figure out an Histogram",
+			reader:   NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
+			views:    []View{defaultView},
+			inst:     instruments[InstrumentKindHistogram],
+			wantKind: internal.NewCumulativeHistogram[N](aggregation.ExplicitBucketHistogram{}),
+			wantLen:  1,
+		},
+		{
+			name:     "reader with default aggregation should figure out an ObservableCounter",
+			reader:   NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
+			views:    []View{defaultView},
+			inst:     instruments[InstrumentKindObservableCounter],
+			wantKind: internal.NewPrecomputedCumulativeSum[N](true),
+			wantLen:  1,
+		},
+		{
+			name:     "reader with default aggregation should figure out an ObservableUpDownCounter",
+			reader:   NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
+			views:    []View{defaultView},
+			inst:     instruments[InstrumentKindObservableUpDownCounter],
+			wantKind: internal.NewPrecomputedCumulativeSum[N](true),
+			wantLen:  1,
+		},
+		{
+			name:     "reader with default aggregation should figure out an ObservableGauge",
+			reader:   NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
+			views:    []View{defaultView},
+			inst:     instruments[InstrumentKindObservableGauge],
+			wantKind: internal.NewLastValue[N](),
+			wantLen:  1,
 		},
 		{
 			name:    "view with invalid aggregation should error",
@@ -594,12 +635,6 @@ func TestIsAggregatorCompatible(t *testing.T) {
 			agg:  aggregation.ExplicitBucketHistogram{},
 			want: errIncompatibleAggregation,
 		},
-		{
-			name: "Default aggregation should error",
-			kind: InstrumentKindCounter,
-			agg:  aggregation.Default{},
-			want: errUnknownAggregation,
-		},
 		{
 			name: "unknown kind with Sum should error",
 			kind: undefinedInstrument,