natefinch / lumberjack

lumberjack is a log rolling package for Go
MIT License
4.76k stars 585 forks source link

v3 Work Thread #115

Closed natefinch closed 2 years ago

natefinch commented 3 years ago

I think Lumberjack needs a v3. This package was written a long time ago, and while it's functional, there were decisions I made that were poor in hindsight. This thread will be a list of what I think needs to be done.

  1. Switch from a struct to a new function to create a new logger. This has several advantages - we can sanity check the log file name, and return an error if you've given us an invalid name. It lets us do some logic before the first write, like setting up the mill goroutine. And it means there's no way to change things after creation that really shouldn't be changed, like changing the maxsize etc.
  2. Switch from a size in megabytes to a size in bytes (many people have asked for this)
  3. Stop using yaml and toml struct tags.... if people want to write a config they can do that in their own code and we don't need to import those packages for no reason if people don't want to use them
  4. We can change the size calculation so that we just sum all the sizes of the current file and backup files, and then it won't matter if they're not all the same size, and then if people want to rotate on a time-based schedule, they can do that without worrying they'll accidentally blow up their disk.
  5. I think we can clean up the code some, so it's a bit easier to reason about.
iredmail commented 3 years ago

Before V3, i believe it's better fix these critical issues and tagged a new v2 release:

81: leak file handles

63: memory leak

56: goroutine leak

Thanks for your great work.

phuslu commented 3 years ago

I also implemented a rolling writer, for the work item 4, I think add a optional callback to do it is a pretty way.

type FileWriter struct {
    Filename string
    MaxSize int64
    MaxBackups int
    // Cleaner specifies an optional cleanup function of log backups after rotation,
    // if not set, the default behavior is to delete more than MaxBackups log files.
    Cleaner func(filename string, maxBackups int, matches []os.FileInfo)

       // ... other fileds
}

In Cleaner callback function, users could sum up the matches total size then do they want to do.

An example here, https://github.com/phuslu/log#rotating-file-writer

natefinch commented 3 years ago

Yeah, I think an OnRollover callback is an interesting idea. There's been a push for something like that for a while. I think if we call do it in a goroutine, that's fine.

phuslu commented 3 years ago

Yes, the callback also can link to https://github.com/natefinch/lumberjack/issues/96

phuslu commented 3 years ago

One more thing :smile: In production environment, I think symlink is more friendly to system(may not friendly to human)

image As above picture, log files wont change its name during rotation, sometimes it's important for a real-time/incremental log collector (e.g. rsync )

This is major reason why I wrote my rolling writer. If lumberjack v3 could supports this feature by default or as a option, I think it would make users happy.

mirecl commented 3 years ago

Hi @natefinch, do you need help implementing v3?

natefinch commented 3 years ago

Yeah, at this point, probably.

natefinch commented 2 years ago

I made a v3 branch.. moved to a NewFoo style, maxsize is in bytes, maxage is a time.Duration. . I'll look at the bugs linked here and see what else needs to be done.

natefinch commented 2 years ago

Dropping this issue, instead gonna take this to Discussions in #160