From 74308d5a9c3d52d99a32c4d19023d1c12294e860 Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Wed, 3 Dec 2025 21:53:42 +0100 Subject: [PATCH 01/14] fix dedup key log msg and add tests --- sdk/log/record.go | 27 ++- sdk/log/record_test.go | 431 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 444 insertions(+), 14 deletions(-) diff --git a/sdk/log/record.go b/sdk/log/record.go index 5b830b7ea8f..970dbc60095 100644 --- a/sdk/log/record.go +++ b/sdk/log/record.go @@ -27,6 +27,10 @@ var logAttrDropped = sync.OnceFunc(func() { global.Warn("limit reached: dropping log Record attributes") }) +var logKeyValuePairDropped = sync.OnceFunc(func() { + global.Warn("key duplication: dropping key-value pair") +}) + // uniquePool is a pool of unique attributes used for attributes de-duplication. var uniquePool = sync.Pool{ New: func() any { return new([]log.KeyValue) }, @@ -132,13 +136,17 @@ type Record struct { } func (r *Record) addDropped(n int) { - logAttrDropped() r.dropped += n + if n > 0 { + logAttrDropped() + } } func (r *Record) setDropped(n int) { - logAttrDropped() r.dropped = n + if n > 0 { + logAttrDropped() + } } // EventName returns the event name. @@ -233,7 +241,9 @@ func (r *Record) AddAttributes(attrs ...log.KeyValue) { var drop int if !r.allowDupKeys { attrs, drop = dedup(attrs) - r.setDropped(drop) + if drop > 0 { + logKeyValuePairDropped() + } } attrs, drop := head(attrs, r.attributeCountLimit) @@ -266,6 +276,7 @@ func (r *Record) AddAttributes(attrs ...log.KeyValue) { idx, found := uIndex[a.Key] if found { dropped++ + logKeyValuePairDropped() (*unique)[idx] = a continue } @@ -274,6 +285,7 @@ func (r *Record) AddAttributes(attrs ...log.KeyValue) { if found { // New attrs overwrite any existing with the same key. dropped++ + logKeyValuePairDropped() if idx < 0 { r.front[-(idx + 1)] = a } else { @@ -289,7 +301,6 @@ func (r *Record) AddAttributes(attrs ...log.KeyValue) { if dropped > 0 { attrs = make([]log.KeyValue, len(*unique)) copy(attrs, *unique) - r.addDropped(dropped) } } @@ -352,7 +363,9 @@ func (r *Record) SetAttributes(attrs ...log.KeyValue) { r.setDropped(0) if !r.allowDupKeys { attrs, drop = dedup(attrs) - r.setDropped(drop) + if drop > 0 { + logKeyValuePairDropped() + } } attrs, drop = head(attrs, r.attributeCountLimit) @@ -515,7 +528,9 @@ func (r *Record) applyValueLimitsAndDedup(val log.Value) log.Value { // Deduplicate then truncate. // Do not do at the same time to avoid wasted truncation operations. newKvs, dropped = dedup(kvs) - r.addDropped(dropped) + if dropped > 0 { + logKeyValuePairDropped() + } } else { newKvs = kvs } diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index 6f363527df0..ae70c21f05f 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -553,17 +553,45 @@ func TestRecordDroppedAttributes(t *testing.T) { attrs := make([]log.KeyValue, i) attrs[0] = log.Bool("only key different then the rest", true) + // The rest have empty keys (duplicates with each other) assert.False(t, called, "non-dropped attributed logged") r.AddAttributes(attrs...) - assert.Equalf(t, i-1, r.DroppedAttributes(), "%d: AddAttributes", i) - assert.True(t, called, "dropped attributes not logged") + // After deduplication: + // - if i==1: only 1 unique key, no dropped (within limit) + // - if i>1: 2 unique keys (first + one empty key), limit=1, so 1 dropped + expectedDropped := 0 + if i > 1 { + expectedDropped = 1 + } + assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: AddAttributes", i) + if i > 1 { + assert.True(t, called, "dropped attributes not logged") + } + + // Reset for second call + called = false + logAttrDropped = func() { called = true } r.AddAttributes(attrs...) - assert.Equalf(t, 2*i-1, r.DroppedAttributes(), "%d: second AddAttributes", i) + // After second add: still limit=1 + // - if i==1: no new drops (still 1 attribute, within limit) + // - if i>1: dropped count increases by 1 + expectedDropped = 0 + if i > 1 { + expectedDropped = 2 + } + assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: second AddAttributes", i) r.SetAttributes(attrs...) - assert.Equalf(t, i-1, r.DroppedAttributes(), "%d: SetAttributes", i) + // SetAttributes resets, then adds + // - if i==1: no dropped + // - if i>1: 1 dropped + expectedDropped = 0 + if i > 1 { + expectedDropped = 1 + } + assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: SetAttributes", i) } } @@ -957,7 +985,7 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { log.String("g", "GG"), log.String("h", "H"), ), - wantDroppedAttrs: 10, + wantDroppedAttrs: 0, // Deduplication doesn't count as dropped }, { name: "EmptyMap", @@ -987,7 +1015,7 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { log.MapValue(log.String("key", "value2")), log.StringValue("normal"), ), - wantDroppedAttrs: 1, + wantDroppedAttrs: 0, // Nested deduplication doesn't count as dropped }, { name: "NestedSliceInMap", @@ -1001,7 +1029,7 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { log.MapValue(log.String("nested", "value2")), ), ), - wantDroppedAttrs: 1, + wantDroppedAttrs: 0, // Nested deduplication doesn't count as dropped }, { name: "DeeplyNestedStructure", @@ -1023,7 +1051,7 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { ), ), ), - wantDroppedAttrs: 1, + wantDroppedAttrs: 0, // Deeply nested deduplication doesn't count as dropped }, { name: "NestedMapWithoutDuplicateKeys", @@ -1060,6 +1088,393 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { } } +func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { + origKeyValueDropped := logKeyValuePairDropped + origAttrDropped := logAttrDropped + t.Cleanup(func() { + logKeyValuePairDropped = origKeyValueDropped + logAttrDropped = origAttrDropped + }) + + testCases := []struct { + name string + setupRecord func() *Record + operation func(*Record) + expectKeyValueDropped bool + expectAttrDropped bool + expectedDroppedCount int + expectedAttributeCount int + description string + }{ + { + name: "SetAttributes with duplicate keys", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + allowDupKeys: false, + } + return r + }, + operation: func(r *Record) { + r.SetAttributes( + log.String("key", "value1"), + log.String("key", "value2"), + ) + }, + expectKeyValueDropped: true, + expectAttrDropped: false, + expectedDroppedCount: 0, + expectedAttributeCount: 1, + description: "SetAttributes with duplicate keys should call logKeyValuePairDropped", + }, + { + name: "AddAttributes with duplicate keys in new attrs", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + allowDupKeys: false, + } + return r + }, + operation: func(r *Record) { + r.AddAttributes( + log.String("key", "value1"), + log.String("key", "value2"), + ) + }, + expectKeyValueDropped: true, + expectAttrDropped: false, + expectedDroppedCount: 0, + expectedAttributeCount: 1, + description: "AddAttributes with duplicate keys should call logKeyValuePairDropped", + }, + { + name: "AddAttributes with duplicate between existing and new", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + allowDupKeys: false, + } + r.SetAttributes(log.String("key1", "value1")) + return r + }, + operation: func(r *Record) { + r.AddAttributes(log.String("key1", "value2")) + }, + expectKeyValueDropped: true, + expectAttrDropped: false, + expectedDroppedCount: 0, + expectedAttributeCount: 1, + description: "AddAttributes with duplicate between existing and new should call logKeyValuePairDropped", + }, + { + name: "AddAttributes with nested map duplicates", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + allowDupKeys: false, + } + return r + }, + operation: func(r *Record) { + r.AddAttributes( + log.Map("outer", + log.String("nested", "value1"), + log.String("nested", "value2"), + ), + ) + }, + expectKeyValueDropped: true, + expectAttrDropped: false, + expectedDroppedCount: 0, + expectedAttributeCount: 1, + description: "Nested map duplicates should call logKeyValuePairDropped", + }, + { + name: "SetAttributes with limit reached (no duplicates)", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + attributeCountLimit: 2, + allowDupKeys: false, + } + return r + }, + operation: func(r *Record) { + r.SetAttributes( + log.String("key1", "value1"), + log.String("key2", "value2"), + log.String("key3", "value3"), + ) + }, + expectKeyValueDropped: false, + expectAttrDropped: true, + expectedDroppedCount: 1, + expectedAttributeCount: 2, + description: "Limit reached without duplicates should call logAttrDropped", + }, + { + name: "SetAttributes with both duplicates and limit", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + attributeCountLimit: 2, + allowDupKeys: false, + } + return r + }, + operation: func(r *Record) { + r.SetAttributes( + log.String("key1", "value1"), + log.String("key1", "value2"), // duplicate + log.String("key2", "value3"), + log.String("key3", "value4"), + log.String("key3", "value5"), // duplicate + ) + }, + expectKeyValueDropped: true, + expectAttrDropped: true, + expectedDroppedCount: 1, + expectedAttributeCount: 2, + description: "Both duplicates and limit should call both log functions", + }, + { + name: "AddAttributes no duplicates no limit", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + attributeCountLimit: 10, + allowDupKeys: false, + } + return r + }, + operation: func(r *Record) { + r.AddAttributes( + log.String("key1", "value1"), + log.String("key2", "value2"), + ) + }, + expectKeyValueDropped: false, + expectAttrDropped: false, + expectedDroppedCount: 0, + expectedAttributeCount: 2, + description: "No duplicates and no limit should not call any log function", + }, + { + name: "SetAttributes with allowDupKeys enabled", + setupRecord: func() *Record { + r := &Record{ + attributeValueLengthLimit: -1, + allowDupKeys: true, + } + return r + }, + operation: func(r *Record) { + r.SetAttributes( + log.String("key", "value1"), + log.String("key", "value2"), + ) + }, + expectKeyValueDropped: false, + expectAttrDropped: false, + expectedDroppedCount: 0, + expectedAttributeCount: 2, + description: "With allowDupKeys=true, should not call logKeyValuePairDropped", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + keyValueDroppedCalled := false + attrDroppedCalled := false + + logKeyValuePairDropped = sync.OnceFunc(func() { + keyValueDroppedCalled = true + }) + logAttrDropped = sync.OnceFunc(func() { + attrDroppedCalled = true + }) + + r := tc.setupRecord() + tc.operation(r) + + assert.Equal(t, tc.expectKeyValueDropped, keyValueDroppedCalled, + "logKeyValuePairDropped call mismatch: %s", tc.description) + assert.Equal(t, tc.expectAttrDropped, attrDroppedCalled, + "logAttrDropped call mismatch: %s", tc.description) + assert.Equal(t, tc.expectedDroppedCount, r.DroppedAttributes(), + "DroppedAttributes count mismatch: %s", tc.description) + assert.Equal(t, tc.expectedAttributeCount, r.AttributesLen(), + "AttributesLen mismatch: %s", tc.description) + }) + } +} + +func TestDroppedCountExcludesDeduplication(t *testing.T) { + testCases := []struct { + name string + attrs []log.KeyValue + attributeCountLimit int + expectedDropped int + expectedAttrCount int + description string + }{ + { + name: "Only deduplication, no limit", + attrs: []log.KeyValue{ + log.String("key", "value1"), + log.String("key", "value2"), + log.String("key", "value3"), + }, + attributeCountLimit: 0, // No limit + expectedDropped: 0, // Deduplication doesn't count + expectedAttrCount: 1, + description: "Multiple duplicates should not increase dropped count", + }, + { + name: "Deduplication and limit", + attrs: []log.KeyValue{ + log.String("a", "value1"), + log.String("a", "value2"), // duplicate + log.String("b", "value3"), + log.String("c", "value4"), + log.String("c", "value5"), // duplicate + }, + attributeCountLimit: 2, + expectedDropped: 1, // Only limit drops count (3 unique -> 2 kept) + expectedAttrCount: 2, + description: "Deduplication then limit: only limit drops should count", + }, + { + name: "No deduplication, only limit", + attrs: []log.KeyValue{ + log.String("a", "value1"), + log.String("b", "value2"), + log.String("c", "value3"), + log.String("d", "value4"), + }, + attributeCountLimit: 2, + expectedDropped: 2, // 2 attributes dropped due to limit + expectedAttrCount: 2, + description: "Only limit without deduplication", + }, + { + name: "Complex: multiple duplicates and limit", + attrs: []log.KeyValue{ + log.String("a", "a1"), + log.String("b", "b1"), + log.String("a", "a2"), // dup + log.String("c", "c1"), + log.String("b", "b2"), // dup + log.String("d", "d1"), + log.String("a", "a3"), // dup + log.String("e", "e1"), + }, + attributeCountLimit: 3, + expectedDropped: 2, // 5 unique keys, limit 3, so 2 dropped + expectedAttrCount: 3, + description: "Complex scenario with multiple duplicates and limit", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + r := &Record{ + attributeValueLengthLimit: -1, + attributeCountLimit: tc.attributeCountLimit, + allowDupKeys: false, + } + + r.SetAttributes(tc.attrs...) + + assert.Equal(t, tc.expectedDropped, r.DroppedAttributes(), + "%s: dropped count mismatch", tc.description) + assert.Equal(t, tc.expectedAttrCount, r.AttributesLen(), + "%s: attribute count mismatch", tc.description) + }) + } +} + +func TestSetDroppedAndAddDroppedWithZero(t *testing.T) { + origAttrDropped := logAttrDropped + t.Cleanup(func() { + logAttrDropped = origAttrDropped + }) + + t.Run("setDropped(0) does not call log", func(t *testing.T) { + called := false + logAttrDropped = sync.OnceFunc(func() { + called = true + }) + + r := &Record{} + r.setDropped(0) + + assert.False(t, called, "logAttrDropped should not be called for setDropped(0)") + assert.Equal(t, 0, r.DroppedAttributes()) + }) + + t.Run("setDropped(n>0) calls log", func(t *testing.T) { + called := false + logAttrDropped = sync.OnceFunc(func() { + called = true + }) + + r := &Record{} + r.setDropped(5) + + assert.True(t, called, "logAttrDropped should be called for setDropped(n>0)") + assert.Equal(t, 5, r.DroppedAttributes()) + }) + + t.Run("addDropped(0) does not call log", func(t *testing.T) { + called := false + logAttrDropped = sync.OnceFunc(func() { + called = true + }) + + r := &Record{} + r.addDropped(0) + + assert.False(t, called, "logAttrDropped should not be called for addDropped(0)") + assert.Equal(t, 0, r.DroppedAttributes()) + }) + + t.Run("addDropped(n>0) calls log", func(t *testing.T) { + called := false + logAttrDropped = sync.OnceFunc(func() { + called = true + }) + + r := &Record{} + r.addDropped(3) + + assert.True(t, called, "logAttrDropped should be called for addDropped(n>0)") + assert.Equal(t, 3, r.DroppedAttributes()) + }) + + t.Run("multiple addDropped accumulates", func(t *testing.T) { + called := false + logAttrDropped = sync.OnceFunc(func() { + called = true + }) + + r := &Record{} + r.addDropped(2) + assert.True(t, called, "first addDropped should call log") + + // Reset for second call + called = false + logAttrDropped = sync.OnceFunc(func() { + called = true + }) + + r.addDropped(3) + assert.True(t, called, "second addDropped should call log") + assert.Equal(t, 5, r.DroppedAttributes(), "dropped count should accumulate") + }) +} + func TestApplyAttrLimitsTruncation(t *testing.T) { testcases := []struct { name string From 89d2390b0c5d1c91f448c14ba0421a47573d317c Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Wed, 3 Dec 2025 22:10:52 +0100 Subject: [PATCH 02/14] clean comments in tests --- sdk/log/record_test.go | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index ae70c21f05f..c7b74d3ea39 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -553,13 +553,9 @@ func TestRecordDroppedAttributes(t *testing.T) { attrs := make([]log.KeyValue, i) attrs[0] = log.Bool("only key different then the rest", true) - // The rest have empty keys (duplicates with each other) assert.False(t, called, "non-dropped attributed logged") r.AddAttributes(attrs...) - // After deduplication: - // - if i==1: only 1 unique key, no dropped (within limit) - // - if i>1: 2 unique keys (first + one empty key), limit=1, so 1 dropped expectedDropped := 0 if i > 1 { expectedDropped = 1 @@ -569,14 +565,10 @@ func TestRecordDroppedAttributes(t *testing.T) { assert.True(t, called, "dropped attributes not logged") } - // Reset for second call called = false logAttrDropped = func() { called = true } r.AddAttributes(attrs...) - // After second add: still limit=1 - // - if i==1: no new drops (still 1 attribute, within limit) - // - if i>1: dropped count increases by 1 expectedDropped = 0 if i > 1 { expectedDropped = 2 @@ -584,9 +576,6 @@ func TestRecordDroppedAttributes(t *testing.T) { assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: second AddAttributes", i) r.SetAttributes(attrs...) - // SetAttributes resets, then adds - // - if i==1: no dropped - // - if i>1: 1 dropped expectedDropped = 0 if i > 1 { expectedDropped = 1 @@ -1226,10 +1215,10 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { operation: func(r *Record) { r.SetAttributes( log.String("key1", "value1"), - log.String("key1", "value2"), // duplicate + log.String("key1", "value2"), log.String("key2", "value3"), log.String("key3", "value4"), - log.String("key3", "value5"), // duplicate + log.String("key3", "value5"), ) }, expectKeyValueDropped: true, @@ -1326,8 +1315,8 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { log.String("key", "value2"), log.String("key", "value3"), }, - attributeCountLimit: 0, // No limit - expectedDropped: 0, // Deduplication doesn't count + attributeCountLimit: 0, + expectedDropped: 0, expectedAttrCount: 1, description: "Multiple duplicates should not increase dropped count", }, @@ -1335,10 +1324,10 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { name: "Deduplication and limit", attrs: []log.KeyValue{ log.String("a", "value1"), - log.String("a", "value2"), // duplicate + log.String("a", "value2"), log.String("b", "value3"), log.String("c", "value4"), - log.String("c", "value5"), // duplicate + log.String("c", "value5"), }, attributeCountLimit: 2, expectedDropped: 1, // Only limit drops count (3 unique -> 2 kept) @@ -1363,11 +1352,11 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { attrs: []log.KeyValue{ log.String("a", "a1"), log.String("b", "b1"), - log.String("a", "a2"), // dup + log.String("a", "a2"), log.String("c", "c1"), - log.String("b", "b2"), // dup + log.String("b", "b2"), log.String("d", "d1"), - log.String("a", "a3"), // dup + log.String("a", "a3"), log.String("e", "e1"), }, attributeCountLimit: 3, From 88668eae7ef7bde0ab75239dd37e2e1f3b92ce00 Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Wed, 3 Dec 2025 22:33:17 +0100 Subject: [PATCH 03/14] add changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc6c571bb4a..132dc9fffb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Return partial OTLP export errors to the caller in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc`. (#7372) - Return partial OTLP export errors to the caller in `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp`. (#7372) - Fix `AddAttributes`, `SetAttributes`, `SetBody` on `Record` in `go.opentelemetry.io/otel/sdk/log` to not mutate input. (#7403) - +- Fix log error message from dropping key-value pair and remove the count (#7662) ### Removed - Drop support for [Go 1.23]. (#7274) From 7eececdfd538c7d2f15b4f8b4795031f9b2641a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 9 Dec 2025 12:45:14 +0100 Subject: [PATCH 04/14] Update changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c87968602e3..483ca364842 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed -- Fix log error message from dropping key-value pair and remove the count in `go.opentelemetry.io/otel/sdk/log`. (#7662) +- Fix bad log message when key-value pairs are dropped because of key duplication in `go.opentelemetry.io/otel/sdk/log`. (#7662) From 065756e99c34515f8b0bec5853a22d3b5d5654a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 9 Dec 2025 12:51:51 +0100 Subject: [PATCH 05/14] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 483ca364842..34ec05ce87d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix bad log message when key-value pairs are dropped because of key duplication in `go.opentelemetry.io/otel/sdk/log`. (#7662) +- Fix `DroppedAttributes` in `go.opentelemetry.io/otel/sdk/log` to not count the dropped non-attribute key-value pairs. (#7662) From 1ab60b48e924d3dd495af9a82464c6808faee8a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Tue, 9 Dec 2025 12:55:53 +0100 Subject: [PATCH 06/14] Update CHANGELOG for key-value pair duplication fix Clarify log message fix for dropped key-value pairs. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34ec05ce87d..d884c905ed0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix bad log message when key-value pairs are dropped because of key duplication in `go.opentelemetry.io/otel/sdk/log`. (#7662) -- Fix `DroppedAttributes` in `go.opentelemetry.io/otel/sdk/log` to not count the dropped non-attribute key-value pairs. (#7662) +- Fix `DroppedAttributes` in `go.opentelemetry.io/otel/sdk/log` to not count the non-attribute key-value pairs dropped because of key duplication. (#7662) From 764fc3fb4c54f9e0d2f8b1e40b947dec241eb30f Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Fri, 26 Dec 2025 19:05:26 +0100 Subject: [PATCH 07/14] Rename test vars and moved descriptions field below name --- sdk/log/record_test.go | 146 ++++++++++++++++++++--------------------- 1 file changed, 73 insertions(+), 73 deletions(-) diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index c7b74d3ea39..3c162359582 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -556,11 +556,11 @@ func TestRecordDroppedAttributes(t *testing.T) { assert.False(t, called, "non-dropped attributed logged") r.AddAttributes(attrs...) - expectedDropped := 0 + wantDropped := 0 if i > 1 { - expectedDropped = 1 + wantDropped = 1 } - assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: AddAttributes", i) + assert.Equalf(t, wantDropped, r.DroppedAttributes(), "%d: AddAttributes", i) if i > 1 { assert.True(t, called, "dropped attributes not logged") } @@ -569,18 +569,18 @@ func TestRecordDroppedAttributes(t *testing.T) { logAttrDropped = func() { called = true } r.AddAttributes(attrs...) - expectedDropped = 0 + wantDropped = 0 if i > 1 { - expectedDropped = 2 + wantDropped = 2 } - assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: second AddAttributes", i) + assert.Equalf(t, wantDropped, r.DroppedAttributes(), "%d: second AddAttributes", i) r.SetAttributes(attrs...) - expectedDropped = 0 + wantDropped = 0 if i > 1 { - expectedDropped = 1 + wantDropped = 1 } - assert.Equalf(t, expectedDropped, r.DroppedAttributes(), "%d: SetAttributes", i) + assert.Equalf(t, wantDropped, r.DroppedAttributes(), "%d: SetAttributes", i) } } @@ -1086,14 +1086,14 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { }) testCases := []struct { - name string - setupRecord func() *Record - operation func(*Record) - expectKeyValueDropped bool - expectAttrDropped bool - expectedDroppedCount int - expectedAttributeCount int - description string + name string + setupRecord func() *Record + operation func(*Record) + wantKeyValueDropped bool + wantAttrDropped bool + wantDroppedCount int + wantAttributeCount int + description string }{ { name: "SetAttributes with duplicate keys", @@ -1110,11 +1110,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { log.String("key", "value2"), ) }, - expectKeyValueDropped: true, - expectAttrDropped: false, - expectedDroppedCount: 0, - expectedAttributeCount: 1, - description: "SetAttributes with duplicate keys should call logKeyValuePairDropped", + wantKeyValueDropped: true, + wantAttrDropped: false, + wantDroppedCount: 0, + wantAttributeCount: 1, + description: "SetAttributes with duplicate keys should call logKeyValuePairDropped", }, { name: "AddAttributes with duplicate keys in new attrs", @@ -1131,11 +1131,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { log.String("key", "value2"), ) }, - expectKeyValueDropped: true, - expectAttrDropped: false, - expectedDroppedCount: 0, - expectedAttributeCount: 1, - description: "AddAttributes with duplicate keys should call logKeyValuePairDropped", + wantKeyValueDropped: true, + wantAttrDropped: false, + wantDroppedCount: 0, + wantAttributeCount: 1, + description: "AddAttributes with duplicate keys should call logKeyValuePairDropped", }, { name: "AddAttributes with duplicate between existing and new", @@ -1150,11 +1150,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { operation: func(r *Record) { r.AddAttributes(log.String("key1", "value2")) }, - expectKeyValueDropped: true, - expectAttrDropped: false, - expectedDroppedCount: 0, - expectedAttributeCount: 1, - description: "AddAttributes with duplicate between existing and new should call logKeyValuePairDropped", + wantKeyValueDropped: true, + wantAttrDropped: false, + wantDroppedCount: 0, + wantAttributeCount: 1, + description: "AddAttributes with duplicate between existing and new should call logKeyValuePairDropped", }, { name: "AddAttributes with nested map duplicates", @@ -1173,11 +1173,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { ), ) }, - expectKeyValueDropped: true, - expectAttrDropped: false, - expectedDroppedCount: 0, - expectedAttributeCount: 1, - description: "Nested map duplicates should call logKeyValuePairDropped", + wantKeyValueDropped: true, + wantAttrDropped: false, + wantDroppedCount: 0, + wantAttributeCount: 1, + description: "Nested map duplicates should call logKeyValuePairDropped", }, { name: "SetAttributes with limit reached (no duplicates)", @@ -1196,11 +1196,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { log.String("key3", "value3"), ) }, - expectKeyValueDropped: false, - expectAttrDropped: true, - expectedDroppedCount: 1, - expectedAttributeCount: 2, - description: "Limit reached without duplicates should call logAttrDropped", + wantKeyValueDropped: false, + wantAttrDropped: true, + wantDroppedCount: 1, + wantAttributeCount: 2, + description: "Limit reached without duplicates should call logAttrDropped", }, { name: "SetAttributes with both duplicates and limit", @@ -1221,11 +1221,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { log.String("key3", "value5"), ) }, - expectKeyValueDropped: true, - expectAttrDropped: true, - expectedDroppedCount: 1, - expectedAttributeCount: 2, - description: "Both duplicates and limit should call both log functions", + wantKeyValueDropped: true, + wantAttrDropped: true, + wantDroppedCount: 1, + wantAttributeCount: 2, + description: "Both duplicates and limit should call both log functions", }, { name: "AddAttributes no duplicates no limit", @@ -1243,11 +1243,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { log.String("key2", "value2"), ) }, - expectKeyValueDropped: false, - expectAttrDropped: false, - expectedDroppedCount: 0, - expectedAttributeCount: 2, - description: "No duplicates and no limit should not call any log function", + wantKeyValueDropped: false, + wantAttrDropped: false, + wantDroppedCount: 0, + wantAttributeCount: 2, + description: "No duplicates and no limit should not call any log function", }, { name: "SetAttributes with allowDupKeys enabled", @@ -1264,11 +1264,11 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { log.String("key", "value2"), ) }, - expectKeyValueDropped: false, - expectAttrDropped: false, - expectedDroppedCount: 0, - expectedAttributeCount: 2, - description: "With allowDupKeys=true, should not call logKeyValuePairDropped", + wantKeyValueDropped: false, + wantAttrDropped: false, + wantDroppedCount: 0, + wantAttributeCount: 2, + description: "With allowDupKeys=true, should not call logKeyValuePairDropped", }, } @@ -1287,13 +1287,13 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { r := tc.setupRecord() tc.operation(r) - assert.Equal(t, tc.expectKeyValueDropped, keyValueDroppedCalled, + assert.Equal(t, tc.wantKeyValueDropped, keyValueDroppedCalled, "logKeyValuePairDropped call mismatch: %s", tc.description) - assert.Equal(t, tc.expectAttrDropped, attrDroppedCalled, + assert.Equal(t, tc.wantAttrDropped, attrDroppedCalled, "logAttrDropped call mismatch: %s", tc.description) - assert.Equal(t, tc.expectedDroppedCount, r.DroppedAttributes(), + assert.Equal(t, tc.wantDroppedCount, r.DroppedAttributes(), "DroppedAttributes count mismatch: %s", tc.description) - assert.Equal(t, tc.expectedAttributeCount, r.AttributesLen(), + assert.Equal(t, tc.wantAttributeCount, r.AttributesLen(), "AttributesLen mismatch: %s", tc.description) }) } @@ -1304,8 +1304,8 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { name string attrs []log.KeyValue attributeCountLimit int - expectedDropped int - expectedAttrCount int + wantDropped int + wantAttrCount int description string }{ { @@ -1316,8 +1316,8 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { log.String("key", "value3"), }, attributeCountLimit: 0, - expectedDropped: 0, - expectedAttrCount: 1, + wantDropped: 0, + wantAttrCount: 1, description: "Multiple duplicates should not increase dropped count", }, { @@ -1330,8 +1330,8 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { log.String("c", "value5"), }, attributeCountLimit: 2, - expectedDropped: 1, // Only limit drops count (3 unique -> 2 kept) - expectedAttrCount: 2, + wantDropped: 1, // Only limit drops count (3 unique -> 2 kept) + wantAttrCount: 2, description: "Deduplication then limit: only limit drops should count", }, { @@ -1343,8 +1343,8 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { log.String("d", "value4"), }, attributeCountLimit: 2, - expectedDropped: 2, // 2 attributes dropped due to limit - expectedAttrCount: 2, + wantDropped: 2, // 2 attributes dropped due to limit + wantAttrCount: 2, description: "Only limit without deduplication", }, { @@ -1360,8 +1360,8 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { log.String("e", "e1"), }, attributeCountLimit: 3, - expectedDropped: 2, // 5 unique keys, limit 3, so 2 dropped - expectedAttrCount: 3, + wantDropped: 2, // 5 unique keys, limit 3, so 2 dropped + wantAttrCount: 3, description: "Complex scenario with multiple duplicates and limit", }, } @@ -1376,9 +1376,9 @@ func TestDroppedCountExcludesDeduplication(t *testing.T) { r.SetAttributes(tc.attrs...) - assert.Equal(t, tc.expectedDropped, r.DroppedAttributes(), + assert.Equal(t, tc.wantDropped, r.DroppedAttributes(), "%s: dropped count mismatch", tc.description) - assert.Equal(t, tc.expectedAttrCount, r.AttributesLen(), + assert.Equal(t, tc.wantAttrCount, r.AttributesLen(), "%s: attribute count mismatch", tc.description) }) } From 630b8fbb8212a14bf8b353aae2ebe64b8e31f336 Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Fri, 26 Dec 2025 19:27:06 +0100 Subject: [PATCH 08/14] minimize amount of written test --- sdk/log/record_test.go | 382 +++++++---------------------------------- 1 file changed, 64 insertions(+), 318 deletions(-) diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index 3c162359582..fb5e91b4195 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -1077,7 +1077,7 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { } } -func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { +func TestDeduplicationBehavior(t *testing.T) { origKeyValueDropped := logKeyValuePairDropped origAttrDropped := logAttrDropped t.Cleanup(func() { @@ -1087,188 +1087,52 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { testCases := []struct { name string - setupRecord func() *Record - operation func(*Record) + attributeCountLimit int + allowDupKeys bool + attrs []log.KeyValue wantKeyValueDropped bool wantAttrDropped bool wantDroppedCount int wantAttributeCount int - description string }{ { - name: "SetAttributes with duplicate keys", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - allowDupKeys: false, - } - return r - }, - operation: func(r *Record) { - r.SetAttributes( - log.String("key", "value1"), - log.String("key", "value2"), - ) - }, - wantKeyValueDropped: true, - wantAttrDropped: false, - wantDroppedCount: 0, - wantAttributeCount: 1, - description: "SetAttributes with duplicate keys should call logKeyValuePairDropped", - }, - { - name: "AddAttributes with duplicate keys in new attrs", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - allowDupKeys: false, - } - return r - }, - operation: func(r *Record) { - r.AddAttributes( - log.String("key", "value1"), - log.String("key", "value2"), - ) - }, - wantKeyValueDropped: true, - wantAttrDropped: false, - wantDroppedCount: 0, - wantAttributeCount: 1, - description: "AddAttributes with duplicate keys should call logKeyValuePairDropped", - }, - { - name: "AddAttributes with duplicate between existing and new", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - allowDupKeys: false, - } - r.SetAttributes(log.String("key1", "value1")) - return r - }, - operation: func(r *Record) { - r.AddAttributes(log.String("key1", "value2")) - }, - wantKeyValueDropped: true, - wantAttrDropped: false, - wantDroppedCount: 0, - wantAttributeCount: 1, - description: "AddAttributes with duplicate between existing and new should call logKeyValuePairDropped", - }, - { - name: "AddAttributes with nested map duplicates", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - allowDupKeys: false, - } - return r - }, - operation: func(r *Record) { - r.AddAttributes( - log.Map("outer", - log.String("nested", "value1"), - log.String("nested", "value2"), - ), - ) - }, + name: "Duplicate keys only", + attrs: []log.KeyValue{log.String("key", "v1"), log.String("key", "v2")}, wantKeyValueDropped: true, - wantAttrDropped: false, - wantDroppedCount: 0, + wantDroppedCount: 0, // Deduplication doesn't count wantAttributeCount: 1, - description: "Nested map duplicates should call logKeyValuePairDropped", }, { - name: "SetAttributes with limit reached (no duplicates)", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - attributeCountLimit: 2, - allowDupKeys: false, - } - return r - }, - operation: func(r *Record) { - r.SetAttributes( - log.String("key1", "value1"), - log.String("key2", "value2"), - log.String("key3", "value3"), - ) - }, - wantKeyValueDropped: false, + name: "Limit exceeded only", + attributeCountLimit: 2, + attrs: []log.KeyValue{log.String("a", "v1"), log.String("b", "v2"), log.String("c", "v3")}, wantAttrDropped: true, wantDroppedCount: 1, wantAttributeCount: 2, - description: "Limit reached without duplicates should call logAttrDropped", }, { - name: "SetAttributes with both duplicates and limit", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - attributeCountLimit: 2, - allowDupKeys: false, - } - return r - }, - operation: func(r *Record) { - r.SetAttributes( - log.String("key1", "value1"), - log.String("key1", "value2"), - log.String("key2", "value3"), - log.String("key3", "value4"), - log.String("key3", "value5"), - ) - }, + name: "Both duplicates and limit", + attributeCountLimit: 2, + attrs: []log.KeyValue{log.String("a", "v1"), log.String("a", "v2"), log.String("b", "v3"), log.String("c", "v4")}, wantKeyValueDropped: true, wantAttrDropped: true, - wantDroppedCount: 1, + wantDroppedCount: 1, // Only limit drops count wantAttributeCount: 2, - description: "Both duplicates and limit should call both log functions", }, { - name: "AddAttributes no duplicates no limit", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - attributeCountLimit: 10, - allowDupKeys: false, - } - return r - }, - operation: func(r *Record) { - r.AddAttributes( - log.String("key1", "value1"), - log.String("key2", "value2"), - ) - }, + name: "allowDupKeys=true", + allowDupKeys: true, + attrs: []log.KeyValue{log.String("key", "v1"), log.String("key", "v2")}, wantKeyValueDropped: false, - wantAttrDropped: false, wantDroppedCount: 0, wantAttributeCount: 2, - description: "No duplicates and no limit should not call any log function", }, { - name: "SetAttributes with allowDupKeys enabled", - setupRecord: func() *Record { - r := &Record{ - attributeValueLengthLimit: -1, - allowDupKeys: true, - } - return r - }, - operation: func(r *Record) { - r.SetAttributes( - log.String("key", "value1"), - log.String("key", "value2"), - ) - }, - wantKeyValueDropped: false, - wantAttrDropped: false, + name: "Nested map duplicates", + attrs: []log.KeyValue{log.Map("outer", log.String("nested", "v1"), log.String("nested", "v2"))}, + wantKeyValueDropped: true, wantDroppedCount: 0, - wantAttributeCount: 2, - description: "With allowDupKeys=true, should not call logKeyValuePairDropped", + wantAttributeCount: 1, }, } @@ -1277,193 +1141,75 @@ func TestLogKeyValuePairDroppedOnDeduplication(t *testing.T) { keyValueDroppedCalled := false attrDroppedCalled := false - logKeyValuePairDropped = sync.OnceFunc(func() { - keyValueDroppedCalled = true - }) - logAttrDropped = sync.OnceFunc(func() { - attrDroppedCalled = true - }) + logKeyValuePairDropped = sync.OnceFunc(func() { keyValueDroppedCalled = true }) + logAttrDropped = sync.OnceFunc(func() { attrDroppedCalled = true }) - r := tc.setupRecord() - tc.operation(r) + r := &Record{ + attributeValueLengthLimit: -1, + attributeCountLimit: tc.attributeCountLimit, + allowDupKeys: tc.allowDupKeys, + } - assert.Equal(t, tc.wantKeyValueDropped, keyValueDroppedCalled, - "logKeyValuePairDropped call mismatch: %s", tc.description) - assert.Equal(t, tc.wantAttrDropped, attrDroppedCalled, - "logAttrDropped call mismatch: %s", tc.description) - assert.Equal(t, tc.wantDroppedCount, r.DroppedAttributes(), - "DroppedAttributes count mismatch: %s", tc.description) - assert.Equal(t, tc.wantAttributeCount, r.AttributesLen(), - "AttributesLen mismatch: %s", tc.description) + r.SetAttributes(tc.attrs...) + + assert.Equal(t, tc.wantKeyValueDropped, keyValueDroppedCalled) + assert.Equal(t, tc.wantAttrDropped, attrDroppedCalled) + assert.Equal(t, tc.wantDroppedCount, r.DroppedAttributes()) + assert.Equal(t, tc.wantAttributeCount, r.AttributesLen()) }) } } -func TestDroppedCountExcludesDeduplication(t *testing.T) { +func TestDroppedLoggingOnlyWhenNonZero(t *testing.T) { + origAttrDropped := logAttrDropped + t.Cleanup(func() { logAttrDropped = origAttrDropped }) + testCases := []struct { - name string - attrs []log.KeyValue - attributeCountLimit int - wantDropped int - wantAttrCount int - description string + name string + operation func(*Record) + wantCalled bool + wantCount int }{ { - name: "Only deduplication, no limit", - attrs: []log.KeyValue{ - log.String("key", "value1"), - log.String("key", "value2"), - log.String("key", "value3"), - }, - attributeCountLimit: 0, - wantDropped: 0, - wantAttrCount: 1, - description: "Multiple duplicates should not increase dropped count", + name: "setDropped(0) does not log", + operation: func(r *Record) { r.setDropped(0) }, + wantCalled: false, + wantCount: 0, }, { - name: "Deduplication and limit", - attrs: []log.KeyValue{ - log.String("a", "value1"), - log.String("a", "value2"), - log.String("b", "value3"), - log.String("c", "value4"), - log.String("c", "value5"), - }, - attributeCountLimit: 2, - wantDropped: 1, // Only limit drops count (3 unique -> 2 kept) - wantAttrCount: 2, - description: "Deduplication then limit: only limit drops should count", + name: "setDropped(n>0) logs", + operation: func(r *Record) { r.setDropped(5) }, + wantCalled: true, + wantCount: 5, }, { - name: "No deduplication, only limit", - attrs: []log.KeyValue{ - log.String("a", "value1"), - log.String("b", "value2"), - log.String("c", "value3"), - log.String("d", "value4"), - }, - attributeCountLimit: 2, - wantDropped: 2, // 2 attributes dropped due to limit - wantAttrCount: 2, - description: "Only limit without deduplication", + name: "addDropped(0) does not log", + operation: func(r *Record) { r.addDropped(0) }, + wantCalled: false, + wantCount: 0, }, { - name: "Complex: multiple duplicates and limit", - attrs: []log.KeyValue{ - log.String("a", "a1"), - log.String("b", "b1"), - log.String("a", "a2"), - log.String("c", "c1"), - log.String("b", "b2"), - log.String("d", "d1"), - log.String("a", "a3"), - log.String("e", "e1"), - }, - attributeCountLimit: 3, - wantDropped: 2, // 5 unique keys, limit 3, so 2 dropped - wantAttrCount: 3, - description: "Complex scenario with multiple duplicates and limit", + name: "addDropped(n>0) logs", + operation: func(r *Record) { r.addDropped(3) }, + wantCalled: true, + wantCount: 3, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - r := &Record{ - attributeValueLengthLimit: -1, - attributeCountLimit: tc.attributeCountLimit, - allowDupKeys: false, - } + called := false + logAttrDropped = sync.OnceFunc(func() { called = true }) - r.SetAttributes(tc.attrs...) + r := &Record{} + tc.operation(r) - assert.Equal(t, tc.wantDropped, r.DroppedAttributes(), - "%s: dropped count mismatch", tc.description) - assert.Equal(t, tc.wantAttrCount, r.AttributesLen(), - "%s: attribute count mismatch", tc.description) + assert.Equal(t, tc.wantCalled, called) + assert.Equal(t, tc.wantCount, r.DroppedAttributes()) }) } } -func TestSetDroppedAndAddDroppedWithZero(t *testing.T) { - origAttrDropped := logAttrDropped - t.Cleanup(func() { - logAttrDropped = origAttrDropped - }) - - t.Run("setDropped(0) does not call log", func(t *testing.T) { - called := false - logAttrDropped = sync.OnceFunc(func() { - called = true - }) - - r := &Record{} - r.setDropped(0) - - assert.False(t, called, "logAttrDropped should not be called for setDropped(0)") - assert.Equal(t, 0, r.DroppedAttributes()) - }) - - t.Run("setDropped(n>0) calls log", func(t *testing.T) { - called := false - logAttrDropped = sync.OnceFunc(func() { - called = true - }) - - r := &Record{} - r.setDropped(5) - - assert.True(t, called, "logAttrDropped should be called for setDropped(n>0)") - assert.Equal(t, 5, r.DroppedAttributes()) - }) - - t.Run("addDropped(0) does not call log", func(t *testing.T) { - called := false - logAttrDropped = sync.OnceFunc(func() { - called = true - }) - - r := &Record{} - r.addDropped(0) - - assert.False(t, called, "logAttrDropped should not be called for addDropped(0)") - assert.Equal(t, 0, r.DroppedAttributes()) - }) - - t.Run("addDropped(n>0) calls log", func(t *testing.T) { - called := false - logAttrDropped = sync.OnceFunc(func() { - called = true - }) - - r := &Record{} - r.addDropped(3) - - assert.True(t, called, "logAttrDropped should be called for addDropped(n>0)") - assert.Equal(t, 3, r.DroppedAttributes()) - }) - - t.Run("multiple addDropped accumulates", func(t *testing.T) { - called := false - logAttrDropped = sync.OnceFunc(func() { - called = true - }) - - r := &Record{} - r.addDropped(2) - assert.True(t, called, "first addDropped should call log") - - // Reset for second call - called = false - logAttrDropped = sync.OnceFunc(func() { - called = true - }) - - r.addDropped(3) - assert.True(t, called, "second addDropped should call log") - assert.Equal(t, 5, r.DroppedAttributes(), "dropped count should accumulate") - }) -} - func TestApplyAttrLimitsTruncation(t *testing.T) { testcases := []struct { name string From abe6f02b99ebb92f1bef44f01b2407264776bded Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri <67772460+mexirica@users.noreply.github.com> Date: Thu, 8 Jan 2026 21:42:42 +0100 Subject: [PATCH 09/14] Update CHANGELOG.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Robert Pająk --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be1f9fe507d..a418ac0d91e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Fix bad log message when key-value pairs are dropped because of key duplication in `go.opentelemetry.io/otel/sdk/log`. (#7662) - Fix `DroppedAttributes` in `go.opentelemetry.io/otel/sdk/log` to not count the non-attribute key-value pairs dropped because of key duplication. (#7662) -- Fix `setDropped` in `go.opentelemetry.io/otel/sdk/log` to log dropped keys only when the input quantity is greater than zero. (#7662) +- Fix `SetAttributes` in `go.opentelemetry.io/otel/sdk/log` to not log that attributes are dropped when they are actually not dropped. (#7662) + ### Changed - `Exporter` in `go.opentelemetry.io/otel/exporter/prometheus` ignores metrics with the scope `go.opentelemetry.io/contrib/bridges/prometheus`. From 6fd3a23fe8123687a5b7b0b3dc0cff0e319be17d Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Thu, 8 Jan 2026 21:47:30 +0100 Subject: [PATCH 10/14] remove setDropped method --- sdk/log/record.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/sdk/log/record.go b/sdk/log/record.go index 970dbc60095..83d1812efe2 100644 --- a/sdk/log/record.go +++ b/sdk/log/record.go @@ -142,13 +142,6 @@ func (r *Record) addDropped(n int) { } } -func (r *Record) setDropped(n int) { - r.dropped = n - if n > 0 { - logAttrDropped() - } -} - // EventName returns the event name. // A log record with non-empty event name is interpreted as an event record. func (r *Record) EventName() string { @@ -360,7 +353,7 @@ func (r *Record) addAttrs(attrs []log.KeyValue) { // SetAttributes sets (and overrides) attributes to the log record. func (r *Record) SetAttributes(attrs ...log.KeyValue) { var drop int - r.setDropped(0) + r.dropped = 0 if !r.allowDupKeys { attrs, drop = dedup(attrs) if drop > 0 { From 79ee73e3e9cc54952d5817ff4b65e5913879d991 Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Thu, 8 Jan 2026 21:50:55 +0100 Subject: [PATCH 11/14] Remove TestDroppedLoggingOnlyWhenNonZero test method --- sdk/log/record_test.go | 50 ------------------------------------------ 1 file changed, 50 deletions(-) diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index fb5e91b4195..f98551c7760 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -1160,56 +1160,6 @@ func TestDeduplicationBehavior(t *testing.T) { } } -func TestDroppedLoggingOnlyWhenNonZero(t *testing.T) { - origAttrDropped := logAttrDropped - t.Cleanup(func() { logAttrDropped = origAttrDropped }) - - testCases := []struct { - name string - operation func(*Record) - wantCalled bool - wantCount int - }{ - { - name: "setDropped(0) does not log", - operation: func(r *Record) { r.setDropped(0) }, - wantCalled: false, - wantCount: 0, - }, - { - name: "setDropped(n>0) logs", - operation: func(r *Record) { r.setDropped(5) }, - wantCalled: true, - wantCount: 5, - }, - { - name: "addDropped(0) does not log", - operation: func(r *Record) { r.addDropped(0) }, - wantCalled: false, - wantCount: 0, - }, - { - name: "addDropped(n>0) logs", - operation: func(r *Record) { r.addDropped(3) }, - wantCalled: true, - wantCount: 3, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - called := false - logAttrDropped = sync.OnceFunc(func() { called = true }) - - r := &Record{} - tc.operation(r) - - assert.Equal(t, tc.wantCalled, called) - assert.Equal(t, tc.wantCount, r.DroppedAttributes()) - }) - } -} - func TestApplyAttrLimitsTruncation(t *testing.T) { testcases := []struct { name string From 8addc853dd73b9b6c4b5025486c0b260364794b4 Mon Sep 17 00:00:00 2001 From: Rodrigo Mecheri Date: Thu, 8 Jan 2026 23:30:55 +0100 Subject: [PATCH 12/14] fix linter --- sdk/log/record_test.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index f98551c7760..18898270337 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -1113,7 +1113,12 @@ func TestDeduplicationBehavior(t *testing.T) { { name: "Both duplicates and limit", attributeCountLimit: 2, - attrs: []log.KeyValue{log.String("a", "v1"), log.String("a", "v2"), log.String("b", "v3"), log.String("c", "v4")}, + attrs: []log.KeyValue{ + log.String("a", "v1"), + log.String("a", "v2"), + log.String("b", "v3"), + log.String("c", "v4"), + }, wantKeyValueDropped: true, wantAttrDropped: true, wantDroppedCount: 1, // Only limit drops count @@ -1128,8 +1133,10 @@ func TestDeduplicationBehavior(t *testing.T) { wantAttributeCount: 2, }, { - name: "Nested map duplicates", - attrs: []log.KeyValue{log.Map("outer", log.String("nested", "v1"), log.String("nested", "v2"))}, + name: "Nested map duplicates", + attrs: []log.KeyValue{ + log.Map("outer", log.String("nested", "v1"), log.String("nested", "v2")), + }, wantKeyValueDropped: true, wantDroppedCount: 0, wantAttributeCount: 1, From 3b3fe59215bcbb7b21063ce0c2acac3a0156efd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 9 Jan 2026 11:29:06 +0100 Subject: [PATCH 13/14] Apply suggestions from code review --- sdk/log/record_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index 18898270337..ec3107c719d 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -553,21 +553,20 @@ func TestRecordDroppedAttributes(t *testing.T) { attrs := make([]log.KeyValue, i) attrs[0] = log.Bool("only key different then the rest", true) - assert.False(t, called, "non-dropped attributed logged") r.AddAttributes(attrs...) + // Deduplication doesn't count as dropped. wantDropped := 0 if i > 1 { wantDropped = 1 } assert.Equalf(t, wantDropped, r.DroppedAttributes(), "%d: AddAttributes", i) - if i > 1 { - assert.True(t, called, "dropped attributes not logged") + if i <= 1 { + assert.False(t, called, "%d: dropped attributes logged", i) + } else { + assert.True(t, called, "%d: dropped attributes not logged", i) } - called = false - logAttrDropped = func() { called = true } - r.AddAttributes(attrs...) wantDropped = 0 if i > 1 { From 2fcb73675f53ab007d3f1be3b9521f78d5b11413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Paj=C4=85k?= Date: Fri, 9 Jan 2026 11:37:53 +0100 Subject: [PATCH 14/14] Update CHANGELOG.md --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a418ac0d91e..2f2fb2432a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Fixed - Fix bad log message when key-value pairs are dropped because of key duplication in `go.opentelemetry.io/otel/sdk/log`. (#7662) -- Fix `DroppedAttributes` in `go.opentelemetry.io/otel/sdk/log` to not count the non-attribute key-value pairs dropped because of key duplication. (#7662) -- Fix `SetAttributes` in `go.opentelemetry.io/otel/sdk/log` to not log that attributes are dropped when they are actually not dropped. (#7662) +- Fix `DroppedAttributes` on `Record` in `go.opentelemetry.io/otel/sdk/log` to not count the non-attribute key-value pairs dropped because of key duplication. (#7662) +- Fix `SetAttributes` on `Record` in `go.opentelemetry.io/otel/sdk/log` to not log that attributes are dropped when they are actually not dropped. (#7662) ### Changed