geth: Implement log rotation using lumberjack (#229) #524
Conversation
geth/log/log.go
Outdated
| return err | ||
| logger.ljWriter = &lumberjack.Logger{ | ||
| Filename: filename, | ||
| MaxBackups: 3, // number of backups |
There was a problem hiding this comment.
Minor: maybe we could describe it as // number of backup files to denote that backup = file in this case. It may not be obvious.
| handler, err := log.FileHandler(filename, log.TerminalFormat(false)) | ||
| if err != nil { | ||
| return err | ||
| logger.ljWriter = &lumberjack.Logger{ |
There was a problem hiding this comment.
How does lubmerjack report problems with e.g. creating a file? We used to have some issues with creating logs files via status-go, thus we have this code: https://github.com/status-im/status-react/blob/6d5251ad2072c949c0608acde4144d4f4cff0d11/modules/react-native-status/ios/RCTStatus/RCTStatus.m#L221-L226.
I will try to test a case when a new log file is created before merging this PR.
There was a problem hiding this comment.
lumberjack errors will not be reported, since the geth logger discards the return value from io.Writer. There's no reason why keeping the code you mentioned wouldn't work though, unless I'm missing something. The code you mentioned is meant to address the scenario where a user/something changes the permissions on a log file, right?
By the way, noticed that I used v1 of lumberjack, but a v2 is available and recommended, so I'll update that.
There was a problem hiding this comment.
There's no reason why keeping the code you mentioned wouldn't work though, unless I'm missing something. The code you mentioned is meant to address the scenario where a user/something changes the permissions on a log file, right?
iOS is a very specific platform. I don't remember what exactly was the problem but ultimately the log file needed to be created in Objective-C before status-go started using it. We just need to test it as a whole and figure out a better solution if there will be a problem.
By the way, noticed that I used v1 of lumberjack, but a v2 is available and recommended, so I'll update that.
👍
|
I believe we don't have to manage log rotation from within our code and the original issue is outdated. We use logs only for debug/test sessions and benefits of using log rotation do not cover the costs of supporting this code. I've never seen a situation where our log fills up all the disk space in practice either. |
|
Closing this after discussion with @pombeirp |
geth: Implement log rotation using lumberjack.
Introduce a lumberjack layer that rotates the logs every 2MB (16MB in debug log level) or 28 days. A maximum of 3 backup log files are kept.
Closes #229