log icon indicating copy to clipboard operation
log copied to clipboard

Allow per-crate compile-time filters

Open glandium opened this issue 2 years ago • 8 comments

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 avatar Jun 01 '22 04:06 glandium

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

glandium avatar Jun 01 '22 04:06 glandium

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.

KodrAus avatar Jun 01 '22 05:06 KodrAus

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 avatar Jun 01 '22 06:06 glandium

The change that broke things was e49f063235752784b0476e74559d142cd7786ca8.

glandium avatar Jun 01 '22 06:06 glandium

@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.

Thomasdezeeuw avatar Jun 01 '22 08:06 Thomasdezeeuw

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.

glandium avatar Jun 02 '22 01:06 glandium

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

sfackler avatar Jun 02 '22 01:06 sfackler

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

glandium avatar Jun 02 '22 01:06 glandium

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!

KodrAus avatar Aug 31 '22 10:08 KodrAus