Skip to content

Commit

Permalink
Rename instrumentID to streamID in metric SDK (#3735)
Browse files Browse the repository at this point in the history
An instrument is defined by a name, description, unit, and kind. The
instrumentID contains more identifying information than these fields.
The additional information it contains relate to what the OTel metric
data-model calls a stream. Match the terminology and remove using the
instrumentID name in case we want to use it for something else (say,
when caching instruments).
  • Loading branch information
MrAlias authored Feb 16, 2023
1 parent c1802c2 commit 0252734
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 26 deletions.
20 changes: 10 additions & 10 deletions sdk/metric/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,21 @@ func (c *cache[K, V]) Lookup(key K, f func() V) V {
type instrumentCache[N int64 | float64] struct {
// aggregators is used to ensure duplicate creations of the same instrument
// return the same instance of that instrument's aggregator.
aggregators *cache[instrumentID, aggVal[N]]
aggregators *cache[streamID, aggVal[N]]
// views is used to ensure if instruments with the same name are created,
// but do not have the same identifying properties, a warning is logged.
views *cache[string, instrumentID]
views *cache[string, streamID]
}

// newInstrumentCache returns a new instrumentCache that uses ac as the
// underlying cache for aggregators and vc as the cache for views. If ac or vc
// are nil, a new empty cache will be used.
func newInstrumentCache[N int64 | float64](ac *cache[instrumentID, aggVal[N]], vc *cache[string, instrumentID]) instrumentCache[N] {
func newInstrumentCache[N int64 | float64](ac *cache[streamID, aggVal[N]], vc *cache[string, streamID]) instrumentCache[N] {
if ac == nil {
ac = &cache[instrumentID, aggVal[N]]{}
ac = &cache[streamID, aggVal[N]]{}
}
if vc == nil {
vc = &cache[string, instrumentID]{}
vc = &cache[string, streamID]{}
}
return instrumentCache[N]{aggregators: ac, views: vc}
}
Expand All @@ -85,7 +85,7 @@ func newInstrumentCache[N int64 | float64](ac *cache[instrumentID, aggVal[N]], v
// in the cache and returned.
//
// LookupAggregator is safe to call concurrently.
func (c instrumentCache[N]) LookupAggregator(id instrumentID, f func() (internal.Aggregator[N], error)) (agg internal.Aggregator[N], err error) {
func (c instrumentCache[N]) LookupAggregator(id streamID, f func() (internal.Aggregator[N], error)) (agg internal.Aggregator[N], err error) {
v := c.aggregators.Lookup(id, func() aggVal[N] {
a, err := f()
return aggVal[N]{Aggregator: a, Err: err}
Expand All @@ -100,11 +100,11 @@ type aggVal[N int64 | float64] struct {
}

// Unique returns if id is unique or a duplicate instrument. If an instrument
// with the same name has already been created, that instrumentID will be
// returned along with false. Otherwise, id is returned with true.
// with the same name has already been created, that streamID will be returned
// along with false. Otherwise, id is returned with true.
//
// Unique is safe to call concurrently.
func (c instrumentCache[N]) Unique(id instrumentID) (instrumentID, bool) {
got := c.views.Lookup(id.Name, func() instrumentID { return id })
func (c instrumentCache[N]) Unique(id streamID) (streamID, bool) {
got := c.views.Lookup(id.Name, func() streamID { return id })
return got, id == got
}
18 changes: 9 additions & 9 deletions sdk/metric/instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,24 +150,24 @@ type Stream struct {
AttributeFilter attribute.Filter
}

// instrumentID are the identifying properties of an instrument.
type instrumentID struct {
// Name is the name of the instrument.
// streamID are the identifying properties of a stream.
type streamID struct {
// Name is the name of the stream.
Name string
// Description is the description of the instrument.
// Description is the description of the stream.
Description string
// Unit is the unit of the instrument.
// Unit is the unit of the stream.
Unit unit.Unit
// Aggregation is the aggregation data type of the instrument.
// Aggregation is the aggregation data type of the stream.
Aggregation string
// Monotonic is the monotonicity of an instruments data type. This field is
// not used for all data types, so a zero value needs to be understood in the
// context of Aggregation.
Monotonic bool
// Temporality is the temporality of an instrument's data type. This field
// is not used by some data types.
// Temporality is the temporality of a stream's data type. This field is
// not used by some data types.
Temporality metricdata.Temporality
// Number is the number type of the instrument.
// Number is the number type of the stream.
Number string
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/metric/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ type meter struct {
func newMeter(s instrumentation.Scope, p pipelines) *meter {
// viewCache ensures instrument conflicts, including number conflicts, this
// meter is asked to create are logged to the user.
var viewCache cache[string, instrumentID]
var viewCache cache[string, streamID]

// Passing nil as the ac parameter to newInstrumentCache will have each
// create its own aggregator cache.
Expand Down
8 changes: 4 additions & 4 deletions sdk/metric/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum
)
}

id := i.instrumentID(kind, stream)
id := i.streamID(kind, stream)
// If there is a conflict, the specification says the view should
// still be applied and a warning should be logged.
i.logConflict(id)
Expand Down Expand Up @@ -316,7 +316,7 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum

// logConflict validates if an instrument with the same name as id has already
// been created. If that instrument conflicts with id, a warning is logged.
func (i *inserter[N]) logConflict(id instrumentID) {
func (i *inserter[N]) logConflict(id streamID) {
existing, unique := i.cache.Unique(id)
if unique {
return
Expand All @@ -334,9 +334,9 @@ func (i *inserter[N]) logConflict(id instrumentID) {
)
}

func (i *inserter[N]) instrumentID(kind InstrumentKind, stream Stream) instrumentID {
func (i *inserter[N]) streamID(kind InstrumentKind, stream Stream) streamID {
var zero N
id := instrumentID{
id := streamID{
Name: stream.Name,
Description: stream.Description,
Unit: stream.Unit,
Expand Down
4 changes: 2 additions & 2 deletions sdk/metric/pipeline_registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ func TestPipelineRegistryCreateAggregatorsIncompatibleInstrument(t *testing.T) {
p := newPipelines(resource.Empty(), readers, views)
inst := Instrument{Name: "foo", Kind: InstrumentKindObservableGauge}

vc := cache[string, instrumentID]{}
vc := cache[string, streamID]{}
ri := newResolver(p, newInstrumentCache[int64](nil, &vc))
intAggs, err := ri.Aggregators(inst)
assert.Error(t, err)
Expand Down Expand Up @@ -397,7 +397,7 @@ func TestResolveAggregatorsDuplicateErrors(t *testing.T) {

p := newPipelines(resource.Empty(), readers, views)

vc := cache[string, instrumentID]{}
vc := cache[string, streamID]{}
ri := newResolver(p, newInstrumentCache[int64](nil, &vc))
intAggs, err := ri.Aggregators(fooInst)
assert.NoError(t, err)
Expand Down

0 comments on commit 0252734

Please sign in to comment.