Add per-serverID log files for easier backend troubleshooting#724
Add per-serverID log files for easier backend troubleshooting#724
Conversation
- Created ServerFileLogger that manages individual log files for each serverID
- Added LogInfoWithServer, LogWarnWithServer, LogErrorWithServer, LogDebugWithServer functions
- Initialize per-serverID logger in main entry point
- Updated backend-related log calls to use per-serverID logging
- Per-serverID logs are written to {serverID}.log files in the log directory
- Logs are also duplicated to the main mcp-gateway.log for unified view
- Added comprehensive tests for per-serverID logging including concurrent access
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
- Added per-serverID logging documentation to logger README - Updated main README to highlight new feature - Documented log directory structure with per-serverID files - Added examples showing how to use per-serverID logging APIs Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Addressed code review feedback: - Fixed double formatting issue when falling back to unified logging - Use proper variadic argument passing to avoid double fmt.Sprintf calls Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Added documentation for: - Per-serverID log files in the log directory structure - LogInfoWithServer/LogWarnWithServer/LogErrorWithServer/LogDebugWithServer functions - Benefits of per-serverID logging for troubleshooting Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
|
@copilot make sure that single-view files are preserved as well as the individual per-serverid log files |
Added TestServerFileLoggerPreservesUnifiedView to verify that: - Per-serverID logs contain only messages for that specific server - Unified mcp-gateway.log contains ALL messages from all servers - Messages in unified log include serverID prefix for identification This confirms single-view files (mcp-gateway.log) are preserved alongside per-serverID logs. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Confirmed! Single-view files are preserved. Each
Added test in commit 7b18f89 that verifies:
The implementation maintains backward compatibility - the unified log continues to work exactly as before, with the addition of per-serverID prefixes and optional per-server isolation files. |
|
@copilot please fix this linting errror make lint |
Fixed formatting issue in server_file_logger_test.go by removing extra blank line at end of file. Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR implements per-serverID log files to improve troubleshooting of individual MCP backend servers. The implementation creates separate log files for each serverID (e.g., github.log, slack.log) while preserving a unified mcp-gateway.log with all messages. This dual logging approach maintains backward compatibility while adding the ability to view server-specific logs in isolation.
Changes:
- Added
ServerFileLoggerimplementation with thread-safe per-serverID file management and automatic fallback to unified logging - Updated backend server logging calls in launcher, unified server, and connection packages to use new
LogInfoWithServer,LogWarnWithServer,LogErrorWithServer, andLogDebugWithServerfunctions - Enhanced documentation in README files to describe the new logging structure and usage patterns
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/logger/server_file_logger.go | Core implementation of ServerFileLogger with per-serverID log file management, concurrent access handling, and dual logging to both per-server and unified logs |
| internal/logger/server_file_logger_test.go | Comprehensive test suite covering initialization, concurrent access, fallback mode, all log levels, multiple initialization, and unified log preservation |
| internal/cmd/root.go | Integration of per-serverID logger initialization during gateway startup with proper cleanup on shutdown |
| internal/launcher/launcher.go | Updated backend server launch and session management logs to use per-serverID logging functions |
| internal/launcher/log_helpers.go | Updated all launcher helper logging calls (security warnings, launch start/success/error, timeout errors) to use per-serverID logging |
| internal/mcp/connection.go | Updated stderr logging to use per-serverID logging with simplified format |
| internal/server/unified.go | Updated tool registration success and failure logs to use per-serverID logging |
| internal/logger/README.md | Added comprehensive documentation for per-serverID logging feature including usage examples, log file structure, and benefits |
| README.md | Updated main documentation to highlight per-serverID logs feature with log directory structure examples |
| AGENTS.md | Enhanced logging documentation with per-serverID logging details and usage patterns |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger.Println(logLine) | ||
|
|
||
| // Flush to disk immediately | ||
| sfl.mu.RLock() | ||
| if file, exists := sfl.files[serverID]; exists { | ||
| if err := file.Sync(); err != nil { | ||
| log.Printf("WARNING: Failed to sync log file for server %s: %v", serverID, err) | ||
| } | ||
| } | ||
| sfl.mu.RUnlock() |
There was a problem hiding this comment.
Potential race condition: After calling logger.Println on line 115 without holding a lock, another goroutine could call Close() which acquires a write lock, closes all files, and clears the maps. This means logger.Println could write to a file handle that's being closed or has been closed. Consider holding the RLock from before the Println call through the Sync call, or use a different synchronization approach to ensure the logger and file aren't closed while being used.
| logger.Println(logLine) | |
| // Flush to disk immediately | |
| sfl.mu.RLock() | |
| if file, exists := sfl.files[serverID]; exists { | |
| if err := file.Sync(); err != nil { | |
| log.Printf("WARNING: Failed to sync log file for server %s: %v", serverID, err) | |
| } | |
| } | |
| sfl.mu.RUnlock() | |
| sfl.mu.RLock() | |
| defer sfl.mu.RUnlock() | |
| logger.Println(logLine) | |
| // Flush to disk immediately | |
| if file, exists := sfl.files[serverID]; exists { | |
| if err := file.Sync(); err != nil { | |
| log.Printf("WARNING: Failed to sync log file for server %s: %v", serverID, err) | |
| } | |
| } |
Per-ServerID Logging Implementation ✅
This PR implements per-serverID log files to make it easier to troubleshoot individual MCP servers.
Completed Tasks
ServerFileLoggerthat manages per-serverID log files{serverID}.login the log directoryServerFileLoggerstruct with map of serverID -> file loggerLogWithServerfunctions that route to per-serverID filesmcp-gateway.logImplementation Summary
Key Feature - Dual Logging:
✅ Each per-serverID log function writes to BOTH:
github.log) - isolated messagesmcp-gateway.log- all messages with[serverID]prefixThis ensures single-view files are preserved while adding per-serverID isolation.
Log Directory Structure:
API Usage:
Testing:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.