added RollingInterval, to rotate file every x sec. #101
Conversation
|
@glaslos Hey:) this is actually a feature that we needed in few project. any chance to review the PR, so we won't used the forked branch? |
changed intervalExceeded to exceedsRollingInterval changed TestRollingInterval to set createdAt 2 seconds backward instead of using time.After fixed comments
|
@natefinch what do you think? |
|
So here's the thing... the whole point of rolling a log file is to avoid filling up your disk. Rolling on a time interval doesn't do that. You could have a script kiddie or runaway process that spams the logs... and you could quickly run out of disk space if you wait to roll once a day or once an hour. This is the main reason why I have resisted adding periodic rolling. What's the use case here? |
|
@natefinch it's not replacing any of the existing measures, so anything spamming would still trigger the MaxSize and MaxBackups. If I interpret it correctly, this solves a use case where you want for example the log to roll over every 24h. |
|
@natefinch as @glaslos said it won't fill up the disk or create unnecessary spamming since the rolling mechanism still exists as it currently is (MaxSize and MaxBackups will still work as usual). |
|
I think we should roll this (heh) into a v3 version of lumberjack. See here: #115 |
|
Time to make a new branch then? :) |
|
please let me know if there is anything that I could do, I'll be more than glad to help :) |
|
rolling interval is hard, we must face to potential goroutine leakage and race condition, maybe it's not a bug finally, it's a wrong design. |
|
And user probably will not be satisfied just rolling by |
|
I can vouch for this feature as well. A simple use case is syncing the logs to s3. In this case, not only do you want to rotate by size so as to avoid filling up disk, you also want to rotate by time so that recent logs are synced and not kept forever. That said, this implementation rotates on a write and not asynchronously, so it's suboptimal for the above use case. |
|
Hey, that is exactly what we need! Is there any time estimation when it is going to be merged into master? |
|
We will also be glad to use the original repository instead of my fork. @natefinch because that it's a fully tested side feature that is not depended on or required by any other part of the project, I think that it's ready to be merged into master and not wait to version 3, |
|
@oryaacov haha, this commit is what we are searching for! Besides, could you add the ability to rotate files exactly on the hour, like "08:00:00" or "12:00:00". I would like to rotate a file daily on "00:00:00" instead of 24 hours after the setup. Thank you in advance for your help. |
|
@natefinch can we merge this commit? |
|
Yes, I've come around on it, mostly because there is a use case that I hadn't considered before. You may not want to keep logs beyond a certain date, as a data retention window. MaxAge will sorta do that, but it only checks when you rotate, so if you happen to not rotate for too long, you'll end up keeping old logs longer than you wanted to. Adding a rolling interval ensures that you can be sure that you'll age out old logs. There's probably a better way to do it, too, but this is a valid feature regardless. Lemme fix whatever bug is screwing up |
|
@natefinch sure, thank you. |
|
@natefinch hey, can we merge? |
|
why dose it still not be merged |
|
Were still waiting for it as well |
|
@natefinch may you please merge/close the PR? |
@natefinch I could tell about my use case. I have a background task, which sends backup-ed log files to S3. So, I want to send a file every 5 minutes, even if it is just a few records (otherwise, I could lose logs for more, than 5 minutes). |
added RollingInterval feature in order to generate new log file every single hour.
RollingInterval is the number of seconds before rotating to a new log file.
the old log files will be deleted by the MaxAge & MaxBackups properties as usual. if the rolling interval is 0 the feature is off, default is 0.