From 4fc7162e89366706f781ac3cb1221a50670dc158 Mon Sep 17 00:00:00 2001 From: Anton Manakin <45166364+amanakin@users.noreply.github.com> Date: Tue, 4 Jun 2024 10:18:14 +0300 Subject: [PATCH] sdk/log: Fix counting number of dropped attributes of log.Record (#5464) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applying attribute limits in `Record` uses value receiver. But it should add count of dropped attrs. In this PR I add using of pointer receiver. Also it's slightly faster with pointer receiver: ``` goos: darwin goarch: arm64 pkg: go.opentelemetry.io/otel/sdk/log │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ SetAddAttributes-10 175.7n ± 3% 159.8n ± 4% -9.08% (p=0.000 n=10) │ old.txt │ new.txt │ │ B/op │ B/op vs base │ SetAddAttributes-10 48.00 ± 0% 48.00 ± 0% ~ (p=1.000 n=10) ¹ ¹ all samples are equal │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ SetAddAttributes-10 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ ¹ all samples are equal ``` --- CHANGELOG.md | 1 + sdk/log/record.go | 6 ++---- sdk/log/record_test.go | 22 +++++++++++++++++++--- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eb1b86f31ed..9021fa522d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Log a warning to the OpenTelemetry internal logger when a `Span` in `go.opentelemetry.io/otel/sdk/trace` drops an attribute, event, or link due to a limit being reached. (#5434) - Document instrument name requirements in `go.opentelemetry.io/otel/metric`. (#5435) - Prevent random number generation data-race for experimental rand exemplars in `go.opentelemetry.io/otel/sdk/metric`. (#5456) +- Fix counting number of dropped attributes of `Record` in `go.opentelemetry.io/otel/sdk/log`. (#5464) ## [1.27.0/0.49.0/0.3.0] 2024-05-21 diff --git a/sdk/log/record.go b/sdk/log/record.go index 9f8c63d2cd3..1c413fa6e24 100644 --- a/sdk/log/record.go +++ b/sdk/log/record.go @@ -275,8 +275,6 @@ func (r *Record) addAttrs(attrs []log.KeyValue) { // SetAttributes sets (and overrides) attributes to the log record. func (r *Record) SetAttributes(attrs ...log.KeyValue) { - // TODO: apply truncation to string and []string values. - // TODO: deduplicate map values. var drop int attrs, drop = dedup(attrs) r.setDropped(drop) @@ -391,12 +389,12 @@ func (r *Record) Clone() Record { return res } -func (r Record) applyAttrLimits(attr log.KeyValue) log.KeyValue { +func (r *Record) applyAttrLimits(attr log.KeyValue) log.KeyValue { attr.Value = r.applyValueLimits(attr.Value) return attr } -func (r Record) applyValueLimits(val log.Value) log.Value { +func (r *Record) applyValueLimits(val log.Value) log.Value { switch val.Kind() { case log.KindString: s := val.AsString() diff --git a/sdk/log/record_test.go b/sdk/log/record_test.go index f76634496dc..cbbd8942420 100644 --- a/sdk/log/record_test.go +++ b/sdk/log/record_test.go @@ -346,9 +346,10 @@ func TestRecordAttrDeduplication(t *testing.T) { func TestApplyAttrLimitsDeduplication(t *testing.T) { testcases := []struct { - name string - limit int - input, want log.Value + name string + limit int + input, want log.Value + wantDroppedAttrs int }{ { // No de-duplication @@ -419,6 +420,7 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { log.String("g", "GG"), log.String("h", "H"), ), + wantDroppedAttrs: 10, }, } @@ -431,11 +433,13 @@ func TestApplyAttrLimitsDeduplication(t *testing.T) { t.Run("AddAttributes", func(t *testing.T) { r.AddAttributes(kv) assertKV(t, r, log.KeyValue{Key: key, Value: tc.want}) + assert.Equal(t, tc.wantDroppedAttrs, r.DroppedAttributes()) }) t.Run("SetAttributes", func(t *testing.T) { r.SetAttributes(kv) assertKV(t, r, log.KeyValue{Key: key, Value: tc.want}) + assert.Equal(t, tc.wantDroppedAttrs, r.DroppedAttributes()) }) }) } @@ -635,3 +639,15 @@ func TestTruncate(t *testing.T) { }) } } + +func BenchmarkSetAddAttributes(b *testing.B) { + kv := log.String("key", "value") + records := make([]Record, b.N) + + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + records[i].SetAttributes(kv) + records[i].AddAttributes(kv) + } +}