Skip to content

Commit 293bc19

Browse files
TiggeZakityranron
andauthored
Fix incorrect log level filtering (rust-mobile#62)
- add `Config::with_max_level()` method accepting `log::LevelFilter` - deprecate `Config::with_min_level()` method accepting `log::Level` Co-authored-by: Kai Ren <[email protected]>
1 parent e8197a6 commit 293bc19

File tree

5 files changed

+69
-27
lines changed

5 files changed

+69
-27
lines changed

CHANGELOG.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,28 @@ All user visible changes to this project will be documented in this file. This p
66

77

88

9+
## [0.12.0] · 2023-01-?? (unreleased)
10+
[0.12.0]: /../../tree/v0.12.0
11+
12+
[Diff](/../../compare/v0.11.3...v0.12.0)
13+
14+
### Added
15+
16+
- `Config::with_max_level()` method to filters logs via `log::LevelFilter`. ([#62])
17+
18+
### Deprecated
19+
20+
- `Config::with_min_level()` method accepting `log::Level`. ([#62])
21+
22+
### Fixed
23+
24+
- Incorrect logs level filtering. ([#62])
25+
26+
[#62]: /../../pull/62
27+
28+
29+
30+
931
## [0.11.3] · 2022-12-20
1032
[0.11.3]: /../../tree/v0.11.3
1133

README.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ Example of initialization on activity creation, with log configuration:
2222
#[macro_use] extern crate log;
2323
extern crate android_logger;
2424

25-
use log::Level;
25+
use log::LevelFilter;
2626
use android_logger::{Config,FilterBuilder};
2727

2828
fn native_activity_create() {
2929
android_logger::init_once(
3030
Config::default()
31-
.with_min_level(Level::Trace) // limit log level
31+
.with_max_level(LevelFilter::Trace) // limit log level
3232
.with_tag("mytag") // logs will show under mytag tag
3333
.with_filter( // configure messages for specific crate
3434
FilterBuilder::new()
@@ -47,12 +47,13 @@ To allow all logs, use the default configuration with min level Trace:
4747
#[macro_use] extern crate log;
4848
extern crate android_logger;
4949

50-
use log::Level;
50+
use log::LevelFilter;
5151
use android_logger::Config;
5252

5353
fn native_activity_create() {
5454
android_logger::init_once(
55-
Config::default().with_min_level(Level::Trace));
55+
Config::default().with_max_level(LevelFilter::Trace),
56+
);
5657
}
5758
```
5859

src/lib.rs

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,13 @@
1313
//! #[macro_use] extern crate log;
1414
//! extern crate android_logger;
1515
//!
16-
//! use log::Level;
16+
//! use log::LevelFilter;
1717
//! use android_logger::Config;
1818
//!
1919
//! /// Android code may not have obvious "main", this is just an example.
2020
//! fn main() {
2121
//! android_logger::init_once(
22-
//! Config::default().with_min_level(Level::Trace),
22+
//! Config::default().with_max_level(LevelFilter::Trace),
2323
//! );
2424
//!
2525
//! debug!("this is a debug {}", "message");
@@ -36,13 +36,13 @@
3636
//! #[macro_use] extern crate log;
3737
//! extern crate android_logger;
3838
//!
39-
//! use log::Level;
39+
//! use log::LevelFilter;
4040
//! use android_logger::{Config,FilterBuilder};
4141
//!
4242
//! fn main() {
4343
//! android_logger::init_once(
4444
//! Config::default()
45-
//! .with_min_level(Level::Trace)
45+
//! .with_max_level(LevelFilter::Trace)
4646
//! .with_tag("mytag")
4747
//! .with_filter(FilterBuilder::new().parse("debug,hello::crate=trace").build()),
4848
//! );
@@ -58,7 +58,7 @@
5858
//!
5959
//! android_logger::init_once(
6060
//! Config::default()
61-
//! .with_min_level(log::Level::Trace)
61+
//! .with_max_level(log::LevelFilter::Trace)
6262
//! .format(|f, record| write!(f, "my_app: {}", record.args()))
6363
//! )
6464
//! ```
@@ -72,7 +72,7 @@ extern crate log;
7272

7373
extern crate env_logger;
7474

75-
use log::{Level, Log, Metadata, Record};
75+
use log::{Level, LevelFilter, Log, Metadata, Record};
7676
#[cfg(target_os = "android")]
7777
use log_ffi::LogPriority;
7878
use std::ffi::{CStr, CString};
@@ -137,7 +137,7 @@ impl Log for AndroidLogger {
137137
fn enabled(&self, metadata: &Metadata) -> bool {
138138
let config = self.config();
139139
// todo: consider __android_log_is_loggable.
140-
Some(metadata.level()) >= config.log_level
140+
metadata.level() <= config.log_level.unwrap_or_else(log::max_level)
141141
}
142142

143143
fn log(&self, record: &Record) {
@@ -214,18 +214,29 @@ impl AndroidLogger {
214214
/// Filter for android logger.
215215
#[derive(Default)]
216216
pub struct Config {
217-
log_level: Option<Level>,
217+
log_level: Option<LevelFilter>,
218218
filter: Option<env_logger::filter::Filter>,
219219
tag: Option<CString>,
220220
custom_format: Option<FormatFn>,
221221
}
222222

223223
impl Config {
224-
/// Change the minimum log level.
224+
// TODO: Remove on 0.13 version release.
225+
/// **DEPRECATED**, use [`Config::with_max_level()`] instead.
226+
#[deprecated(note = "use `.with_max_level()` instead")]
227+
pub fn with_min_level(self, level: Level) -> Self {
228+
self.with_max_level(level.to_level_filter())
229+
}
230+
231+
/// Changes the maximum log level.
232+
///
233+
/// Note, that `Trace` is the maximum level, because it provides the
234+
/// maximum amount of detail in the emitted logs.
225235
///
226-
/// All values above the set level are logged. For example, if
227-
/// `Warn` is set, the `Error` is logged too, but `Info` isn't.
228-
pub fn with_min_level(mut self, level: Level) -> Self {
236+
/// If `Off` level is provided, then nothing is logged at all.
237+
///
238+
/// [`log::max_level()`] is considered as the default level.
239+
pub fn with_max_level(mut self, level: LevelFilter) -> Self {
229240
self.log_level = Some(level);
230241
self
231242
}
@@ -253,7 +264,7 @@ impl Config {
253264
/// # use android_logger::Config;
254265
/// android_logger::init_once(
255266
/// Config::default()
256-
/// .with_min_level(log::Level::Trace)
267+
/// .with_max_level(log::LevelFilter::Trace)
257268
/// .format(|f, record| write!(f, "my_app: {}", record.args()))
258269
/// )
259270
/// ```
@@ -449,7 +460,7 @@ pub fn init_once(config: Config) {
449460
if let Err(err) = log::set_logger(logger) {
450461
debug!("android_logger: log::set_logger failed: {}", err);
451462
} else if let Some(level) = log_level {
452-
log::set_max_level(level.to_level_filter());
463+
log::set_max_level(level);
453464
}
454465
}
455466

@@ -469,18 +480,18 @@ mod tests {
469480
fn check_config_values() {
470481
// Filter is checked in config_filter_match below.
471482
let config = Config::default()
472-
.with_min_level(Level::Trace)
483+
.with_max_level(LevelFilter::Trace)
473484
.with_tag("my_app");
474485

475-
assert_eq!(config.log_level, Some(Level::Trace));
486+
assert_eq!(config.log_level, Some(LevelFilter::Trace));
476487
assert_eq!(config.tag, Some(CString::new("my_app").unwrap()));
477488
}
478489

479490
#[test]
480491
fn log_calls_formatter() {
481492
static FORMAT_FN_WAS_CALLED: AtomicBool = AtomicBool::new(false);
482493
let config = Config::default()
483-
.with_min_level(Level::Info)
494+
.with_max_level(LevelFilter::Info)
484495
.format(|_, _| {
485496
FORMAT_FN_WAS_CALLED.store(true, Ordering::SeqCst);
486497
Ok(())
@@ -493,10 +504,12 @@ mod tests {
493504
}
494505

495506
#[test]
496-
fn logger_always_enabled() {
497-
let logger = AndroidLogger::new(Config::default());
507+
fn logger_enabled_threshold() {
508+
let logger = AndroidLogger::new(Config::default().with_max_level(LevelFilter::Info));
498509

499-
assert!(logger.enabled(&log::MetadataBuilder::new().build()));
510+
assert!(logger.enabled(&log::MetadataBuilder::new().level(Level::Warn).build()));
511+
assert!(logger.enabled(&log::MetadataBuilder::new().level(Level::Info).build()));
512+
assert!(!logger.enabled(&log::MetadataBuilder::new().level(Level::Debug).build()));
500513
}
501514

502515
// Test whether the filter gets called correctly. Not meant to be exhaustive for all filter

tests/config_log_level.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ extern crate log;
33

44
#[test]
55
fn config_log_level() {
6-
android_logger::init_once(android_logger::Config::default().with_min_level(log::Level::Trace));
6+
android_logger::init_once(
7+
android_logger::Config::default().with_max_level(log::LevelFilter::Trace),
8+
);
79

810
assert_eq!(log::max_level(), log::LevelFilter::Trace);
911
}

tests/multiple_init.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@ extern crate log;
33

44
#[test]
55
fn multiple_init() {
6-
android_logger::init_once(android_logger::Config::default().with_min_level(log::Level::Trace));
6+
android_logger::init_once(
7+
android_logger::Config::default().with_max_level(log::LevelFilter::Trace),
8+
);
79

810
// Second initialization should be silently ignored
9-
android_logger::init_once(android_logger::Config::default().with_min_level(log::Level::Error));
11+
android_logger::init_once(
12+
android_logger::Config::default().with_max_level(log::LevelFilter::Error),
13+
);
1014

1115
assert_eq!(log::max_level(), log::LevelFilter::Trace);
1216
}

0 commit comments

Comments
 (0)