Skip to content

Commit

Permalink
Update temporality selector option (#2967)
Browse files Browse the repository at this point in the history
Match the WithAggregationSelector option pattern: define a
TemporalitySelector type, export the DefaultTemporalitySelector
function, and name the option with a Selector suffix.
  • Loading branch information
MrAlias authored Jun 23, 2022
1 parent c6526ec commit 93a1ca6
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 21 deletions.
2 changes: 1 addition & 1 deletion sdk/metric/manual_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions sdk/metric/manual_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/periodic_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions sdk/metric/periodic_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
31 changes: 18 additions & 13 deletions sdk/metric/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}

Expand All @@ -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
Expand All @@ -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 {
Expand Down
14 changes: 14 additions & 0 deletions sdk/metric/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

0 comments on commit 93a1ca6

Please sign in to comment.