Skip to content

otelslog: fix shared kvBuffer append corrupting log attributes - #9229

Merged
MrAlias merged 3 commits into
open-telemetry:mainfrom
somaz94:9046
Jul 24, 2026
Merged

MrAlias merged 3 commits into
open-telemetry:mainfrom
somaz94:9046

Conversation

@somaz94

@somaz94 somaz94 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

kvBuffer.KeyValues returned append(b.data, kvs...). When cap(b.data) > len(b.data) this writes kvs into b.data's spare capacity and returns a slice aliasing that backing array. The buffer is shared across concurrent Handle calls (a Handler's group attributes, bridges/otelslog/handler.go:382), so two records append into the same backing array at the same time, which both races and corrupts each other's log attributes.

The fix clips b.data to its length so the append allocates a new backing array instead of writing into the shared spare capacity. This mirrors the existing slices.Clone use in Clone().

Validation (local, Go 1.26):

  • go test -race ./... passes in bridges/otelslog.
  • Both new tests fail on the pre-fix code: TestKVBufferKeyValuesDoesNotAliasBackingArray (deterministic) fails, and TestKVBufferKeyValuesConcurrent reports a DATA RACE under -race.
  • go vet, gofmt, and golangci-lint v2 are all clean.

Allocation note: the previously-buggy case where kvs fit in b.data's spare capacity now allocates a new backing array (needed for correctness); the empty-kvs path stays zero-alloc via slices.Clip.

related: #9046

@somaz94
somaz94 force-pushed the 9046 branch 2 times, most recently from e599fe0 to 4cbc95b Compare July 14, 2026 07:39
@somaz94
somaz94 marked this pull request as ready for review July 21, 2026 07:19
@somaz94
somaz94 requested review from a team, MrAlias and pellared as code owners July 21, 2026 07:19
@codecov

codecov Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.0%. Comparing base (32be80e) to head (8a58f11).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #9229     +/-   ##
=======================================
+ Coverage   83.8%   84.0%   +0.1%     
=======================================
  Files        198     198             
  Lines      16328   16328             
=======================================
+ Hits       13695   13717     +22     
+ Misses      2157    2135     -22     
  Partials     476     476             
Files with missing lines Coverage Δ
bridges/otelslog/handler.go 97.1% <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.

@MrAlias
MrAlias merged commit 659e6cb into open-telemetry:main Jul 24, 2026
30 checks passed
@MrAlias MrAlias added this to the v1.45.0 milestone Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants