rust-cli / env_logger

A logging implementation for `log` which is configured via an environment variable.
https://docs.rs/env_logger
Apache License 2.0
781 stars 124 forks source link

Option to conditionally log to stderr only above a given level #219

Open abonander opened 2 years ago

abonander commented 2 years ago

When using env_logger in a binary running in Google Kubernetes Engine, everything it logs gets recorded as an error log because it logs to stderr by default:

image

This is just the actix-web logger middleware logging requests from the Gcloud healthcheck service at INFO (plus an easter egg of a web crawler looking for an admin login form?), but Gcloud thinks they're error logs because they were printed to stderr.

Of course, we could just set Target::Stdout and call it a day, but I would like to retain the ability to print actual error logs to stderr as we want to set this up to page someone if our API service is throwing actual errors.

I'm thinking another variant to Target that looks something like:

enum Target {
    // ...

    // Print to `stderr` if the record level is at or above the given `log::Level`, otherwise `stdout`.
    StdoutOrStderrAt(log::Level),
}

I'm also considering, as a workaround, using the Builder::format() method to conditionally print to stderr but I'm not sure if this would break things. It's suboptimal either way:

builder.format(|f, record| {
    if record.level() >= log::Level::Error {
        eprintln!(/* format `record` using the same format as the default formatter */)
    } else {
        write!(fmt, "{}", record.args())
    }
})
abonander commented 2 years ago

I was also thinking as a more flexible option:

enum Target {
   // ...

    Threshold {
        // `Box` needed to keep the size finite
        default_target: Box<Target>,
        threshold_target: Box<Target>,
        threshold: log::Level,
    }
}

The mildly annoying part is the Box required to make the size calculable since otherwise it would be a recursive type.

It could be a separate method on Builder to set this up, maybe:

impl Builder {
    // Log a record to the given target if it is at or above the given threshold, otherwise log to the regular target.
    pub fn threshold_target(&mut self, threshold: log::Level, target: Target) -> &mut Builder { ... }
}
Zarathustra2 commented 2 years ago

Hey @abonander we are also running into this issue, what did you end up doing?

abonander commented 2 years ago

I think we just changed the default sink to stdout so it stopped logging everything as errors.

Zarathustra2 commented 2 years ago

I had a helpful discussion with people on the rust irc, will likely go with this: https://cloud.google.com/logging/docs/structured-logging and make it so that it picks up the correct severity! Thanks for the input though! :)

bonigarcia commented 1 year ago

Any update on this? IMO, it would be interesting to redirect the Level::Error messages to stderr and the rest levels to stdout.

jplatte commented 1 year ago

No, this crate is not actively maintained. I did a release not long ago to fix the most glaring issues, but don't intend to spend more time on it and doesn't seem like any of the other people with access rights want to, either. See #159

epage commented 1 year ago

WG-CLI is picking up passive maintenance of this script.

I'm not familiar with that technology stack but this overall feels like a problem with how output is interacting with the technology stack.

mercuriete commented 1 year ago

I am having the same problem. meilisearch is having the same problem but they tell me its not their fault: https://github.com/meilisearch/meilisearch/discussions/2514

Please do the following. By default, you should output to 2 different files. stdout: when the log is in log level below error. stderr: when the log is equal or above error.

changing the default behaviour should be good enough to make happy to all parts.

Thanks in advance.