Skip to content

Commit

Permalink
Return the first-seen for instrument name conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
MrAlias committed Aug 9, 2023
1 parent 4246678 commit 5b40f35
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
11 changes: 11 additions & 0 deletions sdk/metric/instrument.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"errors"
"fmt"
"strings"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric"
Expand Down Expand Up @@ -187,6 +188,16 @@ type instID struct {
Number string
}

// Returns a normalized version of the instID i.
//
// Instrument names are considered case-insensitive. Standardize the instrument
// name to always be lowercase so comparisons will not be made on the name
// casing.
func (i instID) normalize() instID {
i.Name = strings.ToLower(i.Name)
return i
}

type int64Inst struct {
measures []aggregate.Measure[int64]

Expand Down
16 changes: 12 additions & 4 deletions sdk/metric/pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,12 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum
// If there is a conflict, the specification says the view should
// still be applied and a warning should be logged.
i.logConflict(id)
cv := i.aggregators.Lookup(id, func() aggVal[N] {

// If there are requests for the same instrument with different name
// casing, the first-seen needs to be returned. Use a normalize ID for the
// cache lookup to ensure the correct comparison.
normID := id.normalize()
cv := i.aggregators.Lookup(normID, func() aggVal[N] {
b := aggregate.Builder[N]{
Temporality: i.pipeline.reader.temporality(kind),
}
Expand All @@ -344,6 +349,8 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum
return aggVal[N]{0, nil, nil}
}
i.pipeline.addSync(scope, instrumentSync{
// Use the first-seen name casing for this and all subsequent
// requests of this instrument.
name: stream.Name,
description: stream.Description,
unit: stream.Unit,
Expand All @@ -355,12 +362,13 @@ func (i *inserter[N]) cachedAggregator(scope instrumentation.Scope, kind Instrum
return cv.Measure, cv.ID, cv.Err
}

// 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.
// logConflict validates if an instrument with the same case-insensitive name
// as id has already been created. If that instrument conflicts with id, a
// warning is logged.
func (i *inserter[N]) logConflict(id instID) {
// The API specification defines names as case-insensitive. If there is a
// different casing of a name it needs to be a conflict.
name := strings.ToLower(id.Name)
name := id.normalize().Name
existing := i.views.Lookup(name, func() instID { return id })
if id == existing {
return
Expand Down

0 comments on commit 5b40f35

Please sign in to comment.