sdk/log: Add WithAllowKeyDuplication logger provider option #6968

Mojachieee · 2025-07-07T11:04:13Z

This couldn't be added as an option on a processor, as that would involve moving all the attribute deduplication. logic outside of the record type. Instead this PR provides the same functionality but it is set when creating the log provider

The below benchstat report shows the performance improvement when allowDupKeys is set

goos: darwin
goarch: arm64
pkg: go.opentelemetry.io/otel/sdk/log
cpu: Apple M2 Pro
                                  │ withoutDedup.txt │            withDedup.txt            │
                                  │      sec/op      │   sec/op     vs base                │
SetAddAttributes/SetAttributes-12        141.3n ± 2%   167.4n ± 5%  +18.51% (p=0.000 n=10)
SetAddAttributes/AddAttributes-12        117.5n ± 2%   124.8n ± 5%   +6.17% (p=0.000 n=10)
geomean                                  128.9n        144.5n       +12.17%

                                  │ withoutDedup.txt │            withDedup.txt            │
                                  │       B/op       │    B/op     vs base                 │
SetAddAttributes/SetAttributes-12       48.00 ± 0%     48.00 ± 0%       ~ (p=1.000 n=10) ¹
SetAddAttributes/AddAttributes-12       0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                            ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                  │ withoutDedup.txt │            withDedup.txt            │
                                  │    allocs/op     │ allocs/op   vs base                 │
SetAddAttributes/SetAttributes-12       1.000 ± 0%     1.000 ± 0%       ~ (p=1.000 n=10) ¹
SetAddAttributes/AddAttributes-12       0.000 ± 0%     0.000 ± 0%       ~ (p=1.000 n=10) ¹
geomean                                            ²               +0.00%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

codecov · 2025-07-07T11:06:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.7%. Comparing base (e4c84b9) to head (62ad9ea).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #6968     +/-   ##
=======================================
- Coverage   82.8%   82.7%   -0.1%     
=======================================
  Files        262     262             
  Lines      24325   24339     +14     
=======================================
+ Hits       20142   20152     +10     
- Misses      3808    3812      +4     
  Partials     375     375

Files with missing lines	Coverage Δ
sdk/log/logger.go	`100.0% <100.0%> (ø)`
sdk/log/provider.go	`97.2% <100.0%> (+0.1%)`	⬆️
sdk/log/record.go	`100.0% <100.0%> (ø)`

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

sdk/log/record_test.go

pellared

This looks really nice.

Can you add benchmark results to the PR description showcasing that the performance is improved even for a "happy-path" scenario when there are no duplicate keys. I hope the existing benchmarks would capture this. Use the output of https://pkg.go.dev/golang.org/x/perf/cmd/benchstat when showcasing the diffs.

sdk/log/record.go

sdk/log/provider.go

sdk/log/record_test.go

pellared

I will look deeper in the following days.

CHANGELOG.md

sdk/log/provider.go

Co-authored-by: Robert Pająk <[email protected]>

pellared

Overall LGTM. Just a few last comments.

sdk/log/logger_test.go

sdk/log/provider.go

Co-authored-by: Robert Pająk <[email protected]>

sdk/log/provider.go

pellared

LGTM (just 2 nit comments left)

Co-authored-by: Robert Pająk <[email protected]>

sdk/log/provider.go

Co-authored-by: Damien Mathieu <[email protected]>

sdk/log/provider.go

pellared · 2025-07-11T07:54:55Z

@Mojachieee, thanks for your contribution! Looking forward for your future contributions.

This release is the last to support [Go 1.23]. The next release will require at least [Go 1.24]. ### Added - Add native histogram exemplar support in `go.opentelemetry.io/otel/exporters/prometheus`. (#6772) - Add template attribute functions to the `go.opentelmetry.io/otel/semconv/v1.34.0` package. (#6939) - `ContainerLabel` - `DBOperationParameter` - `DBSystemParameter` - `HTTPRequestHeader` - `HTTPResponseHeader` - `K8SCronJobAnnotation` - `K8SCronJobLabel` - `K8SDaemonSetAnnotation` - `K8SDaemonSetLabel` - `K8SDeploymentAnnotation` - `K8SDeploymentLabel` - `K8SJobAnnotation` - `K8SJobLabel` - `K8SNamespaceAnnotation` - `K8SNamespaceLabel` - `K8SNodeAnnotation` - `K8SNodeLabel` - `K8SPodAnnotation` - `K8SPodLabel` - `K8SReplicaSetAnnotation` - `K8SReplicaSetLabel` - `K8SStatefulSetAnnotation` - `K8SStatefulSetLabel` - `ProcessEnvironmentVariable` - `RPCConnectRPCRequestMetadata` - `RPCConnectRPCResponseMetadata` - `RPCGRPCRequestMetadata` - `RPCGRPCResponseMetadata` - Add `ErrorType` attribute helper function to the `go.opentelmetry.io/otel/semconv/v1.34.0` package. (#6962) - Add `WithAllowKeyDuplication` in `go.opentelemetry.io/otel/sdk/log` which can be used to disable deduplication for log records. (#6968) - Add `WithCardinalityLimit` option to configure the cardinality limit in `go.opentelemetry.io/otel/sdk/metric`. (#6996, #7065, #7081, #7164, #7165, #7179) - Add `Clone` method to `Record` in `go.opentelemetry.io/otel/log` that returns a copy of the record with no shared state. (#7001) - Add experimental self-observability span and batch span processor metrics in `go.opentelemetry.io/otel/sdk/trace`. Check the `go.opentelemetry.io/otel/sdk/trace/internal/x` package documentation for more information. (#7027, #6393, #7209) - The `go.opentelemetry.io/otel/semconv/v1.36.0` package. The package contains semantic conventions from the `v1.36.0` version of the OpenTelemetry Semantic Conventions. See the [migration documentation](./semconv/v1.36.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.34.0.`(#7032, #7041) - Add support for configuring Prometheus name translation using `WithTranslationStrategy` option in `go.opentelemetry.io/otel/exporters/prometheus`. The current default translation strategy when UTF-8 mode is enabled is `NoUTF8EscapingWithSuffixes`, but a future release will change the default strategy to `UnderscoreEscapingWithSuffixes` for compliance with the specification. (#7111) - Add experimental self-observability log metrics in `go.opentelemetry.io/otel/sdk/log`. Check the `go.opentelemetry.io/otel/sdk/log/internal/x` package documentation for more information. (#7121) - Add experimental self-observability trace exporter metrics in `go.opentelemetry.io/otel/exporters/stdout/stdouttrace`. Check the `go.opentelemetry.io/otel/exporters/stdout/stdouttrace/internal/x` package documentation for more information. (#7133) - Support testing of [Go 1.25]. (#7187) - The `go.opentelemetry.io/otel/semconv/v1.37.0` package. The package contains semantic conventions from the `v1.37.0` version of the OpenTelemetry Semantic Conventions. See the [migration documentation](./semconv/v1.37.0/MIGRATION.md) for information on how to upgrade from `go.opentelemetry.io/otel/semconv/v1.36.0.`(#7254) ### Changed - Optimize `TraceIDFromHex` and `SpanIDFromHex` in `go.opentelemetry.io/otel/sdk/trace`. (#6791) - Change `AssertEqual` in `go.opentelemetry.io/otel/log/logtest` to accept `TestingT` in order to support benchmarks and fuzz tests. (#6908) - Change `DefaultExemplarReservoirProviderSelector` in `go.opentelemetry.io/otel/sdk/metric` to use `runtime.GOMAXPROCS(0)` instead of `runtime.NumCPU()` for the `FixedSizeReservoirProvider` default size. (#7094) ### Fixed - `SetBody` method of `Record` in `go.opentelemetry.io/otel/sdk/log` now deduplicates key-value collections (`log.Value` of `log.KindMap` from `go.opentelemetry.io/otel/log`). (#7002) - Fix `go.opentelemetry.io/otel/exporters/prometheus` to not append a suffix if it's already present in metric name. (#7088) - Fix the `go.opentelemetry.io/otel/exporters/stdout/stdouttrace` self-observability component type and name. (#7195) - Fix partial export count metric in `go.opentelemetry.io/otel/exporters/stdout/stdouttrace`. (#7199) ### Deprecated - Deprecate `WithoutUnits` and `WithoutCounterSuffixes` options, preferring `WithTranslationStrategy` instead. (#7111) - Deprecate support for `OTEL_GO_X_CARDINALITY_LIMIT` environment variable in `go.opentelemetry.io/otel/sdk/metric`. Use `WithCardinalityLimit` option instead. (#7166)

Mojachieee added 7 commits July 7, 2025 11:56

introduce flag on log record to stop dedupes

eda672e

add some tests for allowDupAttrs

9ba97c2

add a test that specifically covers duplicate entries in a map

a676514

add benchmarks for AddAttributes with allowed duplicates

d38f4b5

pipework to set WithoutAttributeDeduplication and pass through to record

dfb14a2

remove todo

8154388

lint

be0ae83

Mojachieee requested review from MrAlias, XSAM, dashpole, dmathieu and pellared as code owners July 7, 2025 11:04

update changelog

6e42194

Mojachieee commented Jul 7, 2025

View reviewed changes

sdk/log/record_test.go Show resolved Hide resolved

pellared reviewed Jul 7, 2025

View reviewed changes

sdk/log/record.go Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

sdk/log/record_test.go Show resolved Hide resolved

Mojachieee added 4 commits July 7, 2025 12:59

rename field

5fcd513

allow duplicates in log.KindMap entries

0803dad

fix dedup benchmark bug

61f4060

rename LoggerProviderOption, update GoDoc

49dea12

pellared reviewed Jul 7, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

pellared changed the title ~~sdk/log: Add WithoutAttributeDeduplication to log provider~~ sdk/log: Add AllowKeyDuplication logger provider option Jul 7, 2025

Mojachieee and others added 2 commits July 9, 2025 08:26

Apply suggestions from code review

74d71fe

Co-authored-by: Robert Pająk <[email protected]>

Merge branch 'main' into logs-without-dedup

bff7659

pellared reviewed Jul 9, 2025

View reviewed changes

sdk/log/logger_test.go Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

sdk/log/provider.go Outdated Show resolved Hide resolved

pellared mentioned this pull request Jul 9, 2025

sdk/log: Deduplicate key-value collections in Record.SetBody #6982

Closed

Mojachieee and others added 3 commits July 9, 2025 09:29

Apply suggestions from code review

e20be6f

Co-authored-by: Robert Pająk <[email protected]>

rename keysAllowDuplicates -> allowDupKeys

8558e3c

revert use of r.Clone()

5d73e94

pellared assigned Mojachieee Jul 9, 2025

pellared added this to Go: Logs (Post-GA) Jul 9, 2025

pellared moved this to In progress in Go: Logs (Post-GA) Jul 9, 2025

pellared reviewed Jul 9, 2025

View reviewed changes

sdk/log/provider.go Outdated Show resolved Hide resolved

pellared approved these changes Jul 9, 2025

View reviewed changes

Update AllowKeyDuplication comment

3b270bc

Co-authored-by: Robert Pająk <[email protected]>

pellared added pkg:SDK Related to an SDK package area:logs Part of OpenTelemetry logs labels Jul 9, 2025

pellared mentioned this pull request Jul 9, 2025

log: Add Record.Clone #6986

Closed

dmathieu approved these changes Jul 9, 2025

View reviewed changes

sdk/log/provider.go Outdated Show resolved Hide resolved

Update AllowKeyDuplication comment

b22e822

Co-authored-by: Damien Mathieu <[email protected]>

MrAlias reviewed Jul 9, 2025

View reviewed changes

sdk/log/provider.go Outdated Show resolved Hide resolved

update function name

0b7b350

Mojachieee changed the title ~~sdk/log: Add AllowKeyDuplication logger provider option~~ sdk/log: Add WithAllowKeyDuplication logger provider option Jul 10, 2025

pellared requested a review from MrAlias July 10, 2025 10:43

Merge branch 'main' into logs-without-dedup

62ad9ea

pellared merged commit 5da6cd2 into open-telemetry:main Jul 11, 2025
33 checks passed

github-project-automation bot moved this from In progress to Done in Go: Logs (Post-GA) Jul 11, 2025

Mojachieee deleted the logs-without-dedup branch July 11, 2025 08:53

pellared mentioned this pull request Jul 14, 2025

sdk/log: Deduplicate key-value collections in Record.SetBody #7002

Merged

MrAlias added this to the v1.38.0 milestone Jul 17, 2025

MrAlias mentioned this pull request Aug 29, 2025

Release v1.38.0 #7271

Merged

sdk/log: Add WithAllowKeyDuplication logger provider option #6968

sdk/log: Add WithAllowKeyDuplication logger provider option #6968

Uh oh!

Conversation

Mojachieee commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pellared left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pellared commented Jul 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Mojachieee commented Jul 7, 2025 •

edited

Loading

codecov bot commented Jul 7, 2025 •

edited

Loading