Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix logging from inside the WASM runtime #7355

Merged
merged 2 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions frame/support/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ smallvec = "1.4.1"
pretty_assertions = "0.6.1"
frame-system = { version = "2.0.0", path = "../system" }
parity-util-mem = { version = "0.7.0", features = ["primitive-types"] }
substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" }
sp-api = { version = "2.0.0", default-features = false, path = "../../primitives/api" }

[features]
default = ["std"]
Expand Down
49 changes: 48 additions & 1 deletion frame/support/src/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,16 @@ impl RuntimeLogger {
/// This is a no-op when running natively (`std`).
#[cfg(not(feature = "std"))]
pub fn init() {
static LOGGER: RuntimeLogger = RuntimeLogger;;
static LOGGER: RuntimeLogger = RuntimeLogger;
let _ = log::set_logger(&LOGGER);

// Set max level to `TRACE` to ensure we propagate
// all log entries to the native side that will do the
// final filtering on what should be printed.
//
// If we don't set any level, logging is disabled
// completly.
log::set_max_level(log::LevelFilter::Trace);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIR it shouldn't matter. We already return true in log::Log::enabled which should be responsible for filtering like that? Are you sure it's actually filtered somewhere internally in log crate?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I checked that. Remove the line and the test fails ;)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.rs/log/0.4.11/log/#implementing-a-logger

They also mention it here that you need to call it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.rs/log/0.4.11/src/log/macros.rs.html#46

Here they call the max_level function that checks the value that is set with set_max_value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough!

}
}

Expand All @@ -198,3 +206,42 @@ impl log::Log for RuntimeLogger {

fn flush(&self) {}
}

#[cfg(test)]
mod tests {
use substrate_test_runtime_client::{
ExecutionStrategy, TestClientBuilderExt, DefaultTestClientBuilderExt,
TestClientBuilder, runtime::TestAPI,
};
use sp_api::ProvideRuntimeApi;
use sp_runtime::generic::BlockId;

#[test]
fn ensure_runtime_logger_works() {
let executable = std::env::current_exe().unwrap();
let output = std::process::Command::new(executable)
.env("RUN_TEST", "1")
.env("RUST_LOG", "trace")
.args(&["--nocapture", "ensure_runtime_logger_works_implementation"])
.output()
.unwrap();

let output = dbg!(String::from_utf8(output.stderr).unwrap());
assert!(output.contains("Hey I'm runtime"));
}

/// This is no actual test. It will be called by `ensure_runtime_logger_works`
/// to check that the runtime can print from the wasm side using the
/// `RuntimeLogger`.
#[test]
fn ensure_runtime_logger_works_implementation() {
if std::env::var("RUN_TEST").is_ok() {
sp_tracing::try_init_simple();

let client = TestClientBuilder::new().set_execution_strategy(ExecutionStrategy::AlwaysWasm).build();
let runtime_api = client.runtime_api();
let block_id = BlockId::Number(0);
runtime_api.do_trace_log(&block_id).expect("Logging should not fail");
}
}
}
4 changes: 3 additions & 1 deletion primitives/tracing/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ pub use crate::types::{
/// Ignores any error. Useful for testing.
#[cfg(feature = "std")]
pub fn try_init_simple() {
let _ = tracing_subscriber::fmt().with_writer(std::io::stderr).try_init();
let _ = tracing_subscriber::fmt()
.with_env_filter(tracing_subscriber::EnvFilter::from_default_env())
.with_writer(std::io::stderr).try_init();
}

#[cfg(feature = "std")]
Expand Down
14 changes: 14 additions & 0 deletions test-utils/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,8 @@ cfg_if! {
/// Test that ensures that we can call a function that takes multiple
/// arguments.
fn test_multiple_arguments(data: Vec<u8>, other: Vec<u8>, num: u32);
/// Traces log "Hey I'm runtime."
fn do_trace_log();
}
}
} else {
Expand Down Expand Up @@ -391,6 +393,8 @@ cfg_if! {
/// Test that ensures that we can call a function that takes multiple
/// arguments.
fn test_multiple_arguments(data: Vec<u8>, other: Vec<u8>, num: u32);
/// Traces log "Hey I'm runtime."
fn do_trace_log();
}
}
}
Expand Down Expand Up @@ -698,6 +702,11 @@ cfg_if! {
assert_eq!(&data[..], &other[..]);
assert_eq!(data.len(), num as usize);
}

fn do_trace_log() {
frame_support::debug::RuntimeLogger::init();
frame_support::debug::trace!("Hey I'm runtime");
}
}

impl sp_consensus_aura::AuraApi<Block, AuraId> for Runtime {
Expand Down Expand Up @@ -944,6 +953,11 @@ cfg_if! {
assert_eq!(&data[..], &other[..]);
assert_eq!(data.len(), num as usize);
}

fn do_trace_log() {
frame_support::debug::RuntimeLogger::init();
frame_support::debug::trace!("Hey I'm runtime");
}
}

impl sp_consensus_aura::AuraApi<Block, AuraId> for Runtime {
Expand Down