search

natefinch / lumberjack

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

add: Sync method mapped to the underlying file #47

Closed prasannavl closed 7 years ago

prasannavl commented 7 years ago

Allows flushing the underlying logger. Also, ensures it works with loggers like uber-go/zap out of the box that uses Sync.

natefinch commented 7 years ago

So, I'm not convinced this is actually all that useful. Please correct me if I'm wrong.

Sync() causes the OS to flush the data to disk, which the OS will do by itself anyway (but it might take a small amount of time). The only time when it would not automatically flush to disk is if the machine loses power between when the file was written but before the auto-flush happens.

So, calling defer Flush() in your code (as it the use case with uber-go/zap) only protects against those instances where your application shuts down normally (either gracefully or from a panic) and then the machine loses power before it can flush to disk. Note that if your application is killed or the machine loses power while your application is running, the defer will not run. Note that if you shut down the OS normally, it'll flush to disk.

This seems like an incredibly unlikely edge case, and I'm not convinced it's worth expanding the API of Lumberjack for it.

prasannavl commented 7 years ago

@natefinch - I suppose you're right. I was thinking it terms of higher abstraction, where if the File is buffered. But I guess since lumberjack never exposes it, and always uses just the File - it really is counter-intuitive and just increase the api surface.

Thanks for the review. I'm closing this. :)

fxposter commented 3 years ago

@natefinch There are cases, where doing sync is nice to have (here's one of the examples: https://github.com/uber-go/zap/blob/master/zapcore/core.go#L95-L99)