Conversation
There was a problem hiding this comment.
Pull request overview
Adds log rotation support to the backend Winston logger to prevent log files from growing without bound in production.
Changes:
- Replaced
winston.transports.Filewithwinston-daily-rotate-filetransports for error/complete/activity logs (20MB max size + gzip). - Added rotate/error event listeners for the rotating file transports.
- Added
winston-daily-rotate-filedependency to backendpackage.json.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
backend/utils/logger.js |
Switches file logging to rotating transports and hooks up rotation/error event handlers. |
backend/package.json |
Adds winston-daily-rotate-file dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exports = module.exports = function (service = "log", db = null) { | ||
| const errorTransport = new winston.transports.DailyRotateFile({ | ||
| filename: logging_dir + '/error.log', | ||
| level: 'error', | ||
| maxSize: '20m', | ||
| zippedArchive: true | ||
| }); | ||
|
|
||
| const completeTransport = new winston.transports.DailyRotateFile({ | ||
| filename: logging_dir + '/complete.log', | ||
| level: process.env.LOGGING_LEVEL, | ||
| maxSize: '20m', | ||
| zippedArchive: true | ||
| }); | ||
|
|
||
| const activityTransport = new winston.transports.DailyRotateFile({ | ||
| filename: logging_dir + '/activity.log', | ||
| level: 'info', | ||
| maxSize: '20m', | ||
| zippedArchive: true | ||
| }); | ||
|
|
||
| // Handle transport errors | ||
| [errorTransport, completeTransport, activityTransport].forEach(transport => { | ||
| transport.on('error', error => { | ||
| console.error('Log transport error:', error); | ||
| }); | ||
|
|
||
| transport.on('rotate', (oldFilename, newFilename) => { | ||
| console.log('Log rotated from %s to %s', oldFilename, newFilename); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
These DailyRotateFile transports (and their error/rotate listeners) are instantiated every time the exported logger factory is called. This codebase creates loggers in frequently constructed objects (e.g., per-socket connection), so this can lead to many concurrent file transports writing/rotating the same files (extra file descriptors, duplicated rotate events, potential contention). Consider creating the rotating transports once at module scope and returning baseLogger.child({service}), or otherwise ensuring transports are shared/singleton.
| "uuid": "^8.3.2", | ||
| "winston": "^3.8.1", | ||
| "winston-daily-rotate-file": "^5.0.0", | ||
| "winston-transport": "^4.5.0", |
There was a problem hiding this comment.
package-lock.json is present in the repo, but this PR adds a new dependency. Please update and commit the lockfile as well (e.g., run npm install in backend/) so CI/production installs are deterministic.
| "winston-transport": "^4.5.0", |
Implemented log rotation to prevent unbounded growth of activity logs in production
New Dev Features
winston-daily-rotate-filefor all log files (error, activity, and complete logs)Improvements
Bug Fixes
Known Limitations
Future Steps
maxFilessetting) for environments with strict disk space constraints