From 59563f7ae416c2e2b512d5e2e8036e3012bba390 Mon Sep 17 00:00:00 2001 From: Tyler Yahn Date: Mon, 15 Sep 2025 09:28:12 -0700 Subject: [PATCH] Do not use the user-defined empty set when comparing sets. (#7357) Fix #7356 --- CHANGELOG.md | 1 + attribute/set.go | 28 ++++++++++++++++++---------- attribute/set_test.go | 11 +++++++++++ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95737f417..c4002d5b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### 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) +- The equality of `attribute.Set` when using the `Equal` method is not affected by the user overriding the empty set pointed to by `attribute.EmptySet` in `go.opentelemetry.io/otel/attribute`. (#7357) ### Removed diff --git a/attribute/set.go b/attribute/set.go index 64735d382..764285a90 100644 --- a/attribute/set.go +++ b/attribute/set.go @@ -50,8 +50,18 @@ var ( // keyValueType is used in computeDistinctReflect. keyValueType = reflect.TypeOf(KeyValue{}) - // emptySet is returned for empty attribute sets. - emptySet = &Set{ + // userDefinedEmptySet is an empty set. It was mistakenly exposed to users + // as something they can assign to, so it must remain addressable and + // mutable. + // + // This is kept for backwards compatibility, but should not be used in new code. + userDefinedEmptySet = &Set{ + equivalent: Distinct{ + iface: [0]KeyValue{}, + }, + } + + emptySet = Set{ equivalent: Distinct{ iface: [0]KeyValue{}, }, @@ -62,7 +72,11 @@ var ( // // This is a convenience provided for optimized calling utility. func EmptySet() *Set { - return emptySet + // Continue to return the pointer to the user-defined empty set for + // backwards-compatibility. + // + // New code should not use this, instead use emptySet. + return userDefinedEmptySet } // reflectValue abbreviates reflect.ValueOf(d). @@ -169,12 +183,6 @@ func (l *Set) Encoded(encoder Encoder) string { return encoder.Encode(l.Iter()) } -func empty() Set { - return Set{ - equivalent: emptySet.equivalent, - } -} - // NewSet returns a new Set. See the documentation for // NewSetWithSortableFiltered for more details. // @@ -204,7 +212,7 @@ func NewSetWithSortable(kvs []KeyValue, _ *Sortable) Set { func NewSetWithFiltered(kvs []KeyValue, filter Filter) (Set, []KeyValue) { // Check for empty set. if len(kvs) == 0 { - return empty(), nil + return emptySet, nil } // Stable sort so the following de-duplication can implement diff --git a/attribute/set_test.go b/attribute/set_test.go index 1ba0a6527..b86786784 100644 --- a/attribute/set_test.go +++ b/attribute/set_test.go @@ -469,6 +469,17 @@ func TestMarshalJSON(t *testing.T) { } } +func TestSetEqualsEmpty(t *testing.T) { + e := attribute.EmptySet() + empty := *e + + alt := attribute.NewSet(attribute.String("A", "B")) + *e = alt + + var s attribute.Set + assert.Truef(t, s.Equals(&empty), "expected %v to equal empty set %v", s, attribute.EmptySet()) +} + type simpleStringer struct { val string }