cooperspencer / gickup

https://cooperspencer.github.io/gickup-documentation/
Apache License 2.0
988 stars 38 forks source link

Only log errors go to stderr #51

Closed Gultak closed 2 years ago

Gultak commented 2 years ago

Currently all log messages are logged to stderr. Only errors should be logged to stderr, all other messages should go to stdout.

The current behavior breaks unattended execution where any output on stderr escalates (e.g. sends an email), which is a common pattern to supervise such things.

cooperspencer commented 2 years ago

I see your point. I need to check how to do that with zerolog. I will try to implement it.

colindean commented 2 years ago

I messed around with this a little bit and couldn't really come up with a straightforward solution with zerolog.

This did not work:

        stdErrWriter := zerolog.ConsoleWriter{Out: os.Stderr, TimeFormat: conf.Timeformat}
    errorsLogger := zerolog.New(os.Stderr).Level(zerolog.WarnLevel).Output(stdErrWriter)

    stdOutWriter := zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: conf.Timeformat}
    normalLogger := zerolog.New(os.Stdout).Level(zerolog.InfoLevel).Output(stdOutWriter)

    multi := zerolog.MultiLevelWriter(errorsLogger, normalLogger)

    writers = append(writers, multi)
    if conf.FileLogging.File != "" {
        writers = append(writers, NewRollingFile(conf.FileLogging))
    }
    mw := zerolog.MultiLevelWriter(writers...)

I think gickup might need its own implementation of something that implements the LevelWriter interface and takes two ConsoleWriter instances as its constructor args. I got the idea from SyslogWriter's WriteLevel() impl.

Gultak commented 2 years ago

Instead of rewriting the logging code, a -quiet option or a log:level in the config would also do.

cooperspencer commented 2 years ago

A --quiet flag sounds good. I'll implement that on Monday.

cooperspencer commented 2 years ago

upps, just implemented it.

Gultak commented 2 years ago

Great!

Just a small suggestion: Instead of completely removing console logging when the flag is active, i would recommend to use zerologs global log level. Example:

if cli.Quiet {
  zerolog.SetGlobalLevel(zerolog.ErrorLevel)
}

This way we only silence info messages and retain error output.

Gultak commented 2 years ago

That looks perfect. Many thanks!