Skip to content

[Server] Log JSON-RPC payloads at debug level only - #525

Open
aton-of-data wants to merge 1 commit into
modelcontextprotocol:mainfrom
aton-of-data:protocol-log-payloads-at-debug
Open

aton-of-data wants to merge 1 commit into
modelcontextprotocol:mainfrom
aton-of-data:protocol-log-payloads-at-debug

Conversation

@aton-of-data

@aton-of-data aton-of-data commented Oct 2, 2026 •

Copy link
Copy Markdown

Fixes #524

Problem. At 3175614, src/Server/Protocol.php puts whole JSON-RPC payloads into the log context at info level:

  • :181 info('Received message to process.', ['message' => $input]), the raw wire input
  • :260 info('Handling request.', ['request' => $request])
  • :364 info('Handling response from client.', ['response' => $response]), which carries sampling and elicitation replies
  • :384 info('Handling notification.', ['notification' => $notification])

docs/run/server-builder.md:241 and :271 set up a Monolog StreamHandler at Logger::INFO. With that setup every tool argument, and every elicitation or sampling reply, is written to disk twice per message.

The rest of the SDK already keeps payloads at debug: CallToolHandler.php:69 logs tool arguments at debug and :155 tool failures, the client logs its raw input at debug (src/Client/Protocol.php:576), and StatelessProtocol logs no payloads at all. Protocol was the one place that did not follow that split.

Change. The four info records now carry identifiers only: method and request_id for requests, message_id for client responses, method for notifications. Received message to process. stays at info with no context, and a new debug record, Received message payload., carries the raw input under the same message key. That raw input holds every request, response and notification in a batch, so debug output still has the full payload.

Reproduction. The documented configuration (Monolog StreamHandler('php://stdout', Logger::INFO)), Server::builder()->setLogger($logger)->addTool(fn (string $username, string $password) => 'ok', 'login'), over InMemoryTransport, sending initialize, notifications/initialized, then tools/call with {"username":"alice","password":"hunter2"}.

Before, excerpt:

mcp-server.INFO: Received message to process. {"message":"{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"login\",\"arguments\":{\"username\":\"alice\",\"password\":\"hunter2\"}}}"} []
mcp-server.INFO: Handling request. {"request":{"Mcp\\Schema\\Request\\CallToolRequest":{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"login","arguments":{"username":"alice","password":"hunter2"}}}}} []

After:

mcp-server.INFO: Received message to process. [] []
mcp-server.INFO: Handling request. {"method":"tools/call","request_id":2} []
mcp-server.INFO: Queueing server response {"response_id":2} []

hunter2 appears twice in the INFO log before and not at all after. With the handler at DEBUG it still appears, in Received message payload. and in CallToolHandler's Executing tool.

Test. ProtocolTest::testMessagePayloadsAreOnlyLoggedAtDebugLevel runs three cases (a tools/call request, a client response to an elicitation, a notifications/cancelled notification) and asserts for each that no record at info or above contains the payload, that the method or id is still logged at info, and that the payload is present at debug. It fails 3 of 3 on 3175614 and passes 3 of 3 with the change. No existing test was changed.

Verified on PHP 8.3.6, PHPUnit 10.5.65:

Check Before After
phpunit --testsuite=unit 1582 tests, OK, 4 skipped 1585 tests, OK, same 4 skipped
phpstan no errors no errors
php-cs-fixer fix --dry-run --diff (no cache) 0 of 575 files 0 of 575 files
integration suite, one file at a time 12 of 13 OK same 12 OK
examples suite 5 tests OK

Re-run on 2026-10-06 after rebasing onto 7deb350 (PHPUnit 10.5.66): unit suite 1589 tests before and 1592 after, OK, same 4 skipped; the new test fails 3 of 3 against 7deb350's Protocol.php; phpstan no errors; php-cs-fixer 0 of 575 files. The integration, examples and inspector suites were not re-run after the rebase.

Not verified. DualEraElicitationTest hangs in my environment on the base commit as well, so it was skipped. In the inspector suite the 44 HTTP tests error on base and patched alike (fetch failed ... invalid onRequestStart method from the npx inspector); the stdio inspector tests pass. The conformance tests need Docker and were not run.

Left alone, same family, failure paths only. src/Capability/Discovery/SchemaValidator.php:89-92 logs the tool arguments at error, but only when the validator itself throws; Protocol.php:562-566 logs a client reply at error when it cannot be reconstructed from the session. These are diagnostics at warning or error, not per-message info logging, so I kept them out of this change. The first is worth a follow-up if you agree.

AI disclosure: Claude (Anthropic) was used to diagnose this, write the change and the test, and run the verification. Every result quoted here was executed.

@chr-hertel chr-hertel added Server Issues & PRs related to the Server component enhancement Request for a new feature that's not currently supported labels Oct 5, 2026
@chr-hertel

Copy link
Copy Markdown
Member

THanks @aton-of-data for jumping on that - @mglaman is it what you had in mind?

@chr-hertel chr-hertel added the needs review PR needs code review by maintainer label Oct 5, 2026
Protocol logged the raw input and the full request, client response and
notification objects as context at info level, so a server logging at INFO
wrote every tool argument and every elicitation/sampling reply to its log.

Info records now carry only the method and id; the raw message is logged
once at debug level.

Fixes modelcontextprotocol#524
@aton-of-data
aton-of-data force-pushed the protocol-log-payloads-at-debug branch from 082aba1 to d49f519 Compare October 6, 2026 01:23
@aton-of-data

Copy link
Copy Markdown
Author

Rebased onto current main (7deb350) because the PR had a conflict. The conflict was only in CHANGELOG.md: #520 added its entry on the same line, so I kept both, with ours after it. The code diff is unchanged, and nothing upstream touched Protocol.php since the old base.

After the rebase I re-ran the unit suite (1592 tests, OK), the new test against the old Protocol.php (fails 3 of 3), phpstan and php-cs-fixer, all clean. The body's verification section says what was and wasn't re-run.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Request for a new feature that's not currently supported needs review PR needs code review by maintainer Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server logs full JSON-RPC payloads, tool arguments included, at info level

3 participants