Skip to content

Commit 6d37135

Browse files
authored
Bug 1991777 - Make sure rust-log-forwarder forwards all logs (#6983)
I was seeing an issue with the target list because we spelled things out as `fxa-client` when it should have been `fxa_client`. Let's just install a min_level_event_sink to capture all logs. `simple_event_layer` will filter out tracing logs from non-app-services Rust crates that don't use tracing-support.
1 parent 813f3fa commit 6d37135

File tree

2 files changed

+7
-44
lines changed

2 files changed

+7
-44
lines changed

components/support/rust-log-forwarder/src/lib.rs

Lines changed: 7 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -8,38 +8,6 @@ static MAX_LEVEL: OnceLock<Level> = OnceLock::new();
88
static FOREIGN_LOGGER: OnceLock<Box<dyn AppServicesLogger>> = OnceLock::new();
99
static GLOBAL_SUBSCRIBER: Once = Once::new();
1010

11-
// The "targets" (in practice, crate names) which are hooked up to the `tracing` crate for logging.
12-
// We should improve this, or better still, just kill this crate entirely and move to using
13-
// tracing-support directly, like we (plan to) do on Desktop.
14-
//
15-
// Note also that it is a natural consequence of using `tracing` that each target must be explicitly listened for
16-
// *somewhere*, otherwise logs from that target will not be seen. For now, that list of targets is here, but
17-
// when we move to tracing-support it's likely this list will be pushed down into the clients (so that each
18-
// target can optionally be handled differently from the others).
19-
static TRACING_TARGETS: &[&str] = &[
20-
"autofill",
21-
"error_support",
22-
"fxa-client",
23-
"init_rust_components",
24-
"interrupt_support",
25-
"logins",
26-
"merino",
27-
"nimbus",
28-
"places",
29-
"push",
30-
"rate-limiter",
31-
"rc_crypto",
32-
"relevancy",
33-
"remote_settings",
34-
"search",
35-
"sql_support",
36-
"suggest",
37-
"sync_manager",
38-
"sync15",
39-
"tabs",
40-
"viaduct",
41-
];
42-
4311
#[derive(uniffi::Record, Debug, PartialEq, Eq)]
4412
pub struct Record {
4513
pub level: Level,
@@ -90,7 +58,8 @@ pub trait AppServicesLogger: Sync + Send {
9058

9159
/// Set the logger to forward to.
9260
///
93-
/// Pass in None to disable logging.
61+
/// Once a logger is passed in, it cannot be changed.
62+
/// However, you can pass in `None` to disable logging.
9463
#[uniffi::export]
9564
pub fn set_logger(logger: Option<Box<dyn AppServicesLogger>>) {
9665
GLOBAL_SUBSCRIBER.call_once(|| {
@@ -102,13 +71,13 @@ pub fn set_logger(logger: Option<Box<dyn AppServicesLogger>>) {
10271

10372
let level = MAX_LEVEL.get_or_init(|| Level::Debug);
10473
let sink = Arc::new(ForwarderEventSink {});
105-
// Set up a tracing subscriber for crates which use tracing and forward to the foreign log forwarder.
106-
for target in TRACING_TARGETS {
107-
tracing_support::register_event_sink(target, (*level).into(), sink.clone())
108-
}
109-
// if called before we just ignore the error for now, and also ignored if they supply None.
11074
if let Some(logger) = logger {
75+
// Set up a tracing subscriber for crates which use tracing and forward to the foreign log forwarder.
76+
tracing_support::register_min_level_event_sink((*level).into(), sink.clone());
77+
// Set the `FOREIGN_LOGGER` global. If this was called before we just ignore the error.
11178
FOREIGN_LOGGER.set(logger).ok();
79+
} else {
80+
tracing_support::unregister_min_level_event_sink();
11281
}
11382
}
11483

components/support/tracing/src/filters.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@ use tracing_subscriber::{
1414
/// See `build_targets_from_env` for the exact behavior. If not so configured, the filter will
1515
/// default to the `Level::Error` level.
1616
pub fn init_from_env() {
17-
// This is intended to be equivalent to `env_logger::try_init().ok();`
18-
// `debug!()` output is seen. We could maybe add logging for `#[tracing::instrument]`?
1917
tracing_subscriber::registry()
2018
.with(fmt::layer())
2119
.with(build_targets_from_env(LevelFilter::ERROR, None))
@@ -25,8 +23,6 @@ pub fn init_from_env() {
2523
/// Like `init_for_tests` but uses the specified `level` if logging is not configured in the environment.
2624
pub fn init_from_env_with_level(level: crate::Level) {
2725
let level_filter = LevelFilter::from_level(level.into());
28-
// This is intended to be equivalent to `env_logger::try_init().ok();`
29-
// `debug!()` output is seen. We could maybe add logging for `#[tracing::instrument]`?
3026
tracing_subscriber::registry()
3127
.with(fmt::layer())
3228
.with(build_targets_from_env(level_filter, None))
@@ -35,8 +31,6 @@ pub fn init_from_env_with_level(level: crate::Level) {
3531

3632
/// Like `init_for_tests` but uses a default string if logging is not configured in the environment
3733
pub fn init_from_env_with_default(default_env: &str) {
38-
// This is intended to be equivalent to `env_logger::try_init().ok();`
39-
// `debug!()` output is seen. We could maybe add logging for `#[tracing::instrument]`?
4034
tracing_subscriber::registry()
4135
.with(fmt::layer())
4236
.with(build_targets_from_env(LevelFilter::OFF, Some(default_env)))

0 commit comments

Comments
 (0)