From afa2bc648d5828a77190d35e60bb21170114d541 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Tue, 1 Mar 2022 16:06:05 +0100 Subject: [PATCH 1/6] use 'moonbeam' prefix when emitting prometheus metrics --- Cargo.lock | 13 ++-- node/cli/Cargo.toml | 1 + node/service/Cargo.toml | 2 + node/service/src/lib.rs | 148 ++++++++++++++++++++++++++++++++++++++-- 4 files changed, 154 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00670111c0..2638b01e50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2458,9 +2458,9 @@ checksum = "b02a61fea82de8484f9d7ce99b698b0e190ef8de71035690a961a43d7b18a2ad" [[package]] name = "fastrand" -version = "1.5.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b394ed3d285a429378d3b384b9eb1285267e7df4b166df24b7a6939a04dc392e" +checksum = "c3fcf0cee53519c866c09b5de1f6c56ff9d647101f81c1964fa632e148896cdf" dependencies = [ "instant", ] @@ -5369,6 +5369,7 @@ dependencies = [ "sp-runtime", "structopt", "substrate-build-script-utils", + "substrate-prometheus-endpoint", "try-runtime-cli", ] @@ -5818,6 +5819,7 @@ dependencies = [ "polkadot-primitives", "polkadot-runtime-common", "polkadot-service", + "prometheus", "rand 0.7.3", "sc-basic-authorship", "sc-chain-spec", @@ -5861,6 +5863,7 @@ dependencies = [ "substrate-prometheus-endpoint", "substrate-test-client", "substrate-test-runtime-client", + "tempfile", "tiny-bip39", "tiny-hderive", "tokio", @@ -12963,13 +12966,13 @@ checksum = "b0652da4c4121005e9ed22b79f6c5f2d9e2752906b53a33e9490489ba421a6fb" [[package]] name = "tempfile" -version = "3.2.0" +version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dac1c663cfc93810f88aed9b8941d48cabf856a1b111c29a40439018d870eb22" +checksum = "5cdb1ef4eaeeaddc8fbd371e5017057064af0911902ef36b39801f67cc6d79e4" dependencies = [ "cfg-if 1.0.0", + "fastrand", "libc", - "rand 0.8.4", "redox_syscall 0.2.9", "remove_dir_all", "winapi 0.3.9", diff --git a/node/cli/Cargo.toml b/node/cli/Cargo.toml index 15453581ae..42d26d63b2 100644 --- a/node/cli/Cargo.toml +++ b/node/cli/Cargo.toml @@ -22,6 +22,7 @@ sc-telemetry = { git = "https://github.com/purestake/substrate", branch = "moonb sc-tracing = { git = "https://github.com/purestake/substrate", branch = "moonbeam-polkadot-v0.9.16" } sp-core = { git = "https://github.com/purestake/substrate", branch = "moonbeam-polkadot-v0.9.16" } sp-runtime = { git = "https://github.com/purestake/substrate", branch = "moonbeam-polkadot-v0.9.16" } +substrate-prometheus-endpoint = { git = "https://github.com/purestake/substrate", branch = "moonbeam-polkadot-v0.9.16" } try-runtime-cli = { git = "https://github.com/purestake/substrate", branch = "moonbeam-polkadot-v0.9.16", optional = true } # Cumulus / Nimbus diff --git a/node/service/Cargo.toml b/node/service/Cargo.toml index f98070d15d..363899f50a 100644 --- a/node/service/Cargo.toml +++ b/node/service/Cargo.toml @@ -128,7 +128,9 @@ frame-benchmarking-cli = { git = "https://github.com/purestake/substrate", branc [dev-dependencies] assert_cmd = "0.12" nix = "0.17" +prometheus = { version = "0.13.0", default-features = false } rand = "0.7.3" +tempfile = "3.3.0" # Polkadot dev-dependencies polkadot-runtime-common = { git = "https://github.com/purestake/polkadot", branch = "moonbeam-polkadot-v0.9.16" } diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index bfd817f2e9..977fd5317c 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -50,6 +50,7 @@ use nimbus_consensus::{BuildNimbusConsensusParams, NimbusConsensus}; use nimbus_primitives::NimbusId; use sc_executor::{NativeElseWasmExecutor, NativeExecutionDispatch}; use sc_network::NetworkService; +use sc_service::config::PrometheusConfig; use sc_service::{ error::Error as ServiceError, BasePath, ChainSpec, Configuration, PartialComponents, Role, TFullBackend, TFullClient, TaskManager, @@ -277,13 +278,22 @@ where )) } +// If we're using prometheus, use a registry with a prefix of `moonbeam`. +fn set_prometheus_registry(config: &mut Configuration) -> Result<(), ServiceError> { + if let Some(PrometheusConfig { registry, .. }) = config.prometheus_config.as_mut() { + *registry = Registry::new_custom(Some("moonbeam".into()), None)?; + } + + Ok(()) +} + /// Builds the PartialComponents for a parachain or development service /// /// Use this function if you don't actually need the full service, but just the partial in order to /// be able to perform chain operations. #[allow(clippy::type_complexity)] pub fn new_partial( - config: &Configuration, + config: &mut Configuration, dev_service: bool, ) -> Result< PartialComponents< @@ -314,6 +324,8 @@ where RuntimeApiCollection>, Executor: NativeExecutionDispatch + 'static, { + set_prometheus_registry(config)?; + let telemetry = config .telemetry_endpoints .clone() @@ -443,9 +455,9 @@ where return Err("Light client not supported!".into()); } - let parachain_config = prepare_node_config(parachain_config); + let mut parachain_config = prepare_node_config(parachain_config); - let params = new_partial(¶chain_config, false)?; + let params = new_partial(&mut parachain_config, false)?; let ( _block_import, filter_pool, @@ -723,7 +735,7 @@ where /// Builds a new development service. This service uses manual seal, and mocks /// the parachain inherent. pub fn new_dev( - config: Configuration, + mut config: Configuration, _author_id: Option, sealing: cli_opt::Sealing, rpc_config: RpcConfig, @@ -757,7 +769,7 @@ where frontier_backend, fee_history_cache, ), - } = new_partial::(&config, true)?; + } = new_partial::(&mut config, true)?; let (network, system_rpc_tx, network_starter) = sc_service::build_network(sc_service::BuildNetworkParams { @@ -985,3 +997,129 @@ where network_starter.start_network(); Ok(task_manager) } + +#[cfg(test)] +mod tests { + use prometheus::Counter; + use sc_network::{ + config::{NetworkConfiguration, TransportConfig}, + multiaddr, + }; + use sc_service::{ + config::{BasePath, DatabaseSource, KeystoreConfig}, + Configuration, KeepBlocks, Role, TransactionStorageMode, + }; + use std::{iter, net::Ipv4Addr}; + use tempfile::TempDir; + + use crate::chain_spec::test_spec::staking_spec; + + use super::*; + + #[test] + fn test_prometheus_registry_uses_moonbeam_prefix() { + let counter_name = "my_counter"; + let expected_metric_name = "moonbeam_my_counter"; + let counter = Box::new(Counter::new(counter_name, "foobar").unwrap()); + let mut config = Configuration { + prometheus_config: Some(PrometheusConfig::new_with_default_registry( + "0.0.0.0:8080".parse().unwrap(), + "".into(), + )), + ..test_config() + }; + + set_prometheus_registry(&mut config).unwrap(); + // generate metric + let reg = config.prometheus_registry().unwrap(); + reg.register(counter.clone()).unwrap(); + counter.inc(); + + let actual_metric_name = reg.gather().first().unwrap().get_name().to_string(); + assert_eq!(actual_metric_name.as_str(), expected_metric_name); + } + + fn test_config() -> Configuration { + let index = 0; + let base_port = 9898; + + let root = TempDir::new().unwrap(); + let root = root.path().join(format!("node-{}", index)); + + let mut network_config = NetworkConfiguration::new( + format!("Node {}", index), + "network/test/0.1", + Default::default(), + None, + ); + + network_config.allow_non_globals_in_dht = true; + + network_config.listen_addresses.push( + iter::once(multiaddr::Protocol::Ip4(Ipv4Addr::new(127, 0, 0, 1))) + .chain(iter::once(multiaddr::Protocol::Tcp( + base_port + index as u16, + ))) + .collect(), + ); + + network_config.transport = TransportConfig::Normal { + enable_mdns: false, + allow_private_ipv4: true, + }; + + let runtime = + tokio::runtime::Runtime::new().expect("creating tokio runtime doesn't fail; qed"); + let tokio_handle = runtime.handle().clone(); + let spec = staking_spec(ParaId::new(0)); + + Configuration { + impl_name: String::from("network-test-impl"), + impl_version: String::from("0.1"), + role: Role::Full, + tokio_handle, + transaction_pool: Default::default(), + network: network_config, + keystore_remote: Default::default(), + keystore: KeystoreConfig::Path { + path: root.join("key"), + password: None, + }, + database: DatabaseSource::RocksDb { + path: root.join("db"), + cache_size: 128, + }, + state_cache_size: 16777216, + state_cache_child_ratio: None, + state_pruning: Default::default(), + keep_blocks: KeepBlocks::All, + transaction_storage: TransactionStorageMode::BlockBody, + chain_spec: Box::new(spec), + wasm_method: sc_service::config::WasmExecutionMethod::Interpreted, + wasm_runtime_overrides: Default::default(), + execution_strategies: Default::default(), + rpc_http: None, + rpc_ipc: None, + rpc_ws: None, + rpc_ws_max_connections: None, + rpc_cors: None, + rpc_methods: Default::default(), + rpc_max_payload: None, + ws_max_out_buffer_capacity: None, + prometheus_config: None, + telemetry_endpoints: None, + default_heap_pages: None, + offchain_worker: Default::default(), + force_authoring: false, + disable_grandpa: false, + dev_key_seed: None, + tracing_targets: None, + tracing_receiver: Default::default(), + max_runtime_instances: 8, + announce_block: true, + base_path: Some(BasePath::new(root)), + informant_output_format: Default::default(), + runtime_cache_size: 2, + } + } +} From 1a4537f073cef5af2ab9bfc768bb5d2620978c88 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Tue, 1 Mar 2022 16:11:42 +0100 Subject: [PATCH 2/6] rename test --- node/service/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 977fd5317c..a35a0e0698 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -1017,7 +1017,7 @@ mod tests { use super::*; #[test] - fn test_prometheus_registry_uses_moonbeam_prefix() { + fn test_set_prometheus_registry_uses_moonbeam_prefix() { let counter_name = "my_counter"; let expected_metric_name = "moonbeam_my_counter"; let counter = Box::new(Counter::new(counter_name, "foobar").unwrap()); From e8235fc4047e3ad46e288183a1ba1cc0c004592f Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Tue, 1 Mar 2022 16:29:05 +0100 Subject: [PATCH 3/6] fix argument mutability in perf-test --- node/perf-test/src/command.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/node/perf-test/src/command.rs b/node/perf-test/src/command.rs index 066c6cf4a8..6d38b947ed 100644 --- a/node/perf-test/src/command.rs +++ b/node/perf-test/src/command.rs @@ -75,7 +75,7 @@ where RuntimeApiCollection>, Executor: NativeExecutionDispatch + 'static, { - pub fn from_cmd(config: Configuration, _cmd: &PerfCmd) -> CliResult { + pub fn from_cmd(mut config: Configuration, _cmd: &PerfCmd) -> CliResult { println!("perf-test from_cmd"); let sc_service::PartialComponents { client, @@ -94,7 +94,7 @@ where frontier_backend, fee_history_cache, ), - } = service::new_partial::(&config, true)?; + } = service::new_partial::(&mut config, true)?; // TODO: review -- we don't need any actual networking let (network, system_rpc_tx, network_starter) = From 85894d1817da358573ef5ce8c9930dd639739ef4 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Tue, 1 Mar 2022 23:31:58 +0100 Subject: [PATCH 4/6] use chain-id as const label, simplify tests --- Cargo.lock | 1 - node/service/Cargo.toml | 1 - node/service/src/lib.rs | 121 ++++++++++++++++++++++++++-------------- 3 files changed, 78 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a735560da9..6bcbf75e96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5872,7 +5872,6 @@ dependencies = [ "substrate-prometheus-endpoint 0.10.0-dev (git+https://github.com/purestake/substrate?branch=moonbeam-polkadot-v0.9.17)", "substrate-test-client", "substrate-test-runtime-client", - "tempfile", "tiny-bip39", "tiny-hderive", "tokio", diff --git a/node/service/Cargo.toml b/node/service/Cargo.toml index 166b85a545..1cc0ad835f 100644 --- a/node/service/Cargo.toml +++ b/node/service/Cargo.toml @@ -131,7 +131,6 @@ assert_cmd = "0.12" nix = "0.17" prometheus = { version = "0.13.0", default-features = false } rand = "0.7.3" -tempfile = "3.3.0" # Polkadot dev-dependencies polkadot-runtime-common = { git = "https://github.com/purestake/polkadot", branch = "moonbeam-polkadot-v0.9.17" } diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 59aa61f70c..d80dbffccc 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -32,6 +32,7 @@ pub use moonbase_runtime; pub use moonbeam_runtime; #[cfg(feature = "moonriver-native")] pub use moonriver_runtime; +use std::iter; use std::{collections::BTreeMap, sync::Mutex, time::Duration}; pub mod rpc; use cumulus_client_consensus_common::ParachainConsensus; @@ -281,7 +282,8 @@ where // If we're using prometheus, use a registry with a prefix of `moonbeam`. fn set_prometheus_registry(config: &mut Configuration) -> Result<(), ServiceError> { if let Some(PrometheusConfig { registry, .. }) = config.prometheus_config.as_mut() { - *registry = Registry::new_custom(Some("moonbeam".into()), None)?; + let labels = iter::once((String::from("chain"), config.chain_spec.id().into())).collect(); + *registry = Registry::new_custom(Some("moonbeam".into()), Some(labels))?; } Ok(()) @@ -1018,19 +1020,19 @@ where #[cfg(test)] mod tests { - use prometheus::Counter; - use sc_network::{ - config::{NetworkConfiguration, TransportConfig}, - multiaddr, - }; + use moonbase_runtime::{currency::UNIT, AccountId}; + use prometheus::{proto::LabelPair, Counter}; + use sc_network::config::NetworkConfiguration; + use sc_service::ChainType; use sc_service::{ config::{BasePath, DatabaseSource, KeystoreConfig}, Configuration, KeepBlocks, Role, TransactionStorageMode, }; - use std::{iter, net::Ipv4Addr}; - use tempfile::TempDir; + use std::path::Path; + use std::str::FromStr; - use crate::chain_spec::test_spec::staking_spec; + use crate::chain_spec::moonbase::{testnet_genesis, ChainSpec}; + use crate::chain_spec::Extensions; use super::*; @@ -1044,7 +1046,7 @@ mod tests { "0.0.0.0:8080".parse().unwrap(), "".into(), )), - ..test_config() + ..test_config("test") }; set_prometheus_registry(&mut config).unwrap(); @@ -1057,54 +1059,87 @@ mod tests { assert_eq!(actual_metric_name.as_str(), expected_metric_name); } - fn test_config() -> Configuration { - let index = 0; - let base_port = 9898; + #[test] + fn test_set_prometheus_registry_adds_chain_id_as_label() { + let input_chain_id = "moonriver"; - let root = TempDir::new().unwrap(); - let root = root.path().join(format!("node-{}", index)); + let mut expected_label = LabelPair::default(); + expected_label.set_name("chain".to_owned()); + expected_label.set_value("moonriver".to_owned()); + let expected_chain_label = Some(expected_label); - let mut network_config = NetworkConfiguration::new( - format!("Node {}", index), - "network/test/0.1", - Default::default(), - None, - ); - - network_config.allow_non_globals_in_dht = true; + let counter = Box::new(Counter::new("foo", "foobar").unwrap()); + let mut config = Configuration { + prometheus_config: Some(PrometheusConfig::new_with_default_registry( + "0.0.0.0:8080".parse().unwrap(), + "".into(), + )), + ..test_config(input_chain_id) + }; - network_config.listen_addresses.push( - iter::once(multiaddr::Protocol::Ip4(Ipv4Addr::new(127, 0, 0, 1))) - .chain(iter::once(multiaddr::Protocol::Tcp( - base_port + index as u16, - ))) - .collect(), - ); + set_prometheus_registry(&mut config).unwrap(); + // generate metric + let reg = config.prometheus_registry().unwrap(); + reg.register(counter.clone()).unwrap(); + counter.inc(); - network_config.transport = TransportConfig::Normal { - enable_mdns: false, - allow_private_ipv4: true, - }; + let actual_chain_label = reg + .gather() + .first() + .unwrap() + .get_metric() + .first() + .unwrap() + .get_label() + .into_iter() + .find(|x| x.get_name() == "chain") + .cloned(); + + assert_eq!(actual_chain_label, expected_chain_label); + } - let runtime = - tokio::runtime::Runtime::new().expect("creating tokio runtime doesn't fail; qed"); - let tokio_handle = runtime.handle().clone(); - let spec = staking_spec(ParaId::new(0)); + fn test_config(chain_id: &str) -> Configuration { + let network_config = NetworkConfiguration::new("", "", Default::default(), None); + let runtime = tokio::runtime::Runtime::new().expect("failed creating tokio runtime"); + let spec = ChainSpec::from_genesis( + "test", + chain_id, + ChainType::Local, + move || { + testnet_genesis( + AccountId::from_str("6Be02d1d3665660d22FF9624b7BE0551ee1Ac91b").unwrap(), + vec![], + vec![], + vec![], + vec![], + vec![], + 1000 * UNIT, + ParaId::new(0), + 0, + ) + }, + vec![], + None, + None, + None, + None, + Extensions::default(), + ); Configuration { - impl_name: String::from("network-test-impl"), + impl_name: String::from("test-impl"), impl_version: String::from("0.1"), role: Role::Full, - tokio_handle, + tokio_handle: runtime.handle().clone(), transaction_pool: Default::default(), network: network_config, keystore_remote: Default::default(), keystore: KeystoreConfig::Path { - path: root.join("key"), + path: "key".into(), password: None, }, database: DatabaseSource::RocksDb { - path: root.join("db"), + path: "db".into(), cache_size: 128, }, state_cache_size: 16777216, @@ -1135,7 +1170,7 @@ mod tests { tracing_receiver: Default::default(), max_runtime_instances: 8, announce_block: true, - base_path: Some(BasePath::new(root)), + base_path: Some(BasePath::new(Path::new(""))), informant_output_format: Default::default(), runtime_cache_size: 2, } From ea9762bb6fafcdbf9531531c5a7a1ed0b037d914 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Wed, 2 Mar 2022 11:51:08 +0100 Subject: [PATCH 5/6] use maplit --- Cargo.lock | 1 + node/service/Cargo.toml | 1 + node/service/src/lib.rs | 6 ++++-- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6bcbf75e96..3307b04dbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5800,6 +5800,7 @@ dependencies = [ "libsecp256k1 0.6.0", "log", "manual-xcm-rpc", + "maplit", "moonbase-runtime", "moonbeam-cli-opt", "moonbeam-core-primitives", diff --git a/node/service/Cargo.toml b/node/service/Cargo.toml index 1cc0ad835f..4f325b22ca 100644 --- a/node/service/Cargo.toml +++ b/node/service/Cargo.toml @@ -18,6 +18,7 @@ jsonrpc-core = "18.0.0" jsonrpc-pubsub = "18.0.0" libsecp256k1 = { version = "0.6", features = [ "hmac" ] } log = "0.4" +maplit = "1.0.2" parking_lot = "0.9.0" serde = { version = "1.0.101", features = [ "derive" ] } serde_json = "1.0" diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index d80dbffccc..4df880a54f 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -26,13 +26,13 @@ use cli_opt::{EthApi as EthApiCmd, RpcConfig}; use fc_consensus::FrontierBlockImport; use fc_rpc_core::types::{FeeHistoryCache, FilterPool}; use futures::StreamExt; +use maplit::hashmap; #[cfg(feature = "moonbase-native")] pub use moonbase_runtime; #[cfg(feature = "moonbeam-native")] pub use moonbeam_runtime; #[cfg(feature = "moonriver-native")] pub use moonriver_runtime; -use std::iter; use std::{collections::BTreeMap, sync::Mutex, time::Duration}; pub mod rpc; use cumulus_client_consensus_common::ParachainConsensus; @@ -282,7 +282,9 @@ where // If we're using prometheus, use a registry with a prefix of `moonbeam`. fn set_prometheus_registry(config: &mut Configuration) -> Result<(), ServiceError> { if let Some(PrometheusConfig { registry, .. }) = config.prometheus_config.as_mut() { - let labels = iter::once((String::from("chain"), config.chain_spec.id().into())).collect(); + let labels = hashmap! { + String::from("chain") => config.chain_spec.id().into(), + }; *registry = Registry::new_custom(Some("moonbeam".into()), Some(labels))?; } From 27a6484b0f202ac6258169b3cb2554e06f39b4a5 Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Wed, 2 Mar 2022 20:23:51 +0100 Subject: [PATCH 6/6] use Into::into() instead of String::from() --- node/service/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/service/src/lib.rs b/node/service/src/lib.rs index 4df880a54f..0353e234f2 100644 --- a/node/service/src/lib.rs +++ b/node/service/src/lib.rs @@ -283,7 +283,7 @@ where fn set_prometheus_registry(config: &mut Configuration) -> Result<(), ServiceError> { if let Some(PrometheusConfig { registry, .. }) = config.prometheus_config.as_mut() { let labels = hashmap! { - String::from("chain") => config.chain_spec.id().into(), + "chain".into() => config.chain_spec.id().into(), }; *registry = Registry::new_custom(Some("moonbeam".into()), Some(labels))?; }