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
paritytech · Jun 23, 2020 · ad7b5ef · ad7b5ef
1 parent c771821
commit ad7b5ef
Show file tree

Hide file tree

Showing 10 changed files with 40 additions and 50 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -102,7 +102,6 @@ pub struct ServiceBuilder<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp,
 	remote_backend: Option<Arc<dyn RemoteBlockchain<TBl>>>,
 	marker: PhantomData<(TBl, TRtApi)>,
 	block_announce_validator_builder: Option<Box<dyn FnOnce(Arc<TCl>) -> Box<dyn BlockAnnounceValidator<TBl> + 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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, TRpc, Backend>
 			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<ServiceBuilder<TBl, TRtApi, TCl, TFchr, TSc, TImpQu, TFprb, TFpp,
-		TExPool, TRpc, Backend>, 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
@@ -109,6 +109,8 @@ pub struct Configuration {
 	pub announce_block: bool,
 	/// Base path of the configuration
 	pub base_path: Option<BasePath>,
+	/// 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
@@ -212,6 +212,7 @@ fn node_config<G: RuntimeGenesis + 'static, E: ChainSpecExtension + Clone + 'sta
 		max_runtime_instances: 8,
 		announce_block: true,
 		base_path: Some(BasePath::new(root)),
+		informant_output_format: Default::default(),
 	}
 }
 

diff --git a/primitives/consensus/common/Cargo.toml b/primitives/consensus/common/Cargo.toml
@@ -30,6 +30,7 @@ codec = { package = "parity-scale-codec", version = "1.3.1", 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
@@ -259,7 +259,7 @@ pub(crate) fn import_single_block_metered<B: BlockT, V: Verifier<B>, 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
@@ -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
@@ -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)