Repository navigation
Capture trace context, thread ID and message template in buffered logs - #7818
Open
evgenyfedorov2 wants to merge 1 commit into
Open
evgenyfedorov2 wants to merge 1 commit into
evgenyfedorov2 wants to merge 1 commit into
Conversation
Collaborator
🎉 Good job! The coverage increased 🎉
Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=1628010&view=codecoverage-tab |
evgenyfedorov2
marked this pull request as ready for review
October 9, 2026 14:08
Copilot stopped reviewing on behalf of
evgenyfedorov2 due to an error
October 9, 2026 14:08
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
4 open findings
GlobalBuffer.Flush:bufferedRecordis declared nullable but added… · NewTryDequeue(out SerializedLogRecord? bufferedRecord)makesbufferedRecordnullable, but… · NewBufferedLogRecordproperties only available viaIBufferedLoggerappears… · NewSerializedLogRecordwas changed from a struct to a sealed class with additional fields and a base… · New
What changed in this PR
This PR updates log buffering to preserve additional original-record context (message template, trace/span IDs, managed thread ID) when flushing, and adds tests to validate the behavior.
Changes:
- Add new unit tests for global and per-request log buffer flushing behavior.
- Refactor buffered record representation to use pooled
SerializedLogRecordinstances and capture thread/activity context at record creation time. - Update buffer flush paths to emit
SerializedLogRecorddirectly (removingDeserializedLogRecord).
| File | Description |
|---|---|
| test/Libraries/Microsoft.Extensions.Telemetry.Tests/Buffering/GlobalBufferTests.cs | Adds coverage for global-buffer flush preserving activity/thread/message-template/exception behavior. |
| test/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware.Tests/Buffering/IncomingRequestLogBufferTests.cs | Adds coverage for per-request buffer flush preserving activity/thread/message-template behavior. |
| src/Shared/LogBuffering/SerializedLogRecordFactory.cs | Captures message template + W3C activity IDs + thread ID at record creation and introduces record pooling. |
| src/Shared/LogBuffering/SerializedLogRecord.cs | Changes SerializedLogRecord to a pooled reference type implementing BufferedLogRecord properties. |
| src/Shared/LogBuffering/DeserializedLogRecord.cs | Removes the now-unused deserialized wrapper record type. |
| src/Libraries/Microsoft.Extensions.Telemetry/README.md | Updates documentation about what buffered record data is preserved/available. |
| src/Libraries/Microsoft.Extensions.Telemetry/Buffering/GlobalBuffer.cs | Flush now emits SerializedLogRecord instances directly instead of creating DeserializedLogRecord. |
| src/Libraries/Microsoft.AspNetCore.Diagnostics.Middleware/Buffering/IncomingRequestLogBuffer.cs | Flush now emits SerializedLogRecord instances directly instead of creating DeserializedLogRecord. |
🧠 Review effort: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Adds ActivityTraceId, ActivitySpanId, ManagedThreadId and MessageTemplate to buffered logs. Previously they were dropped. Consequently, we don't need the two identical internal types SerializedLogRecord and DeserializedLogRecord, so one of them is removed. Also, records logged without an exception now report no exception instead of an empty one. Fixes #6502 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
evgenyfedorov2
force-pushed
the
evgenyfedorov2-log-buffering-trace-context
branch
from
October 9, 2026 14:47
d797bf4 to
1783aaa
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds
ActivityTraceId,ActivitySpanId,ManagedThreadIdandMessageTemplateto buffered logs. Previously they were dropped.Consequently, we don't need two identical internal types
SerializedLogRecordandDeserializedLogRecord, so one of them is removed.Fixes: #6502
Also in this PR
null. Loggers that don't implementIBufferedLoggerreceivednew Exception("")for every flushed record,FakeLogRecord.Exceptionwas set, and the console formatters printed an empty exception.Worth a look
IBufferedLoggersee those new values. Loggers that don't implement it still see the flushing thread'sActivity.Current.Microsoft Reviewers: Open in CodeFlow