feat: make the FastMCP access log opt-in - #243
Merged
Merged
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
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.
Closes #240.
FastMCP installed
FastMcpLoggingMiddlewareby default, so every MCP message produced a log line.Litestar's equivalent is off by default and #180 made FastAPI's off by default, leaving FastMCP the
only framework logging every message without being asked.
Breaking, deliberately. This removes log output from services that never configured it, and
removes the
logging_turn_off_middlewarefield outright. That is why it is a separate PR from #180:it needs its own release-note line.
The change
Two lines of production code:
All three frameworks now read the same way:
litestar_logging_middleware_enabledfastapi_logging_middleware_enabledfastmcp_logging_middleware_enabledNo compatibility shim
FastMcpConfigis frozen, soFastMcpConfig(logging_turn_off_middleware=True)now raisesTypeErrorat construction. A test pins that, so the break stays deliberate rather than becoming anaccident someone quietly re-adds a field to fix.
AGENTS.md says "a public rename ships a silent alias", worked as
OpentelemetryConfig = OpenTelemetryConfig. That mechanism cannot express a dataclass field, and asilent alias was never available here anyway: the default flipped from on to off, so a shim would
have had to be a tri-state
bool | Nonedistinguishing "never set" from "set to False", plus awarning, plus a rule for setting both fields. An earlier revision of this PR did exactly that and it
cost more than the break it avoided.
The break is also the better outcome for users. The behaviour change is that log output disappears; a
TypeErrormakes every service that configured this setting re-decide, where a shim would let themupgrade past the change without noticing that the default flipped for everyone else in their
codebase.
Tests
Reverting the production file turns four red: not mounted by default, mounted when enabled, the
removed field raising
TypeError, and the default/enabled paths warning nothing (the repo sets nofilterwarnings, so a stray warning would otherwise fail no test).322 tests,
ruff,ty,mkdocs build --strict, lychee: all clean.Docs
docs/integrations/fastmcp.mdgains a Logging section with the new default, what gets logged, andthe removal.
docs/introduction/configuration.mdgains a### Structlog FastMCPsection, which theissue did not ask for but the other three frameworks already have.