Skip to content

Conversation

@rumpl
Copy link
Member

@rumpl rumpl commented Jan 15, 2026

Summary

When running with --debug, logs are now automatically rotated when they exceed 10MB. Up to 3 backup files are kept (cagent.debug.log.1, .2, .3).

This prevents unbounded log file growth during long running sessions.

Changes

  • Added new pkg/logging package with a RotatingFile writer
  • Updated cmd/root/root.go to use the rotating writer for debug logs
  • Default rotation at 10MB with 3 backup files retained

Testing

  • Added unit tests for rotation behavior
  • All existing tests pass

Fixes #1336

@rumpl rumpl requested a review from a team as a code owner January 15, 2026 13:35
When running with --debug, logs are now automatically rotated when they
exceed 10MB. Up to 3 backup files are kept (cagent.debug.log.1, .2, .3).

This prevents unbounded log file growth during long running sessions.

Fixes docker#1336

Assisted-By: cagent
@rumpl rumpl force-pushed the feature/rotate-debug-logs branch from 0e41c27 to 97e0b43 Compare January 15, 2026 13:39
@rumpl
Copy link
Member Author

rumpl commented Jan 15, 2026

/review

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall Review

Great implementation of log rotation! The code is well-structured, properly tested, and addresses the issue effectively. I have a few suggestions to improve robustness and Go idiomaticity.

Strengths

  • ✅ Clean API with functional options pattern
  • ✅ Comprehensive test coverage including edge cases
  • ✅ Thread-safe implementation with proper mutex usage
  • ✅ Good error handling throughout
  • ✅ No race conditions detected

Key Recommendations

  1. File sync after write - Consider calling Sync() after writes for durability
  2. Nil file check - Add defensive check in Write() method
  3. Error handling in rotate - Handle file close error more robustly
  4. Resource cleanup - Ensure file is closed on all error paths

The changes integrate nicely with the existing codebase and solve the stated problem. With the minor improvements suggested, this will be production-ready.

@dgageot dgageot merged commit a5ec747 into docker:main Jan 15, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rotate debug log when using --debug

2 participants