Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surprising error when using enabled method in logger #606

Closed
AngelicosPhosphoros opened this issue Dec 17, 2023 · 4 comments
Closed

Surprising error when using enabled method in logger #606

AngelicosPhosphoros opened this issue Dec 17, 2023 · 4 comments

Comments

@AngelicosPhosphoros
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

Oh, OK, understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants