Skip to content

Commit

Permalink
Add MatchInstrumentKind filter for Views. (#3037)
Browse files Browse the repository at this point in the history
* Move InstrumentKind to view, Add view filter

* remove TODO

* Add the Option function, fix lint

* use local var over 0

* Fix missing undefinedInstrumnet

Co-authored-by: Tyler Yahn <[email protected]>
  • Loading branch information
MadVikingGod and MrAlias authored Jul 25, 2022
1 parent 91b1a44 commit 67da7d0
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 41 deletions.
7 changes: 4 additions & 3 deletions sdk/metric/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,13 @@ import (

"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
"go.opentelemetry.io/otel/sdk/resource"
)

type reader struct {
producer producer
temporalityFunc func(InstrumentKind) metricdata.Temporality
temporalityFunc func(view.InstrumentKind) metricdata.Temporality
aggregationFunc AggregationSelector
collectFunc func(context.Context) (metricdata.ResourceMetrics, error)
forceFlushFunc func(context.Context) error
Expand All @@ -41,12 +42,12 @@ type reader struct {

var _ Reader = (*reader)(nil)

func (r *reader) aggregation(kind InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type.
func (r *reader) aggregation(kind view.InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type.
return r.aggregationFunc(kind)
}

func (r *reader) register(p producer) { r.producer = p }
func (r *reader) temporality(kind InstrumentKind) metricdata.Temporality {
func (r *reader) temporality(kind view.InstrumentKind) metricdata.Temporality {
return r.temporalityFunc(kind)
}
func (r *reader) Collect(ctx context.Context) (metricdata.ResourceMetrics, error) {
Expand Down
9 changes: 5 additions & 4 deletions sdk/metric/manual_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.opentelemetry.io/otel/internal/global"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

// manualReader is a a simple Reader that allows an application to
Expand All @@ -34,7 +35,7 @@ type manualReader struct {
producer atomic.Value
shutdownOnce sync.Once

temporalitySelector func(InstrumentKind) metricdata.Temporality
temporalitySelector func(view.InstrumentKind) metricdata.Temporality
aggregationSelector AggregationSelector
}

Expand All @@ -61,12 +62,12 @@ func (mr *manualReader) register(p producer) {
}

// temporality reports the Temporality for the instrument kind provided.
func (mr *manualReader) temporality(kind InstrumentKind) metricdata.Temporality {
func (mr *manualReader) temporality(kind view.InstrumentKind) metricdata.Temporality {
return mr.temporalitySelector(kind)
}

// aggregation returns what Aggregation to use for kind.
func (mr *manualReader) aggregation(kind InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type.
func (mr *manualReader) aggregation(kind view.InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type.
return mr.aggregationSelector(kind)
}

Expand Down Expand Up @@ -111,7 +112,7 @@ func (mr *manualReader) Collect(ctx context.Context) (metricdata.ResourceMetrics

// manualReaderConfig contains configuration options for a ManualReader.
type manualReaderConfig struct {
temporalitySelector func(InstrumentKind) metricdata.Temporality
temporalitySelector func(view.InstrumentKind) metricdata.Temporality
aggregationSelector AggregationSelector
}

Expand Down
6 changes: 4 additions & 2 deletions sdk/metric/manual_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/stretchr/testify/suite"

"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

func TestManualReader(t *testing.T) {
Expand All @@ -34,8 +35,8 @@ func BenchmarkManualReader(b *testing.B) {
b.Run("Collect", benchReaderCollectFunc(NewManualReader()))
}

var deltaTemporalitySelector = func(InstrumentKind) metricdata.Temporality { return metricdata.DeltaTemporality }
var cumulativeTemporalitySelector = func(InstrumentKind) metricdata.Temporality { return metricdata.CumulativeTemporality }
var deltaTemporalitySelector = func(view.InstrumentKind) metricdata.Temporality { return metricdata.DeltaTemporality }
var cumulativeTemporalitySelector = func(view.InstrumentKind) metricdata.Temporality { return metricdata.CumulativeTemporality }

func TestManualReaderTemporality(t *testing.T) {
tests := []struct {
Expand Down Expand Up @@ -68,6 +69,7 @@ func TestManualReaderTemporality(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var undefinedInstrument view.InstrumentKind
rdr := NewManualReader(tt.options...)
assert.Equal(t, tt.wantTemporality, rdr.temporality(undefinedInstrument))
})
Expand Down
9 changes: 5 additions & 4 deletions sdk/metric/periodic_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"go.opentelemetry.io/otel/internal/global"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

// Default periodic reader timing.
Expand All @@ -40,7 +41,7 @@ const (
type periodicReaderConfig struct {
interval time.Duration
timeout time.Duration
temporalitySelector func(InstrumentKind) metricdata.Temporality
temporalitySelector func(view.InstrumentKind) metricdata.Temporality
aggregationSelector AggregationSelector
}

Expand Down Expand Up @@ -140,7 +141,7 @@ type periodicReader struct {
timeout time.Duration
exporter Exporter

temporalitySelector func(InstrumentKind) metricdata.Temporality
temporalitySelector func(view.InstrumentKind) metricdata.Temporality
aggregationSelector AggregationSelector

wg sync.WaitGroup
Expand Down Expand Up @@ -185,12 +186,12 @@ func (r *periodicReader) register(p producer) {
}

// temporality reports the Temporality for the instrument kind provided.
func (r *periodicReader) temporality(kind InstrumentKind) metricdata.Temporality {
func (r *periodicReader) temporality(kind view.InstrumentKind) metricdata.Temporality {
return r.temporalitySelector(kind)
}

// aggregation returns what Aggregation to use for kind.
func (r *periodicReader) aggregation(kind InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type.
func (r *periodicReader) aggregation(kind view.InstrumentKind) aggregation.Aggregation { // nolint:revive // import-shadow for method scoped by type.
return r.aggregationSelector(kind)
}

Expand Down
2 changes: 2 additions & 0 deletions sdk/metric/periodic_reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

const testDur = time.Second * 2
Expand Down Expand Up @@ -216,6 +217,7 @@ func TestPeriodiclReaderTemporality(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var undefinedInstrument view.InstrumentKind
rdr := NewPeriodicReader(new(fnExporter), tt.options...)
assert.Equal(t, tt.wantTemporality, rdr.temporality(undefinedInstrument))
})
Expand Down
23 changes: 12 additions & 11 deletions sdk/metric/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"go.opentelemetry.io/otel/internal/global"
"go.opentelemetry.io/otel/sdk/metric/aggregation"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

// errDuplicateRegister is logged by a Reader when an attempt to registered it
Expand Down Expand Up @@ -58,10 +59,10 @@ type Reader interface {
register(producer)

// temporality reports the Temporality for the instrument kind provided.
temporality(InstrumentKind) metricdata.Temporality
temporality(view.InstrumentKind) metricdata.Temporality

// aggregation returns what Aggregation to use for an instrument kind.
aggregation(InstrumentKind) aggregation.Aggregation // nolint:revive // import-shadow for method scoped by type.
aggregation(view.InstrumentKind) aggregation.Aggregation // nolint:revive // import-shadow for method scoped by type.

// Collect gathers and returns all metric data related to the Reader from
// the SDK. An error is returned if this is called after Shutdown.
Expand Down Expand Up @@ -118,12 +119,12 @@ type ReaderOption interface {
}

// TemporalitySelector selects the temporality to use based on the InstrumentKind.
type TemporalitySelector func(InstrumentKind) metricdata.Temporality
type TemporalitySelector func(view.InstrumentKind) metricdata.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) metricdata.Temporality {
func DefaultTemporalitySelector(view.InstrumentKind) metricdata.Temporality {
return metricdata.CumulativeTemporality
}

Expand All @@ -135,7 +136,7 @@ func WithTemporalitySelector(selector TemporalitySelector) ReaderOption {
}

type temporalitySelectorOption struct {
selector func(instrument InstrumentKind) metricdata.Temporality
selector func(instrument view.InstrumentKind) metricdata.Temporality
}

// applyManual returns a manualReaderConfig with option applied.
Expand All @@ -152,21 +153,21 @@ func (t temporalitySelectorOption) applyPeriodic(prc periodicReaderConfig) perio

// AggregationSelector selects the aggregation and the parameters to use for
// that aggregation based on the InstrumentKind.
type AggregationSelector func(InstrumentKind) aggregation.Aggregation
type AggregationSelector func(view.InstrumentKind) aggregation.Aggregation

// DefaultAggregationSelector returns the default aggregation and parameters
// that will be used to summarize measurement made from an instrument of
// InstrumentKind. This AggregationSelector using the following selection
// mapping: Counter ⇨ Sum, Asynchronous Counter ⇨ Sum, UpDownCounter ⇨ Sum,
// Asynchronous UpDownCounter ⇨ Sum, Asynchronous Gauge ⇨ LastValue,
// Histogram ⇨ ExplicitBucketHistogram.
func DefaultAggregationSelector(ik InstrumentKind) aggregation.Aggregation {
func DefaultAggregationSelector(ik view.InstrumentKind) aggregation.Aggregation {
switch ik {
case SyncCounter, SyncUpDownCounter, AsyncCounter, AsyncUpDownCounter:
case view.SyncCounter, view.SyncUpDownCounter, view.AsyncCounter, view.AsyncUpDownCounter:
return aggregation.Sum{}
case AsyncGauge:
case view.AsyncGauge:
return aggregation.LastValue{}
case SyncHistogram:
case view.SyncHistogram:
return aggregation.ExplicitBucketHistogram{
Boundaries: []float64{0, 5, 10, 25, 50, 75, 100, 250, 500, 1000},
NoMinMax: false,
Expand All @@ -181,7 +182,7 @@ func DefaultAggregationSelector(ik InstrumentKind) aggregation.Aggregation {
// 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 {
wrapped := func(ik view.InstrumentKind) aggregation.Aggregation {
a := selector(ik)
cpA := a.Copy()
if err := cpA.Err(); err != nil {
Expand Down
31 changes: 17 additions & 14 deletions sdk/metric/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"go.opentelemetry.io/otel/sdk/metric/view"
)

type readerTestSuite struct {
Expand Down Expand Up @@ -184,15 +185,16 @@ func benchReaderCollectFunc(r Reader) func(*testing.B) {
}

func TestDefaultAggregationSelector(t *testing.T) {
var undefinedInstrument view.InstrumentKind
assert.Panics(t, func() { DefaultAggregationSelector(undefinedInstrument) })

iKinds := []InstrumentKind{
SyncCounter,
SyncUpDownCounter,
SyncHistogram,
AsyncCounter,
AsyncUpDownCounter,
AsyncGauge,
iKinds := []view.InstrumentKind{
view.SyncCounter,
view.SyncUpDownCounter,
view.SyncHistogram,
view.AsyncCounter,
view.AsyncUpDownCounter,
view.AsyncGauge,
}

for _, ik := range iKinds {
Expand All @@ -201,14 +203,15 @@ func TestDefaultAggregationSelector(t *testing.T) {
}

func TestDefaultTemporalitySelector(t *testing.T) {
for _, ik := range []InstrumentKind{
var undefinedInstrument view.InstrumentKind
for _, ik := range []view.InstrumentKind{
undefinedInstrument,
SyncCounter,
SyncUpDownCounter,
SyncHistogram,
AsyncCounter,
AsyncUpDownCounter,
AsyncGauge,
view.SyncCounter,
view.SyncUpDownCounter,
view.SyncHistogram,
view.AsyncCounter,
view.AsyncUpDownCounter,
view.AsyncGauge,
} {
assert.Equal(t, metricdata.CumulativeTemporality, DefaultTemporalitySelector(ik))
}
Expand Down
1 change: 1 addition & 0 deletions sdk/metric/view/instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ type Instrument struct {

Name string
Description string
Kind InstrumentKind
Aggregation aggregation.Aggregation
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
//go:build go1.18
// +build go1.18

package metric // import "go.opentelemetry.io/otel/sdk/metric"
package view // import "go.opentelemetry.io/otel/sdk/metric/view"

// InstrumentKind describes the kind of instrument a Meter can create.
type InstrumentKind uint8
Expand Down
17 changes: 15 additions & 2 deletions sdk/metric/view/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ type View struct {
instrumentName *regexp.Regexp
hasWildcard bool
scope instrumentation.Scope
instrumentKind InstrumentKind

filter attribute.Filter
name string
Expand Down Expand Up @@ -115,12 +116,16 @@ func (v View) matchScopeVersion(version string) bool {
func (v View) matchScopeSchemaURL(schemaURL string) bool {
return v.scope.SchemaURL == "" || schemaURL == v.scope.SchemaURL
}
func (v View) matchInstrumentKind(kind InstrumentKind) bool {
return v.instrumentKind == undefinedInstrument || kind == v.instrumentKind
}

func (v View) match(i Instrument) bool {
return v.matchName(i.Name) &&
v.matchScopeName(i.Scope.Name) &&
v.matchScopeSchemaURL(i.Scope.SchemaURL) &&
v.matchScopeVersion(i.Scope.Version)
v.matchScopeVersion(i.Scope.Version) &&
v.matchInstrumentKind(i.Kind)
}

// Option applies a Configuration option value to a View. All options
Expand Down Expand Up @@ -152,9 +157,17 @@ func MatchInstrumentName(name string) Option {
})
}

// TODO (#2813): Implement MatchInstrumentKind when InstrumentKind is defined.
// TODO (#2813): Implement MatchNumberKind when NumberKind is defined.

// MatchInstrumentKind with match an instrument based on the instrument's kind.
// The default is to match all instrument kinds.
func MatchInstrumentKind(kind InstrumentKind) Option {
return optionFunc(func(v View) View {
v.instrumentKind = kind
return v
})
}

// MatchInstrumentationScope will do an exact match on any
// instrumentation.Scope field that is non-empty (""). The default is to match all
// instrumentation scopes.
Expand Down

0 comments on commit 67da7d0

Please sign in to comment.