Changed logging to warn level when web client is built in release mode. #588
Conversation
There was a problem hiding this comment.
That's not really what I meant in #549 (review):
If trunk doesn't build in release by default, it's definitely needed to do that. And speaking of release, maybe there's too much logging to the console. Instead of use
wasm_logger::Config::default()we should usewasm_logger::Config::new(level)with appropriate level. This probably needs to be done at compile-time using features (one perlog::Level) and set in the host runner when building trunk usinglog::max_level().to_level()to know which feature to use. If no feature is passed (i.e. whento_level()returnsNone), then logging should be completely disabled. But this should be done in a separate PR.
The idea is to have one feature per log level in web-client. Then in runner-host when calling trunk we check log::max_level().to_level() and enable the appropriate feature. If no feature is selected, web-client should not log anything. This lets the user control the level of logging instead of having something hard-coded.
| web_client_log_level_debug = [] | ||
| web_client_log_level_warn = [] | ||
| web_client_log_level_error = [] | ||
| web_client_log_level_trace = [] |
There was a problem hiding this comment.
You need to put this table before the [lints] one, because the sync script assumes [lints] is the last one. We can also use much simple feature names:
[features]
# At most one log level can be selected (defaults to off).
log_debug = []
log_error = []
...| let mut trunk = tokio::process::Command::new("../../scripts/wrapper.sh"); | ||
| trunk.args(["trunk", "build", "--release", "crates/web-client/index.html"]); | ||
| wasefire_cli_tools::cmd::execute(&mut trunk).await?; | ||
| let mut trunk = std::process::Command::new("../../scripts/wrapper.sh"); |
There was a problem hiding this comment.
You reverted the tokio::process and --locked changes while merging.
| trunk.args(["trunk", "build", "--release", "crates/web-client/index.html"]); | ||
| wasefire_cli_tools::cmd::execute(&mut trunk).await?; | ||
| let mut trunk = std::process::Command::new("../../scripts/wrapper.sh"); | ||
| trunk.args(["trunk", "build", "--locked", "--release", "--features=web_client_log_level_warn", "crates/web-client/index.html"]); |
There was a problem hiding this comment.
I guess you're still working on computing this automatically.
| fn main() { | ||
| wasm_logger::init(wasm_logger::Config::default()); | ||
| wasm_logger::init(wasm_logger::Config::new(get_log_level())); |
There was a problem hiding this comment.
This could simply be something like this:
| wasm_logger::init(wasm_logger::Config::new(get_log_level())); | |
| #[cfg(feature = "log_trace")] | |
| wasm_logger::init(wasm_logger::Config::new(log::Level::Trace)); | |
| #[cfg(feature = "log_debug")] | |
| wasm_logger::init(wasm_logger::Config::new(log::Level::Debug)); | |
| #[cfg(feature = "log_info")] | |
| wasm_logger::init(wasm_logger::Config::new(log::Level::Info)); | |
| #[cfg(feature = "log_warn")] | |
| wasm_logger::init(wasm_logger::Config::new(log::Level::Warn)); | |
| #[cfg(feature = "log_error")] | |
| wasm_logger::init(wasm_logger::Config::new(log::Level::Error)); |
Then we don't need the cfg_if dependency and when no features are selected, logging is disabled.
Follow up from web client PR lowering the logging level on release builds.