From e4f9551a4d63f2f1761082439590d9865adf4dec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <bkchr@users.noreply.github.com>
Date: Tue, 25 Jan 2022 20:57:56 +0100
Subject: [PATCH] basic-authorship: Add new metrics for block size limit and
 weight limit (#10697)

* basic-authorship: Add new metriscs for block size limit and weight limit

* Review feedback
---
 .../basic-authorship/src/basic_authorship.rs  | 28 +++++++++------
 client/proposer-metrics/src/lib.rs            | 36 +++++++++++++++++--
 2 files changed, 51 insertions(+), 13 deletions(-)

diff --git a/client/basic-authorship/src/basic_authorship.rs b/client/basic-authorship/src/basic_authorship.rs
index 5ab207e39e583..23725e5138697 100644
--- a/client/basic-authorship/src/basic_authorship.rs
+++ b/client/basic-authorship/src/basic_authorship.rs
@@ -47,7 +47,7 @@ use sp_runtime::{
 use std::{marker::PhantomData, pin::Pin, sync::Arc, time};
 
 use prometheus_endpoint::Registry as PrometheusRegistry;
-use sc_proposer_metrics::MetricsLink as PrometheusMetrics;
+use sc_proposer_metrics::{EndProposingReason, MetricsLink as PrometheusMetrics};
 
 /// Default block size limit in bytes used by [`Proposer`].
 ///
@@ -412,16 +412,21 @@ where
 		debug!("Attempting to push transactions from the pool.");
 		debug!("Pool status: {:?}", self.transaction_pool.status());
 		let mut transaction_pushed = false;
-		let mut hit_block_size_limit = false;
 
-		while let Some(pending_tx) = pending_iterator.next() {
+		let end_reason = loop {
+			let pending_tx = if let Some(pending_tx) = pending_iterator.next() {
+				pending_tx
+			} else {
+				break EndProposingReason::NoMoreTransactions
+			};
+
 			let now = (self.now)();
 			if now > deadline {
 				debug!(
 					"Consensus deadline reached when pushing block transactions, \
 					proceeding with proposing."
 				);
-				break
+				break EndProposingReason::HitDeadline
 			}
 
 			let pending_tx_data = pending_tx.data().clone();
@@ -448,8 +453,7 @@ where
 					continue
 				} else {
 					debug!("Reached block size limit, proceeding with proposing.");
-					hit_block_size_limit = true;
-					break
+					break EndProposingReason::HitBlockSizeLimit
 				}
 			}
 
@@ -473,8 +477,8 @@ where
 							 so we will try a bit more before quitting."
 						);
 					} else {
-						debug!("Block is full, proceed with proposing.");
-						break
+						debug!("Reached block weight limit, proceeding with proposing.");
+						break EndProposingReason::HitBlockWeightLimit
 					}
 				},
 				Err(e) if skipped > 0 => {
@@ -491,9 +495,9 @@ where
 					unqueue_invalid.push(pending_tx_hash);
 				},
 			}
-		}
+		};
 
-		if hit_block_size_limit && !transaction_pushed {
+		if matches!(end_reason, EndProposingReason::HitBlockSizeLimit) && !transaction_pushed {
 			warn!(
 				"Hit block size limit of `{}` without including any transaction!",
 				block_size_limit,
@@ -507,6 +511,8 @@ where
 		self.metrics.report(|metrics| {
 			metrics.number_of_transactions.set(block.extrinsics().len() as u64);
 			metrics.block_constructed.observe(block_timer.elapsed().as_secs_f64());
+
+			metrics.report_end_proposing_reason(end_reason);
 		});
 
 		info!(
@@ -518,7 +524,7 @@ where
 			block.extrinsics().len(),
 			block.extrinsics()
 				.iter()
-				.map(|xt| format!("{}", BlakeTwo256::hash_of(xt)))
+				.map(|xt| BlakeTwo256::hash_of(xt).to_string())
 				.collect::<Vec<_>>()
 				.join(", ")
 		);
diff --git a/client/proposer-metrics/src/lib.rs b/client/proposer-metrics/src/lib.rs
index a34660faab5d3..c27d16ea04d7d 100644
--- a/client/proposer-metrics/src/lib.rs
+++ b/client/proposer-metrics/src/lib.rs
@@ -19,7 +19,8 @@
 //! Prometheus basic proposer metrics.
 
 use prometheus_endpoint::{
-	register, Gauge, Histogram, HistogramOpts, PrometheusError, Registry, U64,
+	prometheus::CounterVec, register, Gauge, Histogram, HistogramOpts, Opts, PrometheusError,
+	Registry, U64,
 };
 
 /// Optional shareable link to basic authorship metrics.
@@ -38,15 +39,24 @@ impl MetricsLink {
 	}
 
 	pub fn report<O>(&self, do_this: impl FnOnce(&Metrics) -> O) -> Option<O> {
-		Some(do_this(self.0.as_ref()?))
+		self.0.as_ref().map(do_this)
 	}
 }
 
+/// The reason why proposing a block ended.
+pub enum EndProposingReason {
+	NoMoreTransactions,
+	HitDeadline,
+	HitBlockSizeLimit,
+	HitBlockWeightLimit,
+}
+
 /// Authorship metrics.
 #[derive(Clone)]
 pub struct Metrics {
 	pub block_constructed: Histogram,
 	pub number_of_transactions: Gauge<U64>,
+	pub end_proposing_reason: CounterVec,
 	pub create_inherents_time: Histogram,
 	pub create_block_proposal_time: Histogram,
 }
@@ -82,6 +92,28 @@ impl Metrics {
 				))?,
 				registry,
 			)?,
+			end_proposing_reason: register(
+				CounterVec::new(
+					Opts::new(
+						"substrate_proposer_end_proposal_reason",
+						"The reason why the block proposing was ended. This doesn't include errors.",
+					),
+					&["reason"],
+				)?,
+				registry,
+			)?,
 		})
 	}
+
+	/// Report the reason why the proposing ended.
+	pub fn report_end_proposing_reason(&self, reason: EndProposingReason) {
+		let reason = match reason {
+			EndProposingReason::HitDeadline => "hit_deadline",
+			EndProposingReason::NoMoreTransactions => "no_more_transactions",
+			EndProposingReason::HitBlockSizeLimit => "hit_block_size_limit",
+			EndProposingReason::HitBlockWeightLimit => "hit_block_weight_limit",
+		};
+
+		self.end_proposing_reason.with_label_values(&[reason]).inc();
+	}
 }