Add more metrics (#2310)

Closes #807

## Description
This PR adds a couple of additional metrics (block importer and p2p) and
also contains slight refactor of how we initialize bucket sized for
histogram based metrics.

The `metrics` command-line parameter has been replaced with
`disable-metrics`. Metrics are now enabled by default, with the option
to disable them entirely or on a per-module basis. This change is
_breaking_ for all dependencies that use CLI to setup the
`fuel-core-client`

```
      --disable-metrics <METRICS>
          Disables all metrics, or specify a comma-separated list of modules to disable metrics for specific ones. Available options: importer, p2p, producer, txpool, graphql
          
          [env: METRICS=]
          [default: ]
```

Startup logs also show the metrics config:
```
2024-10-14T20:17:37.536840Z  INFO fuel_core_bin::cli::run: 308: Metrics config: Disable modules: txpool
```

## Checklist
- [X] Breaking changes are clearly marked as such in the PR description
and changelog
- [X] New behavior is reflected in tests

### Before requesting review
- [X] I have reviewed the code myself

---------

Co-authored-by: acerone85 <[email protected]>
Co-authored-by: rymnc <[email protected]>
Co-authored-by: Green Baneling <[email protected]>

CHANGELOG.md

@@ -15,6 +15,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 - [2334](https://github.com/FuelLabs/fuel-core/pull/2334): Prepare the GraphQL service for the switching to `async` methods.
+- [2310](https://github.com/FuelLabs/fuel-core/pull/2310): New metrics: "The gas prices used in a block" (`importer_gas_price_for_block`), "The total gas used in a block" (`importer_gas_per_block`), "The total fee (gwei) paid by transactions in a block" (`importer_fee_per_block_gwei`), "The total number of transactions in a block" (`importer_transactions_per_block`), P2P metrics for swarm and protocol.
+
+#### Breaking
+- [2310](https://github.com/FuelLabs/fuel-core/pull/2310): The `metrics` command-line parameter has been replaced with `disable-metrics`. Metrics are now enabled by default, with the option to disable them entirely or on a per-module basis.
 - [2341](https://github.com/FuelLabs/fuel-core/pull/2341): Updated all pagination queries to work with the async stream instead of the sync iterator.
 - [2340](https://github.com/FuelLabs/fuel-core/pull/2340): Avoid long heavy tasks in the GraphQL service by splitting work into batches.
 - [2350](https://github.com/FuelLabs/fuel-core/pull/2350): Limited the number of threads used by the GraphQL service.

bin/fuel-core/Cargo.toml

@@ -28,6 +28,7 @@ dotenvy = { version = "0.15", optional = true }
 fuel-core = { workspace = true, features = ["wasm-executor"] }
 fuel-core-chain-config = { workspace = true }
 fuel-core-compression = { workspace = true }
+fuel-core-metrics = { workspace = true }
 fuel-core-poa = { workspace = true }
 fuel-core-types = { workspace = true, features = ["std"] }
 hex = { workspace = true }
@@ -53,6 +54,7 @@ itertools = { workspace = true }
 pretty_assertions = { workspace = true }
 rand = { workspace = true }
 serde = { workspace = true }
+strum = { workspace = true }
 tempfile = { workspace = true }
 test-case = { workspace = true }
 

bin/fuel-core/src/cli/run.rs

@@ -51,6 +51,10 @@ use fuel_core_chain_config::{
     SnapshotMetadata,
     SnapshotReader,
 };
+use fuel_core_metrics::config::{
+    DisableConfig,
+    Module,
+};
 use fuel_core_poa::signer::SignMode;
 use fuel_core_types::blockchain::header::StateTransitionBytecodeVersion;
 use pyroscope::{
@@ -236,8 +240,8 @@ pub struct Command {
     #[cfg(feature = "p2p")]
     pub sync_args: p2p::SyncArgs,
 
-    #[arg(long = "metrics", env)]
-    pub metrics: bool,
+    #[arg(long = "disable-metrics", value_delimiter = ',', help = fuel_core_metrics::config::help_string(), env)]
+    pub disabled_metrics: Vec<Module>,
 
     #[clap(long = "verify-max-da-lag", default_value = "10", env)]
     pub max_da_lag: u64,
@@ -294,7 +298,7 @@ impl Command {
             p2p_args,
             #[cfg(feature = "p2p")]
             sync_args,
-            metrics,
+            disabled_metrics: metrics,
             max_da_lag,
             max_wait_time,
             tx_pool,
@@ -305,6 +309,14 @@ impl Command {
             profiling: _,
         } = self;
 
+        let enabled_metrics = metrics.list_of_enabled();
+
+        if !enabled_metrics.is_empty() {
+            info!("`{:?}` metrics are enabled", enabled_metrics);
+        } else {
+            info!("All metrics are disabled");
+        }
+
         let addr = net::SocketAddr::new(graphql.ip, graphql.port);
 
         let snapshot_reader = match snapshot.as_ref() {
@@ -320,7 +332,10 @@ impl Command {
         let relayer_cfg = relayer_args.into_config();
 
         #[cfg(feature = "p2p")]
-        let p2p_cfg = p2p_args.into_config(chain_config.chain_name.clone(), metrics)?;
+        let p2p_cfg = p2p_args.into_config(
+            chain_config.chain_name.clone(),
+            metrics.is_enabled(Module::P2P),
+        )?;
 
         let trigger: Trigger = poa_trigger.into();
 
@@ -428,8 +443,9 @@ impl Command {
             state_rewind_policy,
         };
 
-        let block_importer =
-            fuel_core::service::config::fuel_core_importer::Config::new();
+        let block_importer = fuel_core::service::config::fuel_core_importer::Config::new(
+            metrics.is_enabled(Module::Importer),
+        );
 
         let da_compression = match da_compression {
             Some(retention) => {
@@ -549,7 +565,7 @@ impl Command {
             },
             block_producer: ProducerConfig {
                 coinbase_recipient,
-                metrics,
+                metrics: metrics.is_enabled(Module::Producer),
             },
             starting_gas_price,
             gas_price_change_percent,
@@ -659,3 +675,85 @@ fn start_pyroscope_agent(
         })
         .transpose()
 }
+
+#[cfg(test)]
+#[allow(non_snake_case)]
+#[allow(clippy::bool_assert_comparison)]
+mod tests {
+    use super::*;
+    use strum::IntoEnumIterator;
+
+    fn parse_command(args: &[&str]) -> anyhow::Result<Command> {
+        Ok(Command::try_parse_from([""].iter().chain(args))?)
+    }
+
+    #[test]
+    fn parse_disabled_metrics__no_value_enables_everything() {
+        // Given
+        let args = [];
+
+        // When
+        let command = parse_command(&args).unwrap();
+
+        // Then
+        let config = command.disabled_metrics;
+        Module::iter().for_each(|module| {
+            assert_eq!(config.is_enabled(module), true);
+        });
+    }
+
+    #[test]
+    fn parse_disabled_metrics__all() {
+        // Given
+        let args = ["--disable-metrics", "all"];
+
+        // When
+        let command = parse_command(&args).unwrap();
+
+        // Then
+        let config = command.disabled_metrics;
+        Module::iter().for_each(|module| {
+            assert_eq!(config.is_enabled(module), false);
+        });
+    }
+
+    #[test]
+    fn parse_disabled_metrics__mixed_args() {
+        // Given
+        let args = [
+            "--disable-metrics",
+            "txpool,importer",
+            "--disable-metrics",
+            "graphql",
+        ];
+
+        // When
+        let command = parse_command(&args).unwrap();
+
+        // Then
+        let config = command.disabled_metrics;
+        assert_eq!(config.is_enabled(Module::TxPool), false);
+        assert_eq!(config.is_enabled(Module::Importer), false);
+        assert_eq!(config.is_enabled(Module::GraphQL), false);
+        assert_eq!(config.is_enabled(Module::P2P), true);
+        assert_eq!(config.is_enabled(Module::Producer), true);
+    }
+
+    #[test]
+    fn parse_disabled_metrics__bad_values() {
+        // Given
+        let args = ["--disable-metrics", "txpool,alpha,bravo"];
+
+        // When
+        let command = parse_command(&args);
+
+        // Then
+        let err = command.expect_err("should fail to parse");
+        assert_eq!(
+            err.to_string(),
+            "error: invalid value 'alpha' for \
+            '--disable-metrics <DISABLED_METRICS>': Matching variant not found\
+            \n\nFor more information, try '--help'.\n"
+        );
+    }
+}

crates/fuel-core/src/service/adapters/block_importer.rs

@@ -62,8 +62,11 @@ impl BlockImporterAdapter {
         executor: ExecutorAdapter,
         verifier: VerifierAdapter,
     ) -> Self {
+        let metrics = config.metrics;
         let importer = Importer::new(chain_id, config, database, executor, verifier);
-        importer.init_metrics();
+        if metrics {
+            importer.init_metrics();
+        }
         Self {
             block_importer: Arc::new(importer),
         }

crates/fuel-core/src/service/config.rs

@@ -104,7 +104,7 @@ impl Config {
 
     #[cfg(feature = "test-helpers")]
     pub fn local_node_with_reader(snapshot_reader: SnapshotReader) -> Self {
-        let block_importer = fuel_core_importer::Config::new();
+        let block_importer = fuel_core_importer::Config::new(false);
         let latest_block = snapshot_reader.last_block_config();
         // In tests, we always want to use the native executor as a default configuration.
         let native_executor_version = latest_block

crates/metrics/Cargo.toml

@@ -11,10 +11,13 @@ repository = { workspace = true }
 description = "Fuel metrics"
 
 [dependencies]
+once_cell = { workspace = true }
 parking_lot = { workspace = true }
 pin-project-lite = { workspace = true }
 prometheus-client = { workspace = true }
 regex = "1"
+strum = { workspace = true }
+strum_macros = { workspace = true }
 tracing = { workspace = true }
 
 [dev-dependencies]

crates/metrics/src/buckets.rs

@@ -0,0 +1,65 @@
+use std::{
+    collections::HashMap,
+    sync::OnceLock,
+};
+#[cfg(test)]
+use strum_macros::EnumIter;
+
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[cfg_attr(test, derive(EnumIter))]
+pub(crate) enum Buckets {
+    Timing,
+}
+static BUCKETS: OnceLock<HashMap<Buckets, Vec<f64>>> = OnceLock::new();
+pub(crate) fn buckets(b: Buckets) -> impl Iterator<Item = f64> {
+    BUCKETS.get_or_init(initialize_buckets)[&b].iter().copied()
+}
+
+#[rustfmt::skip]
+fn initialize_buckets() -> HashMap<Buckets, Vec<f64>> {
+    [
+        (
+            Buckets::Timing,
+            vec![
+                0.005,
+                0.010,
+                0.025,
+                0.050,
+                0.100,
+                0.250,
+                0.500,
+                1.000,
+                2.500,
+                5.000,
+               10.000,
+            ],
+        ),
+    ]
+    .into_iter()
+    .collect()
+}
+
+#[cfg(test)]
+mod tests {
+    use strum::IntoEnumIterator;
+
+    use crate::buckets::Buckets;
+
+    use super::initialize_buckets;
+
+    #[test]
+    fn buckets_are_defined_for_every_variant() {
+        let actual_buckets = initialize_buckets();
+        let actual_buckets = actual_buckets.keys().collect::<Vec<_>>();
+
+        let required_buckets: Vec<_> = Buckets::iter().collect();
+
+        assert_eq!(required_buckets.len(), actual_buckets.len());
+
+        let all_buckets_defined = required_buckets
+            .iter()
+            .all(|required_bucket| actual_buckets.contains(&required_bucket));
+
+        assert!(all_buckets_defined)
+    }
+}

crates/metrics/src/config.rs

@@ -0,0 +1,51 @@
+use once_cell::sync::Lazy;
+use strum::IntoEnumIterator;
+use strum_macros::{
+    Display,
+    EnumIter,
+    EnumString,
+};
+
+#[derive(Debug, Display, Clone, Copy, PartialEq, EnumString, EnumIter)]
+#[strum(serialize_all = "lowercase")]
+pub enum Module {
+    All,
+    Importer,
+    P2P,
+    Producer,
+    TxPool, /* TODO[RC]: Not used. Add support in https://github.com/FuelLabs/fuel-core/pull/2321 */
+    GraphQL, // TODO[RC]: Not used... yet.
+}
+
+/// Configuration for disabling metrics.
+pub trait DisableConfig {
+    /// Returns `true` if the given module is enabled.
+    fn is_enabled(&self, module: Module) -> bool;
+
+    /// Returns the list of enabled modules.
+    fn list_of_enabled(&self) -> Vec<Module>;
+}
+
+impl DisableConfig for Vec<Module> {
+    fn is_enabled(&self, module: Module) -> bool {
+        !self.contains(&module) && !self.contains(&Module::All)
+    }
+
+    fn list_of_enabled(&self) -> Vec<Module> {
+        Module::iter()
+            .filter(|module| self.is_enabled(*module) && *module != Module::All)
+            .collect()
+    }
+}
+
+static HELP_STRING: Lazy<String> = Lazy::new(|| {
+    let all_modules: Vec<_> = Module::iter().map(|module| module.to_string()).collect();
+    format!(
+        "Comma-separated list of modules or 'all' to disable all metrics. Available options: {}, all",
+        all_modules.join(", ")
+    )
+});
+
+pub fn help_string() -> &'static str {
+    &HELP_STRING
+}

crates/metrics/src/futures/future_tracker.rs

@@ -150,7 +150,9 @@ impl<T: Future> Future for FutureTracker<T> {
 
 #[cfg(test)]
 mod tests {
-    use super::*;
+    use std::time::Duration;
+
+    use crate::futures::future_tracker::FutureTracker;
 
     #[tokio::test]
     async fn empty_future() {

crates/metrics/src/graphql_metrics.rs

@@ -1,6 +1,9 @@
 use crate::{
+    buckets::{
+        buckets,
+        Buckets,
+    },
     global_registry,
-    timing_buckets,
 };
 use prometheus_client::{
     encoding::EncodeLabelSet,
@@ -30,7 +33,7 @@ impl GraphqlMetrics {
         let tx_count_gauge = Gauge::default();
         let queries_complexity = Histogram::new(buckets_complexity());
         let requests = Family::<Label, Histogram>::new_with_constructor(|| {
-            Histogram::new(timing_buckets().iter().cloned())
+            Histogram::new(buckets(Buckets::Timing))
         });
         let mut registry = global_registry().registry.lock();
         registry.register("graphql_request_duration_seconds", "", requests.clone());