Parachain runtime metrics followup (#4602)

* initial changes

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fmt

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* remove file

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Remove pallet

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fix copyright year

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Remove metric registration op

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Register runtime metrics in client

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fmt

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fix build without `runtime-metrics`

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* reduce visibility

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* remove metric prefix logic, use hardcoded

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fix

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* review feedback

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Split CounterVec api so it doesn't need mutability

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Const beautify

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fix

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Fix

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* fix docs

Signed-off-by: Andrei Sandu <andrei-mihail@parity.io>

* Merge web ui feedback.

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

polkadot/runtime/metrics/src/with_runtime_metrics.rs

@@ -16,21 +16,24 @@
 
 //! This module provides an implementation for the runtime metrics types: `Counter`
 //! and `CounterVec`. These types expose a Prometheus like interface and same functionality.
-//! Each instance of a runtime metric is mapped to a Prometheus metric on the node side.
+//! Each instance of a runtime metric is mapped to a Prometheus metric on the client side.
+//! The runtime metrics must be registered with the registry in the client, otherwise
+//! they will not be published.
 
 const TRACING_TARGET: &'static str = "metrics";
 
 use parity_scale_codec::Encode;
 use primitives::v1::{
-	RuntimeMetricLabelValues, RuntimeMetricOp, RuntimeMetricRegisterParams, RuntimeMetricUpdate,
+	metric_definitions::{CounterDefinition, CounterVecDefinition},
+	RuntimeMetricLabelValues, RuntimeMetricOp, RuntimeMetricUpdate,
 };
+
 use sp_std::prelude::*;
 
 /// Holds a set of counters that have different values for their labels,
 /// like Prometheus CounterVec.
 pub struct CounterVec {
 	name: &'static str,
-	label_values: Option<RuntimeMetricLabelValues>,
 }
 
 /// A counter metric.
@@ -49,68 +52,57 @@ trait MetricEmitter {
 	}
 }
 
-impl MetricEmitter for CounterVec {}
-impl MetricEmitter for Counter {}
+///
+pub struct LabeledMetric {
+	name: &'static str,
+	label_values: RuntimeMetricLabelValues,
+}
 
-impl CounterVec {
-	/// Create a new metric with specified `name`, `description` and `labels`.
-	pub fn new(name: &'static str, description: &'static str, labels: &[&'static str]) -> Self {
-		// Send a register metric operation to node side.
+impl LabeledMetric {
+	/// Increment the counter by `value`.
+	pub fn inc_by(&self, value: u64) {
 		let metric_update = RuntimeMetricUpdate {
-			metric_name: Vec::from(name),
-			op: RuntimeMetricOp::Register(RuntimeMetricRegisterParams::new(
-				Vec::from(description),
-				Some(labels.into()),
-			)),
+			metric_name: Vec::from(self.name),
+			op: RuntimeMetricOp::IncrementCounterVec(value, self.label_values.clone()),
 		};
 
 		Self::emit(&metric_update);
-
-		CounterVec { name, label_values: None }
-	}
-
-	/// Set the label values. Must be called before each increment operation.
-	pub fn with_label_values(&mut self, label_values: &[&'static str]) -> &mut Self {
-		self.label_values = Some(label_values.into());
-		self
-	}
-
-	/// Increment the counter by `value`.
-	pub fn inc_by(&mut self, value: u64) {
-		self.label_values.take().map(|label_values| {
-			let metric_update = RuntimeMetricUpdate {
-				metric_name: Vec::from(self.name),
-				op: RuntimeMetricOp::IncrementCounterVec(value, label_values),
-			};
-
-			Self::emit(&metric_update);
-		});
 	}
 
 	/// Increment the counter value.
-	pub fn inc(&mut self) {
+	pub fn inc(&self) {
 		self.inc_by(1);
 	}
 }
 
-impl Counter {
-	/// Create a new counter metric with specified `name`, `description`.
-	pub fn new(name: &'static str, description: &'static str) -> Self {
-		// Send a register metric operation to node side.
-		let metric_update = RuntimeMetricUpdate {
-			metric_name: Vec::from(name),
-			op: RuntimeMetricOp::Register(RuntimeMetricRegisterParams::new(
-				Vec::from(description),
-				None,
-			)),
-		};
+impl MetricEmitter for LabeledMetric {}
+impl MetricEmitter for Counter {}
 
-		Self::emit(&metric_update);
-		Counter { name }
+impl CounterVec {
+	/// Create a new counter as specified by `definition`. This metric needs to be registered
+	/// in the client before it can be used.
+	pub const fn new(definition: CounterVecDefinition) -> Self {
+		// No register op is emitted since the metric is supposed to be registered
+		// on the client by the time `inc()` is called.
+		CounterVec { name: definition.name }
+	}
+
+	/// Returns a LabeledMetric instance that provides an interface for incrementing
+	/// the metric.
+	pub fn with_label_values(&self, label_values: &[&'static str]) -> LabeledMetric {
+		LabeledMetric { name: self.name, label_values: label_values.into() }
+	}
+}
+
+impl Counter {
+	/// Create a new counter as specified by `definition`. This metric needs to be registered
+	/// in the client before it can be used.
+	pub const fn new(definition: CounterDefinition) -> Self {
+		Counter { name: definition.name }
 	}
 
 	/// Increment counter by `value`.
-	pub fn inc_by(&mut self, value: u64) {
+	pub fn inc_by(&self, value: u64) {
 		let metric_update = RuntimeMetricUpdate {
 			metric_name: Vec::from(self.name),
 			op: RuntimeMetricOp::IncrementCounter(value),
@@ -120,7 +112,7 @@ impl Counter {
 	}
 
 	/// Increment counter.
-	pub fn inc(&mut self) {
+	pub fn inc(&self) {
 		self.inc_by(1);
 	}
 }

polkadot/runtime/metrics/src/without_runtime_metrics.rs

@@ -18,6 +18,8 @@
 //! provide a dummy implementation for the native runtime to avoid cluttering the runtime code
 //! with `#[cfg(feature = "runtime-metrics")]`.
 
+use primitives::v1::metric_definitions::{CounterDefinition, CounterVecDefinition};
+
 /// A dummy Counter.
 pub struct Counter;
 /// A dummy CounterVec.
@@ -26,27 +28,27 @@ pub struct CounterVec;
 /// Dummy implementation.
 impl CounterVec {
 	/// Constructor.
-	pub fn new(_name: &'static str, _description: &'static str, _labels: &[&'static str]) -> Self {
+	pub const fn new(_definition: CounterVecDefinition) -> Self {
 		CounterVec
 	}
 	/// Sets label values, implementation is a `no op`.
-	pub fn with_label_values(&mut self, _label_values: &[&'static str]) -> &mut Self {
+	pub fn with_label_values(&self, _label_values: &[&'static str]) -> &Self {
 		self
 	}
 	/// Increment counter by value, implementation is a `no op`.
-	pub fn inc_by(&mut self, _: u64) {}
+	pub fn inc_by(&self, _: u64) {}
 	/// Increment counter, implementation is a `no op`.
-	pub fn inc(&mut self) {}
+	pub fn inc(&self) {}
 }
 
 /// Dummy implementation.
 impl Counter {
 	/// Constructor.
-	pub fn new(_name: &'static str, _description: &'static str) -> Self {
+	pub const fn new(_definition: CounterDefinition) -> Self {
 		Counter
 	}
 	/// Increment counter by value, implementation is a `no op`.
-	pub fn inc_by(&mut self, _: u64) {}
+	pub fn inc_by(&self, _: u64) {}
 	/// Increment counter, implementation is a `no op`.
-	pub fn inc(&mut self) {}
+	pub fn inc(&self) {}
 }

polkadot/runtime/parachains/src/lib.rs

@@ -29,6 +29,7 @@ pub mod dmp;
 pub mod hrmp;
 pub mod inclusion;
 pub mod initializer;
+pub mod metrics;
 pub mod origin;
 pub mod paras;
 pub mod paras_inherent;

polkadot/runtime/parachains/src/metrics.rs

@@ -0,0 +1,118 @@
+// Copyright 2021 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot 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.
+
+// Polkadot 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 Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Runtime declaration of the parachain metrics.
+
+use polkadot_runtime_metrics::{Counter, CounterVec};
+use primitives::v1::metric_definitions::{
+	PARACHAIN_CREATE_INHERENT_BITFIELDS_SIGNATURE_CHECKS,
+	PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED, PARACHAIN_INHERENT_DATA_CANDIDATES_PROCESSED,
+	PARACHAIN_INHERENT_DATA_DISPUTE_SETS_INCLUDED, PARACHAIN_INHERENT_DATA_DISPUTE_SETS_PROCESSED,
+	PARACHAIN_INHERENT_DATA_WEIGHT,
+};
+
+pub struct Metrics {
+	/// Samples inherent data weight.
+	inherent_data_weight: CounterVec,
+	/// Counts how many inherent bitfields processed in `enter_inner`.
+	bitfields_processed: Counter,
+	/// Counts how many parachain candidates processed in `enter_inner`.
+	candidates_processed: CounterVec,
+	/// Counts dispute statements sets processed in `enter_inner`.
+	dispute_sets_processed: CounterVec,
+	/// Counts dispute statements sets included in `enter_inner`.
+	disputes_included: Counter,
+	/// Counts bitfield signature checks in `enter_inner`.
+	bitfields_signature_checks: CounterVec,
+}
+
+impl Metrics {
+	/// Sample the inherent data weight metric before filtering.
+	pub fn on_before_filter(&self, value: u64) {
+		self.inherent_data_weight.with_label_values(&["before-filter"]).inc_by(value);
+	}
+
+	/// Sample the inherent data weight metric after filtering.
+	pub fn on_after_filter(&self, value: u64) {
+		self.inherent_data_weight.with_label_values(&["after-filter"]).inc_by(value);
+	}
+
+	/// Increment the number of bitfields processed.
+	pub fn on_bitfields_processed(&self, value: u64) {
+		self.bitfields_processed.inc_by(value);
+	}
+
+	/// Increment the number of parachain candidates included.
+	pub fn on_candidates_included(&self, value: u64) {
+		self.candidates_processed.with_label_values(&["included"]).inc_by(value);
+	}
+
+	/// Increment the number of parachain candidates sanitized.
+	pub fn on_candidates_sanitized(&self, value: u64) {
+		self.candidates_processed.with_label_values(&["sanitized"]).inc_by(value);
+	}
+
+	/// Increment the total number of parachain candidates received in `enter_inner`.
+	pub fn on_candidates_processed_total(&self, value: u64) {
+		self.candidates_processed.with_label_values(&["total"]).inc_by(value);
+	}
+
+	/// Sample the relay chain freeze events causing runtime to not process candidates in
+	/// `enter_inner`.
+	pub fn on_relay_chain_freeze(&self) {
+		self.dispute_sets_processed.with_label_values(&["frozen"]).inc();
+	}
+
+	/// Sample the number of dispute sets processed from the current session.
+	pub fn on_current_session_disputes_processed(&self, value: u64) {
+		self.dispute_sets_processed.with_label_values(&["current"]).inc_by(value);
+	}
+
+	/// Increment the number of disputes that have concluded as invalid.
+	pub fn on_disputes_concluded_invalid(&self, value: u64) {
+		self.dispute_sets_processed
+			.with_label_values(&["concluded_invalid"])
+			.inc_by(value);
+	}
+
+	/// Increment the number of disputes imported.
+	pub fn on_disputes_imported(&self, value: u64) {
+		self.dispute_sets_processed.with_label_values(&["imported"]).inc_by(value);
+	}
+
+	pub fn on_disputes_included(&self, value: u64) {
+		self.disputes_included.inc_by(value);
+	}
+
+	pub fn on_valid_bitfield_signature(&self) {
+		self.bitfields_signature_checks.with_label_values(&["valid"]).inc();
+	}
+
+	pub fn on_invalid_bitfield_signature(&self) {
+		self.bitfields_signature_checks.with_label_values(&["invalid"]).inc();
+	}
+}
+
+pub const METRICS: Metrics = Metrics {
+	inherent_data_weight: CounterVec::new(PARACHAIN_INHERENT_DATA_WEIGHT),
+	bitfields_processed: Counter::new(PARACHAIN_INHERENT_DATA_BITFIELDS_PROCESSED),
+	candidates_processed: CounterVec::new(PARACHAIN_INHERENT_DATA_CANDIDATES_PROCESSED),
+	dispute_sets_processed: CounterVec::new(PARACHAIN_INHERENT_DATA_DISPUTE_SETS_PROCESSED),
+	disputes_included: Counter::new(PARACHAIN_INHERENT_DATA_DISPUTE_SETS_INCLUDED),
+	bitfields_signature_checks: CounterVec::new(
+		PARACHAIN_CREATE_INHERENT_BITFIELDS_SIGNATURE_CHECKS,
+	),
+};

polkadot/runtime/parachains/src/paras_inherent/mod.rs

@@ -26,6 +26,7 @@ use crate::{
 	inclusion,
 	inclusion::{CandidateCheckContext, FullCheck},
 	initializer,
+	metrics::METRICS,
 	scheduler::{self, CoreAssignment, FreedReason},
 	shared, ump, ParaId,
 };
@@ -55,7 +56,6 @@ use sp_std::{
 	vec::Vec,
 };
 
-use polkadot_runtime_metrics::{Counter, CounterVec};
 mod misc;
 mod weights;
 
@@ -264,34 +264,6 @@ impl<T: Config> Pallet<T> {
 		#[cfg(feature = "runtime-metrics")]
 		sp_io::init_tracing();
 
-		let mut weight_metric = CounterVec::new(
-			"parachain_inherent_data_weight",
-			"Inherent data weight before and after filtering",
-			&["when"],
-		);
-
-		let mut bitfields_processed_metric = Counter::new(
-			"parachain_inherent_data_bitfields_processed",
-			"Counts the number of bitfields processed in `enter_inner`.",
-		);
-
-		let mut candidates_processed_metric = CounterVec::new(
-			"parachain_inherent_data_candidates_processed",
-			"Counts the number of parachain block candidates processed in `enter_inner`.",
-			&["category"],
-		);
-
-		let mut dispute_sets_processed_metric = CounterVec::new(
-			"parachain_inherent_data_dispute_sets_processed",
-			"Counts the number of dispute statements sets processed in `enter_inner`.",
-			&["category"],
-		);
-
-		let mut disputes_included_metric = Counter::new(
-			"parachain_inherent_data_disputes_included",
-			"Counts the number of dispute statements sets included in a block in `enter_inner`.",
-		);
-
 		log::debug!(
 			target: LOG_TARGET,
 			"[enter_inner] parent_header={:?} bitfields.len(): {}, backed_candidates.len(): {}, disputes.len(): {}",
@@ -316,9 +288,7 @@ impl<T: Config> Pallet<T> {
 
 		let max_block_weight = <T as frame_system::Config>::BlockWeights::get().max_block;
 
-		weight_metric
-			.with_label_values(&["before-filter"])
-			.inc_by(candidate_weight + bitfields_weight + disputes_weight);
+		METRICS.on_before_filter(candidate_weight + bitfields_weight + disputes_weight);
 
 		// Potentially trim inherent data to ensure processing will be within weight limits
 		let total_weight = {
@@ -366,22 +336,18 @@ impl<T: Config> Pallet<T> {
 			// Note that `provide_multi_dispute_data` will iterate, verify, and import each
 			// dispute; so the input here must be reasonably bounded.
 			let _ = T::DisputesHandler::provide_multi_dispute_data(disputes.clone())?;
-			dispute_sets_processed_metric
-				.with_label_values(&["imported"])
-				.inc_by(disputes.len() as u64);
+			METRICS.on_disputes_imported(disputes.len() as u64);
 
 			if T::DisputesHandler::is_frozen() {
 				// Relay chain freeze, at this point we will not include any parachain blocks.
-				dispute_sets_processed_metric.with_label_values(&["frozen"]).inc();
+				METRICS.on_relay_chain_freeze();
 
 				// The relay chain we are currently on is invalid. Proceed no further on parachains.
 				return Ok(Some(dispute_statements_weight::<T>(&disputes)).into())
 			}
 
 			// Process the dispute sets of the current session.
-			dispute_sets_processed_metric
-				.with_label_values(&["current"])
-				.inc_by(new_current_dispute_sets.len() as u64);
+			METRICS.on_current_session_disputes_processed(new_current_dispute_sets.len() as u64);
 
 			let mut freed_disputed = if !new_current_dispute_sets.is_empty() {
 				let concluded_invalid_disputes = new_current_dispute_sets
@@ -393,9 +359,7 @@ impl<T: Config> Pallet<T> {
 					.collect::<BTreeSet<CandidateHash>>();
 
 				// Count invalid dispute sets.
-				dispute_sets_processed_metric
-					.with_label_values(&["concluded_invalid"])
-					.inc_by(concluded_invalid_disputes.len() as u64);
+				METRICS.on_disputes_concluded_invalid(concluded_invalid_disputes.len() as u64);
 
 				let freed_disputed: Vec<_> =
 					<inclusion::Pallet<T>>::collect_disputed(&concluded_invalid_disputes)
@@ -425,7 +389,8 @@ impl<T: Config> Pallet<T> {
 			disputed_bitfield
 		};
 
-		bitfields_processed_metric.inc_by(signed_bitfields.len() as u64);
+		METRICS.on_bitfields_processed(signed_bitfields.len() as u64);
+
 		// Process new availability bitfields, yielding any availability cores whose
 		// work has now concluded.
 		let freed_concluded = <inclusion::Pallet<T>>::process_bitfields(
@@ -440,17 +405,14 @@ impl<T: Config> Pallet<T> {
 			T::DisputesHandler::note_included(current_session, *candidate_hash, now);
 		}
 
-		candidates_processed_metric
-			.with_label_values(&["included"])
-			.inc_by(freed_concluded.len() as u64);
+		METRICS.on_candidates_included(freed_concluded.len() as u64);
 		let freed = collect_all_freed_cores::<T, _>(freed_concluded.iter().cloned());
 
 		<scheduler::Pallet<T>>::clear();
 		<scheduler::Pallet<T>>::schedule(freed, now);
 
-		candidates_processed_metric
-			.with_label_values(&["total"])
-			.inc_by(backed_candidates.len() as u64);
+		METRICS.on_candidates_processed_total(backed_candidates.len() as u64);
+
 		let scheduled = <scheduler::Pallet<T>>::scheduled();
 		let backed_candidates = sanitize_backed_candidates::<T, _>(
 			parent_hash,
@@ -461,9 +423,8 @@ impl<T: Config> Pallet<T> {
 			},
 			&scheduled[..],
 		);
-		candidates_processed_metric
-			.with_label_values(&["sanitized"])
-			.inc_by(backed_candidates.len() as u64);
+
+		METRICS.on_candidates_sanitized(backed_candidates.len() as u64);
 
 		// Process backed candidates according to scheduled cores.
 		let parent_storage_root = parent_header.state_root().clone();
@@ -478,7 +439,7 @@ impl<T: Config> Pallet<T> {
 			full_check,
 		)?;
 
-		disputes_included_metric.inc_by(disputes.len() as u64);
+		METRICS.on_disputes_included(disputes.len() as u64);
 
 		// The number of disputes included in a block is
 		// limited by the weight as well as the number of candidate blocks.
@@ -495,7 +456,7 @@ impl<T: Config> Pallet<T> {
 		// this is max config.ump_service_total_weight
 		let _ump_weight = <ump::Pallet<T>>::process_pending_upward_messages();
 
-		weight_metric.with_label_values(&vec!["after-filter"]).inc_by(total_weight);
+		METRICS.on_after_filter(total_weight);
 
 		Ok(Some(total_weight).into())
 	}
@@ -865,12 +826,6 @@ pub(crate) fn sanitize_bitfields<T: crate::inclusion::Config>(
 	validators: &[ValidatorId],
 	full_check: FullCheck,
 ) -> UncheckedSignedAvailabilityBitfields {
-	let mut bitfields_signature_checks_metric = CounterVec::new(
-		"create_inherent_bitfields_signature_checks",
-		"Counts the number of bitfields signature checked in `enter_inner`.",
-		&["validity"],
-	);
-
 	let mut bitfields = Vec::with_capacity(unchecked_bitfields.len());
 
 	let mut last_index: Option<ValidatorIndex> = None;
@@ -941,10 +896,10 @@ pub(crate) fn sanitize_bitfields<T: crate::inclusion::Config>(
 				unchecked_bitfield.try_into_checked(&signing_context, validator_public)
 			{
 				bitfields.push(signed_bitfield.into_unchecked());
-				bitfields_signature_checks_metric.with_label_values(&["valid"]).inc();
+				METRICS.on_valid_bitfield_signature();
 			} else {
 				log::warn!(target: LOG_TARGET, "Invalid bitfield signature");
-				bitfields_signature_checks_metric.with_label_values(&["invalid"]).inc();
+				METRICS.on_invalid_bitfield_signature();
 			};
 		} else {
 			bitfields.push(unchecked_bitfield);