Unify metric names (#1209)

* unify metric names

* refactor standalone metrics

* headers sync metrics

* post-merge fix

* fix compilation

* fmt

* fix dashboards

* fix local dashboards

* update Rococo/Wococo runtime version

* remove commented code

* fixed grumbles

* fmt

* fixed widget names

bridges/relays/lib-substrate-relay/src/helpers.rs

@@ -16,17 +16,11 @@
 
 //! Substrate relay helpers
 
-use relay_utils::metrics::{FloatJsonValueMetric, PrometheusError, Registry};
+use relay_utils::metrics::{FloatJsonValueMetric, PrometheusError};
 
 /// Creates standalone token price metric.
-pub fn token_price_metric(
-	registry: &Registry,
-	prefix: Option<&str>,
-	token_id: &str,
-) -> Result<FloatJsonValueMetric, PrometheusError> {
+pub fn token_price_metric(token_id: &str) -> Result<FloatJsonValueMetric, PrometheusError> {
 	FloatJsonValueMetric::new(
-		registry,
-		prefix,
 		format!("https://api.coingecko.com/api/v3/simple/price?ids={}&vs_currencies=btc", token_id),
 		format!("$.{}.btc", token_id),
 		format!("{}_to_base_conversion_rate", token_id.replace("-", "_")),

bridges/relays/lib-substrate-relay/src/messages_lane.rs

@@ -34,7 +34,9 @@ use relay_substrate_client::{
 	BlockNumberOf, Chain, Client, HashOf,
 };
 use relay_utils::{
-	metrics::{F64SharedRef, MetricsParams},
+	metrics::{
+		FloatJsonValueMetric, GlobalMetrics, MetricsParams, PrometheusError, StandaloneMetric,
+	},
 	BlockNumberBase,
 };
 use sp_core::{storage::StorageKey, Bytes};
@@ -63,6 +65,8 @@ pub struct MessagesRelayParams<SC: Chain, SS, TC: Chain, TS, Strategy: RelayStra
 	pub lane_id: LaneId,
 	/// Metrics parameters.
 	pub metrics_params: MetricsParams,
+	/// Pre-registered standalone metrics.
+	pub standalone_metrics: Option<StandaloneMessagesMetrics<SC, TC>>,
 	/// Relay strategy
 	pub relay_strategy: Strategy,
 }
@@ -241,110 +245,155 @@ pub fn select_delivery_transaction_limits<W: pallet_bridge_messages::WeightInfoE
 	(max_number_of_messages, weight_for_messages_dispatch)
 }
 
-/// Shared references to the values of standalone metrics of the message lane relay loop.
+/// Shared references to the standalone metrics of the message lane relay loop.
 #[derive(Debug, Clone)]
-pub struct StandaloneMessagesMetrics {
-	/// Shared reference to the actual target -> <base> chain token conversion rate.
-	pub target_to_base_conversion_rate: Option<F64SharedRef>,
-	/// Shared reference to the actual source -> <base> chain token conversion rate.
-	pub source_to_base_conversion_rate: Option<F64SharedRef>,
-	/// Shared reference to the stored (in the source chain runtime storage) target -> source chain
-	/// conversion rate.
-	pub target_to_source_conversion_rate: Option<F64SharedRef>,
+pub struct StandaloneMessagesMetrics<SC: Chain, TC: Chain> {
+	/// Global metrics.
+	pub global: GlobalMetrics,
+	/// Storage chain proof overhead metric.
+	pub source_storage_proof_overhead: StorageProofOverheadMetric<SC>,
+	/// Target chain proof overhead metric.
+	pub target_storage_proof_overhead: StorageProofOverheadMetric<TC>,
+	/// Source tokens to base conversion rate metric.
+	pub source_to_base_conversion_rate: Option<FloatJsonValueMetric>,
+	/// Target tokens to base conversion rate metric.
+	pub target_to_base_conversion_rate: Option<FloatJsonValueMetric>,
+	/// Source tokens to target tokens conversion rate metric. This rate is stored by the target
+	/// chain.
+	pub source_to_target_conversion_rate:
+		Option<FloatStorageValueMetric<TC, sp_runtime::FixedU128>>,
+	/// Target tokens to source tokens conversion rate metric. This rate is stored by the source
+	/// chain.
+	pub target_to_source_conversion_rate:
+		Option<FloatStorageValueMetric<SC, sp_runtime::FixedU128>>,
 }
 
-impl StandaloneMessagesMetrics {
+impl<SC: Chain, TC: Chain> StandaloneMessagesMetrics<SC, TC> {
+	/// Swap source and target sides.
+	pub fn reverse(self) -> StandaloneMessagesMetrics<TC, SC> {
+		StandaloneMessagesMetrics {
+			global: self.global,
+			source_storage_proof_overhead: self.target_storage_proof_overhead,
+			target_storage_proof_overhead: self.source_storage_proof_overhead,
+			source_to_base_conversion_rate: self.target_to_base_conversion_rate,
+			target_to_base_conversion_rate: self.source_to_base_conversion_rate,
+			source_to_target_conversion_rate: self.target_to_source_conversion_rate,
+			target_to_source_conversion_rate: self.source_to_target_conversion_rate,
+		}
+	}
+
+	/// Register all metrics in the registry.
+	pub fn register_and_spawn(
+		self,
+		metrics: MetricsParams,
+	) -> Result<MetricsParams, PrometheusError> {
+		self.global.register_and_spawn(&metrics.registry)?;
+		self.source_storage_proof_overhead.register_and_spawn(&metrics.registry)?;
+		self.target_storage_proof_overhead.register_and_spawn(&metrics.registry)?;
+		if let Some(m) = self.source_to_base_conversion_rate {
+			m.register_and_spawn(&metrics.registry)?;
+		}
+		if let Some(m) = self.target_to_base_conversion_rate {
+			m.register_and_spawn(&metrics.registry)?;
+		}
+		if let Some(m) = self.target_to_source_conversion_rate {
+			m.register_and_spawn(&metrics.registry)?;
+		}
+		Ok(metrics)
+	}
+
 	/// Return conversion rate from target to source tokens.
 	pub async fn target_to_source_conversion_rate(&self) -> Option<f64> {
-		let target_to_base_conversion_rate =
-			(*self.target_to_base_conversion_rate.as_ref()?.read().await)?;
-		let source_to_base_conversion_rate =
-			(*self.source_to_base_conversion_rate.as_ref()?.read().await)?;
-		Some(source_to_base_conversion_rate / target_to_base_conversion_rate)
+		Self::compute_target_to_source_conversion_rate(
+			*self.target_to_base_conversion_rate.as_ref()?.shared_value_ref().read().await,
+			*self.source_to_base_conversion_rate.as_ref()?.shared_value_ref().read().await,
+		)
+	}
+
+	/// Return conversion rate from target to source tokens, given conversion rates from
+	/// target/source tokens to some base token.
+	fn compute_target_to_source_conversion_rate(
+		target_to_base_conversion_rate: Option<f64>,
+		source_to_base_conversion_rate: Option<f64>,
+	) -> Option<f64> {
+		Some(source_to_base_conversion_rate? / target_to_base_conversion_rate?)
 	}
 }
 
-/// Add general standalone metrics for the message lane relay loop.
-pub fn add_standalone_metrics<P: SubstrateMessageLane>(
-	metrics_prefix: Option<String>,
-	metrics_params: MetricsParams,
-	source_client: Client<P::SourceChain>,
+/// Create standalone metrics for the message lane relay loop.
+///
+/// All metrics returned by this function are exposed by loops that are serving given lane (`P`)
+/// and by loops that are serving reverse lane (`P` with swapped `TargetChain` and `SourceChain`).
+pub fn standalone_metrics<SC: Chain, TC: Chain>(
+	source_client: Client<SC>,
+	target_client: Client<TC>,
 	source_chain_token_id: Option<&str>,
 	target_chain_token_id: Option<&str>,
+	source_to_target_conversion_rate_params: Option<(StorageKey, FixedU128)>,
 	target_to_source_conversion_rate_params: Option<(StorageKey, FixedU128)>,
-) -> anyhow::Result<(MetricsParams, StandaloneMessagesMetrics)> {
-	let mut target_to_source_conversion_rate = None;
-	let mut source_to_base_conversion_rate = None;
-	let mut target_to_base_conversion_rate = None;
-	let mut metrics_params = relay_utils::relay_metrics(metrics_prefix, metrics_params)
-		.standalone_metric(|registry, prefix| {
-			StorageProofOverheadMetric::new(
-				registry,
-				prefix,
-				source_client.clone(),
-				format!("{}_storage_proof_overhead", P::SourceChain::NAME.to_lowercase()),
-				format!("{} storage proof overhead", P::SourceChain::NAME),
-			)
-		})?;
-	if let Some((
-		target_to_source_conversion_rate_storage_key,
-		initial_target_to_source_conversion_rate,
-	)) = target_to_source_conversion_rate_params
-	{
-		metrics_params = metrics_params.standalone_metric(|registry, prefix| {
-			let metric = FloatStorageValueMetric::<_, sp_runtime::FixedU128>::new(
-				registry,
-				prefix,
-				source_client,
-				target_to_source_conversion_rate_storage_key,
-				Some(initial_target_to_source_conversion_rate),
-				format!(
-					"{}_{}_to_{}_conversion_rate",
-					P::SourceChain::NAME,
-					P::TargetChain::NAME,
-					P::SourceChain::NAME
-				),
-				format!(
-					"{} to {} tokens conversion rate (used by {})",
-					P::TargetChain::NAME,
-					P::SourceChain::NAME,
-					P::SourceChain::NAME
-				),
-			)?;
-			target_to_source_conversion_rate = Some(metric.shared_value_ref());
-			Ok(metric)
-		})?;
-	}
-	if let Some(source_chain_token_id) = source_chain_token_id {
-		metrics_params = metrics_params.standalone_metric(|registry, prefix| {
-			let metric =
-				crate::helpers::token_price_metric(registry, prefix, source_chain_token_id)?;
-			source_to_base_conversion_rate = Some(metric.shared_value_ref());
-			Ok(metric)
-		})?;
-	}
-	if let Some(target_chain_token_id) = target_chain_token_id {
-		metrics_params = metrics_params.standalone_metric(|registry, prefix| {
-			let metric =
-				crate::helpers::token_price_metric(registry, prefix, target_chain_token_id)?;
-			target_to_base_conversion_rate = Some(metric.shared_value_ref());
-			Ok(metric)
-		})?;
-	}
-	Ok((
-		metrics_params.into_params(),
-		StandaloneMessagesMetrics {
-			target_to_base_conversion_rate,
-			source_to_base_conversion_rate,
-			target_to_source_conversion_rate,
-		},
-	))
+) -> anyhow::Result<StandaloneMessagesMetrics<SC, TC>> {
+	Ok(StandaloneMessagesMetrics {
+		global: GlobalMetrics::new()?,
+		source_storage_proof_overhead: StorageProofOverheadMetric::new(
+			source_client.clone(),
+			format!("{}_storage_proof_overhead", SC::NAME.to_lowercase()),
+			format!("{} storage proof overhead", SC::NAME),
+		)?,
+		target_storage_proof_overhead: StorageProofOverheadMetric::new(
+			target_client.clone(),
+			format!("{}_storage_proof_overhead", TC::NAME.to_lowercase()),
+			format!("{} storage proof overhead", TC::NAME),
+		)?,
+		source_to_base_conversion_rate: source_chain_token_id
+			.map(|source_chain_token_id| {
+				crate::helpers::token_price_metric(source_chain_token_id).map(Some)
+			})
+			.unwrap_or(Ok(None))?,
+		target_to_base_conversion_rate: target_chain_token_id
+			.map(|target_chain_token_id| {
+				crate::helpers::token_price_metric(target_chain_token_id).map(Some)
+			})
+			.unwrap_or(Ok(None))?,
+		source_to_target_conversion_rate: source_to_target_conversion_rate_params
+			.map(|(key, rate)| {
+				FloatStorageValueMetric::<_, sp_runtime::FixedU128>::new(
+					target_client,
+					key,
+					Some(rate),
+					format!("{}_{}_to_{}_conversion_rate", TC::NAME, SC::NAME, TC::NAME),
+					format!(
+						"{} to {} tokens conversion rate (used by {})",
+						SC::NAME,
+						TC::NAME,
+						TC::NAME
+					),
+				)
+				.map(Some)
+			})
+			.unwrap_or(Ok(None))?,
+		target_to_source_conversion_rate: target_to_source_conversion_rate_params
+			.map(|(key, rate)| {
+				FloatStorageValueMetric::<_, sp_runtime::FixedU128>::new(
+					source_client,
+					key,
+					Some(rate),
+					format!("{}_{}_to_{}_conversion_rate", SC::NAME, TC::NAME, SC::NAME),
+					format!(
+						"{} to {} tokens conversion rate (used by {})",
+						TC::NAME,
+						SC::NAME,
+						SC::NAME
+					),
+				)
+				.map(Some)
+			})
+			.unwrap_or(Ok(None))?,
+	})
 }
 
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use async_std::sync::{Arc, RwLock};
 
 	type RialtoToMillauMessagesWeights =
 		pallet_bridge_messages::weights::RialtoWeight<rialto_runtime::Runtime>;
@@ -369,12 +418,9 @@ mod tests {
 
 	#[async_std::test]
 	async fn target_to_source_conversion_rate_works() {
-		let metrics = StandaloneMessagesMetrics {
-			target_to_base_conversion_rate: Some(Arc::new(RwLock::new(Some(183.15)))),
-			source_to_base_conversion_rate: Some(Arc::new(RwLock::new(Some(12.32)))),
-			target_to_source_conversion_rate: None, // we don't care
-		};
-
-		assert_eq!(metrics.target_to_source_conversion_rate().await, Some(12.32 / 183.15),);
+		assert_eq!(
+			StandaloneMessagesMetrics::<relay_rococo_client::Rococo, relay_wococo_client::Wococo>::compute_target_to_source_conversion_rate(Some(183.15), Some(12.32)),
+			Some(12.32 / 183.15),
+		);
 	}
 }

bridges/relays/lib-substrate-relay/src/messages_target.rs

@@ -55,7 +55,7 @@ pub struct SubstrateMessagesTarget<P: SubstrateMessageLane> {
 	client: Client<P::TargetChain>,
 	lane: P,
 	lane_id: LaneId,
-	metric_values: StandaloneMessagesMetrics,
+	metric_values: StandaloneMessagesMetrics<P::SourceChain, P::TargetChain>,
 	source_to_target_headers_relay: Option<OnDemandHeadersRelay<P::SourceChain>>,
 }
 
@@ -65,7 +65,7 @@ impl<P: SubstrateMessageLane> SubstrateMessagesTarget<P> {
 		client: Client<P::TargetChain>,
 		lane: P,
 		lane_id: LaneId,
-		metric_values: StandaloneMessagesMetrics,
+		metric_values: StandaloneMessagesMetrics<P::SourceChain, P::TargetChain>,
 		source_to_target_headers_relay: Option<OnDemandHeadersRelay<P::SourceChain>>,
 	) -> Self {
 		SubstrateMessagesTarget {