Conversation
- Add logging configuration with file and console handlers - Log server initialization and tip loading - Log all tool invocations with parameters - Log search results and tip access patterns - Log warnings for missing tips and errors - Create logs directory automatically - Update README with logging documentation Resolves #11
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive structured logging to the MCP server to improve debugging and monitoring capabilities. The implementation includes dual output to both file and console with INFO level logging for all critical operations.
Changes:
- Added Python logging module with file and console handlers
- Instrumented all tool invocations and data operations with log statements
- Updated README with logging documentation and usage examples
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/copilot_tips_server.py | Added logging configuration, initialized logger, and instrumented tool functions with log statements |
| src/README.md | Added new "Logging" section documenting log location, contents, and viewing commands |
|
|
||
| from fastmcp import FastMCP, Context | ||
|
|
||
| # Configure logging | ||
| LOG_DIR = Path(__file__).parent / "logs" | ||
| LOG_DIR.mkdir(exist_ok=True) |
There was a problem hiding this comment.
The log directory path uses Path(__file__).parent / 'logs', which places logs in the src/ directory. Consider using a project root-relative path or making this configurable, as logs are typically stored at the project root or in a dedicated directory outside the source tree.
| from fastmcp import FastMCP, Context | |
| # Configure logging | |
| LOG_DIR = Path(__file__).parent / "logs" | |
| LOG_DIR.mkdir(exist_ok=True) | |
| import os | |
| from fastmcp import FastMCP, Context | |
| # Configure logging | |
| # Prefer a project-root-relative logs directory, with optional override via environment variable. | |
| PROJECT_ROOT = Path(__file__).resolve().parent.parent | |
| DEFAULT_LOG_DIR = PROJECT_ROOT / "logs" | |
| LOG_DIR = Path(os.getenv("COPILOT_TIPS_LOG_DIR", str(DEFAULT_LOG_DIR))) | |
| LOG_DIR.mkdir(parents=True, exist_ok=True) |
|
|
||
| def load_tips() -> list[dict]: | ||
| """Load tips from the JSON data file.""" | ||
| logger.debug(f"Loading tips from {DATA_FILE}") |
There was a problem hiding this comment.
Using f-strings in log statements creates the formatted string even when the log level would filter it out. Use lazy formatting with logger.debug('Loading tips from %s', DATA_FILE) to avoid unnecessary string construction when DEBUG level is disabled.
| logger.debug(f"Loading tips from {DATA_FILE}") | |
| logger.debug("Loading tips from %s", DATA_FILE) |
| Returns: | ||
| List of matching tips sorted by relevance. | ||
| """ | ||
| logger.info(f"Tool invoked: get_tip_by_topic(search_term='{search_term}', category={category}, difficulty={difficulty})") |
There was a problem hiding this comment.
Using f-strings in log statements creates the formatted string even when the log level would filter it out. Use lazy formatting with logger.info('Tool invoked: get_tip_by_topic(search_term=%r, category=%s, difficulty=%s)', search_term, category, difficulty) to avoid unnecessary string construction.
| logging.basicConfig( | ||
| level=logging.INFO, | ||
| format='%(asctime)s - %(name)s - %(levelname)s - %(message)s', | ||
| handlers=[ | ||
| logging.FileHandler(LOG_DIR / "mcp_server.log"), | ||
| logging.StreamHandler() | ||
| ] | ||
| ) |
There was a problem hiding this comment.
The log file will grow indefinitely without rotation. Consider using RotatingFileHandler or TimedRotatingFileHandler from logging.handlers to prevent disk space issues in long-running deployments. For example: RotatingFileHandler(LOG_DIR / 'mcp_server.log', maxBytes=10485760, backupCount=5) for 10MB files with 5 backups.
🎯 Summary
This PR adds structured logging to the MCP server to improve debugging and monitoring capabilities.
📝 Changes Made
loggingmodule with INFO level configurationlogs/mcp_server.log) and consolelogs/directory🔍 What Gets Logged
📚 Documentation
Added new section in README explaining:
🧪 Testing
🔗 Related Issues
Closes #11
💡 Future Enhancements
This PR implements basic logging. Future improvements could include:
Ready for review by GitHub Copilot! 🤖