diff --git a/CHANGELOG.md b/CHANGELOG.md index 8494643ff..95737f417 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Rename the `OTEL_GO_X_SELF_OBSERVABILITY` environment variable to `OTEL_GO_X_OBSERVABILITY` in `go.opentelemetry.io/otel/sdk/trace`, `go.opentelemetry.io/otel/sdk/log`, and `go.opentelemetry.io/otel/exporters/stdout/stdouttrace`. (#7302) - Improve performance of histogram `Record` in `go.opentelemetry.io/otel/sdk/metric` when min and max are disabled using `NoMinMax`. (#7306) +- `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/trace` synchronously de-duplicates the passed attributes instead of delegating it to the returned `TracerOption`. (#7266) +- `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/meter` synchronously de-duplicates the passed attributes instead of delegating it to the returned `MeterOption`. (#7266) +- `WithInstrumentationAttributes` in `go.opentelemetry.io/otel/log` synchronously de-duplicates the passed attributes instead of delegating it to the returned `LoggerOption`. (#7266) diff --git a/log/logger.go b/log/logger.go index c2fa322b1..d9decebde 100644 --- a/log/logger.go +++ b/log/logger.go @@ -5,6 +5,7 @@ package log // import "go.opentelemetry.io/otel/log" import ( "context" + "slices" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/log/embedded" @@ -129,30 +130,16 @@ func mergeSets(a, b attribute.Set) attribute.Set { // WithInstrumentationAttributes returns a [LoggerOption] that sets the // instrumentation attributes of a [Logger]. // -// The passed attributes will be de-duplicated. -// -// Note that [WithInstrumentationAttributeSet] is recommended as -// it is more efficient and also allows safely reusing the passed argument. +// This is equivalent to calling WithInstrumentationAttributeSet with an +// [attribute.Set] created from a clone of the passed attributes. +// [WithInstrumentationAttributeSet] is recommended for more control. // // 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 { - return config - }) - } - - return loggerOptionFunc(func(config LoggerConfig) LoggerConfig { - newAttrs := attribute.NewSet(attr...) - if config.attrs.Len() == 0 { - config.attrs = newAttrs - } else { - config.attrs = mergeSets(config.attrs, newAttrs) - } - return config - }) + set := attribute.NewSet(slices.Clone(attr)...) + return WithInstrumentationAttributeSet(set) } // WithInstrumentationAttributeSet returns a [LoggerOption] that adds the diff --git a/log/logger_test.go b/log/logger_test.go index 137a3284c..e2b6e972c 100644 --- a/log/logger_test.go +++ b/log/logger_test.go @@ -31,6 +31,23 @@ func TestNewLoggerConfig(t *testing.T) { assert.Equal(t, attr, c.InstrumentationAttributes(), "instrumentation attributes") } +func TestWithInstrumentationAttributesNotLazy(t *testing.T) { + attrs := []attribute.KeyValue{ + attribute.String("service", "test"), + attribute.Int("three", 3), + } + want := attribute.NewSet(attrs...) + + // WithInstrumentationAttributes is expected to immediately + // create an immutable set from the attributes, so later changes + // to attrs should not affect the config. + opt := log.WithInstrumentationAttributes(attrs...) + attrs[0] = attribute.String("service", "changed") + + c := log.NewLoggerConfig(opt) + assert.Equal(t, want, c.InstrumentationAttributes(), "instrumentation attributes") +} + func TestWithInstrumentationAttributeSet(t *testing.T) { attrs := attribute.NewSet( attribute.String("service", "test"), diff --git a/metric/config.go b/metric/config.go index adcdae5d9..e42dd6e70 100644 --- a/metric/config.go +++ b/metric/config.go @@ -3,7 +3,11 @@ package metric // import "go.opentelemetry.io/otel/metric" -import "go.opentelemetry.io/otel/attribute" +import ( + "slices" + + "go.opentelemetry.io/otel/attribute" +) // MeterConfig contains options for Meters. type MeterConfig struct { @@ -64,30 +68,16 @@ func WithInstrumentationVersion(version string) MeterOption { // WithInstrumentationAttributes adds the instrumentation attributes. // -// The passed attributes will be de-duplicated. -// -// Note that [WithInstrumentationAttributeSet] is recommended as -// it is more efficient and also allows safely reusing the passed argument. +// This is equivalent to calling [WithInstrumentationAttributeSet] with an +// [attribute.Set] created from a clone of the passed attributes. +// [WithInstrumentationAttributeSet] is recommended for more control. // // 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 { - return config - }) - } - - return meterOptionFunc(func(config MeterConfig) MeterConfig { - newAttrs := attribute.NewSet(attr...) - if config.attrs.Len() == 0 { - config.attrs = newAttrs - } else { - config.attrs = mergeSets(config.attrs, newAttrs) - } - return config - }) + set := attribute.NewSet(slices.Clone(attr)...) + return WithInstrumentationAttributeSet(set) } // WithInstrumentationAttributeSet adds the instrumentation attributes. diff --git a/metric/config_test.go b/metric/config_test.go index 07f0088a9..a24ba1ca5 100644 --- a/metric/config_test.go +++ b/metric/config_test.go @@ -31,6 +31,23 @@ func TestConfig(t *testing.T) { assert.Equal(t, attr, c.InstrumentationAttributes(), "instrumentation attributes") } +func TestWithInstrumentationAttributesNotLazy(t *testing.T) { + attrs := []attribute.KeyValue{ + attribute.String("service", "test"), + attribute.Int("three", 3), + } + want := attribute.NewSet(attrs...) + + // WithInstrumentationAttributes is expected to immediately + // create an immutable set from the attributes, so later changes + // to attrs should not affect the config. + opt := metric.WithInstrumentationAttributes(attrs...) + attrs[0] = attribute.String("service", "changed") + + c := metric.NewMeterConfig(opt) + assert.Equal(t, want, c.InstrumentationAttributes(), "instrumentation attributes") +} + func TestWithInstrumentationAttributeSet(t *testing.T) { attrs := attribute.NewSet( attribute.String("service", "test"), diff --git a/trace/config.go b/trace/config.go index 4d83fb059..d9ecef1ca 100644 --- a/trace/config.go +++ b/trace/config.go @@ -4,6 +4,7 @@ package trace // import "go.opentelemetry.io/otel/trace" import ( + "slices" "time" "go.opentelemetry.io/otel/attribute" @@ -318,30 +319,16 @@ func mergeSets(a, b attribute.Set) attribute.Set { // WithInstrumentationAttributes adds the instrumentation attributes. // -// The passed attributes will be de-duplicated. -// -// Note that [WithInstrumentationAttributeSet] is recommended as -// it is more efficient and also allows safely reusing the passed argument. +// This is equivalent to calling [WithInstrumentationAttributeSet] with an +// [attribute.Set] created from a clone of the passed attributes. +// [WithInstrumentationAttributeSet] is recommended for more control. // // 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 { - return config - }) - } - - return tracerOptionFunc(func(config TracerConfig) TracerConfig { - newAttrs := attribute.NewSet(attr...) - if config.attrs.Len() == 0 { - config.attrs = newAttrs - } else { - config.attrs = mergeSets(config.attrs, newAttrs) - } - return config - }) + set := attribute.NewSet(slices.Clone(attr)...) + return WithInstrumentationAttributeSet(set) } // WithInstrumentationAttributeSet adds the instrumentation attributes. diff --git a/trace/config_test.go b/trace/config_test.go index e88415025..40667fa01 100644 --- a/trace/config_test.go +++ b/trace/config_test.go @@ -231,6 +231,23 @@ func TestTracerConfig(t *testing.T) { assert.Equal(t, attrs, c.InstrumentationAttributes(), "instrumentation attributes") } +func TestWithInstrumentationAttributesNotLazy(t *testing.T) { + attrs := []attribute.KeyValue{ + attribute.String("service", "test"), + attribute.Int("three", 3), + } + want := attribute.NewSet(attrs...) + + // WithInstrumentationAttributes is expected to immediately + // create an immutable set from the attributes, so later changes + // to attrs should not affect the config. + opt := WithInstrumentationAttributes(attrs...) + attrs[0] = attribute.String("service", "changed") + + c := NewTracerConfig(opt) + assert.Equal(t, want, c.InstrumentationAttributes(), "instrumentation attributes") +} + func TestWithInstrumentationAttributeSet(t *testing.T) { attrs := attribute.NewSet( attribute.String("service", "test"),