fix(sdk): deduplicate LogRecord attribute keys

[antcybersec]

Summary

• Deduplicate SdkLogRecord attribute keys in add_attribute using last-write-wins semantics.
• Add GrowableArray::get_mut to update existing attribute slots without extra allocations.
• Add unit tests covering inline capacity, batch add_attributes, and overflow paths.

Fixes #3497.

Rationale

The OpenTelemetry spec requires exported attribute collections to contain only unique keys by default. The SDK previously used Vec::push, so duplicate keys could reach exporters (e.g. from tracing events, span-attribute enrichment, or custom appenders).

Per maintainer feedback on #3497, deduplication is eager at add_attribute time (default-on, no builder flag).

Test plan

• cargo test -p opentelemetry-sdk --features testing,rt-tokio --lib logs::record::tests
• cargo test -p opentelemetry-sdk --features testing,rt-tokio --lib logs::
• cargo test -p opentelemetry-appender-tracing --lib
• cargo clippy -p opentelemetry-sdk --lib -- -Dwarnings

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: antcybersec / name: Anant Kumar (309e4c6, ab5f9c4)

[codecov]

Codecov Report

❌ Patch coverage is 97.22222% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.9%. Comparing base (2571776) to head (c04e507).
⚠️ Report is 17 commits behind head on main.

Files with missing lines	Patch %	Lines
opentelemetry-sdk/src/growable_array.rs	93.7%	1 Missing ⚠️
opentelemetry-sdk/src/logs/logger_provider.rs	94.4%	1 Missing ⚠️
opentelemetry-sdk/src/logs/record.rs	98.1%	1 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #3537    +/-   ##
======================================
  Coverage   82.9%   82.9%            
======================================
  Files        130     130            
  Lines      27350   27456   +106     
======================================
+ Hits       22675   22778   +103     
- Misses      4675    4678     +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

[cijothomas]

can you add benchmarks and show before/after, so we can gauge the perf impact easily.
also, we need an opt-out of this behavior flag for users who'd rather not pay the perf penalty (and retain current behavior).

[antcybersec]

@cijothomas — addressed both review points in the latest commits:

Opt-out flag

LoggerProviderBuilder::with_log_record_attribute_deduplication(false) disables dedup and restores the previous push-only behavior. Default remains true (spec-compliant).

let provider = SdkLoggerProvider::builder()
    .with_log_record_attribute_deduplication(false)
    .build();

Benchmarks (before / after)

Run: cargo bench --bench log_record_attribute_dedup --features logs

Results from local criterion run (see also opentelemetry-sdk/benches/log_record_attribute_dedup.rs header):

Scenario	Dedup ON	Dedup OFF (before)	Overhead
1 unique attribute	27.2 ns	24.7 ns	~1.1x
5 unique attributes	135.5 ns	119.3 ns	~1.1x
9 unique attributes	301.2 ns	220.8 ns	~1.4x
5 writes, same key	31.9 ns	32.1 ns	~1.0x

Takeaway: For the typical case (≤5 unique keys, which matches the SDK's inline capacity of 5), overhead is small (~10%). It grows at larger attribute counts (9 attrs ~1.4x), which is the main case where users may want the opt-out.

Criterion output screenshot attached below for reference.

[cijothomas]

if you don't mind could you run this few teams and see if this holds. I cloned an ran once and numbers are telling much more regression. (It does not mean we cannot take the PR - just want to quantify the impact)

[cijothomas]

This is technically a regression, so need a bigger warning so users bumping version will also disable deduplication to get back previous behavior.
@open-telemetry/rust-maintainers Please weigh in to this change - I am okay to accept the regression and be spec-compliant. The opt-out is easy for end users who want to retain original performance.

[cijothomas]

@scottgerring PTAL.

[lalitb]

looks good to me with updated changelog.

EDIT - Given the spec requires exported attribute collections to be unique, did we consider enforcing this closer to export/serialization instead, so the breaking behavior and cost are narrower? I understand SDK-level dedup gives processors/exporters a consistent view, but I want us to be explicit about accepting that GA-wide behavior/perf tradeoff.

[antcybersec]

Hi @cijothomas, ran the benchmarks 3 independent times on Apple M4 Max (macOS 25.3.0) and took the median — numbers are very stable across runs:

Unique keys (typical case — no duplicate writes)

Scenario	Run 1 (dedup ON / OFF)	Run 2 (dedup ON / OFF)	Run 3 (dedup ON / OFF)	Median ON	Median OFF	Overhead
1 attribute	25.5 / 24.9 ns	25.5 / 25.7 ns	25.3 / 25.5 ns	25.3 ns	25.5 ns	~1.0x
5 attributes	128.7 / 120.1 ns	130.9 / 128.2 ns	131.5 / 120.5 ns	130.7 ns	122.9 ns	~1.06x
9 attributes	302.3 / 221.9 ns	293.1 / 219.3 ns	295.5 / 219.7 ns	296.9 ns	220.3 ns	~1.35x

Repeated key (5 writes to the same key)

Run	Dedup ON	Dedup OFF
Run 1	31.9 ns	32.1 ns
Run 2	32.0 ns	32.1 ns
Run 3	32.1 ns	32.0 ns

---

Summary:

• 1 attribute: effectively no overhead (~1.0x) — the common case for most structured log lines
• 5 attributes: ~6% overhead — still negligible in practice
• 9 attributes (worst-case for inline storage, all unique): ~35% overhead — this is the honest number

The 9-attribute case does show meaningful regression when all 9 keys are unique (the linear scan grows). This is exactly the scenario the opt-out flag exists for: users with high-cardinality attribute sets who emit guaranteed-unique keys per record can call .with_log_record_attribute_deduplication(false) on their LoggerProviderBuilder and pay zero overhead.

The benchmark file header has been updated in the latest commit to reflect the median-of-3 numbers. Happy to adjust the benchmark configuration (longer measurement time, more samples) if you'd like even tighter confidence intervals.

[antcybersec]

Good call @cijothomas — updated the CHANGELOG.md entry to make the breaking behavioral nature explicit with a Migration section so users bumping the version know exactly how to opt out.

The new entry reads:

---

Breaking behavioral change: SdkLogRecord::add_attribute now deduplicates attribute keys by default (last-write-wins), so exported log records conform to the OpenTelemetry specification requirement that attributes form a map of unique keys. This changes observable output for any code that previously called add_attribute with the same key more than once.

Migration: If you relied on the previous push-only behavior (e.g. for performance reasons or because downstream consumers tolerated duplicate keys), opt out via the provider builder:

let provider = SdkLoggerProvider::builder()
    .with_log_record_attribute_deduplication(false)
    .build();

---

And agree with your framing — spec-compliance is the right default, the opt-out is a one-liner for the rare case where it matters, and the benchmark numbers show the overhead is only meaningful at 9+ unique attributes per record.

[cijothomas]

@antcybersec I have triggered a perf run so we'll get to see the full set of current numbers and their regression. This is the first time since we added that infra, so need some time to see how to evaluate the results!

(Irrespective, I think you have good support for this direction - please wait couple days to let other maintaienrs/approvers also to chime in)

[github-actions]

Thank you for your contribution! This PR has been automatically marked as stale because it has not had activity in the last 14 days. This may be due to a delay in review on our side or awaiting a response from you; either is fine, and we appreciate your patience.

It will be closed in 14 days if no further activity occurs. Pushing a new commit or leaving a comment will remove the stale label and keep the PR open.