Add Trace-level logging for JSON-RPC payloads in transports #1191
+95
−2
Conversation
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
- Add LogTransportSendingMessage and LogTransportSendingMessageSensitive to TransportBase - Update StreamServerTransport to log sent messages at Trace level - Update StreamClientSessionTransport to log sent messages at Trace level - Update SseClientSessionTransport to log sent messages at Trace level - Update StreamableHttpClientSessionTransport to log sent messages at Trace level - Add tests for Trace-level logging of sent and received messages Co-authored-by: stephentoub <[email protected]>
Copilot
AI
changed the title
[WIP] Add logging for JSON RPC payloads at Trace level
Add Trace-level logging for JSON-RPC payloads in transports
Jan 28, 2026
stephentoub
reviewed
Jan 28, 2026
src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs
Outdated
Show resolved
Hide resolved
…ssages Addresses review feedback to only have Trace-level logging for JSON-RPC payloads. Co-authored-by: stephentoub <[email protected]>
stephentoub
reviewed
Jan 28, 2026
src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs
Outdated
Show resolved
Hide resolved
Contributor
|
@copilot it looks like you've only adding tracing for sent payloads... what about received payloads? |
Wrap logging calls with Logger.IsEnabled(LogLevel.Trace) to avoid expensive JSON serialization when trace logging is not enabled. Co-authored-by: stephentoub <[email protected]>
stephentoub
reviewed
Jan 28, 2026
src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs
Outdated
Show resolved
Hide resolved
Guard LogTransportReceivedMessageSensitive calls with IsEnabled(LogLevel.Trace) checks to avoid unnecessary work when trace logging is disabled. Co-authored-by: stephentoub <[email protected]>
Contributor
Author
The received message logging already existed via |
Add an overload that takes a JsonRpcMessage directly, encapsulating the IsEnabled check and JSON serialization to simplify call sites. Co-authored-by: stephentoub <[email protected]>
stephentoub
reviewed
Jan 28, 2026
src/ModelContextProtocol.Core/Client/SseClientSessionTransport.cs
Outdated
Show resolved
Hide resolved
Since the arguments to logging calls are just passing existing variables (no expensive work), the external guards are redundant - the source-generated logger methods handle the IsEnabled check internally. Co-authored-by: stephentoub <[email protected]>
Meir017
reviewed
Jan 29, 2026
| public async Task SendMessageAsync_Should_Log_At_Trace_Level() | ||
| { | ||
| // Arrange | ||
| var mockLoggerProvider = new MockLoggerProvider(); |
There was a problem hiding this comment.
should we use FakeLogger from https://github.com/dotnet/extensions/tree/main/src/Libraries/Microsoft.Extensions.Diagnostics.Testing here?
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.
This PR enables logging of all JSON-RPC payloads at the Trace level for visibility into wire-level protocol traffic.
Summary
Changes Made
Logger Methods in
TransportBase:LogTransportSendingMessageSensitive(string, string)- (Trace level) logs the full JSON payloadLogTransportSendingMessageSensitive(JsonRpcMessage)- overload that encapsulates the IsEnabled check and serialization (only this overload needs internal guarding since it does serialization)Updated Transports (simplified - no external guards needed since arguments are cheap):
StreamServerTransport- server-side stream transportStreamClientSessionTransport- client-side stream transportSseClientSessionTransport- client-side SSE transportStreamableHttpClientSessionTransport- client-side HTTP transportTests Added:
SendMessageAsync_Should_Log_At_Trace_LevelReadMessagesAsync_Should_Log_Received_At_Trace_LevelUsage
To enable JSON-RPC payload logging, configure the logging level to
Trace:When Trace logging is enabled, you'll see messages like:
The
LogTransportSendingMessageSensitive(JsonRpcMessage)overload internally checksIsEnabled(LogLevel.Trace)before serializing to avoid expensive serialization when trace logging is disabled. For call sites passing already-serialized strings, no external guard is needed since the source-generated logger methods handle the check internally.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.