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

Add new hardware and software metrics #11062

Merged
merged 26 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
dccce70
Add new hardware and software metrics
koute Mar 18, 2022
99633f9
Move sysinfo tests into `mod tests`
koute Mar 23, 2022
55c7358
Correct a typo in a comment
koute Mar 23, 2022
909df21
Remove unnecessary `nix` dependency
koute Mar 23, 2022
706e4d3
Fix the version tests
koute Mar 23, 2022
c1e409f
Add a `--disable-hardware-benchmarks` CLI argument
koute Mar 24, 2022
a05159a
Disable hardware benchmarks in the integration tests
koute Mar 25, 2022
fff4573
Remove unused import
koute Mar 25, 2022
3d15f0e
Fix benchmarks compilation
koute Mar 25, 2022
a8c71c1
Merge remote-tracking branch 'origin/master' into master_hwswinfo
koute Mar 25, 2022
d215b39
Move code to a new `sc-sysinfo` crate
koute Mar 28, 2022
c1da1fa
Correct `impl_version` comment
koute Mar 28, 2022
b98bcfa
Move `--disable-hardware-benchmarks` to the chain-specific bin crate
koute Mar 28, 2022
0d25408
Move printing out of hardware bench results to `sc-sysinfo`
koute Mar 28, 2022
46c9de2
Move hardware benchmarks to a separate messages; trigger them manually
koute Mar 29, 2022
56a1e3a
Rename some of the fields in the `HwBench` struct
koute Mar 31, 2022
457b4da
Revert changes to the telemetry crate; manually send hwbench messages
koute Apr 4, 2022
bb26cfa
Move sysinfo logs into the sysinfo crate
koute Apr 4, 2022
77f9c5f
Move the `TARGET_OS_*` constants into the sysinfo crate
koute Apr 4, 2022
73b230a
Minor cleanups
koute Apr 5, 2022
c3214e5
Move the `HwBench` struct to the sysinfo crate
koute Apr 5, 2022
b6d78a0
Derive `Clone` for `HwBench`
koute Apr 5, 2022
19f43ab
Fix broken telemetry connection notification stream
koute Apr 5, 2022
15e5f9e
Prevent the telemetry connection notifiers from leaking if they're di…
koute Apr 5, 2022
e5f210f
Turn the telemetry notification failure log into a debug log
koute Apr 6, 2022
0cafbf8
Rename `--disable-hardware-benchmarks` to `--no-hardware-benchmarks`
koute Apr 6, 2022
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
3 changes: 3 additions & 0 deletions Cargo.lock

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

3 changes: 2 additions & 1 deletion bin/node/cli/tests/check_block_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ 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", "--disable-hardware-benchmarks"])
.await;

let status = Command::new(cargo_bin("substrate"))
.args(&["check-block", "--dev", "--pruning", "archive", "-d"])
Expand Down
3 changes: 2 additions & 1 deletion bin/node/cli/tests/export_import_flow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ 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", "--disable-hardware-benchmarks"])
.await;

let mut executor = ExportImportRevertExecutor::new(&base_path, &exported_blocks_file, &db_path);

Expand Down
3 changes: 2 additions & 1 deletion bin/node/cli/tests/inspect_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ 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", "--disable-hardware-benchmarks"])
.await;

let status = Command::new(cargo_bin("substrate"))
.args(&["inspect", "--dev", "--pruning", "archive", "-d"])
Expand Down
3 changes: 2 additions & 1 deletion bin/node/cli/tests/purge_chain_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ 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", "--disable-hardware-benchmarks"])
.await;

let status = Command::new(cargo_bin("substrate"))
.args(&["purge-chain", "--dev", "-d"])
Expand Down
3 changes: 2 additions & 1 deletion bin/node/cli/tests/running_the_node_and_interrupt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ async fn running_the_node_works_and_can_be_interrupted() {
Command::new(cargo_bin("substrate"))
.args(&["--dev", "-d"])
.arg(base_path.path())
.arg("--disable-hardware-benchmarks")
.spawn()
.unwrap(),
);
Expand All @@ -64,7 +65,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", "--disable-hardware-benchmarks"])
.spawn()
.unwrap()
}
Expand Down
1 change: 1 addition & 0 deletions bin/node/cli/tests/telemetry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ async fn telemetry_works() {
let mut substrate = substrate
.args(&["--dev", "--tmp", "--telemetry-url"])
.arg(format!("ws://{} 10", addr))
.arg("--disable-hardware-benchmarks")
.stdout(process::Stdio::piped())
.stderr(process::Stdio::piped())
.stdin(process::Stdio::null())
Expand Down
2 changes: 1 addition & 1 deletion bin/node/cli/tests/temp_base_path_works.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,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", "--disable-hardware-benchmarks"])
.stdout(Stdio::piped())
.stderr(Stdio::piped())
.spawn()
Expand Down
23 changes: 3 additions & 20 deletions bin/node/cli/tests/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ 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]
Expand All @@ -37,33 +36,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);
}
14 changes: 14 additions & 0 deletions client/cli/src/commands/run_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,16 @@ pub struct RunCmd {
/// When `--dev` is given and no explicit `--base-path`, this option is implied.
#[clap(long, conflicts_with = "base-path")]
pub tmp: bool,

/// 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 disable_hardware_benchmarks: bool,
}

impl RunCmd {
Expand Down Expand Up @@ -477,6 +487,10 @@ impl CliConfiguration for RunCmd {
}
})
}

fn disable_hardware_benchmarks(&self) -> Result<bool> {
Ok(self.disable_hardware_benchmarks)
}
}

/// Check whether a node name is considered as valid.
Expand Down
8 changes: 8 additions & 0 deletions client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,13 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
Ok(Default::default())
}

/// Returns `Ok(true)` if the hardware benchmarks should be disabled
///
/// By default this is `true` unless overriden.
fn disable_hardware_benchmarks(&self) -> Result<bool> {
Ok(true)
}

/// Get the development key seed from the current object
///
/// By default this is `None`.
Expand Down Expand Up @@ -546,6 +553,7 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {
base_path: Some(base_path),
informant_output_format: Default::default(),
runtime_cache_size,
disable_hardware_benchmarks: self.disable_hardware_benchmarks()?,
})
}

Expand Down
3 changes: 3 additions & 0 deletions client/service/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ futures = "0.3.19"
jsonrpc-pubsub = "18.0"
jsonrpc-core = "18.0"
rand = "0.7.3"
rand_pcg = "0.2.1"
parking_lot = "0.12.0"
log = "0.4.11"
futures-timer = "3.0.1"
Expand Down Expand Up @@ -81,6 +82,8 @@ async-trait = "0.1.50"
tokio = { version = "1.15", features = ["time", "rt-multi-thread", "parking_lot"] }
tempfile = "3.1.0"
directories = "4.0.1"
regex = "1"
libc = "0.2"

[dev-dependencies]
substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/runtime/client" }
Expand Down
31 changes: 31 additions & 0 deletions client/service/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// This file is part of Substrate.

// Copyright (C) 2022 Parity Technologies (UK) Ltd.
// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0

// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

fn main() {
let out_dir = std::env::var("OUT_DIR").expect("OUT_DIR is always set in build scripts; qed");
let out_dir = std::path::PathBuf::from(out_dir);
let target_os = std::env::var("CARGO_CFG_TARGET_OS")
.expect("CARGO_CFG_TARGET_OS is always set in build scripts; qed");
let target_arch = std::env::var("CARGO_CFG_TARGET_ARCH")
.expect("CARGO_CFG_TARGET_ARCH is always set in build scripts; qed");
let target_env = std::env::var("CARGO_CFG_TARGET_ENV")
.expect("CARGO_CFG_TARGET_ENV is always set in build scripts; qed");
std::fs::write(out_dir.join("target_os.txt"), target_os).unwrap();
std::fs::write(out_dir.join("target_arch.txt"), target_arch).unwrap();
std::fs::write(out_dir.join("target_env.txt"), target_env).unwrap();
}
77 changes: 76 additions & 1 deletion client/service/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ use sp_runtime::{
};
use std::{str::FromStr, sync::Arc, time::SystemTime};

const TARGET_OS: &str = include_str!(concat!(env!("OUT_DIR"), "/target_os.txt"));
const TARGET_ARCH: &str = include_str!(concat!(env!("OUT_DIR"), "/target_arch.txt"));
const TARGET_ENV: &str = include_str!(concat!(env!("OUT_DIR"), "/target_env.txt"));

/// A utility trait for building an RPC extension given a `DenyUnsafe` instance.
/// This is useful since at service definition time we don't know whether the
/// specific interface where the RPC extension will be exposed is safe or not.
Expand Down Expand Up @@ -487,8 +491,72 @@ where
)
.map_err(|e| Error::Application(Box::new(e)))?;

let database_path = match config.database {
koute marked this conversation as resolved.
Show resolved Hide resolved
sc_client_db::DatabaseSource::Auto { ref paritydb_path, ref rocksdb_path, .. } =>
if rocksdb_path.exists() {
Some(rocksdb_path.as_path())
} else {
Some(paritydb_path.as_path())
},
sc_client_db::DatabaseSource::RocksDb { ref path, .. } |
sc_client_db::DatabaseSource::ParityDb { ref path, .. } => Some(path.as_path()),
sc_client_db::DatabaseSource::Custom { .. } => None,
};

let sysinfo = crate::sysinfo::gather_sysinfo();
let hwbench = if config.disable_hardware_benchmarks {
None
} else {
Some(crate::sysinfo::gather_hwbench(database_path))
};

info!("💻 Operating system: {}", TARGET_OS);
info!("💻 CPU architecture: {}", TARGET_ARCH);
if !TARGET_ENV.is_empty() {
info!("💻 Target environment: {}", TARGET_ENV);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just looking at these log lines, I would get the impression that the properties of the running system are listed, not those of the build target. I know it is really an edge-case, but foreign ELF formats can be loaded and run emulated on a different system. Are we okay with ignoring those fringe usages?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm... well, that is a good point; I don't think we have to care about this in general though since those should mostly be really fringe cases, and detecting this will most likely not be easy. (That said, if anyone has any counterpoints here or any good ideas how to handle this in a reasonable way I'm all ears.)

I guess the most likely cases here would be either someone running the Linux binary on a BSD system, or someone running an amd64 binary on an M1 Mac (but we don't provide binaries for macOS, so they'd have to compile it themselves, and if they're compiling it themselves then why not compile a native aarch64 binary in the first place and run that?).

if let Some(ref cpu) = sysinfo.cpu {
info!("💻 CPU: {}", cpu);
}
if let Some(core_count) = sysinfo.core_count {
info!("💻 CPU cores: {}", core_count);
}
if let Some(memory) = sysinfo.memory {
info!("💻 Memory: {}MB", memory / (1024 * 1024));
}
if let Some(ref linux_kernel) = sysinfo.linux_kernel {
info!("💻 Kernel: {}", linux_kernel);
}
if let Some(ref linux_distro) = sysinfo.linux_distro {
info!("💻 Linux distribution: {}", linux_distro);
}
if let Some(is_virtual_machine) = sysinfo.is_virtual_machine {
info!("💻 Virtual machine: {}", if is_virtual_machine { "yes" } else { "no" });
}

if let Some(ref hwbench) = hwbench {
info!("🏁 CPU score: {}MB/s", hwbench.cpu_score);
info!("🏁 Memory score: {}MB/s", hwbench.memory_score);

if let Some(score) = hwbench.disk_sequential_write_score {
info!("🏁 Disk score (seq. writes): {}MB/s", score);
}
if let Some(score) = hwbench.disk_random_write_score {
info!("🏁 Disk score (rand. writes): {}MB/s", score);
}
}

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),
hwbench,
)
})
.transpose()?;

info!("📦 Highest known block at #{}", chain_info.best_number);
Expand Down Expand Up @@ -609,12 +677,17 @@ 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>,
hwbench: Option<sc_telemetry::HwBench>,
) -> 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: TARGET_OS.into(),
target_arch: TARGET_ARCH.into(),
target_env: TARGET_ENV.into(),
config: String::new(),
chain: config.chain_spec.name().to_owned(),
genesis_hash: format!("{:?}", genesis_hash),
Expand All @@ -625,6 +698,8 @@ fn init_telemetry<TBl: BlockT, TCl: BlockBackend<TBl>>(
.unwrap_or(0)
.to_string(),
network_id: network.local_peer_id().to_base58(),
sysinfo,
hwbench,
};

telemetry.start_telemetry(connection_message)?;
Expand Down
8 changes: 8 additions & 0 deletions client/service/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,14 @@ pub struct Configuration {
pub informant_output_format: sc_informant::OutputFormat,
/// Maximum number of different runtime versions that can be cached.
pub runtime_cache_size: u8,
/// 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.
pub disable_hardware_benchmarks: bool,
koute marked this conversation as resolved.
Show resolved Hide resolved
}

/// Type for tasks spawned by the executor.
Expand Down
4 changes: 4 additions & 0 deletions client/service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ mod client;
mod metrics;
mod task_manager;

mod sysinfo;
#[cfg(target_os = "linux")]
mod sysinfo_linux;

use std::{collections::HashMap, io, net::SocketAddr, pin::Pin};

use codec::{Decode, Encode};
Expand Down
Loading