Remove max_level in favour of extracting it from the Filter

.github/workflows/ci.yml

@@ -41,6 +41,7 @@ jobs:
           components: rustfmt, clippy
 
       - run: cargo fmt --check
+      - run: cargo clippy --target=${{ matrix.target }} ${{ matrix.features }} -- -Dwarnings
       - run: cargo build --target=${{ matrix.target }} ${{ matrix.features }}
       - run: cargo doc --target=${{ matrix.target }} ${{ matrix.features }}
         env:

CHANGELOG.md

@@ -3,6 +3,12 @@
 
 All user visible changes to this project will be documented in this file. This project uses [Semantic Versioning 2.0.0].
 
+## [TODO]
+
+- Remove `max_level` in favour of extracting it from the `Filter`
+
+// TODO: bunch of releases are missing?
+
 
 
 

README.md

@@ -7,74 +7,77 @@
 This library is a drop-in replacement for `env_logger`. Instead, it outputs messages to
 android's logcat.
 
-This only works on Android and requires linking to `log` which
-is only available under android. With Cargo, it is possible to conditionally require
-this library:
+This only works on Android and requires linking to `liblog` which
+is only available under Android. With Cargo, it is possible to conditionally
+include this crate and library requirement when targeting Android only:
 
 ```toml
 [target.'cfg(target_os = "android")'.dependencies]
 android_logger = "0.15"
 ```
 
-Example of initialization on activity creation, with log configuration:
+### Examples
 
-```rust
-#[macro_use] extern crate log;
-extern crate android_logger;
+#### Example of initialization on activity creation, with log configuration
 
-use log::LevelFilter;
-use android_logger::{Config,FilterBuilder};
+```rust
+use android_logger::Config;
 
 fn native_activity_create() {
     android_logger::init_once(
         Config::default()
-            .with_max_level(LevelFilter::Trace) // limit log level
             .with_tag("mytag") // logs will show under mytag tag
-            .with_filter( // configure messages for specific crate
-                FilterBuilder::new()
-                    .parse("debug,hello::crate=error")
-                    .build())
+            .parse_filters("debug,hello::crate=error") // Limig log level to Debug, and limit the  hello::crate module further to Error.
     );
 
-    trace!("this is a verbose {}", "message");
-    error!("this is printed by default");
+    log::debug!("this is a verbose {}", "message");
+    log::error!("this is printed by default");
 }
 ```
 
-To allow all logs, use the default configuration with min level Trace:
+To allow all logs, use the default configuration with the global module level set to `Trace`
 
 ```rust
-#[macro_use] extern crate log;
-extern crate android_logger;
-
-use log::LevelFilter;
 use android_logger::Config;
 
 fn native_activity_create() {
     android_logger::init_once(
-        Config::default().with_max_level(LevelFilter::Trace),
+        Config::default().filter_level(log::LevelFilter::Trace),
     );
 }
 ```
 
+#### Example with a custom log formatter
+
+```rust
+use android_logger::Config;
+
+android_logger::init_once(
+    Config::default()
+        .format(|f, record| write!(f, "my_app: {}", record.args()))
+)
+```
+
+### Single-initialization guarantee
+
 There is a caveat that this library can only be initialized once
 (hence the `init_once` function name). However, Android native activity can be
 re-created every time the screen is rotated, resulting in multiple initialization calls.
 Therefore this library will only log a warning for subsequent `init_once` calls.
 
+###
+
 This library ensures that logged messages do not overflow Android log message limits
 by efficiently splitting messages into chunks.
 
-## Consistent log filtering in mixed Rust/C/C++ apps
+### Consistent log filtering in mixed Rust/C/C++ apps
 
-Android's C logging API determines the effective log level based on [a
-combination](https://cs.android.com/android/platform/superproject/main/+/main:system/logging/liblog/properties.cpp;l=243;drc=b74a506c1b69f5b295a8cdfd7e2da3b16db15934)
-of a process-wide global variable, [system-wide
-properties](https://cs.android.com/android/platform/superproject/main/+/main:system/logging/logd/README.property;l=45;drc=99c545d3098018a544cb292e1501daca694bee0f),
-and call-specific default. `log` + `android_logger` crates add another layer of
-log filtering on top of that, independent from the C API.
+Android's C logging API determines the effective log level based on [the combination] of a process-wide global variable, [system-wide properties], and call-specific default. `log` + `android_logger` crates add another layer of log filtering on top of that, independent from the C API.
 
-```
+[the combination]: https://cs.android.com/android/platform/superproject/main/+/main:system/logging/liblog/properties.cpp;l=243;drc=b74a506c1b69f5b295a8cdfd7e2da3b16db15934
+[system-wide properties]: https://cs.android.com/android/platform/superproject/main/+/main:system/logging/logd/README.property;l=45;drc=99c545d3098018a544cb292e1501daca694bee0f
+
+```text
     .-----.
     | app |
     '-----'     Rust
@@ -97,23 +100,24 @@ C/C++ | '--------------.
 ```
 
 `liblog` APIs introduced in Android API 30 let `android_logger` delegate log
-filtering decision to `liblog`, making the log level consistent across C, C++
+filtering decisions to `liblog`, making the log level consistent across C, C++
 and Rust calls.
 
-If you build `android_logger` with `android-api-30` feature enabled, the logger
+If you build `android_logger` with the `android-api-30` feature enabled, the logger
 will consider the process-wide global state (set via
 [`__android_log_set_minimum_priority`](https://cs.android.com/android/platform/superproject/main/+/main:prebuilts/runtime/mainline/runtime/sdk/common_os/include/system/logging/liblog/include/android/log.h;l=364;drc=4cf460634134d51dba174f8af60dffb10f703f51))
 and Android system properties when deciding if a message should be logged or
 not. In this case, the effective log level is the _least verbose_ of the levels
-set between those and [Rust log
-facilities](https://docs.rs/log/latest/log/fn.set_max_level.html).
+set between those and [Rust log facilities].
+
+[Rust log facilities]: https://docs.rs/log/latest/log/fn.set_max_level.html
 
-## License
+### License
 
 Licensed under either of
 
- * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or http://www.apache.org/licenses/LICENSE-2.0)
- * MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT)
+ * Apache License, Version 2.0, ([LICENSE-APACHE](LICENSE-APACHE) or <http://www.apache.org/licenses/LICENSE-2.0>)
+ * MIT license ([LICENSE-MIT](LICENSE-MIT) or <http://opensource.org/licenses/MIT>)
 
 at your option.
 

examples/system_log_level_overrides.rs

@@ -1,4 +1,4 @@
-//! An utility for testing the behavior of `android_logger` crate.
+//! An utility for testing the behavior of the `android_logger` crate.
 //!
 //! ## Build
 //!
@@ -37,7 +37,7 @@
 //!    HOME=$PWD bin/launch_cvd
 //!    ```
 //!
-//!    Once emulator launches, `adb` should detect it on `0.0.0.0:6520`
+//!    Once the emulator launches, `adb` should detect it on `0.0.0.0:6520`
 //!    automatically. Shut down the `launch_cvd` command to exit the emulator.
 //!
 //! 3. Upload & run:
@@ -55,7 +55,7 @@
 //!
 //! ```
 //! # default: should print info+ logs in `adb logcat -s log_test`
-//! # hint: use `adb logcat -v color` is awesome too
+//! # hint: `adb logcat -v color` is awesome too
 //! adb shell /data/local/tmp/system_log_level_overrides
 //!
 //! # should print trace+ logs in `adb logcat -s log_test`
@@ -71,9 +71,10 @@ fn main() {
     android_logger::init_once(
         android_logger::Config::default()
             .with_tag("log_test")
-            // If set, this is the highest level to log unless overriddeby by the system.
-            // Note the verbosity can be *increased* through system properties.
-            .with_max_level(log::LevelFilter::Info),
+            // If set, this is the highest level to log unless overridden by the system.
+            // Note the verbosity can be *increased* through system properties, as long
+            // as it is also increased in the `log` crate (see the override below).
+            .filter_level(log::LevelFilter::Info),
     );
     // The log crate applies its filtering before we even get to android_logger.
     // Pass everything down so that Android's liblog can determine the log level instead.

src/config.rs

@@ -4,19 +4,33 @@ use std::ffi::CString;
 use std::fmt;
 
 /// Filter for android logger.
-#[derive(Default)]
+// #[derive(Default)]
+// TODO: Rename to Builder.
 pub struct Config {
-    pub(crate) log_level: Option<LevelFilter>,
     pub(crate) buf_id: Option<LogId>,
-    filter: Option<env_filter::Filter>,
+    pub(crate) filter: env_filter::Builder,
     pub(crate) tag: Option<CString>,
     pub(crate) custom_format: Option<FormatFn>,
 }
 
+impl Default for Config {
+    /// Creates a default config that logs all modules at the [`LevelFilter::Error`] level by
+    /// default, when no other filters are set.
+    // TODO: Parse from env?
+    fn default() -> Self {
+        Self {
+            buf_id: None,
+            // TODO: This doesn't read from an env var like RUST_LOG...
+            filter: env_filter::Builder::new(),
+            tag: None,
+            custom_format: None,
+        }
+    }
+}
+
 impl fmt::Debug for Config {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("Config")
-            .field("log_level", &self.log_level)
             .field("buf_id", &self.buf_id)
             .field("filter", &self.filter)
             .field("tag", &self.tag)
@@ -62,39 +76,32 @@ fn android_is_loggable_len(
 }
 
 #[cfg(not(all(target_os = "android", feature = "android-api-30")))]
-fn default_is_loggable(_tag: &str, record_level: Level, config_level: Option<LevelFilter>) -> bool {
-    record_level <= config_level.unwrap_or_else(log::max_level)
+pub(crate) fn is_loggable(_tag: &str, _record_level: Level) -> bool {
+    // There is nothing to test here, the `log` macros already checked the variable
+    // `log::max_level()` before calling into the implementation.
+    // The tests ensure this by creating and calling into `AndroidLogger::log()` without
+    // `set_max_level()` from `init_once()`, and expect the message to be logged.
+    true
 }
 
 #[cfg(all(target_os = "android", feature = "android-api-30"))]
-fn android_is_loggable(tag: &str, record_level: Level, config_level: Option<LevelFilter>) -> bool {
+pub(crate) fn is_loggable(tag: &str, record_level: Level) -> bool {
     let prio = android_log_priority_from_level(record_level);
     // Priority to use in case no system-wide or process-wide overrides are set.
-    let default_prio = match config_level {
-        Some(level_filter) => match level_filter.to_level() {
-            Some(level) => android_log_priority_from_level(level),
-            // LevelFilter::to_level() returns None only for LevelFilter::Off
-            None => android_log_sys::LogPriority::SILENT,
-        },
-        None => android_log_sys::LogPriority::INFO,
+    // WARNING: Reading live `log::max_level()` state here would break tests, for example when
+    // `AndroidLogger` is constructed and `AndroidLogger::log()` is called _without_ going through
+    // `init_once()` which would call `log::set_max_level()`, leaving this at `Off`.  Currently no
+    // tests exist that run on live Android devices and/or mock `__android_log_is_loggable_len()`
+    // such that this function can be called.
+    let default_prio = match log::max_level().to_level() {
+        Some(level) => android_log_priority_from_level(level),
+        // LevelFilter::to_level() returns None only for LevelFilter::Off
+        None => android_log_sys::LogPriority::SILENT,
     };
     android_is_loggable_len(prio, tag, default_prio)
 }
 
 impl Config {
-    /// Changes the maximum log level.
-    ///
-    /// Note, that `Trace` is the maximum level, because it provides the
-    /// maximum amount of detail in the emitted logs.
-    ///
-    /// If `Off` level is provided, then nothing is logged at all.
-    ///
-    /// [`log::max_level()`] is considered as the default level.
-    pub fn with_max_level(mut self, level: LevelFilter) -> Self {
-        self.log_level = Some(level);
-        self
-    }
-
     /// Changes the Android logging system buffer to be used.
     ///
     /// By default, logs are sent to the [`Main`] log. Other logging buffers may
@@ -106,25 +113,26 @@ impl Config {
         self
     }
 
-    pub(crate) fn filter_matches(&self, record: &Record) -> bool {
-        if let Some(ref filter) = self.filter {
-            filter.matches(record)
-        } else {
-            true
-        }
+    /// Adds a directive to the filter for a specific module.
+    ///
+    /// Note that this replaces the default [`LevelFilter::Error`] for all global modules.
+    pub fn filter_module(mut self, module: &str, level: LevelFilter) -> Self {
+        self.filter.filter_module(module, level);
+        self
     }
 
-    pub(crate) fn is_loggable(&self, tag: &str, level: Level) -> bool {
-        #[cfg(all(target_os = "android", feature = "android-api-30"))]
-        use android_is_loggable as is_loggable;
-        #[cfg(not(all(target_os = "android", feature = "android-api-30")))]
-        use default_is_loggable as is_loggable;
-
-        is_loggable(tag, level, self.log_level)
+    /// Adds a directive to the filter for all modules.
+    pub fn filter_level(mut self, level: LevelFilter) -> Self {
+        self.filter.filter_level(level);
+        self
     }
 
-    pub fn with_filter(mut self, filter: env_filter::Filter) -> Self {
-        self.filter = Some(filter);
+    /// Parses the directives string in the same form as the `RUST_LOG`
+    /// environment variable.
+    ///
+    /// See the `env_logger` module documentation for more details.
+    pub fn parse_filters(mut self, filters: &str) -> Self {
+        self.filter.parse(filters);
         self
     }
 
@@ -138,7 +146,6 @@ impl Config {
     /// # use android_logger::Config;
     /// android_logger::init_once(
     ///     Config::default()
-    ///         .with_max_level(log::LevelFilter::Trace)
     ///         .format(|f, record| write!(f, "my_app: {}", record.args()))
     /// )
     /// ```