search

slog-rs / slog

Structured, contextual, extensible, composable logging for Rust
https://slog.rs/
Apache License 2.0
1.58k stars 95 forks source link

Panic on logger trace level if feature disabled #316

Open andrewbaxter opened 2 years ago

andrewbaxter commented 2 years ago

Could we make the code panic if the filtering is set to Trace despite Trace being disabled by flag?

I can't think of any situation where you'd want to disable Trace logging by feature but then configure a logger at Trace level, so presumably panicking in this situation wouldn't be bad.

Suggesting it after being bitten by the same "why aren't my trace logs appearing" issue noted in other issues.

Techcable commented 2 years ago

I fear this may be a problem when loading options from config files. Remember we have different features to control debug/release builds.

It could be bad for an app config file to succeed in a debug mode (because trace is enabled) but trigger a panic in release. Conventionally, switching from debug -> release should never trigger extra panics.

How about logging a warning message instead? This would avoid a panic (in case behavior is intended) but still inform the user.

andrewbaxter commented 2 years ago

If someone uses a config that specifies trace logging and uses a release build, they're probably expecting trace logging and should either change their config (if they don't really want it) or rebuild with the flag enabled (if they do want it).

AFAICT this current behavior is optimizing for some low-touch developer workflow that involves using a single config for testing and production use rather than end user use.

switching from debug -> release should never trigger extra panics

This seems too general to me.

That said, I'd be happy with a warning here too! I imagine introducing a panic at this point might be a breaking change too, which is probably hard unless there's a reason for a major version bump.