Skip to content

Commit 573a115

Browse files
dmathieupull[bot]
authored andcommitted
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
1 parent b5cc372 commit 573a115

3 files changed

Lines changed: 51 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2626
- `TracerProvider` allows calling `Tracer()` while it's shutting down.
2727
It used to deadlock. (#3924)
2828
- Use the SDK version for the Telemetry SDK resource detector in `go.opentelemetry.io/otel/sdk/resource`. (#3949)
29+
- Automatically figure out the default aggregation with `aggregation.Default`. (#3967)
2930

3031
## [1.15.0-rc.2/0.38.0-rc.2] 2023-03-23
3132

sdk/metric/pipeline.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,8 @@ func (i *inserter[N]) streamID(kind InstrumentKind, stream Stream) streamID {
390390
// returned.
391391
func (i *inserter[N]) aggregator(agg aggregation.Aggregation, kind InstrumentKind, temporality metricdata.Temporality, monotonic bool) (internal.Aggregator[N], error) {
392392
switch a := agg.(type) {
393+
case aggregation.Default:
394+
return i.aggregator(DefaultAggregationSelector(kind), kind, temporality, monotonic)
393395
case aggregation.Drop:
394396
return nil, nil
395397
case aggregation.LastValue:
@@ -444,6 +446,8 @@ func (i *inserter[N]) aggregator(agg aggregation.Aggregation, kind InstrumentKin
444446
// | Observable Gauge | X | X | | | |.
445447
func isAggregatorCompatible(kind InstrumentKind, agg aggregation.Aggregation) error {
446448
switch agg.(type) {
449+
case aggregation.Default:
450+
return nil
447451
case aggregation.ExplicitBucketHistogram:
448452
if kind == InstrumentKindCounter || kind == InstrumentKindHistogram {
449453
return nil

sdk/metric/pipeline_registry_test.go

Lines changed: 46 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,52 @@ func testCreateAggregators[N int64 | float64](t *testing.T) {
199199
wantLen: 2,
200200
},
201201
{
202-
name: "reader with invalid aggregation should error",
203-
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
204-
views: []View{defaultView},
205-
inst: instruments[InstrumentKindCounter],
206-
wantErr: errCreatingAggregators,
202+
name: "reader with default aggregation should figure out a Counter",
203+
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
204+
views: []View{defaultView},
205+
inst: instruments[InstrumentKindCounter],
206+
wantKind: internal.NewCumulativeSum[N](true),
207+
wantLen: 1,
208+
},
209+
{
210+
name: "reader with default aggregation should figure out an UpDownCounter",
211+
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
212+
views: []View{defaultView},
213+
inst: instruments[InstrumentKindUpDownCounter],
214+
wantKind: internal.NewCumulativeSum[N](true),
215+
wantLen: 1,
216+
},
217+
{
218+
name: "reader with default aggregation should figure out an Histogram",
219+
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
220+
views: []View{defaultView},
221+
inst: instruments[InstrumentKindHistogram],
222+
wantKind: internal.NewCumulativeHistogram[N](aggregation.ExplicitBucketHistogram{}),
223+
wantLen: 1,
224+
},
225+
{
226+
name: "reader with default aggregation should figure out an ObservableCounter",
227+
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
228+
views: []View{defaultView},
229+
inst: instruments[InstrumentKindObservableCounter],
230+
wantKind: internal.NewPrecomputedCumulativeSum[N](true),
231+
wantLen: 1,
232+
},
233+
{
234+
name: "reader with default aggregation should figure out an ObservableUpDownCounter",
235+
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
236+
views: []View{defaultView},
237+
inst: instruments[InstrumentKindObservableUpDownCounter],
238+
wantKind: internal.NewPrecomputedCumulativeSum[N](true),
239+
wantLen: 1,
240+
},
241+
{
242+
name: "reader with default aggregation should figure out an ObservableGauge",
243+
reader: NewManualReader(WithAggregationSelector(func(ik InstrumentKind) aggregation.Aggregation { return aggregation.Default{} })),
244+
views: []View{defaultView},
245+
inst: instruments[InstrumentKindObservableGauge],
246+
wantKind: internal.NewLastValue[N](),
247+
wantLen: 1,
207248
},
208249
{
209250
name: "view with invalid aggregation should error",
@@ -594,12 +635,6 @@ func TestIsAggregatorCompatible(t *testing.T) {
594635
agg: aggregation.ExplicitBucketHistogram{},
595636
want: errIncompatibleAggregation,
596637
},
597-
{
598-
name: "Default aggregation should error",
599-
kind: InstrumentKindCounter,
600-
agg: aggregation.Default{},
601-
want: errUnknownAggregation,
602-
},
603638
{
604639
name: "unknown kind with Sum should error",
605640
kind: undefinedInstrument,

0 commit comments

Comments
 (0)