From ad7b5ef7f2131eadceb1f89a69381cd359ef1eb9 Mon Sep 17 00:00:00 2001 From: Ashley Date: Tue, 23 Jun 2020 16:50:33 +0200 Subject: [PATCH] Fix the browser node and ensure it doesn't colour the informant output (#6457) * Fix browser informant * Fix documentation * Add an informant_output_format function to the cli config * Wrap informant output format in an option * Revert batch verifier * Remove wasm-timer from primitives io cargo lock * Drop informant_output_format function * derive debug for output format --- Cargo.lock | 1 + client/cli/src/config.rs | 1 + client/informant/src/lib.rs | 29 ++++++++++-- client/service/src/builder.rs | 46 +------------------ client/service/src/config.rs | 2 + client/service/test/src/lib.rs | 1 + primitives/consensus/common/Cargo.toml | 1 + .../consensus/common/src/import_queue.rs | 2 +- .../consensus/common/src/offline_tracker.rs | 3 +- utils/browser/src/lib.rs | 4 ++ 10 files changed, 40 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4ffae8d5626ac..64524d8c5283d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7442,6 +7442,7 @@ dependencies = [ "sp-utils", "sp-version", "substrate-prometheus-endpoint", + "wasm-timer", ] [[package]] diff --git a/client/cli/src/config.rs b/client/cli/src/config.rs index 0ff2d96b4cc03..598acd0ab913e 100644 --- a/client/cli/src/config.rs +++ b/client/cli/src/config.rs @@ -474,6 +474,7 @@ pub trait CliConfiguration: Sized { announce_block: self.announce_block()?, role, base_path: Some(base_path), + informant_output_format: Default::default(), }) } diff --git a/client/informant/src/lib.rs b/client/informant/src/lib.rs index 6a8acbadc36be..d56afcf335917 100644 --- a/client/informant/src/lib.rs +++ b/client/informant/src/lib.rs @@ -34,14 +34,37 @@ 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. + /// Enable color output in logs. True by default. 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 setting this 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 f492c5d4940ca..eebc825b212d9 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 618cd19692196..fb4dbc666a9d6 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -109,6 +109,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/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 441680e20c0df..4ff89f5319ff4 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -212,6 +212,7 @@ fn node_config, 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/utils/browser/src/lib.rs b/utils/browser/src/lib.rs index badb029bfe414..799fe9788ca57 100644 --- a/utils/browser/src/lib.rs +++ b/utils/browser/src/lib.rs @@ -98,6 +98,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)