Skip to content

Enhance HTTP and MCP session logging #608

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions src/ModelContextProtocol.Core/McpSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ public McpSession(
_requestHandlers = requestHandlers;
_notificationHandlers = notificationHandlers;
_logger = logger ?? NullLogger.Instance;
LogSessionCreated(EndpointName, _sessionId, _transportKind);
}

/// <summary>
Expand Down Expand Up @@ -609,9 +610,9 @@ private static void AddExceptionTags(ref TagList tags, Activity? activity, Excep
e = ae.InnerException;
}

int? intErrorCode =
int? intErrorCode =
(int?)((e as McpException)?.ErrorCode) is int errorCode ? errorCode :
e is JsonException ? (int)McpErrorCode.ParseError :
e is JsonException ? (int)McpErrorCode.ParseError :
null;

string? errorType = intErrorCode?.ToString() ?? e.GetType().FullName;
Expand Down Expand Up @@ -692,6 +693,7 @@ public void Dispose()
}

_pendingRequests.Clear();
LogSessionDisposed(EndpointName, _sessionId, _transportKind);
}

#if !NET
Expand Down Expand Up @@ -774,4 +776,10 @@ private static TimeSpan GetElapsed(long startingTimestamp) =>

[LoggerMessage(Level = LogLevel.Trace, Message = "{EndpointName} sending message. Message: '{Message}'.")]
private partial void LogSendingMessageSensitive(string endpointName, string message);

[LoggerMessage(Level = LogLevel.Trace, Message = "{EndpointName} session {SessionId} created with transport {TransportKind}")]
private partial void LogSessionCreated(string endpointName, string sessionId, string transportKind);
Copy link
Contributor

@halter73 halter73 Jul 10, 2025

Choose a reason for hiding this comment

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

Can we make these trace level logs? The Streamable HTTP transport creates one of these per-request in Stateless mode, so this could get pretty noisy. I also don't think we need both LogSessionCreated and LogSessionConnected. Created and Disposed should be sufficient. "Connected" doesn't make much sense on the server side anyway.

Out of curiosity, what made you think that these events in particular were interesting? I wonder if moving these logs up a level to McpClient and McpServer would allow us to provide more informative log messages.

Copy link
Author

@theojiang25 theojiang25 Jul 10, 2025

Choose a reason for hiding this comment

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

I agree that this could be very noisy, moved it to TRACE level.

Out of curiosity, what made you think that these events in particular were interesting? I wonder if moving these logs up a level to McpClient and McpServer would allow us to provide more informative log messages.

We had encountered issues related to session (session ID not found IIRC). And it's very hard to debug without any logs related to session lifecycle.
I looked into McpServer and McpEndpoint where getting session and creating new sessions are involved, I'm wondering if there's any extra information we can get out of it.


[LoggerMessage(Level = LogLevel.Trace, Message = "{EndpointName} session {SessionId} disposed with transport {TransportKind}")]
private partial void LogSessionDisposed(string endpointName, string sessionId, string transportKind);
}