From 3ef32e1a6ed45d79cb41d024b0d6eda93a73f6df Mon Sep 17 00:00:00 2001 From: dmathieu Date: Tue, 7 May 2024 10:08:06 +0200 Subject: [PATCH 1/8] sort slices and maps key/values when comparing them --- log/keyvalue.go | 19 ++++++++++++++++++- log/keyvalue_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/log/keyvalue.go b/log/keyvalue.go index 10920d21f4a..c9eb9034f80 100644 --- a/log/keyvalue.go +++ b/log/keyvalue.go @@ -7,6 +7,7 @@ package log // import "go.opentelemetry.io/otel/log" import ( "bytes" + "cmp" "errors" "fmt" "math" @@ -254,8 +255,24 @@ func (v Value) Equal(w Value) bool { case KindFloat64: return v.asFloat64() == w.asFloat64() case KindSlice: - return slices.EqualFunc(v.asSlice(), w.asSlice(), Value.Equal) + sv := v.asSlice() + sw := w.asSlice() + slices.SortFunc(sv, func(a, b Value) int { + return cmp.Compare(a.String(), b.String()) + }) + slices.SortFunc(sw, func(a, b Value) int { + return cmp.Compare(a.String(), b.String()) + }) + return slices.EqualFunc(sv, sw, Value.Equal) case KindMap: + sv := v.asMap() + slices.SortFunc(sv, func(a, b KeyValue) int { + return cmp.Compare(a.Key, b.Key) + }) + sw := w.asMap() + slices.SortFunc(sw, func(a, b KeyValue) int { + return cmp.Compare(a.Key, b.Key) + }) return slices.EqualFunc(v.asMap(), w.asMap(), KeyValue.Equal) case KindBytes: return bytes.Equal(v.asBytes(), w.asBytes()) diff --git a/log/keyvalue_test.go b/log/keyvalue_test.go index 2f0211160cf..0a9e62731df 100644 --- a/log/keyvalue_test.go +++ b/log/keyvalue_test.go @@ -70,6 +70,35 @@ func TestValueEqual(t *testing.T) { } } +func TestSortedValueEqual(t *testing.T) { + testCases := []struct { + value log.Value + value2 log.Value + }{ + { + value: log.SliceValue(log.IntValue(3), log.StringValue("foo")), + value2: log.SliceValue(log.StringValue("foo"), log.IntValue(3)), + }, + { + value: log.MapValue( + log.Slice("l", log.IntValue(3), log.StringValue("foo")), + log.Bytes("b", []byte{3, 5, 7}), + log.Empty("e"), + ), + value2: log.MapValue( + log.Bytes("b", []byte{3, 5, 7}), + log.Slice("l", log.IntValue(3), log.StringValue("foo")), + log.Empty("e"), + ), + }, + } + for _, tc := range testCases { + t.Run(tc.value.String(), func(t *testing.T) { + assert.Equal(t, true, tc.value.Equal(tc.value2), "%v.Equal(%v)", tc.value, tc.value2) + }) + } +} + func TestValueEmpty(t *testing.T) { v := log.Value{} t.Run("Value.Empty", func(t *testing.T) { From a9f2f4f14dbb76dac60de1b0483e56bb141c549b Mon Sep 17 00:00:00 2001 From: dmathieu Date: Tue, 7 May 2024 10:09:49 +0200 Subject: [PATCH 2/8] add changelog entry --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 61db0af8b1d..f4eee2a555e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix the empty output of `go.opentelemetry.io/otel/log.Value` in `go.opentelemetry.io/otel/exporters/stdout/stdoutlog`. (#5311) +### Fixed + +- Comparison of unordered slices and maps in `go.opentelemetry.io/otel/log.KeyValue` and `go.opentelemetry.io/otel/log.Value`. (#) + ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24 ### Added From 3f662d4b9afe9990ac53b56b64570e20a7358b3e Mon Sep 17 00:00:00 2001 From: dmathieu Date: Tue, 7 May 2024 10:15:26 +0200 Subject: [PATCH 3/8] add benchmark value equal --- log/keyvalue_bench_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/log/keyvalue_bench_test.go b/log/keyvalue_bench_test.go index ede54e54821..247fdf5b751 100644 --- a/log/keyvalue_bench_test.go +++ b/log/keyvalue_bench_test.go @@ -199,3 +199,34 @@ func BenchmarkString(b *testing.B) { } }) } + +func BenchmarkValueEqual(b *testing.B) { + vals := []log.Value{ + {}, + log.Int64Value(1), + log.Int64Value(2), + log.Float64Value(3.5), + log.Float64Value(3.7), + log.BoolValue(true), + log.BoolValue(false), + log.StringValue("hi"), + log.StringValue("bye"), + log.BytesValue([]byte{1, 3, 5}), + log.SliceValue(log.StringValue("foo")), + log.SliceValue(log.IntValue(3), log.StringValue("foo")), + log.MapValue(log.Bool("b", true), log.Int("i", 3)), + log.MapValue( + log.Slice("l", log.IntValue(3), log.StringValue("foo")), + log.Bytes("b", []byte{3, 5, 7}), + log.Empty("e"), + ), + } + for _, v1 := range vals { + for _, v2 := range vals { + b.Run(v1.String()+" with "+v2.String(), func(b *testing.B) { + b.ReportAllocs() + _ = v1.Equal(v2) + }) + } + } +} From 92e789f0b8d10a921217eb61c200f222a7bd31b1 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Tue, 7 May 2024 10:21:12 +0200 Subject: [PATCH 4/8] pr number in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4eee2a555e..2f439f137e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,7 +34,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- Comparison of unordered slices and maps in `go.opentelemetry.io/otel/log.KeyValue` and `go.opentelemetry.io/otel/log.Value`. (#) +- Comparison of unordered slices and maps in `go.opentelemetry.io/otel/log.KeyValue` and `go.opentelemetry.io/otel/log.Value`. (#5306) ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24 From 5a94996622901dd1eff2fae979338b5a44e7e544 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Tue, 7 May 2024 17:22:52 +0200 Subject: [PATCH 5/8] only sort maps, not slices --- log/keyvalue.go | 10 +--------- log/keyvalue_test.go | 4 ---- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/log/keyvalue.go b/log/keyvalue.go index c9eb9034f80..53bcd52f4d0 100644 --- a/log/keyvalue.go +++ b/log/keyvalue.go @@ -255,15 +255,7 @@ func (v Value) Equal(w Value) bool { case KindFloat64: return v.asFloat64() == w.asFloat64() case KindSlice: - sv := v.asSlice() - sw := w.asSlice() - slices.SortFunc(sv, func(a, b Value) int { - return cmp.Compare(a.String(), b.String()) - }) - slices.SortFunc(sw, func(a, b Value) int { - return cmp.Compare(a.String(), b.String()) - }) - return slices.EqualFunc(sv, sw, Value.Equal) + return slices.EqualFunc(v.asSlice(), w.asSlice(), Value.Equal) case KindMap: sv := v.asMap() slices.SortFunc(sv, func(a, b KeyValue) int { diff --git a/log/keyvalue_test.go b/log/keyvalue_test.go index 0a9e62731df..043fab09dc1 100644 --- a/log/keyvalue_test.go +++ b/log/keyvalue_test.go @@ -75,10 +75,6 @@ func TestSortedValueEqual(t *testing.T) { value log.Value value2 log.Value }{ - { - value: log.SliceValue(log.IntValue(3), log.StringValue("foo")), - value2: log.SliceValue(log.StringValue("foo"), log.IntValue(3)), - }, { value: log.MapValue( log.Slice("l", log.IntValue(3), log.StringValue("foo")), From ed4bb0ee6c4a6f7c74f9fc44b11d744a537db901 Mon Sep 17 00:00:00 2001 From: dmathieu Date: Wed, 8 May 2024 10:26:41 +0200 Subject: [PATCH 6/8] don't mutate the map when comparing it --- log/keyvalue.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/log/keyvalue.go b/log/keyvalue.go index 53bcd52f4d0..8258defe360 100644 --- a/log/keyvalue.go +++ b/log/keyvalue.go @@ -257,15 +257,9 @@ func (v Value) Equal(w Value) bool { case KindSlice: return slices.EqualFunc(v.asSlice(), w.asSlice(), Value.Equal) case KindMap: - sv := v.asMap() - slices.SortFunc(sv, func(a, b KeyValue) int { - return cmp.Compare(a.Key, b.Key) - }) - sw := w.asMap() - slices.SortFunc(sw, func(a, b KeyValue) int { - return cmp.Compare(a.Key, b.Key) - }) - return slices.EqualFunc(v.asMap(), w.asMap(), KeyValue.Equal) + sv := sortMap(v.asMap()) + sw := sortMap(w.asMap()) + return slices.EqualFunc(sv, sw, KeyValue.Equal) case KindBytes: return bytes.Equal(v.asBytes(), w.asBytes()) case KindEmpty: @@ -276,6 +270,16 @@ func (v Value) Equal(w Value) bool { } } +func sortMap(m []KeyValue) []KeyValue { + sm := make([]KeyValue, len(m)) + copy(sm, m) + slices.SortFunc(sm, func(a, b KeyValue) int { + return cmp.Compare(a.Key, b.Key) + }) + + return sm +} + // String returns Value's value as a string, formatted like [fmt.Sprint]. // // The returned string is meant for debugging; From 4e2e11e60173424404513d898296c43c62f4a91b Mon Sep 17 00:00:00 2001 From: dmathieu Date: Wed, 15 May 2024 09:19:45 +0200 Subject: [PATCH 7/8] fix changelog entry --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f439f137e8..d136bd104b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,9 +31,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix the empty output of `go.opentelemetry.io/otel/log.Value` in `go.opentelemetry.io/otel/exporters/stdout/stdoutlog`. (#5311) - -### Fixed - - Comparison of unordered slices and maps in `go.opentelemetry.io/otel/log.KeyValue` and `go.opentelemetry.io/otel/log.Value`. (#5306) ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24 From 5397061c71e61f625a1b0ac5c057e78db7e346a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Wed, 15 May 2024 12:20:07 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d136bd104b8..b0658330c74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix the empty output of `go.opentelemetry.io/otel/log.Value` in `go.opentelemetry.io/otel/exporters/stdout/stdoutlog`. (#5311) -- Comparison of unordered slices and maps in `go.opentelemetry.io/otel/log.KeyValue` and `go.opentelemetry.io/otel/log.Value`. (#5306) +- Comparison of unordered maps in `go.opentelemetry.io/otel/log.KeyValue` and `go.opentelemetry.io/otel/log.Value`. (#5306) ## [1.26.0/0.48.0/0.2.0-alpha] 2024-04-24