search

natefinch / lumberjack

lumberjack is a log rolling package for Go
MIT License
4.8k stars 591 forks source link

expose BackupTimeFormat #32

Closed laeshiny closed 7 years ago

laeshiny commented 7 years ago

I have exposed BackupTimeFormat. If you have ideas to fix better, tell me please

natefinch commented 7 years ago

Can you explain why you need to change the backup name format? One of the reasons it isn't exposed is that it was chosen carefully to avoid name collisions (unless you rotate more than once a millisecond, which seems unlikely :)

laeshiny commented 7 years ago

The reasons are like below

  1. To keep consistency of log name format with other apps not made by Go.
  2. It seems unlikely that the log file is rotated more than once as you mentioned.
  3. Some people might prefer to use "20060102T150405" format or other things instead of "2006-01-02T15-04-05".

I think if someone changes log format in new release, he can avoid name collision through handling existing log file or notify to operators, and above all things changing log format rarely happens.

In conclusion, I think that keeping consistency of bakcup format name is important, but it has not big advantage.

laeshiny commented 7 years ago

Please, could you check my comment again?

natefinch commented 7 years ago

Sorry for the delayed response. One problem is something you hinted at - the log file name is how we control rotation. If someone were to change the log file name after running their application, backup log files would not be removed.

I'll take another look and see if there are any other gotchas that may sway my opinion... but I am thinking this is probably ok.

laeshiny commented 7 years ago

Thank you for your response.

I understand what you concern. I also concern that someone changes the log file name. I think that we have different assumptions.

I supposed that renaming log file name is the problem but rarely happens. And developer or operator who already handle the system consisting of applications made by different languages want to maintain consistency of log file name with other application's log file name.

I think that your approach is good at the system consisting of applications made by only golang and the new project in golang.

lumberjack is a great package. I hope that it covers the system consisting of applications made by different languages.

kuangchanglang commented 7 years ago

LGTM

natefinch commented 7 years ago

I don't think the stated reasons for the change are a good enough reason to add to the package's API and add potential footguns if someone changes the backup time format without realizing it'll mean the code won't clean up old log files.

It seems like the only reason for this change is "I don't like the look of the name". Which is unfortunate, but ultimately, it's not actually a technical problem, as far as I can tell.

kuangchanglang commented 7 years ago

Hi @natefinch, it's a nice consideration for old log files clean up. But I think customized backup-file-name is still reasonable requirement. I rotate my logs each day 00:00, and file name with "HH-mm-ss" is redundant.

There should be a way to reach into an agreement, make backup-file-name customized, as well as make this package consistent.

natefinch commented 7 years ago

Well, I also don't support rotating files per day or other time period. The whole point of rotating files is to conserve disk space. The amount of time that has passed is not important. You might have a day of really heavy traffic that needs three rotations, or you might have 3 weeks of no traffic that doesn't need any rotations.