From e60c1b36592876cfbac501358d38b164b9e1d84d Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Fri, 22 Jul 2022 16:24:03 +0000 Subject: [PATCH 1/5] Move InstrumentKind to view, Add view filter --- sdk/metric/config_test.go | 7 ++--- sdk/metric/manual_reader.go | 9 ++++--- sdk/metric/manual_reader_test.go | 7 ++--- sdk/metric/periodic_reader.go | 9 ++++--- sdk/metric/periodic_reader_test.go | 2 +- sdk/metric/reader.go | 23 ++++++++-------- sdk/metric/reader_test.go | 35 +++++++++++++------------ sdk/metric/view/instrument.go | 1 + sdk/metric/{ => view}/instrumentkind.go | 2 +- sdk/metric/view/view.go | 7 ++++- 10 files changed, 57 insertions(+), 45 deletions(-) rename sdk/metric/{ => view}/instrumentkind.go (95%) diff --git a/sdk/metric/config_test.go b/sdk/metric/config_test.go index ba9aaaa54ea..5439fcdc694 100644 --- a/sdk/metric/config_test.go +++ b/sdk/metric/config_test.go @@ -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 @@ -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) { diff --git a/sdk/metric/manual_reader.go b/sdk/metric/manual_reader.go index a3e5f711c2d..2b62764226c 100644 --- a/sdk/metric/manual_reader.go +++ b/sdk/metric/manual_reader.go @@ -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 @@ -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 } @@ -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) } @@ -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 } diff --git a/sdk/metric/manual_reader_test.go b/sdk/metric/manual_reader_test.go index 47125636375..e296d09878b 100644 --- a/sdk/metric/manual_reader_test.go +++ b/sdk/metric/manual_reader_test.go @@ -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) { @@ -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 { @@ -69,7 +70,7 @@ func TestManualReaderTemporality(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { rdr := NewManualReader(tt.options...) - assert.Equal(t, tt.wantTemporality, rdr.temporality(undefinedInstrument)) + assert.Equal(t, tt.wantTemporality, rdr.temporality(0)) }) } } diff --git a/sdk/metric/periodic_reader.go b/sdk/metric/periodic_reader.go index 034f3373cfd..d18140df20b 100644 --- a/sdk/metric/periodic_reader.go +++ b/sdk/metric/periodic_reader.go @@ -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. @@ -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 } @@ -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 @@ -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) } diff --git a/sdk/metric/periodic_reader_test.go b/sdk/metric/periodic_reader_test.go index c7bb8d0740e..fee0c5ed5cf 100644 --- a/sdk/metric/periodic_reader_test.go +++ b/sdk/metric/periodic_reader_test.go @@ -217,7 +217,7 @@ func TestPeriodiclReaderTemporality(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { rdr := NewPeriodicReader(new(fnExporter), tt.options...) - assert.Equal(t, tt.wantTemporality, rdr.temporality(undefinedInstrument)) + assert.Equal(t, tt.wantTemporality, rdr.temporality(0)) }) } } diff --git a/sdk/metric/reader.go b/sdk/metric/reader.go index 4a260ebc256..ff5f987070d 100644 --- a/sdk/metric/reader.go +++ b/sdk/metric/reader.go @@ -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 @@ -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. @@ -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 } @@ -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. @@ -152,7 +153,7 @@ 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 @@ -160,13 +161,13 @@ type AggregationSelector func(InstrumentKind) aggregation.Aggregation // 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, @@ -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 { diff --git a/sdk/metric/reader_test.go b/sdk/metric/reader_test.go index 0be9cb2efc3..b5e1698bd32 100644 --- a/sdk/metric/reader_test.go +++ b/sdk/metric/reader_test.go @@ -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 { @@ -184,15 +185,15 @@ func benchReaderCollectFunc(r Reader) func(*testing.B) { } func TestDefaultAggregationSelector(t *testing.T) { - assert.Panics(t, func() { DefaultAggregationSelector(undefinedInstrument) }) - - iKinds := []InstrumentKind{ - SyncCounter, - SyncUpDownCounter, - SyncHistogram, - AsyncCounter, - AsyncUpDownCounter, - AsyncGauge, + assert.Panics(t, func() { DefaultAggregationSelector(0) }) + + iKinds := []view.InstrumentKind{ + view.SyncCounter, + view.SyncUpDownCounter, + view.SyncHistogram, + view.AsyncCounter, + view.AsyncUpDownCounter, + view.AsyncGauge, } for _, ik := range iKinds { @@ -201,14 +202,14 @@ func TestDefaultAggregationSelector(t *testing.T) { } func TestDefaultTemporalitySelector(t *testing.T) { - for _, ik := range []InstrumentKind{ - undefinedInstrument, - SyncCounter, - SyncUpDownCounter, - SyncHistogram, - AsyncCounter, - AsyncUpDownCounter, - AsyncGauge, + for _, ik := range []view.InstrumentKind{ + 0, + view.SyncCounter, + view.SyncUpDownCounter, + view.SyncHistogram, + view.AsyncCounter, + view.AsyncUpDownCounter, + view.AsyncGauge, } { assert.Equal(t, metricdata.CumulativeTemporality, DefaultTemporalitySelector(ik)) } diff --git a/sdk/metric/view/instrument.go b/sdk/metric/view/instrument.go index 3df030aa4ba..e024b37a6ff 100644 --- a/sdk/metric/view/instrument.go +++ b/sdk/metric/view/instrument.go @@ -28,5 +28,6 @@ type Instrument struct { Name string Description string + Kind InstrumentKind Aggregation aggregation.Aggregation } diff --git a/sdk/metric/instrumentkind.go b/sdk/metric/view/instrumentkind.go similarity index 95% rename from sdk/metric/instrumentkind.go rename to sdk/metric/view/instrumentkind.go index 20d94fcc43a..e053332bd5d 100644 --- a/sdk/metric/instrumentkind.go +++ b/sdk/metric/view/instrumentkind.go @@ -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/metricdata" // InstrumentKind describes the kind of instrument a Meter can create. type InstrumentKind uint8 diff --git a/sdk/metric/view/view.go b/sdk/metric/view/view.go index 03bfd8eec73..4a1b63bed76 100644 --- a/sdk/metric/view/view.go +++ b/sdk/metric/view/view.go @@ -41,6 +41,7 @@ type View struct { instrumentName *regexp.Regexp hasWildcard bool scope instrumentation.Scope + instrumentKind InstrumentKind filter attribute.Filter name string @@ -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 From 46066116d1c6040156393dafa069fbf98560ae28 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Fri, 22 Jul 2022 18:00:06 +0000 Subject: [PATCH 2/5] remove TODO --- sdk/metric/view/view.go | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/metric/view/view.go b/sdk/metric/view/view.go index 4a1b63bed76..080a67f6f25 100644 --- a/sdk/metric/view/view.go +++ b/sdk/metric/view/view.go @@ -157,7 +157,6 @@ func MatchInstrumentName(name string) Option { }) } -// TODO (#2813): Implement MatchInstrumentKind when InstrumentKind is defined. // TODO (#2813): Implement MatchNumberKind when NumberKind is defined. // MatchInstrumentationScope will do an exact match on any From bdd43c8fa5dca44c2555009af0bbf670fbdde757 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Fri, 22 Jul 2022 18:10:05 +0000 Subject: [PATCH 3/5] Add the Option function, fix lint --- sdk/metric/view/instrumentkind.go | 2 +- sdk/metric/view/view.go | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sdk/metric/view/instrumentkind.go b/sdk/metric/view/instrumentkind.go index e053332bd5d..d5fc67953f9 100644 --- a/sdk/metric/view/instrumentkind.go +++ b/sdk/metric/view/instrumentkind.go @@ -15,7 +15,7 @@ //go:build go1.18 // +build go1.18 -package view // import "go.opentelemetry.io/otel/sdk/metric/metricdata" +package view // import "go.opentelemetry.io/otel/sdk/metric/view" // InstrumentKind describes the kind of instrument a Meter can create. type InstrumentKind uint8 diff --git a/sdk/metric/view/view.go b/sdk/metric/view/view.go index 080a67f6f25..bed7c9a24ee 100644 --- a/sdk/metric/view/view.go +++ b/sdk/metric/view/view.go @@ -159,6 +159,15 @@ func MatchInstrumentName(name string) Option { // 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. From 52849bb1ffd55f4b3c815fefd0d385dce4162770 Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Mon, 25 Jul 2022 12:36:22 +0000 Subject: [PATCH 4/5] use local var over 0 --- sdk/metric/manual_reader_test.go | 3 ++- sdk/metric/periodic_reader_test.go | 4 +++- sdk/metric/reader_test.go | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sdk/metric/manual_reader_test.go b/sdk/metric/manual_reader_test.go index e296d09878b..58b1a85cb1d 100644 --- a/sdk/metric/manual_reader_test.go +++ b/sdk/metric/manual_reader_test.go @@ -69,8 +69,9 @@ 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(0)) + assert.Equal(t, tt.wantTemporality, rdr.temporality(undefinedInstrument)) }) } } diff --git a/sdk/metric/periodic_reader_test.go b/sdk/metric/periodic_reader_test.go index fee0c5ed5cf..5689aa11b85 100644 --- a/sdk/metric/periodic_reader_test.go +++ b/sdk/metric/periodic_reader_test.go @@ -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 @@ -216,8 +217,9 @@ 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(0)) + assert.Equal(t, tt.wantTemporality, rdr.temporality(undefinedInstrument)) }) } } diff --git a/sdk/metric/reader_test.go b/sdk/metric/reader_test.go index b5e1698bd32..f00e14d8ff1 100644 --- a/sdk/metric/reader_test.go +++ b/sdk/metric/reader_test.go @@ -185,7 +185,8 @@ func benchReaderCollectFunc(r Reader) func(*testing.B) { } func TestDefaultAggregationSelector(t *testing.T) { - assert.Panics(t, func() { DefaultAggregationSelector(0) }) + var undefinedInstrument view.InstrumentKind + assert.Panics(t, func() { DefaultAggregationSelector(undefinedInstrument) }) iKinds := []view.InstrumentKind{ view.SyncCounter, From 48079e061932115c52ed9083d1eda07f2fcd0f7a Mon Sep 17 00:00:00 2001 From: Aaron Clawson <3766680+MadVikingGod@users.noreply.github.com> Date: Mon, 25 Jul 2022 15:37:02 +0000 Subject: [PATCH 5/5] Fix missing undefinedInstrumnet --- sdk/metric/reader_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/metric/reader_test.go b/sdk/metric/reader_test.go index f00e14d8ff1..3cc8c8c9264 100644 --- a/sdk/metric/reader_test.go +++ b/sdk/metric/reader_test.go @@ -203,8 +203,9 @@ func TestDefaultAggregationSelector(t *testing.T) { } func TestDefaultTemporalitySelector(t *testing.T) { + var undefinedInstrument view.InstrumentKind for _, ik := range []view.InstrumentKind{ - 0, + undefinedInstrument, view.SyncCounter, view.SyncUpDownCounter, view.SyncHistogram,