rust-lang / log

Logging implementation for Rust
https://docs.rs/log
Apache License 2.0
2.18k stars 252 forks source link

Surprising error when using `enabled` method in logger #606

Closed AngelicosPhosphoros closed 9 months ago

AngelicosPhosphoros commented 9 months ago

Version: 0.4.20

Example code:

use log; // 0.4.20

struct LoggerStdout;

impl log::Log for LoggerStdout {
    fn enabled(&self, metadata: &log::Metadata) -> bool {
        metadata.level() <= log::Level::Info
    }

    fn log(&self, record: &log::Record) {
        println!("{} - {}", record.level(), record.args());
    }

    fn flush(&self) {}
}

fn main(){
    static LOGGER: LoggerStdout = LoggerStdout;
    log::set_logger(&LOGGER).unwrap();
    log::set_max_level(log::LevelFilter::Trace);

    assert_eq!(log::log_enabled!(log::Level::Debug), false);
    log::error!("This is error");
    log::debug!("This is a debug");
}

It prints:

ERROR - This is error
DEBUG - This is a debug

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=8c70eca078d4f717845e436b2f308afe

Expected behaviour:

I expected that if my loggers enabled() returns false, the log would not be called.

sfackler commented 9 months ago

Note that enabled is not necessarily called before this method. Implementations of log should perform all necessary filtering internally.

https://docs.rs/log/latest/log/trait.Log.html#tymethod.log

AngelicosPhosphoros commented 9 months ago

I found it when I get this behaviour but it is still surprising API and I think it can be considered to be changed on major release.

Thomasdezeeuw commented 9 months ago

I found it when I get this behaviour but it is still surprising API and I think it can be considered to be changed on major release.

This won't work due to log::logger, which has direct access to the Log implementation, so any can call the log method with at any log level.

AngelicosPhosphoros commented 9 months ago

Oh, OK, understood.