Skip to content

Commit 25182aa

Browse files
author
Marcin Radomski
committed
Use __android_log_is_loggable in AndroidLogger::enabled
Android's C logging API determines the effective log level based on a combination [1] of a global variable, system-wide properties [2], and call-specific default. log + android_logger crates add another layer of log filtering on top of that, independent from the C API. ``` .-----. | app | '-----' | v .-----------. filter by STATIC_MAX_LEVEL [3] + | log crate | - MAX_LOG_LEVEL_FILTER [4], '-----------' overrideable via log::set_max_level | v .----------------------. | android_logger crate | - filter by Config::max_level [5] '----------------------' | v .--------. | liblog | - filter by global state or system-wide properties [2] '--------' ``` The result is that in mixed C/C++ + Rust applications, logs originating from Rust use the least verbose level configured, which sometimes results in unexpected loss of logs. In addition, adjusting the log level globally via system properties, very useful in work on the AOSP itself, currently works only up to the level android_logger defaults to. This change makes AndroidLogger completely delegate log filtering to the underlying liblog when Config::max_level is unset by using __android_log_is_loggable. Setting Config::max_level, or calling log::set_max_level still keep the existing behavior of filtering the logs before they reach liblog, but the default is now to have the log level consistent between Rust and C/C++ logs. It also removes one TODO from the code :) Tested by: - running a test app that uses Rust logs of all levels on a rooted device, using both Android's liblog C API and android_logger ones - adjusting the log.tag system property [2] via adb shell setprop - verifying the message filtering changes according to log.tag changes consistently for logs from both languages [1] https://cs.android.com/android/platform/superproject/main/+/main:system/logging/liblog/properties.cpp;l=243;drc=b74a506c1b69f5b295a8cdfd7e2da3b16db15934 [2] https://cs.android.com/android/platform/superproject/main/+/main:system/logging/logd/README.property;l=45;drc=99c545d3098018a544cb292e1501daca694bee0f [3] https://github.com/rust-lang/log/blob/0551261bb4588b7f8afc8be05640347c97b67e10/src/lib.rs#L1536 [4] https://github.com/rust-lang/log/blob/0551261bb4588b7f8afc8be05640347c97b67e10/src/lib.rs#L469 [5] https://github.com/rust-mobile/android_logger-rs/blob/d51b7ffdacf20fb09fd36a6b309b50240ef50728/src/lib.rs#L198
1 parent d51b7ff commit 25182aa

File tree

2 files changed

+57
-16
lines changed

2 files changed

+57
-16
lines changed

src/lib.rs

+55-14
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,56 @@ fn android_log(
159159
#[cfg(not(target_os = "android"))]
160160
fn android_log(_buf_id: Option<LogId>, _priority: Level, _tag: &CStr, _msg: &CStr) {}
161161

162+
/// Outputs log to Android system.
163+
#[cfg(target_os = "android")]
164+
fn android_is_loggable_len(
165+
prio: log_ffi::LogPriority,
166+
tag: &str,
167+
default_prio: log_ffi::LogPriority,
168+
) -> bool {
169+
// SAFETY: tag points to a valid string tag.len() bytes long.
170+
unsafe {
171+
log_ffi::__android_log_is_loggable_len(
172+
prio as log_ffi::c_int,
173+
tag.as_ptr() as *const log_ffi::c_char,
174+
tag.len() as log_ffi::c_size_t,
175+
default_prio as log_ffi::c_int,
176+
) != 0
177+
}
178+
}
179+
180+
#[cfg(target_os = "android")]
181+
fn android_log_priority_from_level(level: Level) -> LogPriority {
182+
match level {
183+
Level::Warn => LogPriority::WARN,
184+
Level::Info => LogPriority::INFO,
185+
Level::Debug => LogPriority::DEBUG,
186+
Level::Error => LogPriority::ERROR,
187+
Level::Trace => LogPriority::VERBOSE,
188+
}
189+
}
190+
191+
#[cfg(target_os = "android")]
192+
fn should_log(tag: &str, level: Level, logger_config: &Config) -> bool {
193+
let prio = android_log_priority_from_level(level);
194+
// Priority to use in case no system-wide or process-wide overrides are set.
195+
let default_prio = match logger_config.log_level {
196+
Some(level_filter) => match level_filter.to_level() {
197+
Some(level) => android_log_priority_from_level(level),
198+
// LevelFilter::to_level() returns None only for LevelFilter::Off
199+
None => log_ffi::LogPriority::SILENT,
200+
},
201+
None => log_ffi::LogPriority::INFO,
202+
};
203+
android_is_loggable_len(prio, tag, default_prio)
204+
}
205+
206+
/// Dummy placeholder for tests.
207+
#[cfg(not(target_os = "android"))]
208+
fn should_log(_tag: &str, level: Level, logger_config: &Config) -> bool {
209+
level <= logger_config.log_level.unwrap_or(LevelFilter::Trace)
210+
}
211+
162212
/// Underlying android logger backend
163213
pub struct AndroidLogger {
164214
config: OnceLock<Config>,
@@ -193,9 +243,7 @@ impl Default for AndroidLogger {
193243

194244
impl Log for AndroidLogger {
195245
fn enabled(&self, metadata: &Metadata) -> bool {
196-
let config = self.config();
197-
// todo: consider __android_log_is_loggable.
198-
metadata.level() <= config.log_level.unwrap_or_else(log::max_level)
246+
should_log(metadata.target(), metadata.level(), self.config())
199247
}
200248

201249
fn log(&self, record: &Record) {
@@ -375,17 +423,7 @@ impl<'a> PlatformLogWriter<'a> {
375423

376424
#[cfg(target_os = "android")]
377425
pub fn new(buf_id: Option<LogId>, level: Level, tag: &CStr) -> PlatformLogWriter<'_> {
378-
PlatformLogWriter::new_with_priority(
379-
buf_id,
380-
match level {
381-
Level::Warn => LogPriority::WARN,
382-
Level::Info => LogPriority::INFO,
383-
Level::Debug => LogPriority::DEBUG,
384-
Level::Error => LogPriority::ERROR,
385-
Level::Trace => LogPriority::VERBOSE,
386-
},
387-
tag,
388-
)
426+
PlatformLogWriter::new_with_priority(buf_id, android_log_priority_from_level(level), tag)
389427
}
390428

391429
#[cfg(not(target_os = "android"))]
@@ -535,6 +573,9 @@ pub fn init_once(config: Config) {
535573
log::debug!("android_logger: log::set_logger failed: {}", err);
536574
} else if let Some(level) = log_level {
537575
log::set_max_level(level);
576+
} else {
577+
// Do not filter logs at log crate level. Delegate all filtering to underlying liblog.
578+
log::set_max_level(LevelFilter::Trace);
538579
}
539580
}
540581

tests/default_init.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ extern crate log;
55
fn default_init() {
66
android_logger::init_once(Default::default());
77

8-
// android_logger has default log level "off"
9-
assert_eq!(log::max_level(), log::LevelFilter::Off);
8+
// android_logger has default log level "trace"
9+
assert_eq!(log::max_level(), log::LevelFilter::Trace);
1010
}

0 commit comments

Comments
 (0)