fix: [#847] resolve log file rotation issue in daily logger#1323
fix: [#847] resolve log file rotation issue in daily logger#1323
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1323 +/- ##
==========================================
- Coverage 70.09% 70.07% -0.02%
==========================================
Files 282 282
Lines 16748 16752 +4
==========================================
Hits 11739 11739
- Misses 4519 4522 +3
- Partials 490 491 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a log file rotation issue in the daily logger by ensuring time consistency when using carbon's time mocking functionality. The changes enable proper testing of time-dependent behavior and improve code maintainability.
Key Changes:
- Implemented a custom
rotatelogsClockthat usescarbon.Now().StdTime()to support time mocking in daily log rotation - Updated all log entry timestamps to use
carbon.Now().StdTime()for consistency across the logging system - Added comprehensive test for daily log rotation across different days using
carbon.SetTestNow()
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
log/writer.go |
Updated entry time assignment to use carbon.Now().StdTime() for mocking support |
log/writer_test.go |
Refactored tests with updated imports, added daily rotation test, commented out duplicate test, improved mock consistency |
log/logger/handler.go |
Enhanced error handling for first fprintf call, improved variable reuse |
log/logger/daily.go |
Introduced custom clock implementation to support carbon time mocking in rotatelogs |
.github/workflows/benchmark.yml |
Removed benchmark workflow file (entire file deleted) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📑 Description
Closes goravel/goravel#847
This pull request improves the logging system by ensuring that all log entries consistently use the current time from the
carbonpackage, which is especially important when using time mocking (e.g.,carbon.SetTestNow()). It also adds a new test for daily log rotation and refactors some test and import naming for clarity.Key changes include:
Logging Time Consistency:
logrus_writer.gonow explicitly set the log entry time usingcarbon.Now().StdTime(), ensuring logs reflect the correct (potentially mocked) time.logger/daily.gonow uses a customrotatelogs.Clockimplementation that always returnscarbon.Now().StdTime(), maintaining correct rotation behavior when time is mocked. [1] [2]Testing Improvements:
TestLogrus_DailyLogWithDifferentDaysto verify daily log rotation works correctly across simulated days usingcarbon.SetTestNow().Code and Naming Refactoring:
logrus_writer_test.gofor better clarity and consistency (e.g.,configmock→mocksconfig,logcontracts→contractslog). [1] [2] [3] [4] [5]github.com/dromara/carbon/v2inlogrus_writer.goto support the new time handling.✅ Checks