Pass Prometheus Registry into Client (#5120)

* Add a few metrics to Client

* Improve PrometheusConfig

* Fix client docs

substrate/Cargo.lock

@@ -5749,6 +5749,7 @@ dependencies = [
  "sp-std",
  "sp-trie",
  "sp-version",
+ "substrate-prometheus-endpoint",
  "substrate-test-runtime-client",
  "tempfile",
  "tracing",
@@ -5812,6 +5813,7 @@ dependencies = [
  "sp-runtime",
  "sp-state-machine",
  "sp-trie",
+ "substrate-prometheus-endpoint",
  "substrate-test-runtime-client",
  "tempfile",
 ]

substrate/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)

substrate/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]

substrate/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,12 @@ 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::new_with_default_registry(
+				parse_address(&format!("{}:{}", prometheus_interface, 9615), self.prometheus_port)?,
+			));
 		}
 
 		config.tracing_targets = self.import_params.tracing_targets.clone().into();

substrate/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" }

substrate/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<E, S, Block, RA>(
 	fork_blocks: ForkBlocks<Block>,
 	bad_blocks: BadBlocks<Block>,
 	execution_extensions: ExecutionExtensions<Block>,
+	prometheus_registry: Option<Registry>,
 ) -> Result<(
 		sc_client::Client<
 			Backend<Block>,
@@ -317,6 +319,7 @@ pub fn new_client<E, S, Block, RA>(
 			fork_blocks,
 			bad_blocks,
 			execution_extensions,
+			prometheus_registry,
 		)?,
 		backend,
 	))

substrate/client/service/src/builder.rs

@@ -17,7 +17,7 @@
 use crate::{Service, NetworkStatus, NetworkState, error::Error, DEFAULT_PROTOCOL_ID, MallocSizeOfWasm};
 use crate::{TaskManagerBuilder, 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,
@@ -128,7 +128,6 @@ pub struct ServiceBuilder<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TF
 	remote_backend: Option<Arc<dyn RemoteBlockchain<TBl>>>,
 	marker: PhantomData<(TBl, TRtApi)>,
 	background_tasks: Vec<(&'static str, BackgroundTask)>,
-	prometheus_registry: Option<Registry>
 }
 
 /// Full client type.
@@ -262,6 +261,7 @@ fn new_full_parts<TBl, TRtApi, TExecDisp, TGen, TCSExt>(
 			fork_blocks,
 			bad_blocks,
 			extensions,
+			config.prometheus_config.as_ref().map(|config| config.registry.clone()),
 		)?
 	};
 
@@ -308,7 +308,6 @@ where TGen: RuntimeGenesis, TCSExt: Extension {
 			remote_backend: None,
 			background_tasks: Default::default(),
 			marker: PhantomData,
-			prometheus_registry: None,
 		})
 	}
 
@@ -378,6 +377,7 @@ where TGen: RuntimeGenesis, TCSExt: Extension {
 			backend.clone(),
 			config.expect_chain_spec(),
 			executor,
+			config.prometheus_config.as_ref().map(|config| config.registry.clone()),
 		)?);
 
 		Ok(ServiceBuilder {
@@ -396,7 +396,6 @@ where TGen: RuntimeGenesis, TCSExt: Extension {
 			remote_backend: Some(remote_blockchain),
 			background_tasks: Default::default(),
 			marker: PhantomData,
-			prometheus_registry: None,
 		})
 	}
 }
@@ -470,7 +469,6 @@ impl<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, T
 			remote_backend: self.remote_backend,
 			background_tasks: self.background_tasks,
 			marker: self.marker,
-			prometheus_registry: self.prometheus_registry,
 		})
 	}
 
@@ -514,7 +512,6 @@ impl<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, T
 			remote_backend: self.remote_backend,
 			background_tasks: self.background_tasks,
 			marker: self.marker,
-			prometheus_registry: self.prometheus_registry,
 		})
 	}
 
@@ -555,7 +552,6 @@ impl<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, T
 			remote_backend: self.remote_backend,
 			background_tasks: self.background_tasks,
 			marker: self.marker,
-			prometheus_registry: self.prometheus_registry,
 		})
 	}
 
@@ -620,7 +616,6 @@ impl<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, T
 			remote_backend: self.remote_backend,
 			background_tasks: self.background_tasks,
 			marker: self.marker,
-			prometheus_registry: self.prometheus_registry,
 		})
 	}
 
@@ -681,7 +676,6 @@ impl<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, T
 			remote_backend: self.remote_backend,
 			background_tasks: self.background_tasks,
 			marker: self.marker,
-			prometheus_registry: self.prometheus_registry,
 		})
 	}
 
@@ -710,31 +704,8 @@ impl<TBl, TRtApi, TGen, TCSExt, TCl, TFchr, TSc, TImpQu, TFprb, TFpp, TExPool, T
 			remote_backend: self.remote_backend,
 			background_tasks: self.background_tasks,
 			marker: self.marker,
-			prometheus_registry: self.prometheus_registry,
 		})
 	}
-
-	/// Use an existing prometheus `Registry` to record metrics into.
-	pub fn with_prometheus_registry(self, registry: Registry) -> Self {
-		Self {
-			config: self.config,
-			client: self.client,
-			backend: self.backend,
-			tasks_builder: self.tasks_builder,
-			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
@@ -845,7 +816,6 @@ ServiceBuilder<
 			rpc_extensions,
 			remote_backend,
 			background_tasks,
-			prometheus_registry,
 		} = self;
 
 		sp_session::generate_initial_session_keys(
@@ -897,14 +867,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: {
@@ -922,7 +884,7 @@ ServiceBuilder<
 			import_queue,
 			protocol_id,
 			block_announce_validator,
-			metrics_registry: prometheus_registry_and_port.as_ref().map(|(r, _)| r.clone())
+			metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone())
 		};
 
 		let has_bootnodes = !network_params.network_config.boot_nodes.is_empty();
@@ -1035,7 +997,7 @@ ServiceBuilder<
 		}
 
 		// Prometheus metrics
-		let metrics = if let Some((registry, port)) = prometheus_registry_and_port.clone() {
+		let metrics = if let Some(PrometheusConfig { port, registry }) = config.prometheus_config.clone() {
 			let metrics = ServiceMetrics::register(&registry)?;
 			metrics.node_roles.set(u64::from(config.roles.bits()));
 			spawn_handle.spawn(
@@ -1297,7 +1259,7 @@ ServiceBuilder<
 			_telemetry_on_connect_sinks: telemetry_connection_sinks.clone(),
 			keystore,
 			marker: PhantomData::<TBl>,
-			prometheus_registry: prometheus_registry_and_port.map(|(r, _)| r)
+			prometheus_registry: config.prometheus_config.map(|config| config.registry)
 		})
 	}
 }

substrate/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<G, E = NoExtension> {
 	pub rpc_ws_max_connections: Option<usize>,
 	/// CORS settings for HTTP & WS servers. `None` if all origins are allowed.
 	pub rpc_cors: Option<Vec<String>>,
-	/// Prometheus endpoint Port. `None` if disabled.
-	pub prometheus_port: Option<SocketAddr>,
+	/// Prometheus endpoint configuration. `None` if disabled.
+	pub prometheus_config: Option<PrometheusConfig>,
 	/// Telemetry service URL. `None` if disabled.
 	pub telemetry_endpoints: Option<TelemetryEndpoints>,
 	/// External WASM transport for the telemetry. If `Some`, when connection to a telemetry
@@ -165,6 +166,28 @@ pub enum DatabaseConfig {
 	Custom(Arc<dyn KeyValueDB>),
 }
 
+/// Configuration of the Prometheus endpoint.
+#[derive(Clone)]
+pub struct PrometheusConfig {
+	/// Port to use.
+	pub port: SocketAddr,
+	/// 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<G, E> Default for Configuration<G, E> {
 	/// Create a default config
 	fn default() -> Self {
@@ -190,7 +213,7 @@ impl<G, E> Default for Configuration<G, E> {
 			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,

substrate/client/service/test/src/lib.rs

@@ -199,7 +199,7 @@ fn node_config<G, E: Clone> (
 		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,

substrate/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<E, Block, S, RA>(
 	executor: E,
 	genesis_storage: &S,
 	keystore: Option<sp_core::traits::BareCryptoStorePtr>,
+	prometheus_registry: Option<Registry>,
 ) -> sp_blockchain::Result<Client<
 	in_mem::Backend<Block>,
 	LocalCallExecutor<in_mem::Backend<Block>, E>,
@@ -181,7 +183,7 @@ pub fn new_in_mem<E, Block, S, RA>(
 	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<B, E, Block, S, RA>(
 	executor: E,
 	build_genesis_storage: &S,
 	keystore: Option<sp_core::traits::BareCryptoStorePtr>,
+	prometheus_registry: Option<Registry>,
 ) -> sp_blockchain::Result<Client<B, LocalCallExecutor<B, E>, Block, RA>>
 	where
 		E: CodeExecutor + RuntimeInfo,
@@ -207,6 +210,7 @@ pub fn new_with_backend<B, E, Block, S, RA>(
 		Default::default(),
 		Default::default(),
 		extensions,
+		prometheus_registry,
 	)
 }
 
@@ -286,6 +290,7 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 		fork_blocks: ForkBlocks<Block>,
 		bad_blocks: BadBlocks<Block>,
 		execution_extensions: ExecutionExtensions<Block>,
+		_prometheus_registry: Option<Registry>,
 	) -> sp_blockchain::Result<Self> {
 		if backend.blockchain().header(BlockId::Number(Zero::zero()))?.is_none() {
 			let genesis_storage = build_genesis_storage.build_storage()?;

substrate/client/src/lib.rs

@@ -68,6 +68,7 @@
 //! 	Default::default(),
 //! 	Default::default(),
 //! 	Default::default(),
+//!		None,
 //! );
 //! ```
 //!

substrate/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, HashFor};
 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<B, S, GS, RA, E>(
 	backend: Arc<Backend<S, HashFor<B>>>,
 	genesis_storage: &GS,
 	code_executor: E,
+	prometheus_registry: Option<Registry>,
 ) -> ClientResult<
 		Client<
 			Backend<S, HashFor<B>>,
@@ -84,6 +86,7 @@ pub fn new_light<B, S, GS, RA, E>(
 		Default::default(),
 		Default::default(),
 		Default::default(),
+		prometheus_registry,
 	)
 }
 

substrate/test-utils/client/src/lib.rs

@@ -210,7 +210,8 @@ impl<Block: BlockT, Executor, Backend, G: GenesisInit> TestClientBuilder<Block,
 			ExecutionExtensions::new(
 				self.execution_strategies,
 				self.keystore.clone(),
-			)
+			),
+			None,
 		).expect("Creates new client");
 
 		let longest_chain = sc_client::LongestChain::new(self.backend);