rust-lang / log

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

Allow per-crate compile-time filters #517

Closed glandium closed 2 years ago

glandium commented 2 years ago

When building very large projects with multiple layers of dependent crates, it can be useful to limit, at compile-time, the overall logging to a less verbose level like Info or Warn, but still allow some crates to have a more verbose compile-time limit.

A practical example is release builds of Firefox set a compile-time limit to info, but the neqo-common crate wants to still allow to enable debug-level logging at run-time, and current does it via an ad-hoc wrapper that calls into log internal macros (and broke when it changed recently-ish).

With this change, a crate can define the relevant features of its own, and decide which to enable by default for itself.

Caveat: this adds a relatively large amount of code to each logging macro expansion.

glandium commented 2 years ago

Note: this obviously requires documentation, but I wanted to go through bikeshedding/discussion/rejection first.

KodrAus commented 2 years ago

Hi @glandium :wave:

Thanks for the PR! I can appreciate the need to manage logging at a finer grained level than the final binary, but am hesitant to encourage spreading the "Cargo features as level filters" approach currently taken by log. It's a misuse of Cargo features I'd like to get away from that will become harder to do if the pattern is enshrined in consumers.

Do you happen to have a patch handy for what broke when the macros changed? If you were simply calling log's top-level info!, error! etc macros then we shouldn't have broken you.

glandium commented 2 years ago

See https://github.com/mozilla/neqo/issues/1332 The problem is that e.g. debug! does nothing when enabling release_max_level_info, and there's no way around it other than not enabling that feature. So neqo-common has been reimplementing log by wrapping __private_api_log, so when things broke, it had it coming, but there was nothing else it could have done.

glandium commented 2 years ago

The change that broke things was e49f063235752784b0476e74559d142cd7786ca8.

Thomasdezeeuw commented 2 years ago

@glandium I agree with @KodrAus that this is probably not the right forward for the log crate. Wouldn't it be possible to create your own __private_api_log function (and macros) using log::RecordBuilder?

Also sorry for breaking your stuff.

glandium commented 2 years ago

Wouldn't it be possible to create your own __private_api_log function (and macros) using log::RecordBuilder?

It would, but that still requires to use log internals for module path, file and line. That's also a lot of boilerplate.

sfackler commented 2 years ago

but that still requires to use log internals for module path, file and line

It does not: https://doc.rust-lang.org/std/macro.file.html https://doc.rust-lang.org/std/macro.line.html https://doc.rust-lang.org/std/macro.module_path.html

glandium commented 2 years ago

Doh, I missed the fact that those _log* macros are simple wrappers.

KodrAus commented 2 years ago

It's been a little while now, since we weren't looking to dig deeper into Cargo features I'll go ahead and close this one, but I hope you found your way through this ok!