Skip to content

[OpenTelemetry] Fixed dropped self-diagnostics - #7543

Merged
martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:fixed-dropped-logs
Jul 27, 2026
Merged

martincostello merged 3 commits into
open-telemetry:mainfrom
martincostello:fixed-dropped-logs

Conversation

@martincostello

Copy link
Copy Markdown
Member

Changes

Fix self-diagnostic logs being dropped if they contained too many 3-byte characters. Now the content is only truncated.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

Fix self-diagnostic logs being dropped if they contained too many 3-byte characters.
Comment thread src/OpenTelemetry/CHANGELOG.md Outdated
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.27%. Comparing base (f101632) to head (f1abd10).
⚠️ Report is 14 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 95.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #7543      +/-   ##
==========================================
+ Coverage   90.23%   90.27%   +0.04%     
==========================================
  Files         290      290              
  Lines       15969    15989      +20     
==========================================
+ Hits        14409    14434      +25     
+ Misses       1560     1555       -5     
Flag Coverage Δ
unittests-Project-Experimental 90.21% <95.00%> (-0.03%) ⬇️
unittests-Project-Stable 90.25% <95.00%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
...Telemetry/Internal/SelfDiagnosticsEventListener.cs 94.77% <95.00%> (+0.03%) ⬆️

... and 4 files with indirect coverage changes

@github-actions github-actions Bot added the pkg:OpenTelemetry Issues related to OpenTelemetry NuGet package label Jul 21, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 21, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-27 21:51:58 UTC.

  • Status: Merged.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected.

@martincostello
martincostello marked this pull request as ready for review July 21, 2026 15:50
@martincostello
martincostello requested a review from a team as a code owner July 21, 2026 15:50
Copilot AI review requested due to automatic review settings July 21, 2026 15:50

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.

Pull request overview

This PR fixes an edge case in OpenTelemetry’s self-diagnostics logging where UTF-8 byte-size estimation could overflow the internal buffer when messages/parameters contain enough 3-byte UTF-8 characters, causing Encoding.UTF8.GetBytes to throw and the entire log line to be dropped. The change adds a UTF-8 byte-counting walk to safely truncate content and adds regression tests plus a CHANGELOG entry.

Changes:

  • Adjusted SelfDiagnosticsEventListener.EncodeInBuffer to compute how many UTF-16 code units actually fit in the remaining buffer based on UTF-8 byte length, preventing exceptions and log-line drops.
  • Added unit tests covering truncation behavior for 3-byte UTF-8 characters (both message and parameter cases).
  • Added a CHANGELOG entry describing the behavior fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/OpenTelemetry/Internal/SelfDiagnosticsEventListener.cs Adds UTF-8-aware truncation logic to avoid buffer-overflow exceptions that previously dropped self-diagnostic log lines.
test/OpenTelemetry.Tests/Internal/SelfDiagnosticsEventListenerTests.cs Adds regression tests validating truncation for 3-byte UTF-8 characters and keeps source ASCII-only via escape sequence.
src/OpenTelemetry/CHANGELOG.md Documents the self-diagnostics truncation fix in the Unreleased section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/OpenTelemetry/Internal/SelfDiagnosticsEventListener.cs
Fix handling of low/high surrogates.
@martincostello
martincostello enabled auto-merge July 21, 2026 16:32
@martincostello martincostello added the keep-open Prevents issues and pull requests being closed as stale label Jul 22, 2026

@stevejgordon stevejgordon 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.

Looks good as proposed

@xiang17 xiang17 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.

Looks good. This rare corner case has been causing silent drops that's hard to find.

var endPos = SelfDiagnosticsEventListener.EncodeInBuffer(str, false, buffer, 0);

var written = Encoding.UTF8.GetString(buffer, 0, endPos);
Assert.StartsWith(ThreeByteChar.ToString(), written, StringComparison.Ordinal);

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.

nit: would it be better to verify the exact number of ThreeByteChars?

@martincostello
martincostello added this pull request to the merge queue Jul 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 27, 2026
@martincostello
martincostello added this pull request to the merge queue Jul 27, 2026
Merged via the queue into open-telemetry:main with commit 008e176 Jul 27, 2026
75 checks passed
@martincostello
martincostello deleted the fixed-dropped-logs branch July 27, 2026 21:51
@martincostello martincostello removed the keep-open Prevents issues and pull requests being closed as stale label Aug 13, 2026
@martincostello martincostello added this to the v1.18.0 milestone Aug 13, 2026
This was referenced Sep 14, 2026
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.

5 participants