[MM-63462] Use local time for backup log filenames - #60
Open
hanzei wants to merge 4 commits into
Open
Conversation
Backup filenames previously used UTC regardless of the server's local timezone, while log entries used local time. This produced a confusing mismatch for admins correlating rotations with events in their local clock. Setting LocalTime: true aligns filenames with log entry timestamps. Admins who want UTC behaviour can achieve it by running the server process in the UTC timezone. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mutating time.Local races with lumberjack's internal mill goroutine, which reads time.Local via time.Parse even after the logger is shut down. There's no way to drain that goroutine from outside lumberjack. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthrough
ChangesFile target logging
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to Rotated file-log names now use the server’s local timezone, matching log-entry timestamps. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backup log filenames were formatted in UTC (lumberjack default) while log entry timestamps used the server's local timezone. This caused a confusing mismatch for admins trying to correlate a rotation event with something that happened at a specific local time — the filename would show e.g.
T09-14-56(UTC) while the last entry inside the file said14:44:55 +05:30.Setting
LocalTime: trueon the lumberjack logger aligns backup filenames with log entry timestamps. Admins who prefer UTC can achieve consistent UTC output by running the server process in the UTC timezone (TZ=UTC).Testing
A test for this was explored and then removed. The only meaningful approach was to mutate the global
time.Localto a non-UTC timezone, trigger rotation, and verify the backup filename reflected local time. However, lumberjack's internalmillRungoroutine readstime.Localviatime.Parseeven after the logger is shut down (lumberjack'sClose()does not drain that goroutine), so restoringtime.Localin a defer races with it. There is no way to wait for the mill goroutine from outside the library.The fix is a one-liner against a well-understood lumberjack flag; the behavior is covered by lumberjack's own test suite (
TestLocalTime).Ticket
https://mattermost.atlassian.net/browse/MM-63462
🤖 Generated with Claude Code