trace,metric,log: WithInstrumentationAttributes options to merge attributes

CHANGELOG.md

@@ -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)

log/logger.go

@@ -114,13 +114,36 @@ 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 {
 	return loggerOptionFunc(func(config LoggerConfig) LoggerConfig {
-		config.attrs = attribute.NewSet(attr...)
+		newAttrs := attribute.NewSet(attr...)
+		switch {
+		case newAttrs.Len() == 0:
+		case config.attrs.Len() == 0:
+			config.attrs = newAttrs
+		default:
+			config.attrs = mergeSets(config.attrs, newAttrs)
+		}
 		return config
 	})
 }

log/logger_test.go

@@ -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.")
+	})
+}

metric/config.go

@@ -65,9 +65,20 @@ 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 {
 	return meterOptionFunc(func(config MeterConfig) MeterConfig {
-		config.attrs = attribute.NewSet(attr...)
+		newAttrs := attribute.NewSet(attr...)
+		switch {
+		case newAttrs.Len() == 0:
+		case config.attrs.Len() == 0:
+			config.attrs = newAttrs
+		default:
+			config.attrs = mergeSets(config.attrs, newAttrs)
+		}
 		return config
 	})
 }

metric/config_test.go

@@ -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.")
+	})
+}

trace/config.go

@@ -304,12 +304,35 @@ 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 {
 	return tracerOptionFunc(func(config TracerConfig) TracerConfig {
-		config.attrs = attribute.NewSet(attr...)
+		newAttrs := attribute.NewSet(attr...)
+		switch {
+		case newAttrs.Len() == 0:
+		case config.attrs.Len() == 0:
+			config.attrs = newAttrs
+		default:
+			config.attrs = mergeSets(config.attrs, newAttrs)
+		}
 		return config
 	})
 }

trace/config_test.go

@@ -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.")
+	})
+}