phuslu / log

Fastest structured logging
MIT License
672 stars 44 forks source link

Gzip examples writes truncated file #57

Closed cgrinds closed 2 years ago

cgrinds commented 2 years ago

Following the example in #19

async := &log.AsyncWriter{
    ChannelSize: 1,
    Writer: log.IOWriter{
        Writer: gzip.NewWriter(&log.FileWriter{
            Filename:   "foo.log.gz",
            MaxSize:    50 * 1024 * 1024,
            MaxBackups: 7,
        }),
    },
}
logger := log.Logger{
    Writer: async,
}
logger.Info().Msg("hello phuslu")
async.Close()
ls -la
total 40
-rw-r--r--  1 cgrindst  staff    10B May 20 11:07 foo.log.2022-05-20T15-07-25.gz
lrwxr-xr-x  1 cgrindst  staff    30B May 20 11:07 foo.log.gz@ -> foo.log.2022-05-20T15-07-25.gz

file foo.log.2022-05-20T15-07-25.gz 
foo.log.2022-05-20T15-07-25.gz: gzip compressed data, truncated

gunzip foo.log.2022-05-20T15-07-25.gz 
gunzip: foo.log.2022-05-20T15-07-25.gz: unexpected end of file
gunzip: foo.log.2022-05-20T15-07-25.gz: uncompress failed
phuslu commented 2 years ago

try a quick fix at master https://github.com/phuslu/log/commit/522857ba2f232b43ddbbc5f4246c194432984256 could you please give a verification? thanks!

phuslu commented 2 years ago

Oh, this commit will close os.Stderr in tests, let me try to give another fix.

phuslu commented 2 years ago

seems I cannot fixed IOWriter without side effects, so I added a io.WriteCloser,

async := &log.AsyncWriter{
    ChannelSize: 1,
    Writer: log.IOWriteCloser{
        WriteCloser: gzip.NewWriter(&log.FileWriter{
            Filename:   "foo.log.gz",
            MaxSize:    50 * 1024 * 1024,
            MaxBackups: 7,
        }),
    },
}
logger := log.Logger{
    Writer: async,
}
logger.Info().Msg("hello phuslu")
async.Close()

could you please try above code with go get -v github.com/phuslu/log@d69c0401ec34366e92e9bb2897b00d7a9d28d6e0? thanks

cgrinds commented 2 years ago

thanks for the quick reply @phuslu

Using your latest fixes the issues. 👍

phuslu commented 2 years ago

I think your case

gzip.NewWriter(&log.FileWriter{
            Filename:   "foo.log.gz",
            MaxSize:    50 * 1024 * 1024,
            MaxBackups: 7,
        }

is misuse. Because the FileWriter will rotate at your unexpected time.

So the proper way is using gzip.NewWriter + os.File, then you need implement your self rotation logic with "async.Close()"

phuslu commented 2 years ago

So as you see, compression + rotation is hard

The most popular way is compress after rotation (nginx,lumberjack, etc...), In this lib,you can provide the Cleaner callback to FileWriter for similar thing.

But, please aware this will cause peak pressure to system. Thanks.

cgrinds commented 2 years ago

Thanks @phuslu. gzip.NewWriter + os.File works.

Might be worth updating the README examples with that one or maybe you're saying that gziping doesn't belong in your lib, similar to your comment about MaxAge? That's probably worth highlighting if so.

I understand the appeal of keeping your core fast and simple and letting others handle the details of compression, rolling, deletion, etc.

I'll try using phuslu with lumberjack as the last writer instead of implementing a Cleaner adding Cron, etc. I think lumberjack handles all of that and MaxAge already and looks like v3 will round off some rough edges.

phuslu commented 2 years ago

thanks. I'd like keep this lib to be a simple, plain, "static" logger library. I've always been against introducing hidden goroutines&timers into FileWriter.

I added an example at https://github.com/phuslu/log#rotating-file-writer-with-compression to demo the "compress after rotation" with this lib. It is more complicated than using lumberjack directly, but it's doable :)

NOTE: FileWriter rotation logic already recognized ".log.gz" files in https://github.com/phuslu/log/blob/master/file.go#L205-L217

cgrinds commented 2 years ago

thanks!