From 5aea817f1f2542f51cc63a723ef1a442121423f4 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 13:45:18 -0800 Subject: [PATCH 01/15] Add the InstrumentKind type and vars to sdk/metric --- sdk/metric/instrument.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 48440280e42..43498a96e92 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -28,6 +28,36 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) +// InstrumentKind is the identifier of a group of instruments that all +// performing the same function. +type InstrumentKind uint8 + +const ( + // instrumentKindUndefined is an undefined instrument kind, it should not + // be used by any initialized type. + instrumentKindUndefined InstrumentKind = iota // nolint:deadcode,varcheck,unused + // InstrumentKindSyncCounter identifies a group of instruments that record + // increasing values synchronously with the code path they are measuring. + InstrumentKindSyncCounter + // InstrumentKindSyncUpDownCounter identifies a group of instruments that + // record increasing and decreasing values synchronously with the code path + // they are measuring. + InstrumentKindSyncUpDownCounter + // InstrumentKindSyncHistogram identifies a group of instruments that + // record a distribution of values synchronously with the code path they + // are measuring. + InstrumentKindSyncHistogram + // InstrumentKindAsyncCounter identifies a group of instruments that record + // increasing values in an asynchronous callback. + InstrumentKindAsyncCounter + // InstrumentKindAsyncUpDownCounter identifies a group of instruments that + // record increasing and decreasing values in an asynchronous callback. + InstrumentKindAsyncUpDownCounter + // InstrumentKindAsyncGauge identifies a group of instruments that record + // current values in an asynchronous callback. + InstrumentKindAsyncGauge +) + // instrumentID are the identifying properties of an instrument. type instrumentID struct { // Name is the name of the instrument. From 5b5d7e447e18a57fa3df9652f1b5c58ddde76156 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 13:46:41 -0800 Subject: [PATCH 02/15] Add the Instrument type to sdk/metric --- sdk/metric/instrument.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 43498a96e92..9abac13269e 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -24,6 +24,7 @@ import ( "go.opentelemetry.io/otel/metric/instrument/syncfloat64" "go.opentelemetry.io/otel/metric/instrument/syncint64" "go.opentelemetry.io/otel/metric/unit" + "go.opentelemetry.io/otel/sdk/instrumentation" "go.opentelemetry.io/otel/sdk/metric/internal" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -58,6 +59,25 @@ const ( InstrumentKindAsyncGauge ) +type nonComparable [0]func() // nolint: unused // This is indeed used. + +// Instrument describes properties an instrument is created with. +type Instrument struct { + // Name is the human-readable identifier of the instrument. + Name string + // Description describes the purpose of the instrument. + Description string + // Kind defines the functional group of the instrument. + Kind InstrumentKind + // Unit is the unit of measurement recorded by the instrument. + Unit unit.Unit + // Scope identifies the instrumentation that created the instrument. + Scope instrumentation.Scope + + // Ensure forward compatibility if non-comparable fields need to be added. + nonComparable // nolint: unused +} + // instrumentID are the identifying properties of an instrument. type instrumentID struct { // Name is the name of the instrument. From 80061df40da9bd66c7e27161c36e7a5a18fd5997 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 13:46:57 -0800 Subject: [PATCH 03/15] Add the Stream type to sdk/metric --- sdk/metric/instrument.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 9abac13269e..fe65a8c028a 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -25,6 +25,7 @@ import ( "go.opentelemetry.io/otel/metric/instrument/syncint64" "go.opentelemetry.io/otel/metric/unit" "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/metric/aggregation" "go.opentelemetry.io/otel/sdk/metric/internal" "go.opentelemetry.io/otel/sdk/metric/metricdata" ) @@ -78,6 +79,16 @@ type Instrument struct { nonComparable // nolint: unused } +// Stream describes the stream of data an instrument produces. +type Stream struct { + Instrument + + // Aggregation the stream uses for an instrument. + Aggregation aggregation.Aggregation + // AttributeFilter applied to all attributes recorded for an instrument. + AttributeFilter attribute.Filter +} + // instrumentID are the identifying properties of an instrument. type instrumentID struct { // Name is the name of the instrument. From c772a394962c046f1ae568728cbb517d430bf933 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 13:47:11 -0800 Subject: [PATCH 04/15] Add the View type to sdk/metric --- sdk/metric/view.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 sdk/metric/view.go diff --git a/sdk/metric/view.go b/sdk/metric/view.go new file mode 100644 index 00000000000..fa3d38f0d8b --- /dev/null +++ b/sdk/metric/view.go @@ -0,0 +1,21 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metric // import "go.opentelemetry.io/otel/sdk/metric" + +// View is an override to the default behavior of the SDK. It defines how data +// should be collected for certain instruments. It returns true and the exact +// Stream to use for matching Instruments. Otherwise, if the view does not +// match, false is returned. +type View func(Instrument) (Stream, bool) From 791245ade2c20badc0cf6ec3fae585932ae89a3c Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 14:05:44 -0800 Subject: [PATCH 05/15] Add NewView to create Views matching OTel spec --- sdk/metric/instrument.go | 86 ++++++++++++++++++++++++++++++++++++++++ sdk/metric/view.go | 74 ++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index fe65a8c028a..589bcacb977 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -30,6 +30,12 @@ import ( "go.opentelemetry.io/otel/sdk/metric/metricdata" ) +var ( + zeroUnit unit.Unit + zeroInstrumentKind InstrumentKind + zeroScope instrumentation.Scope +) + // InstrumentKind is the identifier of a group of instruments that all // performing the same function. type InstrumentKind uint8 @@ -79,6 +85,86 @@ type Instrument struct { nonComparable // nolint: unused } +// mask returns a copy of p with all non-zero-value fields of m replacing the +// fields of the returned copy. +func (p Instrument) mask(m Instrument) Instrument { + if m.Name != "" { + p.Name = m.Name + } + if m.Description != "" { + p.Description = m.Description + } + if m.Kind != zeroInstrumentKind { + p.Kind = m.Kind + } + if m.Unit != zeroUnit { + p.Unit = m.Unit + } + if m.Scope.Name != "" { + p.Scope.Name = m.Scope.Name + } + if m.Scope.Version != "" { + p.Scope.Version = m.Scope.Version + } + if m.Scope.SchemaURL != "" { + p.Scope.SchemaURL = m.Scope.SchemaURL + } + return p +} + +// empty returns if all fields of p are their zero-value. +func (p Instrument) empty() bool { + return p.Name == "" && + p.Description == "" && + p.Kind == zeroInstrumentKind && + p.Unit == zeroUnit && + p.Scope == zeroScope +} + +// matches returns if all the non-zero-value fields of o match the +// corresponding fields of p. +// +// If o is empty true is returned. +func (p Instrument) matches(o Instrument) bool { + return p.matchesName(o) && + p.matchesDescription(o) && + p.matchesKind(o) && + p.matchesUnit(o) && + p.matchesScope(o) +} + +// matchesName returns true if the Name field of o is a non-zero-value and +// equals the Name field of p, otherwise false. +func (p Instrument) matchesName(o Instrument) bool { + return p.Name == "" || p.Name == o.Name +} + +// matchesDescription returns true if the Description field of o is a +// non-zero-value and equals the Description field of p, otherwise false. +func (p Instrument) matchesDescription(o Instrument) bool { + return p.Description == "" || p.Description == o.Description +} + +// matchesKind returns true if the Kind field of o is a non-zero-value and +// equals the Kind field of p, otherwise false. +func (p Instrument) matchesKind(o Instrument) bool { + return p.Kind == zeroInstrumentKind || p.Kind == o.Kind +} + +// matchesUnit returns true if the Unit field of o is a non-zero-value and +// equals the Unit field of p, otherwise false. +func (p Instrument) matchesUnit(o Instrument) bool { + return p.Unit == zeroUnit || p.Unit == o.Unit +} + +// matchesScope returns true if the Scope field of o is a non-zero-value and +// equals the Scope field of p, otherwise false. +func (p Instrument) matchesScope(o Instrument) bool { + return (p.Scope.Name == "" || p.Scope.Name == o.Scope.Name) && + (p.Scope.Version == "" || p.Scope.Version == o.Scope.Version) && + (p.Scope.SchemaURL == "" || p.Scope.SchemaURL == o.Scope.SchemaURL) +} + // Stream describes the stream of data an instrument produces. type Stream struct { Instrument diff --git a/sdk/metric/view.go b/sdk/metric/view.go index fa3d38f0d8b..6858bb74646 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -14,8 +14,82 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" +import ( + "regexp" + "strings" + + "go.opentelemetry.io/otel/internal/global" + "go.opentelemetry.io/otel/sdk/metric/aggregation" +) + // View is an override to the default behavior of the SDK. It defines how data // should be collected for certain instruments. It returns true and the exact // Stream to use for matching Instruments. Otherwise, if the view does not // match, false is returned. type View func(Instrument) (Stream, bool) + +// NewView returns a View that applies the Stream mask for all instruments that +// match criteria. The returned View will only apply mask if all non-zero-value +// fields of criteria match the corresponding Instrument passed to the view. If +// no criteria are provided, all field of criteria are their zero-values, a +// view that matches no instruments is returned. +// +// The Name field of criteria supports wildcard pattern matching. The wildcard +// "*" is recognized as matching zero or more characters, and "?" is recognized +// as matching exactly one character. For example, a pattern of "*" will match +// all instrument names. +// +// The Stream mask only applies updates for non-zero-value fields. By default, +// the Instrument the View matches against will be use for the returned Stream +// and no Aggregation or AttributeFilter are set. If mask has a non-zero-value +// value for any of the Aggregation or AttributeFilter fields, or any of the +// Instrument fields, that value is used instead of the default. If you need to +// zero out an Stream field returned from a View, create a View directly. +func NewView(criteria Instrument, mask Stream) View { + if criteria.empty() { + return func(Instrument) (Stream, bool) { return Stream{}, false } + } + + var matchFunc func(Instrument) bool + if strings.ContainsAny(criteria.Name, "*?") { + pattern := regexp.QuoteMeta(criteria.Name) + pattern = "^" + pattern + "$" + pattern = strings.ReplaceAll(pattern, "\\?", ".") + pattern = strings.ReplaceAll(pattern, "\\*", ".*") + re := regexp.MustCompile(pattern) + matchFunc = func(p Instrument) bool { + return re.MatchString(p.Name) && + criteria.matchesDescription(p) && + criteria.matchesKind(p) && + criteria.matchesUnit(p) && + criteria.matchesScope(p) + } + } else { + matchFunc = criteria.matches + } + + var agg aggregation.Aggregation + if mask.Aggregation != nil { + agg = mask.Aggregation.Copy() + if err := agg.Err(); err != nil { + global.Error( + err, "not using aggregation with view", + "aggregation", agg, + "view", criteria, + ) + agg = nil + } + } + + return func(p Instrument) (Stream, bool) { + if matchFunc(p) { + stream := Stream{ + Instrument: p.mask(mask.Instrument), + Aggregation: agg, + AttributeFilter: mask.AttributeFilter, + } + return stream, true + } + return Stream{}, false + } +} From 8e591c5aa3dc38d356b6541512dda54d4f7a7e5a Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 14:50:42 -0800 Subject: [PATCH 06/15] Add unit tests for NewView --- sdk/metric/pipeline_registry_test.go | 10 + sdk/metric/view_test.go | 513 +++++++++++++++++++++++++++ 2 files changed, 523 insertions(+) create mode 100644 sdk/metric/view_test.go diff --git a/sdk/metric/pipeline_registry_test.go b/sdk/metric/pipeline_registry_test.go index a6ad6f81d23..76fae178ddd 100644 --- a/sdk/metric/pipeline_registry_test.go +++ b/sdk/metric/pipeline_registry_test.go @@ -369,6 +369,7 @@ func TestPipelineRegistryCreateAggregatorsIncompatibleInstrument(t *testing.T) { type logCounter struct { logr.LogSink + errN uint32 infoN uint32 } @@ -381,6 +382,15 @@ func (l *logCounter) InfoN() int { return int(atomic.SwapUint32(&l.infoN, 0)) } +func (l *logCounter) Error(err error, msg string, keysAndValues ...interface{}) { + atomic.AddUint32(&l.errN, 1) + l.LogSink.Error(err, msg, keysAndValues...) +} + +func (l *logCounter) ErrorN() int { + return int(atomic.SwapUint32(&l.errN, 0)) +} + func TestResolveAggregatorsDuplicateErrors(t *testing.T) { tLog := testr.NewWithOptions(t, testr.Options{Verbosity: 6}) l := &logCounter{LogSink: tLog.GetSink()} diff --git a/sdk/metric/view_test.go b/sdk/metric/view_test.go new file mode 100644 index 00000000000..9d075a8ffd6 --- /dev/null +++ b/sdk/metric/view_test.go @@ -0,0 +1,513 @@ +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package metric // import "go.opentelemetry.io/otel/sdk/metric" + +import ( + "testing" + + "github.com/go-logr/logr" + "github.com/go-logr/logr/testr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/metric/unit" + "go.opentelemetry.io/otel/sdk/instrumentation" + "go.opentelemetry.io/otel/sdk/metric/aggregation" +) + +var ( + schemaURL = "https://opentelemetry.io/schemas/1.0.0" + completeIP = Instrument{ + Name: "foo", + Description: "foo desc", + Kind: InstrumentKindSyncCounter, + Unit: unit.Bytes, + Scope: instrumentation.Scope{ + Name: "TestNewViewMatch", + Version: "v0.1.0", + SchemaURL: schemaURL, + }, + } +) + +func scope(name, ver, url string) instrumentation.Scope { + return instrumentation.Scope{Name: name, Version: ver, SchemaURL: url} +} + +func testNewViewMatchName() func(t *testing.T) { + tests := []struct { + name string + criteria string + match []string + notMatch []string + }{ + { + name: "Exact", + criteria: "foo", + match: []string{"foo"}, + notMatch: []string{"", "bar", "foobar", "barfoo", "ffooo"}, + }, + { + name: "Wildcard/*", + criteria: "*", + match: []string{"", "foo", "foobar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Front?", + criteria: "?oo", + match: []string{"foo", "1oo"}, + notMatch: []string{"", "bar", "foobar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Back?", + criteria: "fo?", + match: []string{"foo", "fo1"}, + notMatch: []string{"", "bar", "foobar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Front*", + criteria: "*foo", + match: []string{"foo", "123foo", "barfoo"}, + notMatch: []string{"", "bar", "foobar", "barfoobaz"}, + }, + { + name: "Wildcard/Back*", + criteria: "foo*", + match: []string{"foo", "foo1", "foobar"}, + notMatch: []string{"", "bar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/FrontBack*", + criteria: "*foo*", + match: []string{"foo", "foo1", "1foo", "1foo1", "foobar", "barfoobaz"}, + notMatch: []string{"", "bar"}, + }, + { + name: "Wildcard/Front**", + criteria: "**foo", + match: []string{"foo", "123foo", "barfoo", "afoo"}, + notMatch: []string{"", "bar", "foobar", "barfoobaz"}, + }, + { + name: "Wildcard/Back**", + criteria: "foo**", + match: []string{"foo", "foo1", "fooa", "foobar"}, + notMatch: []string{"", "bar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Front*?", + criteria: "*?oo", + match: []string{"foo", "123foo", "barfoo", "afoo"}, + notMatch: []string{"", "fo", "bar", "foobar", "barfoobaz"}, + }, + { + name: "Wildcard/Back*?", + criteria: "fo*?", + match: []string{"foo", "foo1", "fooa", "foobar"}, + notMatch: []string{"", "bar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Front?*", + criteria: "?*oo", + match: []string{"foo", "123foo", "barfoo", "afoo"}, + notMatch: []string{"", "oo", "fo", "bar", "foobar", "barfoobaz"}, + }, + { + name: "Wildcard/Back?*", + criteria: "fo?*", + match: []string{"foo", "foo1", "fooa", "foobar"}, + notMatch: []string{"", "fo", "bar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Middle*", + criteria: "f*o", + match: []string{"fo", "foo", "fooo", "fo12baro"}, + notMatch: []string{"", "bar", "barfoo", "barfoobaz"}, + }, + { + name: "Wildcard/Middle?", + criteria: "f?o", + match: []string{"foo", "f1o"}, + notMatch: []string{"", "fo", "fooo", "fo12baro", "bar"}, + }, + { + name: "Wildcard/MetaCharacters", + criteria: "*.+()|[]{}^$-_?", + match: []string{"aa.+()|[]{}^$-_b", ".+()|[]{}^$-_b"}, + notMatch: []string{"", "foo", ".+()|[]{}^$-_"}, + }, + } + + return func(t *testing.T) { + for _, test := range tests { + v := NewView(Instrument{Name: test.criteria}, Stream{}) + t.Run(test.name, func(t *testing.T) { + for _, n := range test.match { + _, matches := v(Instrument{Name: n}) + assert.Truef(t, matches, "%s does not match %s", test.criteria, n) + } + for _, n := range test.notMatch { + _, matches := v(Instrument{Name: n}) + assert.Falsef(t, matches, "%s matches %s", test.criteria, n) + } + }) + } + } +} + +func TestNewViewMatch(t *testing.T) { + // Avoid boilerplate for name match testing. + t.Run("Name", testNewViewMatchName()) + + tests := []struct { + name string + criteria Instrument + matches []Instrument + notMatches []Instrument + }{ + { + name: "Empty", + notMatches: []Instrument{{}, {Name: "foo"}, completeIP}, + }, + { + name: "Description", + criteria: Instrument{Description: "foo desc"}, + matches: []Instrument{{Description: "foo desc"}, completeIP}, + notMatches: []Instrument{{}, {Description: "foo"}, {Description: "desc"}}, + }, + { + name: "Kind", + criteria: Instrument{Kind: InstrumentKindSyncCounter}, + matches: []Instrument{{Kind: InstrumentKindSyncCounter}, completeIP}, + notMatches: []Instrument{ + {}, + {Kind: InstrumentKindSyncUpDownCounter}, + {Kind: InstrumentKindSyncHistogram}, + {Kind: InstrumentKindAsyncCounter}, + {Kind: InstrumentKindAsyncUpDownCounter}, + {Kind: InstrumentKindAsyncGauge}, + }, + }, + { + name: "Unit", + criteria: Instrument{Unit: unit.Bytes}, + matches: []Instrument{{Unit: unit.Bytes}, completeIP}, + notMatches: []Instrument{ + {}, + {Unit: unit.Dimensionless}, + {Unit: unit.Unit("K")}, + }, + }, + { + name: "ScopeName", + criteria: Instrument{Scope: scope("TestNewViewMatch", "", "")}, + matches: []Instrument{ + {Scope: scope("TestNewViewMatch", "", "")}, + completeIP, + }, + notMatches: []Instrument{ + {}, + {Scope: scope("PrefixTestNewViewMatch", "", "")}, + {Scope: scope("TestNewViewMatchSuffix", "", "")}, + {Scope: scope("alt", "", "")}, + }, + }, + { + name: "ScopeVersion", + criteria: Instrument{Scope: scope("", "v0.1.0", "")}, + matches: []Instrument{ + {Scope: scope("", "v0.1.0", "")}, + completeIP, + }, + notMatches: []Instrument{ + {}, + {Scope: scope("", "v0.1.0-RC1", "")}, + {Scope: scope("", "v0.1.1", "")}, + }, + }, + { + name: "ScopeSchemaURL", + criteria: Instrument{Scope: scope("", "", schemaURL)}, + matches: []Instrument{ + {Scope: scope("", "", schemaURL)}, + completeIP, + }, + notMatches: []Instrument{ + {}, + {Scope: scope("", "", schemaURL+"/path")}, + {Scope: scope("", "", "https://go.dev")}, + }, + }, + { + name: "Scope", + criteria: Instrument{Scope: scope("TestNewViewMatch", "v0.1.0", schemaURL)}, + matches: []Instrument{ + {Scope: scope("TestNewViewMatch", "v0.1.0", schemaURL)}, + completeIP, + }, + notMatches: []Instrument{ + {}, + {Scope: scope("CompleteMisMatch", "v0.2.0", "https://go.dev")}, + {Scope: scope("NameMisMatch", "v0.1.0", schemaURL)}, + }, + }, + { + name: "Complete", + criteria: completeIP, + matches: []Instrument{completeIP}, + notMatches: []Instrument{ + {}, + {Name: "foo"}, + { + Name: "Wrong Name", + Description: "foo desc", + Kind: InstrumentKindSyncCounter, + Unit: unit.Bytes, + Scope: scope("TestNewViewMatch", "v0.1.0", schemaURL), + }, + { + Name: "foo", + Description: "Wrong Description", + Kind: InstrumentKindSyncCounter, + Unit: unit.Bytes, + Scope: scope("TestNewViewMatch", "v0.1.0", schemaURL), + }, + { + Name: "foo", + Description: "foo desc", + Kind: InstrumentKindAsyncUpDownCounter, + Unit: unit.Bytes, + Scope: scope("TestNewViewMatch", "v0.1.0", schemaURL), + }, + { + Name: "foo", + Description: "foo desc", + Kind: InstrumentKindSyncCounter, + Unit: unit.Dimensionless, + Scope: scope("TestNewViewMatch", "v0.1.0", schemaURL), + }, + { + Name: "foo", + Description: "foo desc", + Kind: InstrumentKindSyncCounter, + Unit: unit.Bytes, + Scope: scope("Wrong Scope Name", "v0.1.0", schemaURL), + }, + { + Name: "foo", + Description: "foo desc", + Kind: InstrumentKindSyncCounter, + Unit: unit.Bytes, + Scope: scope("TestNewViewMatch", "v1.4.3", schemaURL), + }, + { + Name: "foo", + Description: "foo desc", + Kind: InstrumentKindSyncCounter, + Unit: unit.Bytes, + Scope: scope("TestNewViewMatch", "v0.1.0", "https://go.dev"), + }, + }, + }, + } + + for _, test := range tests { + v := NewView(test.criteria, Stream{}) + t.Run(test.name, func(t *testing.T) { + for _, instrument := range test.matches { + _, matches := v(instrument) + assert.Truef(t, matches, "view does not match %#v", instrument) + } + + for _, instrument := range test.notMatches { + _, matches := v(instrument) + assert.Falsef(t, matches, "view matches %#v", instrument) + } + }) + } +} + +func TestNewViewReplace(t *testing.T) { + alt := "alternative value" + tests := []struct { + name string + mask Stream + want func(Instrument) Stream + }{ + { + name: "Nothing", + want: func(ip Instrument) Stream { + return Stream{Instrument: ip} + }, + }, + { + name: "Name", + mask: Stream{Instrument: Instrument{Name: alt}}, + want: func(ip Instrument) Stream { + ip.Name = alt + return Stream{Instrument: ip} + }, + }, + { + name: "Description", + mask: Stream{Instrument: Instrument{Description: alt}}, + want: func(ip Instrument) Stream { + ip.Description = alt + return Stream{Instrument: ip} + }, + }, + { + name: "Kind", + mask: Stream{Instrument: Instrument{ + Kind: InstrumentKindAsyncUpDownCounter, + }}, + want: func(ip Instrument) Stream { + ip.Kind = InstrumentKindAsyncUpDownCounter + return Stream{Instrument: ip} + }, + }, + { + name: "Unit", + mask: Stream{Instrument: Instrument{Unit: unit.Dimensionless}}, + want: func(ip Instrument) Stream { + ip.Unit = unit.Dimensionless + return Stream{Instrument: ip} + }, + }, + { + name: "ScopeName", + mask: Stream{Instrument: Instrument{Scope: scope(alt, "", "")}}, + want: func(ip Instrument) Stream { + ip.Scope.Name = alt + return Stream{Instrument: ip} + }, + }, + { + name: "ScopeVersion", + mask: Stream{Instrument: Instrument{Scope: scope("", alt, "")}}, + want: func(ip Instrument) Stream { + ip.Scope.Version = alt + return Stream{Instrument: ip} + }, + }, + { + name: "ScopeSchemaURL", + mask: Stream{Instrument: Instrument{Scope: scope("", "", alt)}}, + want: func(ip Instrument) Stream { + ip.Scope.SchemaURL = alt + return Stream{Instrument: ip} + }, + }, + { + name: "Scope", + mask: Stream{Instrument: Instrument{ + Scope: scope("Alt Scope Name", "1.1.1", "https://go.dev"), + }}, + want: func(ip Instrument) Stream { + ip.Scope.Name = "Alt Scope Name" + ip.Scope.Version = "1.1.1" + ip.Scope.SchemaURL = "https://go.dev" + return Stream{Instrument: ip} + }, + }, + { + name: "Aggregation", + mask: Stream{ + Aggregation: aggregation.LastValue{}, + }, + want: func(ip Instrument) Stream { + return Stream{ + Instrument: ip, + Aggregation: aggregation.LastValue{}, + } + }, + }, + { + name: "Complete", + mask: Stream{ + Instrument: Instrument{ + Name: alt, + Description: alt, + Kind: InstrumentKindAsyncUpDownCounter, + Unit: unit.Dimensionless, + Scope: scope(alt, alt, alt), + }, + Aggregation: aggregation.LastValue{}, + }, + want: func(ip Instrument) Stream { + ip.Name = alt + ip.Description = alt + ip.Kind = InstrumentKindAsyncUpDownCounter + ip.Unit = unit.Dimensionless + ip.Scope.Name = alt + ip.Scope.Version = alt + ip.Scope.SchemaURL = alt + return Stream{ + Instrument: ip, + Aggregation: aggregation.LastValue{}, + } + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + got, match := NewView(completeIP, test.mask)(completeIP) + require.True(t, match, "view did not match exact criteria") + assert.Equal(t, test.want(completeIP), got) + }) + } + + // Go does not allow for the comparison of function values, even their + // addresses. Therefore, the AttributeFilter field needs an alternative + // testing strategy. + t.Run("AttributeFilter", func(t *testing.T) { + allowed := attribute.String("key", "val") + filter := func(kv attribute.KeyValue) bool { + return kv == allowed + } + mask := Stream{AttributeFilter: filter} + got, match := NewView(completeIP, mask)(completeIP) + require.True(t, match, "view did not match exact criteria") + require.NotNil(t, got.AttributeFilter, "AttributeFilter not set") + assert.True(t, got.AttributeFilter(allowed), "wrong AttributeFilter") + other := attribute.String("key", "other val") + assert.False(t, got.AttributeFilter(other), "wrong AttributeFilter") + }) +} + +type badAgg struct { + aggregation.Aggregation + err error +} + +func (a badAgg) Copy() aggregation.Aggregation { return a } + +func (a badAgg) Err() error { return a.err } + +func TestNewViewAggregationErrorLogged(t *testing.T) { + tLog := testr.NewWithOptions(t, testr.Options{Verbosity: 6}) + l := &logCounter{LogSink: tLog.GetSink()} + otel.SetLogger(logr.New(l)) + + agg := badAgg{err: assert.AnError} + mask := Stream{Aggregation: agg} + got, match := NewView(completeIP, mask)(completeIP) + require.True(t, match, "view did not match exact criteria") + assert.Nil(t, got.Aggregation, "erroring aggregation used") + assert.Equal(t, 1, l.ErrorN()) +} From dd3512d9b43f0e130d97503d7b7d6a7f31527a20 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 15:03:08 -0800 Subject: [PATCH 07/15] Add changes to changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b81d70ccdc..6d33e08b5a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `OTEL_EXPORTER_OTLP_CLIENT_CERTIFICATE` - `OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE` - `OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE` +- The `View` type and related `NewView` function to create a view according to the OpenTelemetry specification are added to `go.opentelemetry.io/otel/sdk/metric`. + These additions are uses as replacements for the `View` type and `New` function from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) +- The `Instrument` and `InstrumentKind` type are added to `go.opentelemetry.io/otel/sdk/metric`. + These additions are uses as replacements for the `Instrument` and `InstrumentKind` types from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) +- The `Stream` type is added to `go.opentelemetry.io/otel/sdk/metric` to define a metric data stream a view will produce. (#3459) ### Changed From b33ebafd07639f14c3c9d7a0469c182795b5f7ba Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 16:07:32 -0800 Subject: [PATCH 08/15] Apply suggestions from code review Co-authored-by: Anthony Mirabella --- CHANGELOG.md | 2 +- sdk/metric/instrument.go | 2 +- sdk/metric/view.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d33e08b5a7..00c24e49c91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - `OTEL_EXPORTER_OTLP_TRACES_CLIENT_CERTIFICATE` - `OTEL_EXPORTER_OTLP_METRICS_CLIENT_CERTIFICATE` - The `View` type and related `NewView` function to create a view according to the OpenTelemetry specification are added to `go.opentelemetry.io/otel/sdk/metric`. - These additions are uses as replacements for the `View` type and `New` function from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) + These additions are replacements for the `View` type and `New` function from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) - The `Instrument` and `InstrumentKind` type are added to `go.opentelemetry.io/otel/sdk/metric`. These additions are uses as replacements for the `Instrument` and `InstrumentKind` types from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) - The `Stream` type is added to `go.opentelemetry.io/otel/sdk/metric` to define a metric data stream a view will produce. (#3459) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 589bcacb977..1886f53c797 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -121,7 +121,7 @@ func (p Instrument) empty() bool { p.Scope == zeroScope } -// matches returns if all the non-zero-value fields of o match the +// matches returns whether all the non-zero-value fields of o match the // corresponding fields of p. // // If o is empty true is returned. diff --git a/sdk/metric/view.go b/sdk/metric/view.go index 6858bb74646..cb743b58c9b 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -54,8 +54,8 @@ func NewView(criteria Instrument, mask Stream) View { if strings.ContainsAny(criteria.Name, "*?") { pattern := regexp.QuoteMeta(criteria.Name) pattern = "^" + pattern + "$" - pattern = strings.ReplaceAll(pattern, "\\?", ".") - pattern = strings.ReplaceAll(pattern, "\\*", ".*") + pattern = strings.ReplaceAll(pattern, `\?`, ".") + pattern = strings.ReplaceAll(pattern, `\*`, ".*") re := regexp.MustCompile(pattern) matchFunc = func(p Instrument) bool { return re.MatchString(p.Name) && From 9519abd3e3768599aad14b3d7a0a640c1a29d2e8 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 16:08:39 -0800 Subject: [PATCH 09/15] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00c24e49c91..afe8936112d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `View` type and related `NewView` function to create a view according to the OpenTelemetry specification are added to `go.opentelemetry.io/otel/sdk/metric`. These additions are replacements for the `View` type and `New` function from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) - The `Instrument` and `InstrumentKind` type are added to `go.opentelemetry.io/otel/sdk/metric`. - These additions are uses as replacements for the `Instrument` and `InstrumentKind` types from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) + These additions are replacements for the `Instrument` and `InstrumentKind` types from `go.opentelemetry.io/otel/sdk/metric/view`. (#3459) - The `Stream` type is added to `go.opentelemetry.io/otel/sdk/metric` to define a metric data stream a view will produce. (#3459) ### Changed From f7f7d0e65fa52394d084032984a0cba3f871afbf Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Thu, 10 Nov 2022 16:50:11 -0800 Subject: [PATCH 10/15] Update match and mask comments of Instrument --- sdk/metric/instrument.go | 100 +++++++++++++++++++-------------------- sdk/metric/view.go | 18 +++---- 2 files changed, 59 insertions(+), 59 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index 1886f53c797..ab265c5e3df 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -85,84 +85,84 @@ type Instrument struct { nonComparable // nolint: unused } -// mask returns a copy of p with all non-zero-value fields of m replacing the -// fields of the returned copy. -func (p Instrument) mask(m Instrument) Instrument { +// mask returns a copy of base with all non-zero-value fields of m replacing +// the fields of the returned copy. +func (base Instrument) mask(m Instrument) Instrument { + cp := base if m.Name != "" { - p.Name = m.Name + cp.Name = m.Name } if m.Description != "" { - p.Description = m.Description + cp.Description = m.Description } if m.Kind != zeroInstrumentKind { - p.Kind = m.Kind + cp.Kind = m.Kind } if m.Unit != zeroUnit { - p.Unit = m.Unit + cp.Unit = m.Unit } if m.Scope.Name != "" { - p.Scope.Name = m.Scope.Name + cp.Scope.Name = m.Scope.Name } if m.Scope.Version != "" { - p.Scope.Version = m.Scope.Version + cp.Scope.Version = m.Scope.Version } if m.Scope.SchemaURL != "" { - p.Scope.SchemaURL = m.Scope.SchemaURL + cp.Scope.SchemaURL = m.Scope.SchemaURL } - return p + return cp } -// empty returns if all fields of p are their zero-value. -func (p Instrument) empty() bool { - return p.Name == "" && - p.Description == "" && - p.Kind == zeroInstrumentKind && - p.Unit == zeroUnit && - p.Scope == zeroScope +// empty returns if all fields of i are their zero-value. +func (i Instrument) empty() bool { + return i.Name == "" && + i.Description == "" && + i.Kind == zeroInstrumentKind && + i.Unit == zeroUnit && + i.Scope == zeroScope } -// matches returns whether all the non-zero-value fields of o match the -// corresponding fields of p. -// -// If o is empty true is returned. -func (p Instrument) matches(o Instrument) bool { - return p.matchesName(o) && - p.matchesDescription(o) && - p.matchesKind(o) && - p.matchesUnit(o) && - p.matchesScope(o) +// matches returns whether all the non-zero-value fields of i match the +// corresponding fields of other. If i is empty it will match all other, and +// true will always be returned. +func (i Instrument) matches(other Instrument) bool { + return i.matchesName(other) && + i.matchesDescription(other) && + i.matchesKind(other) && + i.matchesUnit(other) && + i.matchesScope(other) } -// matchesName returns true if the Name field of o is a non-zero-value and -// equals the Name field of p, otherwise false. -func (p Instrument) matchesName(o Instrument) bool { - return p.Name == "" || p.Name == o.Name +// matchesName returns true if the Name of i is "" or it equals the Name of +// other, otherwise false. +func (i Instrument) matchesName(other Instrument) bool { + return i.Name == "" || i.Name == other.Name } -// matchesDescription returns true if the Description field of o is a -// non-zero-value and equals the Description field of p, otherwise false. -func (p Instrument) matchesDescription(o Instrument) bool { - return p.Description == "" || p.Description == o.Description +// matchesDescription returns true if the Description of i is "" or it equals +// the Description of other, otherwise false. +func (i Instrument) matchesDescription(other Instrument) bool { + return i.Description == "" || i.Description == other.Description } -// matchesKind returns true if the Kind field of o is a non-zero-value and -// equals the Kind field of p, otherwise false. -func (p Instrument) matchesKind(o Instrument) bool { - return p.Kind == zeroInstrumentKind || p.Kind == o.Kind +// matchesKind returns true if the Kind of i is its zero-value or it equals the +// Kind of other, otherwise false. +func (i Instrument) matchesKind(other Instrument) bool { + return i.Kind == zeroInstrumentKind || i.Kind == other.Kind } -// matchesUnit returns true if the Unit field of o is a non-zero-value and -// equals the Unit field of p, otherwise false. -func (p Instrument) matchesUnit(o Instrument) bool { - return p.Unit == zeroUnit || p.Unit == o.Unit +// matchesUnit returns true if the Unit of i is its zero-value or it equals the +// Unit of other, otherwise false. +func (i Instrument) matchesUnit(other Instrument) bool { + return i.Unit == zeroUnit || i.Unit == other.Unit } -// matchesScope returns true if the Scope field of o is a non-zero-value and -// equals the Scope field of p, otherwise false. -func (p Instrument) matchesScope(o Instrument) bool { - return (p.Scope.Name == "" || p.Scope.Name == o.Scope.Name) && - (p.Scope.Version == "" || p.Scope.Version == o.Scope.Version) && - (p.Scope.SchemaURL == "" || p.Scope.SchemaURL == o.Scope.SchemaURL) +// matchesScope returns true if the Scope of i is its zero-value or it equals +// the Scope of other, otherwise false. +func (i Instrument) matchesScope(other Instrument) bool { + return (i.Scope.Name == "" || i.Scope.Name == other.Scope.Name) && + (i.Scope.Version == "" || i.Scope.Version == other.Scope.Version) && + (i.Scope.SchemaURL == "" || i.Scope.SchemaURL == other.Scope.SchemaURL) } // Stream describes the stream of data an instrument produces. diff --git a/sdk/metric/view.go b/sdk/metric/view.go index cb743b58c9b..458450028f0 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -57,12 +57,12 @@ func NewView(criteria Instrument, mask Stream) View { pattern = strings.ReplaceAll(pattern, `\?`, ".") pattern = strings.ReplaceAll(pattern, `\*`, ".*") re := regexp.MustCompile(pattern) - matchFunc = func(p Instrument) bool { - return re.MatchString(p.Name) && - criteria.matchesDescription(p) && - criteria.matchesKind(p) && - criteria.matchesUnit(p) && - criteria.matchesScope(p) + matchFunc = func(i Instrument) bool { + return re.MatchString(i.Name) && + criteria.matchesDescription(i) && + criteria.matchesKind(i) && + criteria.matchesUnit(i) && + criteria.matchesScope(i) } } else { matchFunc = criteria.matches @@ -81,10 +81,10 @@ func NewView(criteria Instrument, mask Stream) View { } } - return func(p Instrument) (Stream, bool) { - if matchFunc(p) { + return func(i Instrument) (Stream, bool) { + if matchFunc(i) { stream := Stream{ - Instrument: p.mask(mask.Instrument), + Instrument: i.mask(mask.Instrument), Aggregation: agg, AttributeFilter: mask.AttributeFilter, } From 623525e369d3af47a65294c649a1791e518ec1aa Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 11 Nov 2022 12:22:28 -0800 Subject: [PATCH 11/15] Explain wildcard logic in NewView with comment --- sdk/metric/view.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sdk/metric/view.go b/sdk/metric/view.go index 458450028f0..330df323803 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -52,6 +52,8 @@ func NewView(criteria Instrument, mask Stream) View { var matchFunc func(Instrument) bool if strings.ContainsAny(criteria.Name, "*?") { + // Handle branching here in NewView instead of criteria.matches so + // criteria.matches remains inlinable for the simple case. pattern := regexp.QuoteMeta(criteria.Name) pattern = "^" + pattern + "$" pattern = strings.ReplaceAll(pattern, `\?`, ".") From 56ef349c60faba4c4d4d79da3e37204f90b3bb50 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 11 Nov 2022 12:33:57 -0800 Subject: [PATCH 12/15] Drop views that replace name for multi-inst match --- sdk/metric/view.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/sdk/metric/view.go b/sdk/metric/view.go index 330df323803..e82bcba1be0 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -15,6 +15,7 @@ package metric // import "go.opentelemetry.io/otel/sdk/metric" import ( + "errors" "regexp" "strings" @@ -22,6 +23,12 @@ import ( "go.opentelemetry.io/otel/sdk/metric/aggregation" ) +var ( + errMultiInst = errors.New("name replacement for multiple instruments") + + emptyView = func(Instrument) (Stream, bool) { return Stream{}, false } +) + // View is an override to the default behavior of the SDK. It defines how data // should be collected for certain instruments. It returns true and the exact // Stream to use for matching Instruments. Otherwise, if the view does not @@ -47,11 +54,20 @@ type View func(Instrument) (Stream, bool) // zero out an Stream field returned from a View, create a View directly. func NewView(criteria Instrument, mask Stream) View { if criteria.empty() { - return func(Instrument) (Stream, bool) { return Stream{}, false } + return emptyView } var matchFunc func(Instrument) bool if strings.ContainsAny(criteria.Name, "*?") { + if mask.Name != "" { + global.Error( + errMultiInst, "dropping view", + "criteria", criteria, + "mask", mask, + ) + return emptyView + } + // Handle branching here in NewView instead of criteria.matches so // criteria.matches remains inlinable for the simple case. pattern := regexp.QuoteMeta(criteria.Name) @@ -76,8 +92,8 @@ func NewView(criteria Instrument, mask Stream) View { if err := agg.Err(); err != nil { global.Error( err, "not using aggregation with view", - "aggregation", agg, - "view", criteria, + "criteria", criteria, + "mask", mask, ) agg = nil } From a3505888d0d0859afb68abc010d78e23712399f4 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 11 Nov 2022 12:36:55 -0800 Subject: [PATCH 13/15] Comment how users are expected to match zero-vals --- sdk/metric/view.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/metric/view.go b/sdk/metric/view.go index e82bcba1be0..82eb7743af6 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -39,7 +39,8 @@ type View func(Instrument) (Stream, bool) // match criteria. The returned View will only apply mask if all non-zero-value // fields of criteria match the corresponding Instrument passed to the view. If // no criteria are provided, all field of criteria are their zero-values, a -// view that matches no instruments is returned. +// view that matches no instruments is returned. If you need to match a +// zero-value field, create a View directly. // // The Name field of criteria supports wildcard pattern matching. The wildcard // "*" is recognized as matching zero or more characters, and "?" is recognized From 3bdcf8ad06df389d36ca5fb15570af443c28710d Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 11 Nov 2022 14:49:40 -0800 Subject: [PATCH 14/15] Remove InstrumentKind and Scope from Stream --- sdk/metric/instrument.go | 36 ++---------- sdk/metric/view.go | 28 ++++++--- sdk/metric/view_test.go | 120 +++++++++++++-------------------------- 3 files changed, 64 insertions(+), 120 deletions(-) diff --git a/sdk/metric/instrument.go b/sdk/metric/instrument.go index ab265c5e3df..8009540e6b2 100644 --- a/sdk/metric/instrument.go +++ b/sdk/metric/instrument.go @@ -85,34 +85,6 @@ type Instrument struct { nonComparable // nolint: unused } -// mask returns a copy of base with all non-zero-value fields of m replacing -// the fields of the returned copy. -func (base Instrument) mask(m Instrument) Instrument { - cp := base - if m.Name != "" { - cp.Name = m.Name - } - if m.Description != "" { - cp.Description = m.Description - } - if m.Kind != zeroInstrumentKind { - cp.Kind = m.Kind - } - if m.Unit != zeroUnit { - cp.Unit = m.Unit - } - if m.Scope.Name != "" { - cp.Scope.Name = m.Scope.Name - } - if m.Scope.Version != "" { - cp.Scope.Version = m.Scope.Version - } - if m.Scope.SchemaURL != "" { - cp.Scope.SchemaURL = m.Scope.SchemaURL - } - return cp -} - // empty returns if all fields of i are their zero-value. func (i Instrument) empty() bool { return i.Name == "" && @@ -167,8 +139,12 @@ func (i Instrument) matchesScope(other Instrument) bool { // Stream describes the stream of data an instrument produces. type Stream struct { - Instrument - + // Name is the human-readable identifier of the stream. + Name string + // Description describes the purpose of the data. + Description string + // Unit is the unit of measurement recorded. + Unit unit.Unit // Aggregation the stream uses for an instrument. Aggregation aggregation.Aggregation // AttributeFilter applied to all attributes recorded for an instrument. diff --git a/sdk/metric/view.go b/sdk/metric/view.go index 82eb7743af6..7f8da2d5892 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -48,11 +48,11 @@ type View func(Instrument) (Stream, bool) // all instrument names. // // The Stream mask only applies updates for non-zero-value fields. By default, -// the Instrument the View matches against will be use for the returned Stream -// and no Aggregation or AttributeFilter are set. If mask has a non-zero-value -// value for any of the Aggregation or AttributeFilter fields, or any of the -// Instrument fields, that value is used instead of the default. If you need to -// zero out an Stream field returned from a View, create a View directly. +// the Instrument the View matches against will be use for the Name, +// Description, and Unit of the returned Stream and no Aggregation or +// AttributeFilter are set. All non-zero-value value fields of mask are used +// instead of the default. If you need to zero out an Stream field returned +// from a View, create a View directly. func NewView(criteria Instrument, mask Stream) View { if criteria.empty() { return emptyView @@ -102,13 +102,23 @@ func NewView(criteria Instrument, mask Stream) View { return func(i Instrument) (Stream, bool) { if matchFunc(i) { - stream := Stream{ - Instrument: i.mask(mask.Instrument), + return Stream{ + Name: nonZero(mask.Name, i.Name), + Description: nonZero(mask.Description, i.Description), + Unit: nonZero(mask.Unit, i.Unit), Aggregation: agg, AttributeFilter: mask.AttributeFilter, - } - return stream, true + }, true } return Stream{}, false } } + +// nonZero returns v if it is non-zero-valued, otherwise alt. +func nonZero[T comparable](v, alt T) T { + var zero T + if v != zero { + return v + } + return alt +} diff --git a/sdk/metric/view_test.go b/sdk/metric/view_test.go index 9d075a8ffd6..082b0106ebd 100644 --- a/sdk/metric/view_test.go +++ b/sdk/metric/view_test.go @@ -350,88 +350,55 @@ func TestNewViewReplace(t *testing.T) { }{ { name: "Nothing", - want: func(ip Instrument) Stream { - return Stream{Instrument: ip} + want: func(i Instrument) Stream { + return Stream{ + Name: i.Name, + Description: i.Description, + Unit: i.Unit, + } }, }, { name: "Name", - mask: Stream{Instrument: Instrument{Name: alt}}, - want: func(ip Instrument) Stream { - ip.Name = alt - return Stream{Instrument: ip} + mask: Stream{Name: alt}, + want: func(i Instrument) Stream { + return Stream{ + Name: alt, + Description: i.Description, + Unit: i.Unit, + } }, }, { name: "Description", - mask: Stream{Instrument: Instrument{Description: alt}}, - want: func(ip Instrument) Stream { - ip.Description = alt - return Stream{Instrument: ip} - }, - }, - { - name: "Kind", - mask: Stream{Instrument: Instrument{ - Kind: InstrumentKindAsyncUpDownCounter, - }}, - want: func(ip Instrument) Stream { - ip.Kind = InstrumentKindAsyncUpDownCounter - return Stream{Instrument: ip} + mask: Stream{Description: alt}, + want: func(i Instrument) Stream { + return Stream{ + Name: i.Name, + Description: alt, + Unit: i.Unit, + } }, }, { name: "Unit", - mask: Stream{Instrument: Instrument{Unit: unit.Dimensionless}}, - want: func(ip Instrument) Stream { - ip.Unit = unit.Dimensionless - return Stream{Instrument: ip} - }, - }, - { - name: "ScopeName", - mask: Stream{Instrument: Instrument{Scope: scope(alt, "", "")}}, - want: func(ip Instrument) Stream { - ip.Scope.Name = alt - return Stream{Instrument: ip} - }, - }, - { - name: "ScopeVersion", - mask: Stream{Instrument: Instrument{Scope: scope("", alt, "")}}, - want: func(ip Instrument) Stream { - ip.Scope.Version = alt - return Stream{Instrument: ip} - }, - }, - { - name: "ScopeSchemaURL", - mask: Stream{Instrument: Instrument{Scope: scope("", "", alt)}}, - want: func(ip Instrument) Stream { - ip.Scope.SchemaURL = alt - return Stream{Instrument: ip} - }, - }, - { - name: "Scope", - mask: Stream{Instrument: Instrument{ - Scope: scope("Alt Scope Name", "1.1.1", "https://go.dev"), - }}, - want: func(ip Instrument) Stream { - ip.Scope.Name = "Alt Scope Name" - ip.Scope.Version = "1.1.1" - ip.Scope.SchemaURL = "https://go.dev" - return Stream{Instrument: ip} + mask: Stream{Unit: unit.Dimensionless}, + want: func(i Instrument) Stream { + return Stream{ + Name: i.Name, + Description: i.Description, + Unit: unit.Dimensionless, + } }, }, { name: "Aggregation", - mask: Stream{ - Aggregation: aggregation.LastValue{}, - }, - want: func(ip Instrument) Stream { + mask: Stream{Aggregation: aggregation.LastValue{}}, + want: func(i Instrument) Stream { return Stream{ - Instrument: ip, + Name: i.Name, + Description: i.Description, + Unit: i.Unit, Aggregation: aggregation.LastValue{}, } }, @@ -439,25 +406,16 @@ func TestNewViewReplace(t *testing.T) { { name: "Complete", mask: Stream{ - Instrument: Instrument{ - Name: alt, - Description: alt, - Kind: InstrumentKindAsyncUpDownCounter, - Unit: unit.Dimensionless, - Scope: scope(alt, alt, alt), - }, + Name: alt, + Description: alt, + Unit: unit.Dimensionless, Aggregation: aggregation.LastValue{}, }, - want: func(ip Instrument) Stream { - ip.Name = alt - ip.Description = alt - ip.Kind = InstrumentKindAsyncUpDownCounter - ip.Unit = unit.Dimensionless - ip.Scope.Name = alt - ip.Scope.Version = alt - ip.Scope.SchemaURL = alt + want: func(i Instrument) Stream { return Stream{ - Instrument: ip, + Name: alt, + Description: alt, + Unit: unit.Dimensionless, Aggregation: aggregation.LastValue{}, } }, From f2c47b9b66b51e942cdfd349e180659b8200f013 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Fri, 11 Nov 2022 14:58:43 -0800 Subject: [PATCH 15/15] Fix redundant word in NewView comment --- sdk/metric/view.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/metric/view.go b/sdk/metric/view.go index 7f8da2d5892..b8fc363a0a7 100644 --- a/sdk/metric/view.go +++ b/sdk/metric/view.go @@ -50,9 +50,9 @@ type View func(Instrument) (Stream, bool) // The Stream mask only applies updates for non-zero-value fields. By default, // the Instrument the View matches against will be use for the Name, // Description, and Unit of the returned Stream and no Aggregation or -// AttributeFilter are set. All non-zero-value value fields of mask are used -// instead of the default. If you need to zero out an Stream field returned -// from a View, create a View directly. +// AttributeFilter are set. All non-zero-value fields of mask are used instead +// of the default. If you need to zero out an Stream field returned from a +// View, create a View directly. func NewView(criteria Instrument, mask Stream) View { if criteria.empty() { return emptyView