feat: use macro to declare metric constants (#1129)

## Summary Introduce a macro `declare_metric_const!` to avoid repetition and common typos when declaring string consts used as metric names. ## Background Noticed some common errors in reviews and when writing metrics: mismatch between the const's name and the actual value. Missing underscores. Annoying repetiton of `env!("CARGO_CRATE_NAME")`; ## Changes - Declare a macro `declare_metric_const!` and define all consts in terms of it. - Add unit tests for all macros to ensure they don't accidentally break. ## Testing - Unit tests for all const declaration - Simple unit test for the macro ## Metrics - `astria_composer_transactions_per_submission` replaces `astria_composer_transaction_per_submission`
astriaorg · Jun 4, 2024 · fb1d7b8 · fb1d7b8
1 parent 67dacb7
commit fb1d7b8
Show file tree

Hide file tree

Showing 11 changed files with 366 additions and 158 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -57,6 +57,7 @@ bytes = "1"
 celestia-tendermint = "0.32.1"
 celestia-types = "0.1.1"
 clap = "4.5.4"
+const_format = "0.2.32"
 ethers = "2.0.11"
 futures = "0.3"
 hex = "0.4"

diff --git a/crates/astria-bridge-withdrawer/src/metrics_init.rs b/crates/astria-bridge-withdrawer/src/metrics_init.rs
@@ -8,6 +8,7 @@ use metrics::{
     describe_histogram,
     Unit,
 };
+use telemetry::metric_name;
 
 /// Registers all metrics used by this crate.
 pub fn register() {
@@ -39,19 +40,42 @@ pub fn register() {
     );
 }
 
-pub const NONCE_FETCH_COUNT: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_count");
+metric_name!(pub const CURRENT_NONCE);
+metric_name!(pub const NONCE_FETCH_COUNT);
+metric_name!(pub const NONCE_FETCH_FAILURE_COUNT);
+metric_name!(pub const NONCE_FETCH_LATENCY);
+metric_name!(pub const SEQUENCER_SUBMISSION_FAILURE_COUNT);
+metric_name!(pub const SEQUENCER_SUBMISSION_LATENCY);
 
-pub const NONCE_FETCH_FAILURE_COUNT: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_failure_count");
-
-pub const NONCE_FETCH_LATENCY: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_latency");
-
-pub const CURRENT_NONCE: &str = concat!(env!("CARGO_CRATE_NAME"), "_current_nonce");
+#[cfg(test)]
+mod tests {
+    use super::{
+        CURRENT_NONCE,
+        NONCE_FETCH_COUNT,
+        NONCE_FETCH_FAILURE_COUNT,
+        NONCE_FETCH_LATENCY,
+        SEQUENCER_SUBMISSION_FAILURE_COUNT,
+        SEQUENCER_SUBMISSION_LATENCY,
+    };
 
-pub const SEQUENCER_SUBMISSION_FAILURE_COUNT: &str = concat!(
-    env!("CARGO_CRATE_NAME"),
-    "_sequencer_submission_failure_count"
-);
+    #[track_caller]
+    fn assert_const(actual: &'static str, suffix: &str) {
+        // XXX: hard-code this so the crate name isn't accidentally changed.
+        const CRATE_NAME: &str = "astria_bridge_withdrawer";
+        let expected = format!("{CRATE_NAME}_{suffix}");
+        assert_eq!(expected, actual);
+    }
 
-pub const SEQUENCER_SUBMISSION_LATENCY: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_sequencer_submission_latency");
+    #[test]
+    fn metrics_are_as_expected() {
+        assert_const(CURRENT_NONCE, "current_nonce");
+        assert_const(NONCE_FETCH_COUNT, "nonce_fetch_count");
+        assert_const(NONCE_FETCH_FAILURE_COUNT, "nonce_fetch_failure_count");
+        assert_const(NONCE_FETCH_LATENCY, "nonce_fetch_latency");
+        assert_const(
+            SEQUENCER_SUBMISSION_FAILURE_COUNT,
+            "sequencer_submission_failure_count",
+        );
+        assert_const(SEQUENCER_SUBMISSION_LATENCY, "sequencer_submission_latency");
+    }
+}
diff --git a/crates/astria-composer/src/metrics_init.rs b/crates/astria-composer/src/metrics_init.rs
@@ -8,6 +8,7 @@ use metrics::{
     describe_histogram,
     Unit,
 };
+use telemetry::metric_name;
 
 /// Labels
 pub(crate) const ROLLUP_ID_LABEL: &str = "rollup_id";
@@ -73,31 +74,60 @@ pub fn register() {
     );
 }
 
-pub const TRANSACTIONS_RECEIVED: &str = concat!(env!("CARGO_CRATE_NAME"), "_transactions_received");
+metric_name!(pub const TRANSACTIONS_RECEIVED);
+metric_name!(pub const TRANSACTIONS_DROPPED);
+metric_name!(pub const TRANSACTIONS_DROPPED_TOO_LARGE);
+metric_name!(pub const NONCE_FETCH_COUNT);
+metric_name!(pub const NONCE_FETCH_FAILURE_COUNT);
+metric_name!(pub const NONCE_FETCH_LATENCY);
+metric_name!(pub const CURRENT_NONCE);
+metric_name!(pub const SEQUENCER_SUBMISSION_LATENCY);
+metric_name!(pub const SEQUENCER_SUBMISSION_FAILURE_COUNT);
+metric_name!(pub const TRANSACTIONS_PER_SUBMISSION);
+metric_name!(pub const BYTES_PER_SUBMISSION);
 
-pub const TRANSACTIONS_DROPPED: &str = concat!(env!("CARGO_CRATE_NAME"), "_transactions_dropped");
-
-pub const TRANSACTIONS_DROPPED_TOO_LARGE: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_transactions_dropped_too_large");
-
-pub const NONCE_FETCH_COUNT: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_count");
-
-pub const NONCE_FETCH_FAILURE_COUNT: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_failure_count");
-
-pub const NONCE_FETCH_LATENCY: &str = concat!(env!("CARGO_CRATE_NAME"), "_nonce_fetch_latency");
-
-pub const CURRENT_NONCE: &str = concat!(env!("CARGO_CRATE_NAME"), "_current_nonce");
-
-pub const SEQUENCER_SUBMISSION_LATENCY: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_sequencer_submission_latency");
-
-pub const SEQUENCER_SUBMISSION_FAILURE_COUNT: &str = concat!(
-    env!("CARGO_CRATE_NAME"),
-    "_sequencer_submission_failure_count"
-);
+#[cfg(test)]
+mod tests {
+    use super::{
+        BYTES_PER_SUBMISSION,
+        CURRENT_NONCE,
+        NONCE_FETCH_COUNT,
+        NONCE_FETCH_FAILURE_COUNT,
+        NONCE_FETCH_LATENCY,
+        SEQUENCER_SUBMISSION_FAILURE_COUNT,
+        SEQUENCER_SUBMISSION_LATENCY,
+        TRANSACTIONS_DROPPED,
+        TRANSACTIONS_DROPPED_TOO_LARGE,
+        TRANSACTIONS_PER_SUBMISSION,
+        TRANSACTIONS_RECEIVED,
+    };
 
-pub const TRANSACTIONS_PER_SUBMISSION: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_transaction_per_submission");
+    #[track_caller]
+    fn assert_const(actual: &'static str, suffix: &str) {
+        // XXX: hard-code this so the crate name isn't accidentally changed.
+        const CRATE_NAME: &str = "astria_composer";
+        let expected = format!("{CRATE_NAME}_{suffix}");
+        assert_eq!(expected, actual);
+    }
 
-pub const BYTES_PER_SUBMISSION: &str = concat!(env!("CARGO_CRATE_NAME"), "_bytes_per_submission");
+    #[test]
+    fn metrics_are_as_expected() {
+        assert_const(TRANSACTIONS_RECEIVED, "transactions_received");
+        assert_const(TRANSACTIONS_DROPPED, "transactions_dropped");
+        assert_const(
+            TRANSACTIONS_DROPPED_TOO_LARGE,
+            "transactions_dropped_too_large",
+        );
+        assert_const(NONCE_FETCH_COUNT, "nonce_fetch_count");
+        assert_const(NONCE_FETCH_FAILURE_COUNT, "nonce_fetch_failure_count");
+        assert_const(NONCE_FETCH_LATENCY, "nonce_fetch_latency");
+        assert_const(CURRENT_NONCE, "current_nonce");
+        assert_const(SEQUENCER_SUBMISSION_LATENCY, "sequencer_submission_latency");
+        assert_const(
+            SEQUENCER_SUBMISSION_FAILURE_COUNT,
+            "sequencer_submission_failure_count",
+        );
+        assert_const(TRANSACTIONS_PER_SUBMISSION, "transactions_per_submission");
+        assert_const(BYTES_PER_SUBMISSION, "bytes_per_submission");
+    }
+}
diff --git a/crates/astria-conductor/src/metrics_init.rs b/crates/astria-conductor/src/metrics_init.rs
@@ -7,6 +7,7 @@ use metrics::{
     describe_histogram,
     Unit,
 };
+use telemetry::metric_name;
 
 pub(crate) const NAMESPACE_TYPE_LABEL: &str = "namespace_type";
 pub(crate) const NAMESPACE_TYPE_METADATA: &str = "metadata";
@@ -58,31 +59,63 @@ pub fn register() {
     );
 }
 
-pub const BLOBS_PER_CELESTIA_FETCH: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_blobs_per_celestia_fetch",);
+metric_name!(pub const BLOBS_PER_CELESTIA_FETCH);
+metric_name!(pub const CELESTIA_BLOB_FETCH_ERROR_COUNT);
+metric_name!(pub const DECODED_ITEMS_PER_CELESTIA_FETCH);
+metric_name!(pub const SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH);
+metric_name!(pub const SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH);
 
-pub const CELESTIA_BLOB_FETCH_ERROR_COUNT: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_celestia_blob_fetch_error_count");
+metric_name!(pub const EXECUTED_FIRM_BLOCK_NUMBER);
+metric_name!(pub const EXECUTED_SOFT_BLOCK_NUMBER);
+metric_name!(pub const TRANSACTIONS_PER_EXECUTED_BLOCK);
 
-pub const DECODED_ITEMS_PER_CELESTIA_FETCH: &str = concat!(
-    env!("CARGO_CRATE_NAME"),
-    "_decoded_items_per_celestia_fetch",
-);
-
-pub const SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH: &str = concat!(
-    env!("CARGO_CRATE_NAME"),
-    "_sequencer_blocks_metadata_verified_per_celestia_fetch",
-);
-
-pub const SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH: &str = concat!(
-    env!("CARGO_CRATE_NAME"),
-    "_sequencer_block_information_reconstructed_per_celestia_fetch",
-);
-
-pub const EXECUTED_FIRM_BLOCK_NUMBER: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_executed_firm_block_number");
-pub const EXECUTED_SOFT_BLOCK_NUMBER: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_executed_soft_block_number");
-
-pub const TRANSACTIONS_PER_EXECUTED_BLOCK: &str =
-    concat!(env!("CARGO_CRATE_NAME"), "_transactions_per_executed_block",);
+#[cfg(test)]
+mod tests {
+    use super::TRANSACTIONS_PER_EXECUTED_BLOCK;
+    use crate::metrics_init::{
+        BLOBS_PER_CELESTIA_FETCH,
+        CELESTIA_BLOB_FETCH_ERROR_COUNT,
+        DECODED_ITEMS_PER_CELESTIA_FETCH,
+        EXECUTED_FIRM_BLOCK_NUMBER,
+        EXECUTED_SOFT_BLOCK_NUMBER,
+        SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH,
+        SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH,
+    };
+
+    #[track_caller]
+    fn assert_const(actual: &'static str, suffix: &str) {
+        // XXX: hard-code this so the crate name isn't accidentally changed.
+        const CRATE_NAME: &str = "astria_conductor";
+        let expected = format!("{CRATE_NAME}_{suffix}");
+        assert_eq!(expected, actual);
+    }
+
+    #[test]
+    fn metrics_are_as_expected() {
+        assert_const(BLOBS_PER_CELESTIA_FETCH, "blobs_per_celestia_fetch");
+        assert_const(
+            CELESTIA_BLOB_FETCH_ERROR_COUNT,
+            "celestia_blob_fetch_error_count",
+        );
+        assert_const(
+            DECODED_ITEMS_PER_CELESTIA_FETCH,
+            "decoded_items_per_celestia_fetch",
+        );
+
+        assert_const(
+            SEQUENCER_BLOCKS_METADATA_VERIFIED_PER_CELESTIA_FETCH,
+            "sequencer_blocks_metadata_verified_per_celestia_fetch",
+        );
+
+        assert_const(
+            SEQUENCER_BLOCK_INFORMATION_RECONSTRUCTED_PER_CELESTIA_FETCH,
+            "sequencer_block_information_reconstructed_per_celestia_fetch",
+        );
+        assert_const(EXECUTED_FIRM_BLOCK_NUMBER, "executed_firm_block_number");
+        assert_const(EXECUTED_SOFT_BLOCK_NUMBER, "executed_soft_block_number");
+        assert_const(
+            TRANSACTIONS_PER_EXECUTED_BLOCK,
+            "transactions_per_executed_block",
+        );
+    }
+}
diff --git a/crates/astria-sequencer-relayer/Cargo.toml b/crates/astria-sequencer-relayer/Cargo.toml
@@ -24,6 +24,7 @@ axum = { workspace = true }
 base64 = { workspace = true }
 base64-serde = { workspace = true }
 celestia-types = { workspace = true }
+const_format = { workspace = true }
 futures = { workspace = true }
 hex = { workspace = true, features = ["serde"] }
 humantime = { workspace = true }
@@ -43,6 +44,7 @@ thiserror = { workspace = true }
 tracing = { workspace = true }
 tryhard = { workspace = true }
 tokio = { workspace = true, features = ["macros", "rt-multi-thread", "signal"] }
+tokio-stream = { workspace = true }
 tokio-util = { workspace = true }
 tonic = { workspace = true }
 
@@ -61,8 +63,6 @@ sequencer-client = { package = "astria-sequencer-client", path = "../astria-sequ
 telemetry = { package = "astria-telemetry", path = "../astria-telemetry", features = [
   "display",
 ] }
-tokio-stream = { workspace = true }
-const_format = "0.2.32"
 
 [dev-dependencies]
 astria-grpc-mock = { path = "../astria-grpc-mock" }