Add blob count label to DATA_COLUMN_SIDECAR_COMPUTATION metric (#6340)

* Add blob count label to `DATA_COLUMN_SIDECAR_COMPUTATION` metric, and move metrics into the compute function, recording only successful computation.

* Move `discard_timer_on_break` usage to caller site.

* Merge branch 'unstable' into compute-data-column-metric

* Merge branch 'unstable' into compute-data-column-metric

beacon_node/beacon_chain/src/block_verification.rs

@@ -74,6 +74,7 @@ use derivative::Derivative;
 use eth2::types::{BlockGossip, EventKind, PublishBlockRequest};
 use execution_layer::PayloadStatus;
 pub use fork_choice::{AttestationFromBlock, PayloadVerificationStatus};
+use lighthouse_metrics::TryExt;
 use parking_lot::RwLockReadGuard;
 use proto_array::Block as ProtoBlock;
 use safe_arith::ArithError;
@@ -796,8 +797,12 @@ fn build_gossip_verified_data_columns<T: BeaconChainTypes>(
                     GossipDataColumnError::KzgNotInitialized,
                 ))?;
 
-            let timer = metrics::start_timer(&metrics::DATA_COLUMN_SIDECAR_COMPUTATION);
-            let sidecars = blobs_to_data_column_sidecars(&blobs, block, kzg, &chain.spec)?;
+            let mut timer = metrics::start_timer_vec(
+                &metrics::DATA_COLUMN_SIDECAR_COMPUTATION,
+                &[&blobs.len().to_string()],
+            );
+            let sidecars = blobs_to_data_column_sidecars(&blobs, block, kzg, &chain.spec)
+                .discard_timer_on_break(&mut timer)?;
             drop(timer);
             let mut gossip_verified_data_columns = vec![];
             for sidecar in sidecars {

beacon_node/beacon_chain/src/metrics.rs

@@ -1647,11 +1647,12 @@ pub static BLOB_SIDECAR_INCLUSION_PROOF_COMPUTATION: LazyLock<Result<Histogram>>
             "Time taken to compute blob sidecar inclusion proof",
         )
     });
-pub static DATA_COLUMN_SIDECAR_COMPUTATION: LazyLock<Result<Histogram>> = LazyLock::new(|| {
-    try_create_histogram_with_buckets(
+pub static DATA_COLUMN_SIDECAR_COMPUTATION: LazyLock<Result<HistogramVec>> = LazyLock::new(|| {
+    try_create_histogram_vec_with_buckets(
         "data_column_sidecar_computation_seconds",
         "Time taken to compute data column sidecar, including cells, proofs and inclusion proof",
-        Ok(vec![0.04, 0.05, 0.1, 0.2, 0.3, 0.5, 0.7, 1.0]),
+        Ok(vec![0.1, 0.25, 0.5, 1.0, 2.5, 5.0, 10.0]),
+        &["blob_count"],
     )
 });
 pub static DATA_COLUMN_SIDECAR_INCLUSION_PROOF_VERIFICATION: LazyLock<Result<Histogram>> =

common/lighthouse_metrics/src/lib.rs

@@ -400,3 +400,31 @@ pub fn decimal_buckets(min_power: i32, max_power: i32) -> Result<Vec<f64>> {
     }
     Ok(buckets)
 }
+
+/// Would be nice to use the `Try` trait bound and have a single implementation, but try_trait_v2
+/// is not a stable feature yet.
+pub trait TryExt {
+    fn discard_timer_on_break(self, timer: &mut Option<HistogramTimer>) -> Self;
+}
+
+impl<T, E> TryExt for std::result::Result<T, E> {
+    fn discard_timer_on_break(self, timer_opt: &mut Option<HistogramTimer>) -> Self {
+        if self.is_err() {
+            if let Some(timer) = timer_opt.take() {
+                timer.stop_and_discard();
+            }
+        }
+        self
+    }
+}
+
+impl<T> TryExt for Option<T> {
+    fn discard_timer_on_break(self, timer_opt: &mut Option<HistogramTimer>) -> Self {
+        if self.is_none() {
+            if let Some(timer) = timer_opt.take() {
+                timer.stop_and_discard();
+            }
+        }
+        self
+    }
+}