From 93a1ca6b51d2f551753b58f6f7dc55a20f53fb88 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 23 Jun 2022 08:32:39 -0700 Subject: [PATCH] Update temporality selector option (#2967) Match the WithAggregationSelector option pattern: define a TemporalitySelector type, export the DefaultTemporalitySelector function, and name the option with a Selector suffix. --- sdk/metric/manual_reader.go | 2 +- sdk/metric/manual_reader_test.go | 6 +++--- sdk/metric/periodic_reader.go | 2 +- sdk/metric/periodic_reader_test.go | 6 +++--- sdk/metric/reader.go | 31 +++++++++++++++++------------- sdk/metric/reader_test.go | 14 ++++++++++++++ 6 files changed, 40 insertions(+), 21 deletions(-) diff --git a/sdk/metric/manual_reader.go b/sdk/metric/manual_reader.go index 83f0328604b..d807079f46d 100644 --- a/sdk/metric/manual_reader.go +++ b/sdk/metric/manual_reader.go @@ -117,7 +117,7 @@ type manualReaderConfig struct { // newManualReaderConfig returns a manualReaderConfig configured with options. func newManualReaderConfig(opts []ManualReaderOption) manualReaderConfig { cfg := manualReaderConfig{ - temporalitySelector: defaultTemporalitySelector, + temporalitySelector: DefaultTemporalitySelector, aggregationSelector: DefaultAggregationSelector, } for _, opt := range opts { diff --git a/sdk/metric/manual_reader_test.go b/sdk/metric/manual_reader_test.go index 61b9ec74291..3df2e938844 100644 --- a/sdk/metric/manual_reader_test.go +++ b/sdk/metric/manual_reader_test.go @@ -50,15 +50,15 @@ func TestManualReaderTemporality(t *testing.T) { { name: "delta", options: []ManualReaderOption{ - WithTemporality(deltaTemporalitySelector), + WithTemporalitySelector(deltaTemporalitySelector), }, wantTemporality: DeltaTemporality, }, { name: "repeats overwrite", options: []ManualReaderOption{ - WithTemporality(deltaTemporalitySelector), - WithTemporality(cumulativeTemporalitySelector), + WithTemporalitySelector(deltaTemporalitySelector), + WithTemporalitySelector(cumulativeTemporalitySelector), }, wantTemporality: CumulativeTemporality, }, diff --git a/sdk/metric/periodic_reader.go b/sdk/metric/periodic_reader.go index 8bc27e0abb9..4bd735571b6 100644 --- a/sdk/metric/periodic_reader.go +++ b/sdk/metric/periodic_reader.go @@ -50,7 +50,7 @@ func newPeriodicReaderConfig(options []PeriodicReaderOption) periodicReaderConfi c := periodicReaderConfig{ interval: defaultInterval, timeout: defaultTimeout, - temporalitySelector: defaultTemporalitySelector, + temporalitySelector: DefaultTemporalitySelector, aggregationSelector: DefaultAggregationSelector, } for _, o := range options { diff --git a/sdk/metric/periodic_reader_test.go b/sdk/metric/periodic_reader_test.go index ae5e40f2a20..1dbefe028ba 100644 --- a/sdk/metric/periodic_reader_test.go +++ b/sdk/metric/periodic_reader_test.go @@ -200,15 +200,15 @@ func TestPeriodiclReaderTemporality(t *testing.T) { { name: "delta", options: []PeriodicReaderOption{ - WithTemporality(deltaTemporalitySelector), + WithTemporalitySelector(deltaTemporalitySelector), }, wantTemporality: DeltaTemporality, }, { name: "repeats overwrite", options: []PeriodicReaderOption{ - WithTemporality(deltaTemporalitySelector), - WithTemporality(cumulativeTemporalitySelector), + WithTemporalitySelector(deltaTemporalitySelector), + WithTemporalitySelector(cumulativeTemporalitySelector), }, wantTemporality: CumulativeTemporality, }, diff --git a/sdk/metric/reader.go b/sdk/metric/reader.go index 1e5888ee4c7..1a0c360af95 100644 --- a/sdk/metric/reader.go +++ b/sdk/metric/reader.go @@ -117,9 +117,20 @@ type ReaderOption interface { PeriodicReaderOption } -// WithTemporality uses the selector to determine the Temporality measurements -// from instrument should be recorded with. -func WithTemporality(selector func(instrument InstrumentKind) Temporality) ReaderOption { +// TemporalitySelector selects the temporality to use based on the InstrumentKind. +type TemporalitySelector func(InstrumentKind) Temporality + +// DefaultTemporalitySelector is the default TemporalitySelector used if +// WithTemporalitySelector is not provided. CumulativeTemporality will be used +// for all instrument kinds if this TemporalitySelector is used. +func DefaultTemporalitySelector(InstrumentKind) Temporality { + return CumulativeTemporality +} + +// WithTemporalitySelector sets the TemporalitySelector a reader will use to +// determine the Temporality of an instrument based on its kind. If this +// option is not used, the reader will use the DefaultTemporalitySelector. +func WithTemporalitySelector(selector TemporalitySelector) ReaderOption { return temporalitySelectorOption{selector: selector} } @@ -139,12 +150,6 @@ func (t temporalitySelectorOption) applyPeriodic(prc periodicReaderConfig) perio return prc } -// defaultTemporalitySelector returns the default Temporality measurements -// from instrument should be recorded with: cumulative. -func defaultTemporalitySelector(InstrumentKind) Temporality { - return CumulativeTemporality -} - // AggregationSelector selects the aggregation and the parameters to use for // that aggregation based on the InstrumentKind. type AggregationSelector func(InstrumentKind) aggregation.Aggregation @@ -170,10 +175,10 @@ func DefaultAggregationSelector(ik InstrumentKind) aggregation.Aggregation { panic("unknown instrument kind") } -// WithAggregation sets the default aggregation a reader will use for an -// instrument based on the returned value from the selector. If this option is -// not used, the reader will use the DefaultAggregationSelector or the -// aggregation explicitly passed for a view matching an instrument. +// WithAggregationSelector sets the AggregationSelector a reader will use to +// determine the aggregation to use for an instrument based on its kind. If +// this option is not used, the reader will use the DefaultAggregationSelector +// or the aggregation explicitly passed for a view matching an instrument. func WithAggregationSelector(selector AggregationSelector) ReaderOption { // Deep copy and validate before using. wrapped := func(ik InstrumentKind) aggregation.Aggregation { diff --git a/sdk/metric/reader_test.go b/sdk/metric/reader_test.go index 374dc09e909..9b9836f1af1 100644 --- a/sdk/metric/reader_test.go +++ b/sdk/metric/reader_test.go @@ -201,3 +201,17 @@ func TestDefaultAggregationSelector(t *testing.T) { assert.NoError(t, DefaultAggregationSelector(ik).Err(), ik) } } + +func TestDefaultTemporalitySelector(t *testing.T) { + for _, ik := range []InstrumentKind{ + undefinedInstrument, + SyncCounter, + SyncUpDownCounter, + SyncHistogram, + AsyncCounter, + AsyncUpDownCounter, + AsyncGauge, + } { + assert.Equal(t, CumulativeTemporality, DefaultTemporalitySelector(ik)) + } +}