Add op pool metrics for attestations (#2758)

## Proposed Changes

Add several metrics for the number of attestations in the op pool. These give us a way to observe the number of valid, non-trivial attestations during block packing rather than just the size of the entire op pool.

beacon_node/beacon_chain/src/metrics.rs

@@ -365,6 +365,10 @@ lazy_static! {
      */
     pub static ref OP_POOL_NUM_ATTESTATIONS: Result<IntGauge> =
         try_create_int_gauge("beacon_op_pool_attestations_total", "Count of attestations in the op pool");
+    pub static ref OP_POOL_NUM_ATTESTATION_DATA: Result<IntGauge> =
+        try_create_int_gauge("beacon_op_pool_attestation_data_total", "Count of attestation data in the op pool");
+    pub static ref OP_POOL_MAX_AGGREGATES_PER_DATA: Result<IntGauge> =
+        try_create_int_gauge("beacon_op_pool_max_aggregates_per_data", "Max aggregates per AttestationData");
     pub static ref OP_POOL_NUM_ATTESTER_SLASHINGS: Result<IntGauge> =
         try_create_int_gauge("beacon_op_pool_attester_slashings_total", "Count of attester slashings in the op pool");
     pub static ref OP_POOL_NUM_PROPOSER_SLASHINGS: Result<IntGauge> =
@@ -886,9 +890,19 @@ pub fn scrape_for_metrics<T: BeaconChainTypes>(beacon_chain: &BeaconChain<T>) {
         scrape_sync_committee_observation(slot, beacon_chain);
     }
 
+    let attestation_stats = beacon_chain.op_pool.attestation_stats();
+
     set_gauge_by_usize(
         &OP_POOL_NUM_ATTESTATIONS,
-        beacon_chain.op_pool.num_attestations(),
+        attestation_stats.num_attestations,
+    );
+    set_gauge_by_usize(
+        &OP_POOL_NUM_ATTESTATION_DATA,
+        attestation_stats.num_attestation_data,
+    );
+    set_gauge_by_usize(
+        &OP_POOL_MAX_AGGREGATES_PER_DATA,
+        attestation_stats.max_aggregates_per_data,
     );
     set_gauge_by_usize(
         &OP_POOL_NUM_ATTESTER_SLASHINGS,

beacon_node/operation_pool/src/lib.rs

@@ -57,6 +57,15 @@ pub enum OpPoolError {
     IncorrectOpPoolVariant,
 }
 
+pub struct AttestationStats {
+    /// Total number of attestations for all committeees/indices/votes.
+    pub num_attestations: usize,
+    /// Number of unique `AttestationData` attested to.
+    pub num_attestation_data: usize,
+    /// Maximum number of aggregates for a single `AttestationData`.
+    pub max_aggregates_per_data: usize,
+}
+
 impl From<SyncAggregateError> for OpPoolError {
     fn from(e: SyncAggregateError) -> Self {
         OpPoolError::SyncAggregateError(e)
@@ -207,6 +216,23 @@ impl<T: EthSpec> OperationPool<T> {
         self.attestations.read().values().map(Vec::len).sum()
     }
 
+    pub fn attestation_stats(&self) -> AttestationStats {
+        let mut num_attestations = 0;
+        let mut num_attestation_data = 0;
+        let mut max_aggregates_per_data = 0;
+
+        for aggregates in self.attestations.read().values() {
+            num_attestations += aggregates.len();
+            num_attestation_data += 1;
+            max_aggregates_per_data = std::cmp::max(max_aggregates_per_data, aggregates.len());
+        }
+        AttestationStats {
+            num_attestations,
+            num_attestation_data,
+            max_aggregates_per_data,
+        }
+    }
+
     /// Return all valid attestations for the given epoch, for use in max cover.
     fn get_valid_attestations_for_epoch<'a>(
         &'a self,
@@ -265,22 +291,29 @@ impl<T: EthSpec> OperationPool<T> {
 
         // Split attestations for the previous & current epochs, so that we
         // can optimise them individually in parallel.
-        let prev_epoch_att = self.get_valid_attestations_for_epoch(
+        let mut num_prev_valid = 0_i64;
+        let mut num_curr_valid = 0_i64;
+
+        let prev_epoch_att = self
+            .get_valid_attestations_for_epoch(
                 prev_epoch,
                 &*all_attestations,
                 state,
                 total_active_balance,
                 prev_epoch_validity_filter,
                 spec,
-        );
+            )
-        let curr_epoch_att = self.get_valid_attestations_for_epoch(
+            .inspect(|_| num_prev_valid += 1);
+        let curr_epoch_att = self
+            .get_valid_attestations_for_epoch(
                 current_epoch,
                 &*all_attestations,
                 state,
                 total_active_balance,
                 curr_epoch_validity_filter,
                 spec,
-        );
+            )
+            .inspect(|_| num_curr_valid += 1);
 
         let prev_epoch_limit = if let BeaconState::Base(base_state) = state {
             std::cmp::min(
@@ -299,15 +332,22 @@ impl<T: EthSpec> OperationPool<T> {
                 if prev_epoch == current_epoch {
                     vec![]
                 } else {
-                    maximum_cover(prev_epoch_att, prev_epoch_limit)
+                    maximum_cover(prev_epoch_att, prev_epoch_limit, "prev_epoch_attestations")
                 }
             },
             move || {
                 let _timer = metrics::start_timer(&metrics::ATTESTATION_CURR_EPOCH_PACKING_TIME);
-                maximum_cover(curr_epoch_att, T::MaxAttestations::to_usize())
+                maximum_cover(
+                    curr_epoch_att,
+                    T::MaxAttestations::to_usize(),
+                    "curr_epoch_attestations",
+                )
             },
         );
 
+        metrics::set_gauge(&metrics::NUM_PREV_EPOCH_ATTESTATIONS, num_prev_valid);
+        metrics::set_gauge(&metrics::NUM_CURR_EPOCH_ATTESTATIONS, num_curr_valid);
+
         Ok(max_cover::merge_solutions(
             curr_cover,
             prev_cover,
@@ -394,6 +434,7 @@ impl<T: EthSpec> OperationPool<T> {
         let attester_slashings = maximum_cover(
             relevant_attester_slashings,
             T::MaxAttesterSlashings::to_usize(),
+            "attester_slashings",
         )
         .into_iter()
         .map(|cover| {

beacon_node/operation_pool/src/max_cover.rs

@@ -1,3 +1,4 @@
+use crate::metrics;
 use itertools::Itertools;
 
 /// Trait for types that we can compute a maximum cover for.
@@ -44,7 +45,7 @@ impl<T> MaxCoverItem<T> {
 ///
 /// * Time complexity: `O(limit * items_iter.len())`
 /// * Space complexity: `O(item_iter.len())`
-pub fn maximum_cover<I, T>(items_iter: I, limit: usize) -> Vec<T>
+pub fn maximum_cover<I, T>(items_iter: I, limit: usize, label: &str) -> Vec<T>
 where
     I: IntoIterator<Item = T>,
     T: MaxCover,
@@ -56,6 +57,12 @@ where
         .filter(|x| x.item.score() != 0)
         .collect();
 
+    metrics::set_int_gauge(
+        &metrics::MAX_COVER_NON_ZERO_ITEMS,
+        &[label],
+        all_items.len() as i64,
+    );
+
     let mut result = vec![];
 
     for _ in 0..limit {
@@ -146,14 +153,14 @@ mod test {
 
     #[test]
     fn zero_limit() {
-        let cover = maximum_cover(example_system(), 0);
+        let cover = maximum_cover(example_system(), 0, "test");
         assert_eq!(cover.len(), 0);
     }
 
     #[test]
     fn one_limit() {
         let sets = example_system();
-        let cover = maximum_cover(sets.clone(), 1);
+        let cover = maximum_cover(sets.clone(), 1, "test");
         assert_eq!(cover.len(), 1);
         assert_eq!(cover[0], sets[1]);
     }
@@ -163,7 +170,7 @@ mod test {
     fn exclude_zero_score() {
         let sets = example_system();
         for k in 2..10 {
-            let cover = maximum_cover(sets.clone(), k);
+            let cover = maximum_cover(sets.clone(), k, "test");
             assert_eq!(cover.len(), 2);
             assert_eq!(cover[0], sets[1]);
             assert_eq!(cover[1], sets[0]);
@@ -187,7 +194,7 @@ mod test {
             HashSet::from_iter(vec![5, 6, 7, 8]),      // 4, 4*
             HashSet::from_iter(vec![0, 1, 2, 3, 4]),   // 5*
         ];
-        let cover = maximum_cover(sets, 3);
+        let cover = maximum_cover(sets, 3, "test");
         assert_eq!(quality(&cover), 11);
     }
 
@@ -202,7 +209,7 @@ mod test {
             HashSet::from_iter(vec![1, 5, 6, 8]),
             HashSet::from_iter(vec![1, 7, 11, 19]),
         ];
-        let cover = maximum_cover(sets, 5);
+        let cover = maximum_cover(sets, 5, "test");
         assert_eq!(quality(&cover), 19);
         assert_eq!(cover.len(), 5);
     }

beacon_node/operation_pool/src/metrics.rs

@@ -11,4 +11,17 @@ lazy_static! {
         "op_pool_attestation_curr_epoch_packing_time",
         "Time to pack current epoch attestations"
     );
+    pub static ref NUM_PREV_EPOCH_ATTESTATIONS: Result<IntGauge> = try_create_int_gauge(
+        "op_pool_prev_epoch_attestations",
+        "Number of valid attestations considered for packing from the previous epoch"
+    );
+    pub static ref NUM_CURR_EPOCH_ATTESTATIONS: Result<IntGauge> = try_create_int_gauge(
+        "op_pool_curr_epoch_attestations",
+        "Number of valid attestations considered for packing from the current epoch"
+    );
+    pub static ref MAX_COVER_NON_ZERO_ITEMS: Result<IntGaugeVec> = try_create_int_gauge_vec(
+        "op_pool_max_cover_non_zero_items",
+        "Number of non-trivial items considered in a max coverage optimisation",
+        &["label"]
+    );
 }