Add new hardware and software metrics (paritytech#11062)

* Add new hardware and software metrics * Move sysinfo tests into `mod tests` * Correct a typo in a comment * Remove unnecessary `nix` dependency * Fix the version tests * Add a `--disable-hardware-benchmarks` CLI argument * Disable hardware benchmarks in the integration tests * Remove unused import * Fix benchmarks compilation * Move code to a new `sc-sysinfo` crate * Correct `impl_version` comment * Move `--disable-hardware-benchmarks` to the chain-specific bin crate * Move printing out of hardware bench results to `sc-sysinfo` * Move hardware benchmarks to a separate messages; trigger them manually * Rename some of the fields in the `HwBench` struct * Revert changes to the telemetry crate; manually send hwbench messages * Move sysinfo logs into the sysinfo crate * Move the `TARGET_OS_*` constants into the sysinfo crate * Minor cleanups * Move the `HwBench` struct to the sysinfo crate * Derive `Clone` for `HwBench` * Fix broken telemetry connection notification stream * Prevent the telemetry connection notifiers from leaking if they're disconnected * Turn the telemetry notification failure log into a debug log * Rename `--disable-hardware-benchmarks` to `--no-hardware-benchmarks`
gear-tech · Feb 27, 2023 · c2d39de · c2d39de
1 parent c75db3e
commit c2d39de
Show file tree

Hide file tree

Showing 29 changed files with 808 additions and 63 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -54,6 +54,7 @@ members = [
 	"client/service",
 	"client/service/test",
 	"client/state-db",
+	"client/sysinfo",
 	"client/sync-state-rpc",
 	"client/telemetry",
 	"client/tracing",

diff --git a/bin/node/cli/Cargo.toml b/bin/node/cli/Cargo.toml
@@ -76,6 +76,7 @@ sc-telemetry = { version = "4.0.0-dev", path = "../../../client/telemetry" }
 sc-executor = { version = "0.10.0-dev", path = "../../../client/executor" }
 sc-authority-discovery = { version = "0.10.0-dev", path = "../../../client/authority-discovery" }
 sc-sync-state-rpc = { version = "0.10.0-dev", path = "../../../client/sync-state-rpc" }
+sc-sysinfo = { version = "6.0.0-dev", path = "../../../client/sysinfo" }
 
 # frame dependencies
 frame-system = { version = "4.0.0-dev", path = "../../../frame/system" }

diff --git a/bin/node/cli/benches/block_production.rs b/bin/node/cli/benches/block_production.rs
@@ -110,7 +110,8 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
 		wasm_runtime_overrides: None,
 	};
 
-	node_cli::service::new_full_base(config, |_, _| ()).expect("creating a full node doesn't fail")
+	node_cli::service::new_full_base(config, false, |_, _| ())
+		.expect("creating a full node doesn't fail")
 }
 
 fn extrinsic_set_time(now: u64) -> OpaqueExtrinsic {

diff --git a/bin/node/cli/benches/transaction_pool.rs b/bin/node/cli/benches/transaction_pool.rs
@@ -102,7 +102,7 @@ fn new_node(tokio_handle: Handle) -> node_cli::service::NewFullBase {
 		wasm_runtime_overrides: None,
 	};
 
-	node_cli::service::new_full_base(config, |_, _| ()).expect("Creates node")
+	node_cli::service::new_full_base(config, false, |_, _| ()).expect("Creates node")
 }
 
 fn create_accounts(num: usize) -> Vec<sr25519::Pair> {

diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs
@@ -471,7 +471,7 @@ pub(crate) mod tests {
 
 		sc_service_test::connectivity(integration_test_config_with_two_authorities(), |config| {
 			let NewFullBase { task_manager, client, network, transaction_pool, .. } =
-				new_full_base(config, |_, _| ())?;
+				new_full_base(config, false, |_, _| ())?;
 			Ok(sc_service_test::TestNetComponents::new(
 				task_manager,
 				client,

diff --git a/bin/node/cli/src/cli.rs b/bin/node/cli/src/cli.rs
@@ -26,6 +26,16 @@ pub struct Cli {
 	#[allow(missing_docs)]
 	#[clap(flatten)]
 	pub run: sc_cli::RunCmd,
+
+	/// Disable automatic hardware benchmarks.
+	///
+	/// By default these benchmarks are automatically ran at startup and measure
+	/// the CPU speed, the memory bandwidth and the disk speed.
+	///
+	/// The results are then printed out in the logs, and also sent as part of
+	/// telemetry, if telemetry is enabled.
+	#[clap(long)]
+	pub no_hardware_benchmarks: bool,
 }
 
 /// Possible subcommands of the main binary.

diff --git a/bin/node/cli/src/command.rs b/bin/node/cli/src/command.rs
@@ -86,7 +86,8 @@ pub fn run() -> Result<()> {
 		None => {
 			let runner = cli.create_runner(&cli.run)?;
 			runner.run_node_until_exit(|config| async move {
-				service::new_full(config).map_err(sc_cli::Error::Service)
+				service::new_full(config, cli.no_hardware_benchmarks)
+					.map_err(sc_cli::Error::Service)
 			})
 		},
 		Some(Subcommand::Inspect(cmd)) => {

diff --git a/bin/node/cli/src/service.rs b/bin/node/cli/src/service.rs
@@ -309,11 +309,21 @@ pub struct NewFullBase {
 /// Creates a full service from the configuration.
 pub fn new_full_base(
 	mut config: Configuration,
+	disable_hardware_benchmarks: bool,
 	with_startup_data: impl FnOnce(
 		&sc_consensus_babe::BabeBlockImport<Block, FullClient, FullGrandpaBlockImport>,
 		&sc_consensus_babe::BabeLink<Block>,
 	),
 ) -> Result<NewFullBase, ServiceError> {
+	let hwbench = if !disable_hardware_benchmarks {
+		config.database.path().map(|database_path| {
+			let _ = std::fs::create_dir_all(&database_path);
+			sc_sysinfo::gather_hwbench(Some(database_path))
+		})
+	} else {
+		None
+	};
+
 	let sc_service::PartialComponents {
 		client,
 		backend,
@@ -383,6 +393,19 @@ pub fn new_full_base(
 		telemetry: telemetry.as_mut(),
 	})?;
 
+	if let Some(hwbench) = hwbench {
+		sc_sysinfo::print_hwbench(&hwbench);
+
+		if let Some(ref mut telemetry) = telemetry {
+			let telemetry_handle = telemetry.handle();
+			task_manager.spawn_handle().spawn(
+				"telemetry_hwbench",
+				None,
+				sc_sysinfo::initialize_hwbench_telemetry(telemetry_handle, hwbench),
+			);
+		}
+	}
+
 	let (block_import, grandpa_link, babe_link) = import_setup;
 
 	(with_startup_data)(&block_import, &babe_link);
@@ -530,8 +553,12 @@ pub fn new_full_base(
 }
 
 /// Builds a new service for a full client.
-pub fn new_full(config: Configuration) -> Result<TaskManager, ServiceError> {
-	new_full_base(config, |_, _| ()).map(|NewFullBase { task_manager, .. }| task_manager)
+pub fn new_full(
+	config: Configuration,
+	disable_hardware_benchmarks: bool,
+) -> Result<TaskManager, ServiceError> {
+	new_full_base(config, disable_hardware_benchmarks, |_, _| ())
+		.map(|NewFullBase { task_manager, .. }| task_manager)
 }
 
 #[cfg(test)]
@@ -598,6 +625,7 @@ mod tests {
 				let NewFullBase { task_manager, client, network, transaction_pool, .. } =
 					new_full_base(
 						config,
+						false,
 						|block_import: &sc_consensus_babe::BabeBlockImport<Block, _, _>,
 						 babe_link: &sc_consensus_babe::BabeLink<Block>| {
 							setup_handles = Some((block_import.clone(), babe_link.clone()));
@@ -775,7 +803,7 @@ mod tests {
 			crate::chain_spec::tests::integration_test_config_with_two_authorities(),
 			|config| {
 				let NewFullBase { task_manager, client, network, transaction_pool, .. } =
-					new_full_base(config, |_, _| ())?;
+					new_full_base(config, false, |_, _| ())?;
 				Ok(sc_service_test::TestNetComponents::new(
 					task_manager,
 					client,

diff --git a/bin/node/cli/tests/check_block_works.rs b/bin/node/cli/tests/check_block_works.rs
@@ -28,7 +28,7 @@ pub mod common;
 async fn check_block_works() {
 	let base_path = tempdir().expect("could not create a temp dir");
 
-	common::run_node_for_a_while(base_path.path(), &["--dev"]).await;
+	common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await;
 
 	let status = Command::new(cargo_bin("substrate"))
 		.args(&["check-block", "--dev", "--pruning", "archive", "-d"])

diff --git a/bin/node/cli/tests/export_import_flow.rs b/bin/node/cli/tests/export_import_flow.rs
@@ -188,7 +188,7 @@ async fn export_import_revert() {
 	let exported_blocks_file = base_path.path().join("exported_blocks");
 	let db_path = base_path.path().join("db");
 
-	common::run_node_for_a_while(base_path.path(), &["--dev"]).await;
+	common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await;
 
 	let mut executor = ExportImportRevertExecutor::new(&base_path, &exported_blocks_file, &db_path);
 

diff --git a/bin/node/cli/tests/inspect_works.rs b/bin/node/cli/tests/inspect_works.rs
@@ -28,7 +28,7 @@ pub mod common;
 async fn inspect_works() {
 	let base_path = tempdir().expect("could not create a temp dir");
 
-	common::run_node_for_a_while(base_path.path(), &["--dev"]).await;
+	common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await;
 
 	let status = Command::new(cargo_bin("substrate"))
 		.args(&["inspect", "--dev", "--pruning", "archive", "-d"])

diff --git a/bin/node/cli/tests/purge_chain_works.rs b/bin/node/cli/tests/purge_chain_works.rs
@@ -27,7 +27,7 @@ pub mod common;
 async fn purge_chain_works() {
 	let base_path = tempdir().expect("could not create a temp dir");
 
-	common::run_node_for_a_while(base_path.path(), &["--dev"]).await;
+	common::run_node_for_a_while(base_path.path(), &["--dev", "--no-hardware-benchmarks"]).await;
 
 	let status = Command::new(cargo_bin("substrate"))
 		.args(&["purge-chain", "--dev", "-d"])

diff --git a/bin/node/cli/tests/running_the_node_and_interrupt.rs b/bin/node/cli/tests/running_the_node_and_interrupt.rs
@@ -38,6 +38,7 @@ async fn running_the_node_works_and_can_be_interrupted() {
 			Command::new(cargo_bin("substrate"))
 				.args(&["--dev", "-d"])
 				.arg(base_path.path())
+				.arg("--no-hardware-benchmarks")
 				.spawn()
 				.unwrap(),
 		);
@@ -61,7 +62,7 @@ async fn running_the_node_works_and_can_be_interrupted() {
 async fn running_two_nodes_with_the_same_ws_port_should_work() {
 	fn start_node() -> Child {
 		Command::new(cargo_bin("substrate"))
-			.args(&["--dev", "--tmp", "--ws-port=45789"])
+			.args(&["--dev", "--tmp", "--ws-port=45789", "--no-hardware-benchmarks"])
 			.spawn()
 			.unwrap()
 	}

diff --git a/bin/node/cli/tests/telemetry.rs b/bin/node/cli/tests/telemetry.rs
@@ -71,6 +71,7 @@ async fn telemetry_works() {
 	let mut substrate = substrate
 		.args(&["--dev", "--tmp", "--telemetry-url"])
 		.arg(format!("ws://{} 10", addr))
+		.arg("--no-hardware-benchmarks")
 		.stdout(process::Stdio::piped())
 		.stderr(process::Stdio::piped())
 		.stdin(process::Stdio::null())

diff --git a/bin/node/cli/tests/temp_base_path_works.rs b/bin/node/cli/tests/temp_base_path_works.rs
@@ -36,7 +36,7 @@ pub mod common;
 async fn temp_base_path_works() {
 	let mut cmd = Command::new(cargo_bin("substrate"));
 	let mut child = common::KillChildOnDrop(
-		cmd.args(&["--dev", "--tmp"])
+		cmd.args(&["--dev", "--tmp", "--no-hardware-benchmarks"])
 			.stdout(Stdio::piped())
 			.stderr(Stdio::piped())
 			.spawn()

diff --git a/bin/node/cli/tests/version.rs b/bin/node/cli/tests/version.rs
@@ -17,13 +17,11 @@
 // along with this program. If not, see <https://www.gnu.org/licenses/>.
 
 use assert_cmd::cargo::cargo_bin;
-use platforms::*;
 use regex::Regex;
 use std::process::Command;
 
 fn expected_regex() -> Regex {
-	Regex::new(r"^substrate (\d+\.\d+\.\d+(?:-.+?)?)-([a-f\d]+|unknown)-(.+?)-(.+?)(?:-(.+))?$")
-		.unwrap()
+	Regex::new(r"^substrate (.+)-([a-f\d]+)$").unwrap()
 }
 
 #[test]
@@ -37,33 +35,17 @@ fn version_is_full() {
 	let captures = expected.captures(output.as_str()).expect("could not parse version in output");
 
 	assert_eq!(&captures[1], env!("CARGO_PKG_VERSION"));
-	assert_eq!(&captures[3], TARGET_ARCH.as_str());
-	assert_eq!(&captures[4], TARGET_OS.as_str());
-	assert_eq!(captures.get(5).map(|x| x.as_str()), TARGET_ENV.map(|x| x.as_str()));
 }
 
 #[test]
 fn test_regex_matches_properly() {
 	let expected = expected_regex();
 
-	let captures = expected.captures("substrate 2.0.0-da487d19d-x86_64-linux-gnu").unwrap();
+	let captures = expected.captures("substrate 2.0.0-da487d19d").unwrap();
 	assert_eq!(&captures[1], "2.0.0");
 	assert_eq!(&captures[2], "da487d19d");
-	assert_eq!(&captures[3], "x86_64");
-	assert_eq!(&captures[4], "linux");
-	assert_eq!(captures.get(5).map(|x| x.as_str()), Some("gnu"));
 
-	let captures = expected.captures("substrate 2.0.0-alpha.5-da487d19d-x86_64-linux-gnu").unwrap();
+	let captures = expected.captures("substrate 2.0.0-alpha.5-da487d19d").unwrap();
 	assert_eq!(&captures[1], "2.0.0-alpha.5");
 	assert_eq!(&captures[2], "da487d19d");
-	assert_eq!(&captures[3], "x86_64");
-	assert_eq!(&captures[4], "linux");
-	assert_eq!(captures.get(5).map(|x| x.as_str()), Some("gnu"));
-
-	let captures = expected.captures("substrate 2.0.0-alpha.5-da487d19d-x86_64-linux").unwrap();
-	assert_eq!(&captures[1], "2.0.0-alpha.5");
-	assert_eq!(&captures[2], "da487d19d");
-	assert_eq!(&captures[3], "x86_64");
-	assert_eq!(&captures[4], "linux");
-	assert_eq!(captures.get(5).map(|x| x.as_str()), None);
 }
diff --git a/client/cli/src/lib.rs b/client/cli/src/lib.rs
@@ -56,7 +56,7 @@ pub trait SubstrateCli: Sized {
 	///
 	/// By default this will look like this:
 	///
-	/// `2.0.0-b950f731c-x86_64-linux-gnu`
+	/// `2.0.0-b950f731c`
 	///
 	/// Where the hash is the short commit hash of the commit of in the Git repository.
 	fn impl_version() -> String;

diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml
@@ -72,6 +72,7 @@ sc-offchain = { version = "4.0.0-dev", path = "../offchain" }
 prometheus-endpoint = { package = "substrate-prometheus-endpoint", path = "../../utils/prometheus", version = "0.10.0-dev" }
 sc-tracing = { version = "4.0.0-dev", path = "../tracing" }
 sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" }
+sc-sysinfo = { version = "6.0.0-dev", path = "../sysinfo" }
 tracing = "0.1.29"
 tracing-futures = { version = "0.2.4" }
 parity-util-mem = { version = "0.11.0", default-features = false, features = [

diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs
@@ -487,8 +487,13 @@ where
 	)
 	.map_err(|e| Error::Application(Box::new(e)))?;
 
+	let sysinfo = sc_sysinfo::gather_sysinfo();
+	sc_sysinfo::print_sysinfo(&sysinfo);
+
 	let telemetry = telemetry
-		.map(|telemetry| init_telemetry(&mut config, network.clone(), client.clone(), telemetry))
+		.map(|telemetry| {
+			init_telemetry(&mut config, network.clone(), client.clone(), telemetry, Some(sysinfo))
+		})
 		.transpose()?;
 
 	info!("📦 Highest known block at #{}", chain_info.best_number);
@@ -609,12 +614,16 @@ fn init_telemetry<TBl: BlockT, TCl: BlockBackend<TBl>>(
 	network: Arc<NetworkService<TBl, <TBl as BlockT>::Hash>>,
 	client: Arc<TCl>,
 	telemetry: &mut Telemetry,
+	sysinfo: Option<sc_telemetry::SysInfo>,
 ) -> sc_telemetry::Result<TelemetryHandle> {
 	let genesis_hash = client.block_hash(Zero::zero()).ok().flatten().unwrap_or_default();
 	let connection_message = ConnectionMessage {
 		name: config.network.node_name.to_owned(),
 		implementation: config.impl_name.to_owned(),
 		version: config.impl_version.to_owned(),
+		target_os: sc_sysinfo::TARGET_OS.into(),
+		target_arch: sc_sysinfo::TARGET_ARCH.into(),
+		target_env: sc_sysinfo::TARGET_ENV.into(),
 		config: String::new(),
 		chain: config.chain_spec.name().to_owned(),
 		genesis_hash: format!("{:?}", genesis_hash),
@@ -625,6 +634,7 @@ fn init_telemetry<TBl: BlockT, TCl: BlockBackend<TBl>>(
 			.unwrap_or(0)
 			.to_string(),
 		network_id: network.local_peer_id().to_base58(),
+		sysinfo,
 	};
 
 	telemetry.start_telemetry(connection_message)?;