From 6085f3a6838683eefbc251967310d40f51e180fc Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Tue, 3 Mar 2020 17:43:33 +0100 Subject: [PATCH 1/3] Add a few metrics to Client --- Cargo.lock | 2 + bin/node/testing/src/bench.rs | 1 + client/Cargo.toml | 1 + client/cli/src/commands/runcmd.rs | 12 +++-- client/db/Cargo.toml | 1 + client/db/src/lib.rs | 3 ++ client/service/src/builder.rs | 79 +++++++++++++------------------ client/service/src/config.rs | 15 ++++-- client/service/test/src/lib.rs | 2 +- client/src/client.rs | 7 ++- client/src/light/mod.rs | 3 ++ test-utils/client/src/lib.rs | 3 +- 12 files changed, 73 insertions(+), 56 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d651563f137a3..34dbaaac27801 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5732,6 +5732,7 @@ dependencies = [ "sp-std", "sp-trie", "sp-version", + "substrate-prometheus-endpoint", "substrate-test-runtime-client", "tempfile", "tracing", @@ -5795,6 +5796,7 @@ dependencies = [ "sp-runtime", "sp-state-machine", "sp-trie", + "substrate-prometheus-endpoint", "substrate-test-runtime-client", "tempfile", ] diff --git a/bin/node/testing/src/bench.rs b/bin/node/testing/src/bench.rs index 58a7ab933eeb9..19906dd6a1bf5 100644 --- a/bin/node/testing/src/bench.rs +++ b/bin/node/testing/src/bench.rs @@ -155,6 +155,7 @@ impl BenchDb { None, None, ExecutionExtensions::new(profile.into_execution_strategies(), None), + None, ).expect("Should not fail"); (client, backend) diff --git a/client/Cargo.toml b/client/Cargo.toml index 91f3578ce2147..0afd17fb0cc76 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -34,6 +34,7 @@ sp-blockchain = { version = "2.0.0-alpha.2", path = "../primitives/blockchain" } sp-state-machine = { version = "0.8.0-alpha.2", path = "../primitives/state-machine" } sc-telemetry = { version = "2.0.0-alpha.2", path = "telemetry" } sp-trie = { version = "2.0.0-alpha.2", path = "../primitives/trie" } +prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.8.0-alpha.2", path = "../utils/prometheus" } tracing = "0.1.10" [dev-dependencies] diff --git a/client/cli/src/commands/runcmd.rs b/client/cli/src/commands/runcmd.rs index f29bc3c743b64..b3ee35e43cb39 100644 --- a/client/cli/src/commands/runcmd.rs +++ b/client/cli/src/commands/runcmd.rs @@ -24,7 +24,7 @@ use regex::Regex; use chrono::prelude::*; use sc_service::{ AbstractService, Configuration, ChainSpecExtension, RuntimeGenesis, ChainSpec, Roles, - config::KeystoreConfig, + config::{KeystoreConfig, PrometheusConfig}, }; use sc_telemetry::TelemetryEndpoints; @@ -423,11 +423,13 @@ impl RunCmd { // Override prometheus if self.no_prometheus { - config.prometheus_port = None; - } else if config.prometheus_port.is_none() { + config.prometheus_config = None; + } else if config.prometheus_config.is_none() { let prometheus_interface: &str = if self.prometheus_external { "0.0.0.0" } else { "127.0.0.1" }; - config.prometheus_port = Some( - parse_address(&format!("{}:{}", prometheus_interface, 9615), self.prometheus_port)?); + config.prometheus_config = Some(PrometheusConfig { + port: parse_address(&format!("{}:{}", prometheus_interface, 9615), self.prometheus_port)?, + registry: None + }); } config.tracing_targets = self.import_params.tracing_targets.clone().into(); diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index 68460f42a6190..885864fb5f87f 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -30,6 +30,7 @@ sc-state-db = { version = "0.8.0-alpha.2", path = "../state-db" } sp-trie = { version = "2.0.0-alpha.2", path = "../../primitives/trie" } sp-consensus = { version = "0.8.0-alpha.2", path = "../../primitives/consensus/common" } sp-blockchain = { version = "2.0.0-alpha.2", path = "../../primitives/blockchain" } +prometheus-endpoint = { package = "substrate-prometheus-endpoint", version = "0.8.0-alpha.2", path = "../../utils/prometheus" } [dev-dependencies] sp-keyring = { version = "2.0.0-alpha.2", path = "../../primitives/keyring" } diff --git a/client/db/src/lib.rs b/client/db/src/lib.rs index b4dd98457d4f8..c1e540f78e2aa 100644 --- a/client/db/src/lib.rs +++ b/client/db/src/lib.rs @@ -84,6 +84,7 @@ use crate::storage_cache::{CachingState, SharedCache, new_shared_cache}; use crate::stats::StateUsageStats; use log::{trace, debug, warn}; pub use sc_state_db::PruningMode; +use prometheus_endpoint::Registry; #[cfg(any(feature = "kvdb-rocksdb", test))] pub use bench::BenchmarkingState; @@ -291,6 +292,7 @@ pub fn new_client( fork_blocks: ForkBlocks, bad_blocks: BadBlocks, execution_extensions: ExecutionExtensions, + prometheus_registry: Option, ) -> Result<( sc_client::Client< Backend, @@ -317,6 +319,7 @@ pub fn new_client( fork_blocks, bad_blocks, execution_extensions, + prometheus_registry, )?, backend, )) diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 01bba286c64ea..249ac85fae502 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -17,7 +17,7 @@ use crate::{Service, NetworkStatus, NetworkState, error::Error, DEFAULT_PROTOCOL_ID, MallocSizeOfWasm}; use crate::{SpawnTaskHandle, start_rpc_servers, build_network_future, TransactionPoolAdapter}; use crate::status_sinks; -use crate::config::{Configuration, DatabaseConfig, KeystoreConfig}; +use crate::config::{Configuration, DatabaseConfig, KeystoreConfig, PrometheusConfig}; use sc_client_api::{ self, BlockchainEvents, @@ -46,7 +46,7 @@ use sc_executor::{NativeExecutor, NativeExecutionDispatch}; use std::{ borrow::Cow, io::{Read, Write, Seek}, - marker::PhantomData, sync::Arc, pin::Pin + marker::PhantomData, sync::Arc, pin::Pin, net::SocketAddr, }; use wasm_timer::SystemTime; use sysinfo::{get_current_pid, ProcessExt, System, SystemExt}; @@ -92,6 +92,16 @@ impl ServiceMetrics { } } +fn get_registry_and_port(config: Option<&PrometheusConfig>) -> Result, PrometheusError> { + Ok(match config { + Some(config) => match config.registry.as_ref() { + Some(registry) => Some((registry.clone(), config.port)), + None => Some((Registry::new_custom(Some("substrate".into()), None)?, config.port)) + }, + None => None + }) +} + pub type BackgroundTask = Pin + Send>>; /// Aggregator for the components required to build a service. @@ -128,7 +138,7 @@ pub struct ServiceBuilder>>, marker: PhantomData<(TBl, TRtApi)>, background_tasks: Vec<(&'static str, BackgroundTask)>, - prometheus_registry: Option + prometheus_registry_and_port: Option<(Registry, SocketAddr)>, } /// Full client type. @@ -181,6 +191,7 @@ type TFullParts = ( TFullClient, Arc>, Arc>, + Option<(Registry, SocketAddr)>, ); /// Creates a new full client for the given config. @@ -230,6 +241,8 @@ fn new_full_parts( .cloned() .unwrap_or_default(); + let prometheus_registry_and_port = get_registry_and_port(config.prometheus_config.as_ref())?; + let (client, backend) = { let db_config = sc_client_db::DatabaseSettings { state_cache_size: config.state_cache_size, @@ -259,10 +272,11 @@ fn new_full_parts( fork_blocks, bad_blocks, extensions, + prometheus_registry_and_port.as_ref().map(|(r, _)| r.clone()), )? }; - Ok((client, backend, keystore)) + Ok((client, backend, keystore, prometheus_registry_and_port)) } impl ServiceBuilder<(), (), TGen, TCSExt, (), (), (), (), (), (), (), (), ()> @@ -285,12 +299,11 @@ where TGen: RuntimeGenesis, TCSExt: Extension { (), TFullBackend, >, Error> { - let (client, backend, keystore) = new_full_parts(&config)?; + let (client, backend, keystore, prometheus_registry_and_port) = new_full_parts(&config)?; let client = Arc::new(client); Ok(ServiceBuilder { - config, client, backend, keystore, @@ -304,7 +317,8 @@ where TGen: RuntimeGenesis, TCSExt: Extension { remote_backend: None, background_tasks: Default::default(), marker: PhantomData, - prometheus_registry: None, + prometheus_registry_and_port, + config, }) } @@ -368,14 +382,17 @@ where TGen: RuntimeGenesis, TCSExt: Extension { let fetcher = Arc::new(sc_network::config::OnDemand::new(fetch_checker)); let backend = sc_client::light::new_light_backend(light_blockchain); let remote_blockchain = backend.remote_blockchain(); + + let prometheus_registry_and_port = get_registry_and_port(config.prometheus_config.as_ref())?; + let client = Arc::new(sc_client::light::new_light( backend.clone(), config.expect_chain_spec(), executor, + prometheus_registry_and_port.as_ref().map(|(r, _)| r.clone()), )?); Ok(ServiceBuilder { - config, client, backend, keystore, @@ -389,7 +406,8 @@ where TGen: RuntimeGenesis, TCSExt: Extension { remote_backend: Some(remote_blockchain), background_tasks: Default::default(), marker: PhantomData, - prometheus_registry: None, + prometheus_registry_and_port, + config, }) } } @@ -462,7 +480,7 @@ impl Self { - Self { - config: self.config, - client: self.client, - backend: self.backend, - keystore: self.keystore, - fetcher: self.fetcher, - select_chain: self.select_chain, - import_queue: self.import_queue, - finality_proof_request_builder: self.finality_proof_request_builder, - finality_proof_provider: self.finality_proof_provider, - transaction_pool: self.transaction_pool, - rpc_extensions: self.rpc_extensions, - remote_backend: self.remote_backend, - background_tasks: self.background_tasks, - marker: self.marker, - prometheus_registry: Some(registry), - } - } } /// Implemented on `ServiceBuilder`. Allows running block commands, such as import/export/validate @@ -830,7 +827,7 @@ ServiceBuilder< rpc_extensions, remote_backend, background_tasks, - prometheus_registry, + prometheus_registry_and_port, } = self; sp_session::generate_initial_session_keys( @@ -888,14 +885,6 @@ ServiceBuilder< let block_announce_validator = Box::new(sp_consensus::block_validation::DefaultBlockAnnounceValidator::new(client.clone())); - let prometheus_registry_and_port = match config.prometheus_port { - Some(port) => match prometheus_registry { - Some(registry) => Some((registry, port)), - None => Some((Registry::new_custom(Some("substrate".into()), None)?, port)) - }, - None => None - }; - let network_params = sc_network::config::Params { roles: config.roles, executor: { diff --git a/client/service/src/config.rs b/client/service/src/config.rs index 974dac588def9..c3c3374de9b8a 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -27,6 +27,7 @@ use sc_chain_spec::{ChainSpec, NoExtension}; use sp_core::crypto::Protected; use target_info::Target; use sc_telemetry::TelemetryEndpoints; +use prometheus_endpoint::Registry; /// Executable version. Used to pass version information from the root crate. #[derive(Clone)] @@ -93,8 +94,8 @@ pub struct Configuration { pub rpc_ws_max_connections: Option, /// CORS settings for HTTP & WS servers. `None` if all origins are allowed. pub rpc_cors: Option>, - /// Prometheus endpoint Port. `None` if disabled. - pub prometheus_port: Option, + /// Prometheus endpoint configuration. `None` if disabled. + pub prometheus_config: Option, /// Telemetry service URL. `None` if disabled. pub telemetry_endpoints: Option, /// External WASM transport for the telemetry. If `Some`, when connection to a telemetry @@ -165,6 +166,14 @@ pub enum DatabaseConfig { Custom(Arc), } +/// Configuration of the Prometheus endpoint. +pub struct PrometheusConfig { + /// Port to use. + pub port: SocketAddr, + /// A metrics registry to use. One will be created if `None`. + pub registry: Option, +} + impl Default for Configuration { /// Create a default config fn default() -> Self { @@ -190,7 +199,7 @@ impl Default for Configuration { rpc_ws: None, rpc_ws_max_connections: None, rpc_cors: Some(vec![]), - prometheus_port: None, + prometheus_config: None, telemetry_endpoints: None, telemetry_external_transport: None, default_heap_pages: None, diff --git a/client/service/test/src/lib.rs b/client/service/test/src/lib.rs index 5f679b82b39a4..03cccbe4a053a 100644 --- a/client/service/test/src/lib.rs +++ b/client/service/test/src/lib.rs @@ -199,7 +199,7 @@ fn node_config ( rpc_ws: None, rpc_ws_max_connections: None, rpc_cors: None, - prometheus_port: None, + prometheus_config: None, telemetry_endpoints: None, telemetry_external_transport: None, default_heap_pages: None, diff --git a/client/src/client.rs b/client/src/client.rs index d461a17ded75f..4406373405ba9 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -78,6 +78,7 @@ pub use sc_client_api::{ CallExecutor, }; use sp_blockchain::Error; +use prometheus_endpoint::Registry; use crate::{ call_executor::LocalCallExecutor, @@ -171,6 +172,7 @@ pub fn new_in_mem( executor: E, genesis_storage: &S, keystore: Option, + prometheus_registry: Option, ) -> sp_blockchain::Result, LocalCallExecutor, E>, @@ -181,7 +183,7 @@ pub fn new_in_mem( S: BuildStorage, Block: BlockT, { - new_with_backend(Arc::new(in_mem::Backend::new()), executor, genesis_storage, keystore) + new_with_backend(Arc::new(in_mem::Backend::new()), executor, genesis_storage, keystore, prometheus_registry) } /// Create a client with the explicitly provided backend. @@ -191,6 +193,7 @@ pub fn new_with_backend( executor: E, build_genesis_storage: &S, keystore: Option, + prometheus_registry: Option, ) -> sp_blockchain::Result, Block, RA>> where E: CodeExecutor + RuntimeInfo, @@ -207,6 +210,7 @@ pub fn new_with_backend( Default::default(), Default::default(), extensions, + prometheus_registry, ) } @@ -286,6 +290,7 @@ impl Client where fork_blocks: ForkBlocks, bad_blocks: BadBlocks, execution_extensions: ExecutionExtensions, + _prometheus_registry: Option, ) -> sp_blockchain::Result { if backend.blockchain().header(BlockId::Number(Zero::zero()))?.is_none() { let genesis_storage = build_genesis_storage.build_storage()?; diff --git a/client/src/light/mod.rs b/client/src/light/mod.rs index d65fdef71193d..765f581d31b5d 100644 --- a/client/src/light/mod.rs +++ b/client/src/light/mod.rs @@ -28,6 +28,7 @@ use sp_core::traits::CodeExecutor; use sp_runtime::BuildStorage; use sp_runtime::traits::{Block as BlockT, HasherFor}; use sp_blockchain::Result as ClientResult; +use prometheus_endpoint::Registry; use crate::call_executor::LocalCallExecutor; use crate::client::Client; @@ -58,6 +59,7 @@ pub fn new_light( backend: Arc>>, genesis_storage: &GS, code_executor: E, + prometheus_registry: Option, ) -> ClientResult< Client< Backend>, @@ -84,6 +86,7 @@ pub fn new_light( Default::default(), Default::default(), Default::default(), + prometheus_registry, ) } diff --git a/test-utils/client/src/lib.rs b/test-utils/client/src/lib.rs index afe11903d5be9..47ae5eb92e1f7 100644 --- a/test-utils/client/src/lib.rs +++ b/test-utils/client/src/lib.rs @@ -210,7 +210,8 @@ impl TestClientBuilder Date: Wed, 4 Mar 2020 17:32:01 +0100 Subject: [PATCH 2/3] Improve PrometheusConfig --- client/cli/src/commands/runcmd.rs | 7 ++-- client/service/src/builder.rs | 55 +++++++++---------------------- client/service/src/config.rs | 18 ++++++++-- 3 files changed, 35 insertions(+), 45 deletions(-) diff --git a/client/cli/src/commands/runcmd.rs b/client/cli/src/commands/runcmd.rs index b3ee35e43cb39..0ad6fefcce8cb 100644 --- a/client/cli/src/commands/runcmd.rs +++ b/client/cli/src/commands/runcmd.rs @@ -426,10 +426,9 @@ impl RunCmd { config.prometheus_config = None; } else if config.prometheus_config.is_none() { let prometheus_interface: &str = if self.prometheus_external { "0.0.0.0" } else { "127.0.0.1" }; - config.prometheus_config = Some(PrometheusConfig { - port: parse_address(&format!("{}:{}", prometheus_interface, 9615), self.prometheus_port)?, - registry: None - }); + config.prometheus_config = Some(PrometheusConfig::new_with_default_registry( + parse_address(&format!("{}:{}", prometheus_interface, 9615), self.prometheus_port)?, + )); } config.tracing_targets = self.import_params.tracing_targets.clone().into(); diff --git a/client/service/src/builder.rs b/client/service/src/builder.rs index 249ac85fae502..3c1f549f7395a 100644 --- a/client/service/src/builder.rs +++ b/client/service/src/builder.rs @@ -17,7 +17,7 @@ use crate::{Service, NetworkStatus, NetworkState, error::Error, DEFAULT_PROTOCOL_ID, MallocSizeOfWasm}; use crate::{SpawnTaskHandle, start_rpc_servers, build_network_future, TransactionPoolAdapter}; use crate::status_sinks; -use crate::config::{Configuration, DatabaseConfig, KeystoreConfig, PrometheusConfig}; +use crate::config::{Configuration, DatabaseConfig, KeystoreConfig}; use sc_client_api::{ self, BlockchainEvents, @@ -46,7 +46,7 @@ use sc_executor::{NativeExecutor, NativeExecutionDispatch}; use std::{ borrow::Cow, io::{Read, Write, Seek}, - marker::PhantomData, sync::Arc, pin::Pin, net::SocketAddr, + marker::PhantomData, sync::Arc, pin::Pin }; use wasm_timer::SystemTime; use sysinfo::{get_current_pid, ProcessExt, System, SystemExt}; @@ -92,16 +92,6 @@ impl ServiceMetrics { } } -fn get_registry_and_port(config: Option<&PrometheusConfig>) -> Result, PrometheusError> { - Ok(match config { - Some(config) => match config.registry.as_ref() { - Some(registry) => Some((registry.clone(), config.port)), - None => Some((Registry::new_custom(Some("substrate".into()), None)?, config.port)) - }, - None => None - }) -} - pub type BackgroundTask = Pin + Send>>; /// Aggregator for the components required to build a service. @@ -138,7 +128,6 @@ pub struct ServiceBuilder>>, marker: PhantomData<(TBl, TRtApi)>, background_tasks: Vec<(&'static str, BackgroundTask)>, - prometheus_registry_and_port: Option<(Registry, SocketAddr)>, } /// Full client type. @@ -191,7 +180,6 @@ type TFullParts = ( TFullClient, Arc>, Arc>, - Option<(Registry, SocketAddr)>, ); /// Creates a new full client for the given config. @@ -241,8 +229,6 @@ fn new_full_parts( .cloned() .unwrap_or_default(); - let prometheus_registry_and_port = get_registry_and_port(config.prometheus_config.as_ref())?; - let (client, backend) = { let db_config = sc_client_db::DatabaseSettings { state_cache_size: config.state_cache_size, @@ -272,11 +258,11 @@ fn new_full_parts( fork_blocks, bad_blocks, extensions, - prometheus_registry_and_port.as_ref().map(|(r, _)| r.clone()), + config.prometheus_config.as_ref().map(|config| config.registry.clone()), )? }; - Ok((client, backend, keystore, prometheus_registry_and_port)) + Ok((client, backend, keystore)) } impl ServiceBuilder<(), (), TGen, TCSExt, (), (), (), (), (), (), (), (), ()> @@ -299,11 +285,12 @@ where TGen: RuntimeGenesis, TCSExt: Extension { (), TFullBackend, >, Error> { - let (client, backend, keystore, prometheus_registry_and_port) = new_full_parts(&config)?; + let (client, backend, keystore) = new_full_parts(&config)?; let client = Arc::new(client); Ok(ServiceBuilder { + config, client, backend, keystore, @@ -317,8 +304,6 @@ where TGen: RuntimeGenesis, TCSExt: Extension { remote_backend: None, background_tasks: Default::default(), marker: PhantomData, - prometheus_registry_and_port, - config, }) } @@ -382,17 +367,15 @@ where TGen: RuntimeGenesis, TCSExt: Extension { let fetcher = Arc::new(sc_network::config::OnDemand::new(fetch_checker)); let backend = sc_client::light::new_light_backend(light_blockchain); let remote_blockchain = backend.remote_blockchain(); - - let prometheus_registry_and_port = get_registry_and_port(config.prometheus_config.as_ref())?; - let client = Arc::new(sc_client::light::new_light( backend.clone(), config.expect_chain_spec(), executor, - prometheus_registry_and_port.as_ref().map(|(r, _)| r.clone()), + config.prometheus_config.as_ref().map(|config| config.registry.clone()), )?); Ok(ServiceBuilder { + config, client, backend, keystore, @@ -406,8 +389,6 @@ where TGen: RuntimeGenesis, TCSExt: Extension { remote_backend: Some(remote_blockchain), background_tasks: Default::default(), marker: PhantomData, - prometheus_registry_and_port, - config, }) } } @@ -480,7 +461,6 @@ impl, - prometheus_registry: prometheus_registry_and_port.map(|(r, _)| r) + prometheus_registry: config.prometheus_config.map(|config| config.registry) }) } } diff --git a/client/service/src/config.rs b/client/service/src/config.rs index c3c3374de9b8a..6400712cad3b9 100644 --- a/client/service/src/config.rs +++ b/client/service/src/config.rs @@ -167,11 +167,25 @@ pub enum DatabaseConfig { } /// Configuration of the Prometheus endpoint. +#[derive(Clone)] pub struct PrometheusConfig { /// Port to use. pub port: SocketAddr, - /// A metrics registry to use. One will be created if `None`. - pub registry: Option, + /// A metrics registry to use. Useful for setting the metric prefix. + pub registry: Registry, +} + +impl PrometheusConfig { + /// Create a new config using the default registry. + /// + /// The default registry prefixes metrics with `substrate`. + pub fn new_with_default_registry(port: SocketAddr) -> Self { + Self { + port, + registry: Registry::new_custom(Some("substrate".into()), None) + .expect("this can only fail if the prefix is empty") + } + } } impl Default for Configuration { From ed7d4fe00755ddb63e9a4433f8fdc55eaf0de36c Mon Sep 17 00:00:00 2001 From: Ashley Ruglys Date: Wed, 4 Mar 2020 19:46:11 +0100 Subject: [PATCH 3/3] Fix client docs --- client/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/client/src/lib.rs b/client/src/lib.rs index 1d279cabad499..6519dfb5c5c5a 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -69,6 +69,7 @@ //! Default::default(), //! Default::default(), //! Default::default(), +//! None, //! ); //! ``` //!