-
Notifications
You must be signed in to change notification settings - Fork 77
chore: add json based logging #86
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import os | ||
|
|
||
| from fastmcp import FastMCP | ||
| from fastmcp.server.middleware.logging import StructuredLoggingMiddleware | ||
| from key_value.aio.stores.memory import MemoryStore | ||
| from key_value.aio.stores.redis import RedisStore | ||
| from mcp.types import Icon | ||
|
|
@@ -57,6 +58,7 @@ def get_oauth_mcp(base_path: str = "/"): | |
| ], | ||
| ), | ||
| ) | ||
| oauth_mcp.add_middleware(StructuredLoggingMiddleware(include_payloads=True)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider security implications of Enabling If sensitive data exposure in logs is a concern, consider:
🤖 Prompt for AI Agents |
||
| register_tools(oauth_mcp) | ||
| return oauth_mcp | ||
|
|
||
|
|
@@ -68,6 +70,7 @@ def get_header_mcp(): | |
| required_scopes=["read", "write"], | ||
| ), | ||
| ) | ||
| header_mcp.add_middleware(StructuredLoggingMiddleware(include_payloads=True)) | ||
| register_tools(header_mcp) | ||
| return header_mcp | ||
|
|
||
|
|
@@ -76,5 +79,6 @@ def get_stdio_mcp(): | |
| stdio_mcp = FastMCP( | ||
| "Plane MCP Server (stdio)", | ||
| ) | ||
| stdio_mcp.add_middleware(StructuredLoggingMiddleware(include_payloads=True)) | ||
| register_tools(stdio_mcp) | ||
| return stdio_mcp | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent logging configuration across logger hierarchies.
configure_json_logging()only configures the"fastmcp"logger, but other modules in this codebase use loggers under the"plane_mcp"hierarchy:plane_mcp/server.pyuseslogging.getLogger(__name__)→"plane_mcp.server"plane_mcp/auth/usesget_logger(__name__)→"plane_mcp.auth.*"plane_mcp/client.pyusesget_logger(__name__)→"plane_mcp.client"These loggers are not children of
"fastmcp", so they won't use the JSON formatter and will output in a different format (or propagate to root with default formatting).Consider also configuring the
"plane_mcp"logger hierarchy for consistent JSON output:🔧 Proposed fix for consistent logging
def configure_json_logging(): """Replace FastMCP's Rich handlers with a JSON formatter on the fastmcp logger.""" - fastmcp_logger = logging.getLogger("fastmcp") - - # Remove all existing handlers (Rich) - for handler in fastmcp_logger.handlers[:]: - fastmcp_logger.removeHandler(handler) - - handler = logging.StreamHandler(sys.stderr) - handler.setFormatter(JSONFormatter()) - fastmcp_logger.addHandler(handler) - fastmcp_logger.setLevel(logging.INFO) - fastmcp_logger.propagate = False + json_handler = logging.StreamHandler(sys.stderr) + json_handler.setFormatter(JSONFormatter()) + + for logger_name in ("fastmcp", "plane_mcp"): + target_logger = logging.getLogger(logger_name) + for h in target_logger.handlers[:]: + target_logger.removeHandler(h) + target_logger.addHandler(json_handler) + target_logger.setLevel(logging.INFO) + target_logger.propagate = False📝 Committable suggestion
🤖 Prompt for AI Agents