Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

## [Unreleased]

### Fixed

- Fix `WithInstrumentationAttributes` options in `go.opentelemetry.io/otel/trace`, `go.opentelemetry.io/otel/metric`, and `go.opentelemetry.io/otel/log` to properly merge attributes when passed multiple times instead of replacing them. Attributes with duplicate keys will use the last value passed. (#7300)

### Removed

- Drop support for [Go 1.23]. (#7274)
Expand Down
29 changes: 28 additions & 1 deletion log/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,40 @@ func WithInstrumentationVersion(version string) LoggerOption {
})
}

// mergeSets returns the union of keys between a and b. Any duplicate keys will
// use the value associated with b.
func mergeSets(a, b attribute.Set) attribute.Set {
// NewMergeIterator uses the first value for any duplicates.
iter := attribute.NewMergeIterator(&b, &a)
merged := make([]attribute.KeyValue, 0, a.Len()+b.Len())
for iter.Next() {
merged = append(merged, iter.Attribute())
}
return attribute.NewSet(merged...)
}

// WithInstrumentationAttributes returns a [LoggerOption] that sets the
// instrumentation attributes of a [Logger].
//
// The passed attributes will be de-duplicated.
//
// If multiple WithInstrumentationAttributes options are passed the
// attributes will be merged together in the order they are passed. Attributes
// with duplicate keys will use the last value passed.
func WithInstrumentationAttributes(attr ...attribute.KeyValue) LoggerOption {
if len(attr) == 0 {
return loggerOptionFunc(func(config LoggerConfig) LoggerConfig {
return config
})
}

return loggerOptionFunc(func(config LoggerConfig) LoggerConfig {
config.attrs = attribute.NewSet(attr...)
newAttrs := attribute.NewSet(attr...)
Comment thread
flc1125 marked this conversation as resolved.
if config.attrs.Len() == 0 {
config.attrs = newAttrs
} else {
config.attrs = mergeSets(config.attrs, newAttrs)
}
return config
})
}
Expand Down
47 changes: 47 additions & 0 deletions log/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,50 @@ func TestNewLoggerConfig(t *testing.T) {
assert.Equal(t, schemaURL, c.SchemaURL(), "schema URL")
assert.Equal(t, attr, c.InstrumentationAttributes(), "instrumentation attributes")
}

func TestWithInstrumentationAttributesMerge(t *testing.T) {
aliceAttr := attribute.String("user", "Alice")
bobAttr := attribute.String("user", "Bob")
adminAttr := attribute.Bool("admin", true)

alice := attribute.NewSet(aliceAttr)
bob := attribute.NewSet(bobAttr)
aliceAdmin := attribute.NewSet(aliceAttr, adminAttr)
bobAdmin := attribute.NewSet(bobAttr, adminAttr)

t.Run("SameKey", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributes(aliceAttr),
log.WithInstrumentationAttributes(bobAttr),
)
assert.Equal(t, bob, c.InstrumentationAttributes(),
"Later values for the same key should overwrite earlier ones.")
})

t.Run("DifferentKeys", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributes(aliceAttr),
log.WithInstrumentationAttributes(adminAttr),
)
assert.Equal(t, aliceAdmin, c.InstrumentationAttributes(),
"Different keys should be merged.")
})

t.Run("Mixed", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributes(aliceAttr, adminAttr),
log.WithInstrumentationAttributes(bobAttr),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Combination of same and different keys should be merged.")
})

t.Run("MergedEmpty", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributes(aliceAttr),
log.WithInstrumentationAttributes(),
)
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attributes should not affect existing ones.")
})
}
17 changes: 16 additions & 1 deletion metric/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,24 @@ func WithInstrumentationVersion(version string) MeterOption {
// WithInstrumentationAttributes sets the instrumentation attributes.
//
// The passed attributes will be de-duplicated.
//
// If multiple WithInstrumentationAttributes options are passed the
// attributes will be merged together in the order they are passed. Attributes
// with duplicate keys will use the last value passed.
func WithInstrumentationAttributes(attr ...attribute.KeyValue) MeterOption {
if len(attr) == 0 {
return meterOptionFunc(func(config MeterConfig) MeterConfig {
return config
})
}

return meterOptionFunc(func(config MeterConfig) MeterConfig {
config.attrs = attribute.NewSet(attr...)
newAttrs := attribute.NewSet(attr...)
if config.attrs.Len() == 0 {
config.attrs = newAttrs
} else {
config.attrs = mergeSets(config.attrs, newAttrs)
Comment thread
pellared marked this conversation as resolved.
}
return config
})
}
Expand Down
48 changes: 48 additions & 0 deletions metric/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,51 @@ func TestConfig(t *testing.T) {
assert.Equal(t, schemaURL, c.SchemaURL(), "schema URL")
assert.Equal(t, attr, c.InstrumentationAttributes(), "instrumentation attributes")
}

func TestWithInstrumentationAttributesMerge(t *testing.T) {
aliceAttr := attribute.String("user", "Alice")
bobAttr := attribute.String("user", "Bob")
adminAttr := attribute.Bool("admin", true)

alice := attribute.NewSet(aliceAttr)
bob := attribute.NewSet(bobAttr)
aliceAdmin := attribute.NewSet(aliceAttr, adminAttr)
bobAdmin := attribute.NewSet(bobAttr, adminAttr)

t.Run("SameKey", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributes(aliceAttr),
metric.WithInstrumentationAttributes(bobAttr),
)
assert.Equal(t, bob, c.InstrumentationAttributes(),
"Later values for the same key should overwrite earlier ones.")
})

t.Run("DifferentKeys", func(t *testing.T) {
// Different keys should be merged
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributes(aliceAttr),
metric.WithInstrumentationAttributes(adminAttr),
)
assert.Equal(t, aliceAdmin, c.InstrumentationAttributes(),
"Different keys should be merged.")
})

t.Run("Mixed", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributes(aliceAttr, adminAttr),
metric.WithInstrumentationAttributes(bobAttr),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Combination of same and different keys should be merged.")
})

t.Run("MergedEmpty", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributes(aliceAttr),
metric.WithInstrumentationAttributes(),
)
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attributes should not affect existing ones.")
})
}
29 changes: 28 additions & 1 deletion trace/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,12 +304,39 @@ func WithInstrumentationVersion(version string) TracerOption {
})
}

// mergeSets returns the union of keys between a and b. Any duplicate keys will
// use the value associated with b.
func mergeSets(a, b attribute.Set) attribute.Set {
// NewMergeIterator uses the first value for any duplicates.
iter := attribute.NewMergeIterator(&b, &a)
merged := make([]attribute.KeyValue, 0, a.Len()+b.Len())
for iter.Next() {
merged = append(merged, iter.Attribute())
}
return attribute.NewSet(merged...)
}

// WithInstrumentationAttributes sets the instrumentation attributes.
//
// The passed attributes will be de-duplicated.
//
// If multiple WithInstrumentationAttributes options are passed the
// attributes will be merged together in the order they are passed. Attributes
// with duplicate keys will use the last value passed.
func WithInstrumentationAttributes(attr ...attribute.KeyValue) TracerOption {
if len(attr) == 0 {
return tracerOptionFunc(func(config TracerConfig) TracerConfig {
return config
})
}

return tracerOptionFunc(func(config TracerConfig) TracerConfig {
config.attrs = attribute.NewSet(attr...)
newAttrs := attribute.NewSet(attr...)
if config.attrs.Len() == 0 {
config.attrs = newAttrs
} else {
config.attrs = mergeSets(config.attrs, newAttrs)
}
return config
})
}
Expand Down
47 changes: 47 additions & 0 deletions trace/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,50 @@ func BenchmarkNewEventConfig(b *testing.B) {
})
}
}

func TestWithInstrumentationAttributesMerge(t *testing.T) {
aliceAttr := attribute.String("user", "Alice")
bobAttr := attribute.String("user", "Bob")
adminAttr := attribute.Bool("admin", true)

alice := attribute.NewSet(aliceAttr)
bob := attribute.NewSet(bobAttr)
aliceAdmin := attribute.NewSet(aliceAttr, adminAttr)
bobAdmin := attribute.NewSet(bobAttr, adminAttr)

t.Run("SameKey", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributes(aliceAttr),
WithInstrumentationAttributes(bobAttr),
)
assert.Equal(t, bob, c.InstrumentationAttributes(),
"Later values for the same key should overwrite earlier ones.")
})

t.Run("DifferentKeys", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributes(aliceAttr),
WithInstrumentationAttributes(adminAttr),
)
assert.Equal(t, aliceAdmin, c.InstrumentationAttributes(),
"Different keys should be merged")
})

t.Run("Mixed", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributes(aliceAttr, adminAttr),
WithInstrumentationAttributes(bobAttr),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Combination of same and different keys should be merged.")
})

t.Run("MergedEmpty", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributes(aliceAttr),
WithInstrumentationAttributes(),
)
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attributes should not affect existing ones.")
})
}
Loading