search

clux / loggerv

A minimalistic stdout/stderr logger for rust
https://docs.rs/loggerv
MIT License
21 stars 6 forks source link

Filters and RUST_LOG support #14

Open volks73 opened 6 years ago

volks73 commented 6 years ago

This pull request contains changes to implement the filter method and support for inheriting logging configuration from the environment via the RUST_LOG variable, similar to the env_logger crate as discussed in #13. In fact, the env_logger crate already has extensive environment and filter support that is exposed in the public API for re-use in other loggers. Thus, the functionality is re-used in this logger.

The base_level method has been removed because that specific functionality is now covered by using the RUST_LOG environment variable. However, a side effect of this implementation is that the default log level is ERROR, not WARN. I cannot figure out a good way to change this default without causing odd or unexpected behavior related to verbosity and the use of the RUST_LOG environment variable when no filters are applied. The tradeoff is that all filter-related functionality that is implemented in the env_logger is also implemented in this logger without duplicating efforts.

Other than the removal of the base_level method, the rest of the public API remains the same, but the internals have changed significantly. Based on initial tests and examples, the functionality and behavior is the same, but the source code contains numerous changes. Most notably, an internal logger is used with closures for the logging component. This avoids weirdness with the builder pattern and the filter builder from the env_logger crate, and it avoids having a bunch of Option types for field members to the logger. The env_logger crate uses a similar implementation and style.

I have also added a new example that shows using the new filter method with the clap-rs crate. I am not sure if the new functionality yields the desired and expected behavior with relation to the RUST_LOG support, so this should be viewed as a first attempt and some additional refinement might be needed.

clux commented 6 years ago

Hey, sorry I have taken so long to look at this. This is quite a lot more involved than I expected.

Had a go at testing it, though. Think the filter method does not work as intended. It certainly has weird interactions:

(Ignoring RUST_LOG entirely atm) I've tried adding:

.filter("tokio_reactor=warn,tokio_core=warn,hyper=warn,hyper::proto=warn,tokio_core::reactor=warn")

to a spammy module when .verbosity(3) was set, and I'm actually getting more logs with that filter enabled than I am without.

Even with .filter("tokio_core=warn"), I am still seeing trace messages from tokio_core::reactor at verbosity 3.

In fact adding a single .filter("tokio_core=trace") to an app with .verbosity(1) gives me trace messages from all modules, which is very weird.

volks73 commented 6 years ago

...This is quite a lot more involved than I expected.

Yes, I thought so too. I thought I could just re-use the functionality from the env_logger crate, but it ended up being more than expected.

I was experiencing similar weirdness with the filters, especially related to values set with the RUST_LOG environment variable. I think part of the problem is that I did not have a clear picture of the expected behavior with relation to setting filters via environment variables versus the CLI verbosity. It has a lot to do with the "base" level, or lack there of.

I still submitted the pull request because I wanted to make sure the weirdness wan't just me and to use this to get some clarity on the expected behavior.