mcp: gate session lifecycle log demotion behind MCPGODEBUG - #1208
mcp: gate session lifecycle log demotion behind MCPGODEBUG#1208anneheartrecord wants to merge 3 commits into
Conversation
|
I agree overall with the change, but this introduces a behavioral change for existing users. We should gate the change behind an MCPGODEBUG variable |
Session bookkeeping records (server connecting, server session connected/disconnected, session initialized, client log level set) logged at info. On stateless streamable HTTP a session is minted per POST, so every request replays the whole lifecycle: four info lines of bookkeeping per tool call before the server logs anything of its own. The setLevel record additionally fires on every request carrying _meta under the 2026-07-28 spec, on all transports. Demote these five records to debug only when MCPGODEBUG=demotesessionlifecyclelog=1; the default stays at info so users who rely on the records see no change. If the demotion should become the default in a future minor, flipping the gate is a one-line change. Fixes modelcontextprotocol#1204
9ed4506 to
9eed2ee
Compare
anneheartrecord
changed the title
|
@guglielmo-san done — force-pushed as a single commit ( Tests cover both paths (visible at info by default; silent at info and visible at debug with the variable set), |
| // demotesessionlifecyclelog, when set to "1" via MCPGODEBUG, demotes routine | ||
| // session bookkeeping records ("server connecting", "server session | ||
| // connected", "server session disconnected", "session initialized", "client | ||
| // log level set") from info to debug. On stateless streamable HTTP a session | ||
| // is minted per request, so these fire constantly and drown out the server's | ||
| // own logs (#1204). It is gated behind MCPGODEBUG because demoting them | ||
| // changes log output that existing users may rely on. | ||
| var demotesessionlifecyclelog = mcpgodebug.Value("demotesessionlifecyclelog") |
There was a problem hiding this comment.
a description should be added in mcpgodebug.rc.md files
There was a problem hiding this comment.
Added in a9b6d0d — an entry under 1.8.0 next to blockingcancelnotify, in both internal/docs/mcpgodebug.src.md and the generated docs/mcpgodebug.md. Re-ran go generate ./internal/docs/ so the two agree, and go test ./internal/docs/ passes.
One thing worth flagging while you are looking at it: every other option on that page is opt-out (set to 1 to get the old behavior back), whereas this one is opt-in — the default keeps logging at info, which is what avoids the behavior change you raised. I said that explicitly in the entry, but it does sit a bit awkwardly next to the section's "will be removed in the 1.9.0 version" line, since dropping the knob would make the demotion unreachable rather than permanent.
If you would rather it follow the page convention, I can flip it to keepsessionlifecycleloginfo: demote by default, 1 restores info, and removal in 1.9.0 then means the demotion becomes permanent. Your call — happy either way.
2 participants
Session bookkeeping records (
server connecting,server session connected/disconnected,session initialized,client log level set) log at info. On stateless streamable HTTP a session is minted per POST, so every request replays the whole lifecycle — four info lines of bookkeeping per tool call before the server logs anything of its own. Theclient log level setrecord additionally fires on every request carrying_metaunder the 2026-07-28 spec, on all transports, even when the client set no level.This gates the demotion of the five lifecycle records to debug behind
MCPGODEBUG=demotesessionlifecyclelog=1, so the default behavior is unchanged for existing users (per review). Warn/error records are unchanged. If the demotion should become the default in a future minor, flipping the gate is a one-line change. Adds a test covering both paths: lifecycle visible at info by default; silent at info and visible at debug with the variable set.Fixes #1204