1
0
mirror of https://github.com/open-telemetry/opentelemetry-go.git synced 2026-06-03 18:35:08 +02:00

trace,metric,log: add WithInstrumentationAttributeSet option (#7287)

Per
https://github.com/open-telemetry/opentelemetry-go/pull/7266#issuecomment-3237027300

Related to
https://github.com/open-telemetry/opentelemetry-go/issues/7217

## What

This PR adds `WithInstrumentationAttributeSet` option functions to the
`log`, `metric`, and `trace` packages as suggested in
https://github.com/open-telemetry/opentelemetry-go/pull/7266#issuecomment-3237027300.
These new functions provide a more concurrent-safe alternative to the
existing `WithInstrumentationAttributes` functions by accepting a
pre-constructed `attribute.Set` instead of variadic `attribute.KeyValue`
parameters.

## Why

As discussed in #7266, the existing `WithInstrumentationAttributes`
functions can lead to data races when used concurrently because
`attribute.NewSet()` may mutate the passed slice in-place. While the
issue was partially addressed by moving the `attribute.NewSet()` call
outside the closure, the best long-term solution is to provide an
alternative that accepts an immutable `attribute.Set`.

**Benefits:**

1. **Concurrent Safety**: Since `attribute.Set` is immutable, these
functions are inherently safe for concurrent use
2. **Performance**: Avoids repeated calls to `attribute.NewSet()` when
the same attributes are used multiple times
3. **Consistency**: Matches the existing pattern used in
`metric.WithAttributeSet()`
4. **Flexibility**: Allows users to pre-compute attribute sets and reuse
them

Deprecating `WithInstrumentationAttributes` is out of scope. See
https://github.com/open-telemetry/opentelemetry-go/pull/7287#issuecomment-3245820459.

## Benchmarks

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/trace
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
BenchmarkNewTracerConfig/with_no_options-20         	280298306	         4.268 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewTracerConfig/with_an_instrumentation_version-20         	33389427	        30.84 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewTracerConfig/with_a_schema_url-20                       	35441077	        30.46 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewTracerConfig/with_instrumentation_attribute-20          	17607649	        88.23 ns/op	      64 B/op	       1 allocs/op
BenchmarkNewTracerConfig/with_instrumentation_attribute_set-20      	38336211	        31.30 ns/op	       0 B/op	       0 allocs/op
```

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/metric
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
BenchmarkNewMeterConfig/with_no_options-20         	262998199	         4.525 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewMeterConfig/with_an_instrumentation_version-20         	40483780	        29.31 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewMeterConfig/with_a_schema_url-20                       	39162420	        30.58 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewMeterConfig/with_instrumentation_attribute-20          	19900275	        77.50 ns/op	      64 B/op	       1 allocs/op
BenchmarkNewMeterConfig/with_instrumentation_attribute_set-20      	37519020	        31.93 ns/op	       0 B/op	       0 allocs/op
```

```
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/log
cpu: 13th Gen Intel(R) Core(TM) i7-13800H
BenchmarkNewLoggerConfig/with_no_options-20         	271100760	         4.322 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewLoggerConfig/with_an_instrumentation_version-20         	38392390	        30.77 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewLoggerConfig/with_a_schema_url-20                       	39615074	        30.25 ns/op	       0 B/op	       0 allocs/op
BenchmarkNewLoggerConfig/with_instrumentation_attribute-20          	17108463	        82.51 ns/op	      64 B/op	       1 allocs/op
BenchmarkNewLoggerConfig/with_instrumentation_attribute_set-20      	37746534	        31.70 ns/op	       0 B/op	       0 allocs/op
```
This commit is contained in:
Robert Pająk
2025-09-08 17:21:37 +02:00
committed by GitHub
parent b335c0795c
commit 07a91dd2b0
7 changed files with 370 additions and 16 deletions
+5
View File
@@ -8,6 +8,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
## [Unreleased]
### Added
- Add `WithInstrumentationAttributeSet` option to `go.opentelemetry.io/otel/log`, `go.opentelemetry.io/otel/metric`, and `go.opentelemetry.io/otel/trace` packages.
This provides a concurrent-safe and performant alternative to `WithInstrumentationAttributes` by accepting a pre-constructed `attribute.Set`. (#7287)
### 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)
+29 -3
View File
@@ -131,9 +131,12 @@ func mergeSets(a, b attribute.Set) attribute.Set {
//
// 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.
// Note that [WithInstrumentationAttributeSet] is recommended as
// it is more efficient and also allows safely reusing the passed argument.
//
// If multiple [WithInstrumentationAttributes] or [WithInstrumentationAttributeSet]
// 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 {
@@ -152,6 +155,29 @@ func WithInstrumentationAttributes(attr ...attribute.KeyValue) LoggerOption {
})
}
// WithInstrumentationAttributeSet returns a [LoggerOption] that adds the
// instrumentation attributes of a [Logger].
//
// If multiple [WithInstrumentationAttributes] or [WithInstrumentationAttributeSet]
// 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 WithInstrumentationAttributeSet(set attribute.Set) LoggerOption {
if set.Len() == 0 {
return loggerOptionFunc(func(config LoggerConfig) LoggerConfig {
return config
})
}
return loggerOptionFunc(func(config LoggerConfig) LoggerConfig {
if config.attrs.Len() == 0 {
config.attrs = set
} else {
config.attrs = mergeSets(config.attrs, set)
}
return config
})
}
// WithSchemaURL returns a [LoggerOption] that sets the schema URL for a
// [Logger].
func WithSchemaURL(schemaURL string) LoggerOption {
+102
View File
@@ -31,6 +31,19 @@ func TestNewLoggerConfig(t *testing.T) {
assert.Equal(t, attr, c.InstrumentationAttributes(), "instrumentation attributes")
}
func TestWithInstrumentationAttributeSet(t *testing.T) {
attrs := attribute.NewSet(
attribute.String("service", "test"),
attribute.Int("three", 3),
)
c := log.NewLoggerConfig(
log.WithInstrumentationAttributeSet(attrs),
)
assert.Equal(t, attrs, c.InstrumentationAttributes(), "instrumentation attributes")
}
func TestWithInstrumentationAttributesMerge(t *testing.T) {
aliceAttr := attribute.String("user", "Alice")
bobAttr := attribute.String("user", "Bob")
@@ -76,4 +89,93 @@ func TestWithInstrumentationAttributesMerge(t *testing.T) {
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attributes should not affect existing ones.")
})
t.Run("SameKeyWithSet", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributeSet(alice),
log.WithInstrumentationAttributeSet(bob),
)
assert.Equal(t, bob, c.InstrumentationAttributes(),
"Later values for the same key should overwrite earlier ones.")
})
t.Run("DifferentKeysWithSet", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributeSet(alice),
log.WithInstrumentationAttributeSet(attribute.NewSet(adminAttr)),
)
assert.Equal(t, aliceAdmin, c.InstrumentationAttributes(),
"Different keys should be merged.")
})
t.Run("MixedWithSet", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributeSet(aliceAdmin),
log.WithInstrumentationAttributeSet(bob),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Combination of same and different keys should be merged.")
})
t.Run("MergedEmptyWithSet", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributeSet(alice),
log.WithInstrumentationAttributeSet(attribute.NewSet()),
)
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attribute set should not affect existing ones.")
})
t.Run("MixedAttributesAndSet", func(t *testing.T) {
c := log.NewLoggerConfig(
log.WithInstrumentationAttributes(aliceAttr),
log.WithInstrumentationAttributeSet(attribute.NewSet(bobAttr, adminAttr)),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Attributes and attribute sets should be merged together.")
})
}
func BenchmarkNewLoggerConfig(b *testing.B) {
for _, bb := range []struct {
name string
options []log.LoggerOption
}{
{
name: "with no options",
},
{
name: "with an instrumentation version",
options: []log.LoggerOption{
log.WithInstrumentationVersion("testing version"),
},
},
{
name: "with a schema url",
options: []log.LoggerOption{
log.WithSchemaURL("testing URL"),
},
},
{
name: "with instrumentation attribute",
options: []log.LoggerOption{
log.WithInstrumentationAttributes(attribute.String("foo", "value")),
},
},
{
name: "with instrumentation attribute set",
options: []log.LoggerOption{
log.WithInstrumentationAttributeSet(attribute.NewSet(attribute.String("bar", "value"))),
},
},
} {
b.Run(bb.name, func(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for b.Loop() {
log.NewLoggerConfig(bb.options...)
}
})
}
}
+29 -4
View File
@@ -62,13 +62,16 @@ func WithInstrumentationVersion(version string) MeterOption {
})
}
// WithInstrumentationAttributes sets the instrumentation attributes.
// WithInstrumentationAttributes adds 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.
// Note that [WithInstrumentationAttributeSet] is recommended as
// it is more efficient and also allows safely reusing the passed argument.
//
// If multiple [WithInstrumentationAttributes] or [WithInstrumentationAttributeSet]
// 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 {
@@ -87,6 +90,28 @@ func WithInstrumentationAttributes(attr ...attribute.KeyValue) MeterOption {
})
}
// WithInstrumentationAttributeSet adds the instrumentation attributes.
//
// If multiple [WithInstrumentationAttributes] or [WithInstrumentationAttributeSet]
// 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 WithInstrumentationAttributeSet(set attribute.Set) MeterOption {
if set.Len() == 0 {
return meterOptionFunc(func(config MeterConfig) MeterConfig {
return config
})
}
return meterOptionFunc(func(config MeterConfig) MeterConfig {
if config.attrs.Len() == 0 {
config.attrs = set
} else {
config.attrs = mergeSets(config.attrs, set)
}
return config
})
}
// WithSchemaURL sets the schema URL.
func WithSchemaURL(schemaURL string) MeterOption {
return meterOptionFunc(func(config MeterConfig) MeterConfig {
+102
View File
@@ -31,6 +31,19 @@ func TestConfig(t *testing.T) {
assert.Equal(t, attr, c.InstrumentationAttributes(), "instrumentation attributes")
}
func TestWithInstrumentationAttributeSet(t *testing.T) {
attrs := attribute.NewSet(
attribute.String("service", "test"),
attribute.Int("three", 3),
)
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributeSet(attrs),
)
assert.Equal(t, attrs, c.InstrumentationAttributes(), "instrumentation attributes")
}
func TestWithInstrumentationAttributesMerge(t *testing.T) {
aliceAttr := attribute.String("user", "Alice")
bobAttr := attribute.String("user", "Bob")
@@ -77,4 +90,93 @@ func TestWithInstrumentationAttributesMerge(t *testing.T) {
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attributes should not affect existing ones.")
})
t.Run("SameKeyWithSet", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributeSet(alice),
metric.WithInstrumentationAttributeSet(bob),
)
assert.Equal(t, bob, c.InstrumentationAttributes(),
"Later values for the same key should overwrite earlier ones.")
})
t.Run("DifferentKeysWithSet", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributeSet(alice),
metric.WithInstrumentationAttributeSet(attribute.NewSet(adminAttr)),
)
assert.Equal(t, aliceAdmin, c.InstrumentationAttributes(),
"Different keys should be merged.")
})
t.Run("MixedWithSet", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributeSet(aliceAdmin),
metric.WithInstrumentationAttributeSet(bob),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Combination of same and different keys should be merged.")
})
t.Run("MergedEmptyWithSet", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributeSet(alice),
metric.WithInstrumentationAttributeSet(attribute.NewSet()),
)
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attribute set should not affect existing ones.")
})
t.Run("MixedAttributesAndSet", func(t *testing.T) {
c := metric.NewMeterConfig(
metric.WithInstrumentationAttributes(aliceAttr),
metric.WithInstrumentationAttributeSet(attribute.NewSet(bobAttr, adminAttr)),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Attributes and attribute sets should be merged together.")
})
}
func BenchmarkNewMeterConfig(b *testing.B) {
for _, bb := range []struct {
name string
options []metric.MeterOption
}{
{
name: "with no options",
},
{
name: "with an instrumentation version",
options: []metric.MeterOption{
metric.WithInstrumentationVersion("testing version"),
},
},
{
name: "with a schema url",
options: []metric.MeterOption{
metric.WithSchemaURL("testing URL"),
},
},
{
name: "with instrumentation attribute",
options: []metric.MeterOption{
metric.WithInstrumentationAttributes(attribute.String("key", "value")),
},
},
{
name: "with instrumentation attribute set",
options: []metric.MeterOption{
metric.WithInstrumentationAttributeSet(attribute.NewSet(attribute.String("key", "value"))),
},
},
} {
b.Run(bb.name, func(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for b.Loop() {
metric.NewMeterConfig(bb.options...)
}
})
}
}
+29 -4
View File
@@ -316,13 +316,16 @@ func mergeSets(a, b attribute.Set) attribute.Set {
return attribute.NewSet(merged...)
}
// WithInstrumentationAttributes sets the instrumentation attributes.
// WithInstrumentationAttributes adds 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.
// Note that [WithInstrumentationAttributeSet] is recommended as
// it is more efficient and also allows safely reusing the passed argument.
//
// If multiple [WithInstrumentationAttributes] or [WithInstrumentationAttributeSet]
// 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 {
@@ -341,6 +344,28 @@ func WithInstrumentationAttributes(attr ...attribute.KeyValue) TracerOption {
})
}
// WithInstrumentationAttributeSet adds the instrumentation attributes.
//
// If multiple [WithInstrumentationAttributes] or [WithInstrumentationAttributeSet]
// 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 WithInstrumentationAttributeSet(set attribute.Set) TracerOption {
if set.Len() == 0 {
return tracerOptionFunc(func(config TracerConfig) TracerConfig {
return config
})
}
return tracerOptionFunc(func(config TracerConfig) TracerConfig {
if config.attrs.Len() == 0 {
config.attrs = set
} else {
config.attrs = mergeSets(config.attrs, set)
}
return config
})
}
// WithSchemaURL sets the schema URL for the Tracer.
func WithSchemaURL(schemaURL string) TracerOption {
return tracerOptionFunc(func(cfg TracerConfig) TracerConfig {
+74 -5
View File
@@ -231,12 +231,24 @@ func TestTracerConfig(t *testing.T) {
assert.Equal(t, attrs, c.InstrumentationAttributes(), "instrumentation attributes")
}
func TestWithInstrumentationAttributeSet(t *testing.T) {
attrs := attribute.NewSet(
attribute.String("service", "test"),
attribute.Int("three", 3),
)
c := NewTracerConfig(
WithInstrumentationAttributeSet(attrs),
)
assert.Equal(t, attrs, c.InstrumentationAttributes(), "instrumentation attributes")
}
// Save benchmark results to a file level var to avoid the compiler optimizing
// away the actual work.
var (
tracerConfig TracerConfig
spanConfig SpanConfig
eventConfig EventConfig
spanConfig SpanConfig
eventConfig EventConfig
)
func BenchmarkNewTracerConfig(b *testing.B) {
@@ -259,13 +271,25 @@ func BenchmarkNewTracerConfig(b *testing.B) {
WithSchemaURL("testing URL"),
},
},
{
name: "with instrumentation attribute",
options: []TracerOption{
WithInstrumentationAttributes(attribute.String("key", "value")),
},
},
{
name: "with instrumentation attribute set",
options: []TracerOption{
WithInstrumentationAttributeSet(attribute.NewSet(attribute.String("key", "value"))),
},
},
} {
b.Run(bb.name, func(b *testing.B) {
b.ReportAllocs()
b.ResetTimer()
for i := 0; i < b.N; i++ {
tracerConfig = NewTracerConfig(bb.options...)
for b.Loop() {
NewTracerConfig(bb.options...)
}
})
}
@@ -457,4 +481,49 @@ func TestWithInstrumentationAttributesMerge(t *testing.T) {
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attributes should not affect existing ones.")
})
t.Run("SameKeyWithSet", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributeSet(alice),
WithInstrumentationAttributeSet(bob),
)
assert.Equal(t, bob, c.InstrumentationAttributes(),
"Later values for the same key should overwrite earlier ones.")
})
t.Run("DifferentKeysWithSet", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributeSet(alice),
WithInstrumentationAttributeSet(attribute.NewSet(adminAttr)),
)
assert.Equal(t, aliceAdmin, c.InstrumentationAttributes(),
"Different keys should be merged.")
})
t.Run("MixedWithSet", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributeSet(aliceAdmin),
WithInstrumentationAttributeSet(bob),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Combination of same and different keys should be merged.")
})
t.Run("MergedEmptyWithSet", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributeSet(alice),
WithInstrumentationAttributeSet(attribute.NewSet()),
)
assert.Equal(t, alice, c.InstrumentationAttributes(),
"Empty attribute set should not affect existing ones.")
})
t.Run("MixedAttributesAndSet", func(t *testing.T) {
c := NewTracerConfig(
WithInstrumentationAttributes(aliceAttr),
WithInstrumentationAttributeSet(attribute.NewSet(bobAttr, adminAttr)),
)
assert.Equal(t, bobAdmin, c.InstrumentationAttributes(),
"Attributes and attribute sets should be merged together.")
})
}