SourceHorizon / logger

Small, easy to use and extensible logger which prints beautiful logs.
https://pub.dev/packages/logger
MIT License
197 stars 33 forks source link

Allow to limit the number of files created by AdvancedFileOutput #70

Closed lomby92 closed 4 months ago

lomby92 commented 4 months ago

Log rotation is not only about splitting logs between multiple files but also about limiting the number of those files. What do you think about a setting in the AdvancedFileOutput class to limit the count of archived files?

Suggested logic:

I will be happy to provide a PR, let me know if it could be reasonable

Bungeefan commented 4 months ago

Hi! This sounds like a good idea, but I am not sure if we can really rely on the creation date of the files. AFAIK, some file systems and programs tend to be funny with it and either set it to 0 (1970-01-01) or update it along with the modification date. Additionally, it could be re-set to the current date/time when moved between disks (old file deleted -> new file created).

I would like to have these things checked first. However, as this would be opt-in anyway, I don't see a reason against providing this option. :+1:

lomby92 commented 4 months ago

Doing a fast search, also the Dart documentation doesn't recommend to rely on changed property of FileStat: https://api.dart.dev/stable/3.3.3/dart-io/FileStat/changed.html

What do you think about rely on filenames sorted by natural order? The main problem could be the fileNameFormatter property but this function takes a DateTime as input so it's implicit that every filename will have different names. If this could be acceptable, the documentation must specify this logic to prevent wrong usages

Bungeefan commented 4 months ago

TBH, not really a fan of it :/

this function takes a DateTime as input so it's implicit that every filename will have different names

I don't think so, the time is basically optional and nothing guarantees that it is the right order (e.g. could be random UUIDs).

Leaning on e.g. Log4j, the default PathSorter (which is invoked prior to deletion) sorts after "most recently modified". Maybe even make it customizable for the user, this could range from a simple boolean flag (use FileStat.modified vs. changed), over a metadata callback for a single file to a whole PathSorter-like class.

As it's the default in a fairly popular logger library, I guess it's reasonable to go this or a similar route (it's opt-in after all). What do you think?

lomby92 commented 4 months ago

It should be ok.

I will try to open a PR, ok?

Bungeefan commented 4 months ago

Yeah, sure, go ahead!

lomby92 commented 4 months ago

PR ready for review🤞🏼