From c413a51da3e832b5d50a41234c5a8aed572f8af7 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 22 Jun 2020 10:48:16 +0200 Subject: [PATCH 1/8] Fix browser informant --- Cargo.lock | 2 + client/cli/src/config.rs | 1 + client/informant/src/lib.rs | 31 ++++++++++++- client/service/src/builder.rs | 46 +------------------ client/service/src/config.rs | 2 + primitives/consensus/common/Cargo.toml | 1 + .../consensus/common/src/import_queue.rs | 2 +- .../consensus/common/src/offline_tracker.rs | 3 +- primitives/io/Cargo.toml | 1 + primitives/io/src/batch_verifier.rs | 2 +- utils/browser/src/lib.rs | 4 ++ 11 files changed, 46 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 86744c2537bfe..b9e7006cd87ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7433,6 +7433,7 @@ dependencies = [ "sp-utils", "sp-version", "substrate-prometheus-endpoint", + "wasm-timer", ] [[package]] @@ -7614,6 +7615,7 @@ dependencies = [ "sp-tracing", "sp-trie", "sp-wasm-interface", + "wasm-timer", ] [[package]] diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 2c3cfe84199d2..1cb51937fb95f 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -477,6 +477,7 @@ pub trait CliConfiguration: Sized { announce_block: self.announce_block()?, role, base_path: Some(base_path), + informant_output_format: sc_informant::OutputFormat::default(), }) } diff --git a/client/informant/src/lib.rs b/client/informant/src/lib.rs index 6a8acbadc36be..8b27132a773c7 100644 --- a/client/informant/src/lib.rs +++ b/client/informant/src/lib.rs @@ -38,10 +38,39 @@ mod display; pub struct OutputFormat { /// Enable color output in logs. pub enable_color: bool, - /// Add a prefix before every log line + /// Defines the informant's prefix for the logs. An empty string by default. + /// + /// By default substrate will show logs without a prefix. Example: + /// + /// ```text + /// 2020-05-28 15:11:06 ✨ Imported #2 (0xc21c…2ca8) + /// 2020-05-28 15:11:07 💤 Idle (0 peers), best: #2 (0xc21c…2ca8), finalized #0 (0x7299…e6df), ⬇ 0 ⬆ 0 + /// ``` + /// + /// But you can define a prefix by using this function. Example: + /// + /// ```rust,ignore + /// service.with_informant_prefix("[Prefix] ".to_string()); + /// ``` + /// + /// This will output: + /// + /// ```text + /// 2020-05-28 15:11:06 ✨ [Prefix] Imported #2 (0xc21c…2ca8) + /// 2020-05-28 15:11:07 💤 [Prefix] Idle (0 peers), best: #2 (0xc21c…2ca8), finalized #0 (0x7299…e6df), ⬇ 0 ⬆ 0 + /// ``` pub prefix: String, } +impl Default for OutputFormat { + fn default() -> Self { + Self { + enable_color: true, + prefix: String::new(), + } + } +} + /// Marker trait for a type that implements `TransactionPool` and `MallocSizeOf` on `not(target_os = "unknown")`. #[cfg(target_os = "unknown")] pub trait TransactionPoolAndMaybeMallogSizeOf: TransactionPool {} diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 23d736d98b6c9..fb0f9bf93f074 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -102,7 +102,6 @@ pub struct ServiceBuilder>>, marker: PhantomData<(TBl, TRtApi)>, block_announce_validator_builder: Option) -> Box + Send> + Send>>, - informant_prefix: String, } /// A utility trait for building an RPC extension given a `DenyUnsafe` instance. @@ -366,7 +365,6 @@ impl ServiceBuilder<(), (), (), (), (), (), (), (), (), (), ()> { rpc_extensions_builder: Box::new(|_| ()), remote_backend: None, block_announce_validator_builder: None, - informant_prefix: Default::default(), marker: PhantomData, }) } @@ -450,7 +448,6 @@ impl ServiceBuilder<(), (), (), (), (), (), (), (), (), (), ()> { rpc_extensions_builder: Box::new(|_| ()), remote_backend: Some(remote_blockchain), block_announce_validator_builder: None, - informant_prefix: Default::default(), marker: PhantomData, }) } @@ -545,7 +542,6 @@ impl rpc_extensions_builder: self.rpc_extensions_builder, remote_backend: self.remote_backend, block_announce_validator_builder: self.block_announce_validator_builder, - informant_prefix: self.informant_prefix, marker: self.marker, }) } @@ -591,7 +587,6 @@ impl rpc_extensions_builder: self.rpc_extensions_builder, remote_backend: self.remote_backend, block_announce_validator_builder: self.block_announce_validator_builder, - informant_prefix: self.informant_prefix, marker: self.marker, }) } @@ -630,7 +625,6 @@ impl rpc_extensions_builder: self.rpc_extensions_builder, remote_backend: self.remote_backend, block_announce_validator_builder: self.block_announce_validator_builder, - informant_prefix: self.informant_prefix, marker: self.marker, }) } @@ -697,7 +691,6 @@ impl rpc_extensions_builder: self.rpc_extensions_builder, remote_backend: self.remote_backend, block_announce_validator_builder: self.block_announce_validator_builder, - informant_prefix: self.informant_prefix, marker: self.marker, }) } @@ -754,7 +747,6 @@ impl rpc_extensions_builder: self.rpc_extensions_builder, remote_backend: self.remote_backend, block_announce_validator_builder: self.block_announce_validator_builder, - informant_prefix: self.informant_prefix, marker: self.marker, }) } @@ -792,7 +784,6 @@ impl rpc_extensions_builder: Box::new(rpc_extensions_builder), remote_backend: self.remote_backend, block_announce_validator_builder: self.block_announce_validator_builder, - informant_prefix: self.informant_prefix, marker: self.marker, }) } @@ -838,43 +829,9 @@ impl rpc_extensions_builder: self.rpc_extensions_builder, remote_backend: self.remote_backend, block_announce_validator_builder: Some(Box::new(block_announce_validator_builder)), - informant_prefix: self.informant_prefix, marker: self.marker, }) } - - /// Defines the informant's prefix for the logs. An empty string by default. - /// - /// By default substrate will show logs without a prefix. Example: - /// - /// ```text - /// 2020-05-28 15:11:06 ✨ Imported #2 (0xc21c…2ca8) - /// 2020-05-28 15:11:07 💤 Idle (0 peers), best: #2 (0xc21c…2ca8), finalized #0 (0x7299…e6df), ⬇ 0 ⬆ 0 - /// ``` - /// - /// But you can define a prefix by using this function. Example: - /// - /// ```rust,ignore - /// service.with_informant_prefix("[Prefix] ".to_string()); - /// ``` - /// - /// This will output: - /// - /// ```text - /// 2020-05-28 15:11:06 ✨ [Prefix] Imported #2 (0xc21c…2ca8) - /// 2020-05-28 15:11:07 💤 [Prefix] Idle (0 peers), best: #2 (0xc21c…2ca8), finalized #0 (0x7299…e6df), ⬇ 0 ⬆ 0 - /// ``` - pub fn with_informant_prefix( - self, - informant_prefix: String, - ) -> Result, Error> - where TSc: Clone, TFchr: Clone { - Ok(ServiceBuilder { - informant_prefix: informant_prefix, - ..self - }) - } } /// Implemented on `ServiceBuilder`. Allows running block commands, such as import/export/validate @@ -990,7 +947,6 @@ ServiceBuilder< rpc_extensions_builder, remote_backend, block_announce_validator_builder, - informant_prefix, } = self; sp_session::generate_initial_session_keys( @@ -1142,7 +1098,7 @@ ServiceBuilder< client.clone(), network_status_sinks.clone(), transaction_pool.clone(), - sc_informant::OutputFormat { enable_color: true, prefix: informant_prefix }, + config.informant_output_format, )); Ok(Service { diff --git a/client/service/src/config.rs b/client/service/src/config.rs index b79831d57bba3..cfda41040de99 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -108,6 +108,8 @@ pub struct Configuration { pub announce_block: bool, /// Base path of the configuration pub base_path: Option, + /// Configuration of the output format that the informant uses. + pub informant_output_format: sc_informant::OutputFormat, } /// Type for tasks spawned by the executor. diff --git a/primitives/consensus/common/Cargo.toml b/primitives/consensus/common/Cargo.toml index 3f256d3f736ce..9fa4baeb69434 100644 --- a/primitives/consensus/common/Cargo.toml +++ b/primitives/consensus/common/Cargo.toml @@ -30,6 +30,7 @@ codec = { package = "parity-scale-codec", version = "1.3.0", features = ["derive parking_lot = "0.10.0" serde = { version = "1.0", features = ["derive"] } prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../../utils/prometheus", version = "0.8.0-rc3"} +wasm-timer = "0.2.4" [dev-dependencies] sp-test-primitives = { version = "2.0.0-rc3", path = "../../test-primitives" } diff --git a/primitives/consensus/common/src/import_queue.rs b/primitives/consensus/common/src/import_queue.rs index 1a034e11d6644..94228a266385f 100644 --- a/primitives/consensus/common/src/import_queue.rs +++ b/primitives/consensus/common/src/import_queue.rs @@ -259,7 +259,7 @@ pub(crate) fn import_single_block_metered, Transaction r => return Ok(r), // Any other successful result means that the block is already imported. } - let started = std::time::Instant::now(); + let started = wasm_timer::Instant::now(); let (mut import_block, maybe_keys) = verifier.verify(block_origin, header, justification, block.body) .map_err(|msg| { if let Some(ref peer) = peer { diff --git a/primitives/consensus/common/src/offline_tracker.rs b/primitives/consensus/common/src/offline_tracker.rs index 9269640ffc8e4..b96498041f25d 100644 --- a/primitives/consensus/common/src/offline_tracker.rs +++ b/primitives/consensus/common/src/offline_tracker.rs @@ -18,7 +18,8 @@ //! Tracks offline validators. use std::collections::HashMap; -use std::time::{Instant, Duration}; +use std::time::Duration; +use wasm_timer::Instant; // time before we report a validator. const REPORT_TIME: Duration = Duration::from_secs(60 * 5); diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index df66740d657ee..bb83e559af5c5 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -28,6 +28,7 @@ sp-tracing = { version = "2.0.0-rc3", default-features = false, path = "../traci log = { version = "0.4.8", optional = true } futures = { version = "0.3.1", features = ["thread-pool"], optional = true } parking_lot = { version = "0.10.0", optional = true } +wasm-timer = "0.2.4" [features] default = ["std"] diff --git a/primitives/io/src/batch_verifier.rs b/primitives/io/src/batch_verifier.rs index 6a78070b38b55..4ce564e89bf81 100644 --- a/primitives/io/src/batch_verifier.rs +++ b/primitives/io/src/batch_verifier.rs @@ -160,7 +160,7 @@ impl BatchVerifier { #[must_use] pub fn verify_and_clear(&mut self) -> bool { let pending = std::mem::take(&mut self.pending_tasks); - let started = std::time::Instant::now(); + let started = wasm_timer::Instant::now(); log::trace!( target: "runtime", diff --git a/utils/browser/src/lib.rs b/utils/browser/src/lib.rs index e804af6094da4..4bd1f1597f90b 100644 --- a/utils/browser/src/lib.rs +++ b/utils/browser/src/lib.rs @@ -99,6 +99,10 @@ where max_runtime_instances: 8, announce_block: true, base_path: None, + informant_output_format: sc_informant::OutputFormat { + enable_color: false, + prefix: String::new(), + }, }; Ok(config) From cbe8b387658d65080db5984fbb48cf9589734c28 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 22 Jun 2020 10:52:44 +0200 Subject: [PATCH 2/8] Fix documentation --- client/informant/src/lib.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/client/informant/src/lib.rs b/client/informant/src/lib.rs index 8b27132a773c7..a4eafdddcc8d2 100644 --- a/client/informant/src/lib.rs +++ b/client/informant/src/lib.rs @@ -36,7 +36,7 @@ mod display; /// The format to print telemetry output in. #[derive(Clone)] pub struct OutputFormat { - /// Enable color output in logs. + /// Enable color output in logs. True by default. pub enable_color: bool, /// Defines the informant's prefix for the logs. An empty string by default. /// @@ -47,13 +47,7 @@ pub struct OutputFormat { /// 2020-05-28 15:11:07 💤 Idle (0 peers), best: #2 (0xc21c…2ca8), finalized #0 (0x7299…e6df), ⬇ 0 ⬆ 0 /// ``` /// - /// But you can define a prefix by using this function. Example: - /// - /// ```rust,ignore - /// service.with_informant_prefix("[Prefix] ".to_string()); - /// ``` - /// - /// This will output: + /// But you can define a prefix by setting this string. This will output: /// /// ```text /// 2020-05-28 15:11:06 ✨ [Prefix] Imported #2 (0xc21c…2ca8) From 4930288744ad37400e1cce7757eb0afa31234753 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 22 Jun 2020 11:41:55 +0200 Subject: [PATCH 3/8] Add an informant_output_format function to the cli config --- client/cli/src/config.rs | 9 ++++++++- client/service/test/src/lib.rs | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 1cb51937fb95f..775dcaa9ebee1 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -405,6 +405,13 @@ pub trait CliConfiguration: Sized { Ok(true) } + /// Get the format for the informant + /// + /// By default this enables colour and does not use a prefix. + fn informant_output_format(&self) -> sc_informant::OutputFormat { + Default::default() + } + /// Create a Configuration object from the current object fn create_configuration( &self, @@ -477,7 +484,7 @@ pub trait CliConfiguration: Sized { announce_block: self.announce_block()?, role, base_path: Some(base_path), - informant_output_format: sc_informant::OutputFormat::default(), + informant_output_format: self.informant_output_format(), }) } diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index c440b118d54a4..10b766ea34867 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -212,6 +212,7 @@ fn node_config Date: Mon, 22 Jun 2020 11:53:43 +0200 Subject: [PATCH 4/8] Wrap informant output format in an option --- client/cli/src/config.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 775dcaa9ebee1..d47ed343a443d 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -408,8 +408,8 @@ pub trait CliConfiguration: Sized { /// Get the format for the informant /// /// By default this enables colour and does not use a prefix. - fn informant_output_format(&self) -> sc_informant::OutputFormat { - Default::default() + fn informant_output_format(&self) -> Result { + Ok(Default::default()) } /// Create a Configuration object from the current object @@ -484,7 +484,7 @@ pub trait CliConfiguration: Sized { announce_block: self.announce_block()?, role, base_path: Some(base_path), - informant_output_format: self.informant_output_format(), + informant_output_format: self.informant_output_format()?, }) } From e0a6361fcaf4c0834fbb92e3afc62949243fe5d2 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 22 Jun 2020 12:32:02 +0200 Subject: [PATCH 5/8] Revert batch verifier --- primitives/io/src/batch_verifier.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/io/src/batch_verifier.rs b/primitives/io/src/batch_verifier.rs index 4ce564e89bf81..6a78070b38b55 100644 --- a/primitives/io/src/batch_verifier.rs +++ b/primitives/io/src/batch_verifier.rs @@ -160,7 +160,7 @@ impl BatchVerifier { #[must_use] pub fn verify_and_clear(&mut self) -> bool { let pending = std::mem::take(&mut self.pending_tasks); - let started = wasm_timer::Instant::now(); + let started = std::time::Instant::now(); log::trace!( target: "runtime", From 924c6879772b0f4fc57cd8738baa8cc0fc139f14 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Mon, 22 Jun 2020 12:36:41 +0200 Subject: [PATCH 6/8] Remove wasm-timer from primitives io cargo lock --- Cargo.lock | 1 - primitives/io/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b9e7006cd87ab..6d276efc32071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7615,7 +7615,6 @@ dependencies = [ "sp-tracing", "sp-trie", "sp-wasm-interface", - "wasm-timer", ] [[package]] diff --git a/primitives/io/Cargo.toml b/primitives/io/Cargo.toml index bb83e559af5c5..df66740d657ee 100644 --- a/primitives/io/Cargo.toml +++ b/primitives/io/Cargo.toml @@ -28,7 +28,6 @@ sp-tracing = { version = "2.0.0-rc3", default-features = false, path = "../traci log = { version = "0.4.8", optional = true } futures = { version = "0.3.1", features = ["thread-pool"], optional = true } parking_lot = { version = "0.10.0", optional = true } -wasm-timer = "0.2.4" [features] default = ["std"] From c409052883d20e963512c395283107dc9926c2ab Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 23 Jun 2020 12:01:23 +0200 Subject: [PATCH 7/8] Drop informant_output_format function --- client/cli/src/config.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index d47ed343a443d..8222ed274228b 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -405,13 +405,6 @@ pub trait CliConfiguration: Sized { Ok(true) } - /// Get the format for the informant - /// - /// By default this enables colour and does not use a prefix. - fn informant_output_format(&self) -> Result { - Ok(Default::default()) - } - /// Create a Configuration object from the current object fn create_configuration( &self, @@ -484,7 +477,7 @@ pub trait CliConfiguration: Sized { announce_block: self.announce_block()?, role, base_path: Some(base_path), - informant_output_format: self.informant_output_format()?, + informant_output_format: Default::default(), }) } From c0d4bbeb9a1acf9540cb0cca26eb4aff86ce3d29 Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 23 Jun 2020 16:02:05 +0200 Subject: [PATCH 8/8] derive debug for output format --- client/informant/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/informant/src/lib.rs b/client/informant/src/lib.rs index a4eafdddcc8d2..d56afcf335917 100644 --- a/client/informant/src/lib.rs +++ b/client/informant/src/lib.rs @@ -34,7 +34,7 @@ use parking_lot::Mutex; mod display; /// The format to print telemetry output in. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct OutputFormat { /// Enable color output in logs. True by default. pub enable_color: bool,