[OpenTelemetry] Fix LogRecord returned to pool twice by martincostello · Pull Request #7965 · open-telemetry/opentelemetry-dotnet · GitHub
Skip to content

[OpenTelemetry] Fix LogRecord returned to pool twice - #7965

Merged
martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:ANT-2026-9R51ZCPL
Oct 16, 2026
Merged

martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:ANT-2026-9R51ZCPL

Conversation

@martincostello

@martincostello martincostello commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Discovered by Anthropic's OSS Scanner, as vulnerability ANT-2026-9R51ZCPL.

Changes

Fix LogRecord instances being returned to the shared pool while they are still queued for export.

Batch<T>.Enumerator.Dispose() returned the current LogRecord to LogRecordSharedPool for every kind of Batch<LogRecord>, but only batches read from the circular buffer of a BatchLogRecordExportProcessor own a pool reference to their records. When an exporter behind a SimpleLogRecordExportProcessor stopped enumerating its single-item batch early (for example by returning ExportResult.Failure from inside the foreach), the logger's only reference was released. The record was then cleared and reused while a BatchLogRecordExportProcessor on the same provider still had it queued, so the batch exporter could export records with missing or mixed-up attributes.

The enumerator now only returns the current record to the pool when the batch was read from a circular buffer, which matches the guard Batch<T>.Dispose() already uses.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, successful build/tests, etc.)
  • Tests added/updated
  • CHANGELOG.md file(s) updated for non-trivial changes
  • Changes in public API reviewed (if applicable)
  • AI usage disclosure:
    • No AI used
    • AI-assisted
    • Bulk AI-generated
  • I have the experience and knowledge necessary to understand, review, and validate all content in this PR1

Note

The changes in this PR and its description were generated with Claude Code (Claude Opus 5.5).

Footnotes

  1. Yes, I can answer maintainer questions about the content of this PR without using AI. ↩

Only return the current `LogRecord` to the shared pool when a batch enumerator is disposed early if the batch owns a reference to it.

Assisted-by: Claude Code (Claude Opus 5.5) <noreply@anthropic.com>

Discovered by Anthropic's OSS Scanner, as vulnerability ANT-2026-9R51ZCPL.
@github-actions github-actions Bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Oct 16, 2026
Add PR number.

Assisted-by: Claude Code (Claude Opus 5.5) <noreply@anthropic.com>

Discovered by Anthropic's OSS Scanner, as vulnerability ANT-2026-9R51ZCPL.
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The narrow ownership guard and targeted regression tests address the reported failure with no unresolved findings.

0 open findings

What changed in this PR

Fixes premature LogRecord pool returns that could corrupt data queued for batch export.

Changes:

  • Limits enumerator pool cleanup to circular-buffer batches.
  • Adds ownership and mixed-processor regression tests.
  • Documents the fix in the SDK changelog.

[!NOTE]
This review was generated with GitHub Copilot.

File Description
test/​OpenTelemetry.Tests/​Trace/​BatchTests.cs Tests caller-owned and batch-owned record cleanup.
test/​OpenTelemetry.Tests/​Logs/​BatchLogRecordExportProcessorTests.cs Tests early exporter exit with both processor orders.
src/​OpenTelemetry/​CHANGELOG.md Documents the pooling fix.
src/​OpenTelemetry/​Batch.cs Restricts pool returns to circular-buffer batches.

🧠 Review effort: Balanced


💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@martincostello
martincostello marked this pull request as ready for review October 16, 2026 16:43
@martincostello
martincostello requested a review from a team as a code owner October 16, 2026 16:43
@codecov

codecov Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.39%. Comparing base (407091f) to head (361dcb5).
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7965      +/-   ##
==========================================
+ Coverage   92.37%   92.39%   +0.01%     
==========================================
  Files         365      365              
  Lines       21185    21185              
==========================================
+ Hits        19569    19573       +4     
+ Misses       1616     1612       -4     
Flag Coverage Δ
unittests-Project-Experimental 92.51% <100.00%> (+0.01%) ⬆️
unittests-Project-Stable 92.51% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/OpenTelemetry/Batch.cs 99.22% <100.00%> (+3.10%) ⬆️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants