From 4fa34992e8c908dffe314c1a6f15a0004087281e Mon Sep 17 00:00:00 2001
From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Date: Fri, 22 Sep 2023 12:21:44 +0200
Subject: [PATCH] improve metrics macros

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
---
 apollo-router/src/metrics/mod.rs           | 222 +++++++++++++++++----
 apollo-router/src/plugins/telemetry/mod.rs | 140 ++++++++++---
 2 files changed, 290 insertions(+), 72 deletions(-)

diff --git a/apollo-router/src/metrics/mod.rs b/apollo-router/src/metrics/mod.rs
index 16211a9215..f0bfa62b57 100644
--- a/apollo-router/src/metrics/mod.rs
+++ b/apollo-router/src/metrics/mod.rs
@@ -367,11 +367,26 @@ pub(crate) fn meter_provider() -> AggregateMeterProvider {
 /// New metrics should be added using these macros.
 #[allow(unused_macros)]
 macro_rules! u64_counter {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(u64, counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(u64, counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
         metric!(u64, counter, add, $name, $description, $value, &attributes);
     };
 
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(u64, counter, add, $name, $description, $value, &attributes);
+    };
+
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
         metric!(u64, counter, add, $name, $description, $value, $attrs);
     };
@@ -391,11 +406,25 @@ macro_rules! u64_counter {
 /// New metrics should be added using these macros.
 #[allow(unused_macros)]
 macro_rules! f64_counter {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(f64, counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(f64, counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
         metric!(f64, counter, add, $name, $description, $value, &attributes);
     };
 
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(f64, counter, add, $name, $description, $value, &attributes);
+    };
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
         metric!(f64, counter, add, $name, $description, $value, $attrs);
     };
@@ -416,17 +445,32 @@ macro_rules! f64_counter {
 
 #[allow(unused_macros)]
 macro_rules! i64_up_down_counter {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
-        metric!(i64, histogram, record, $name, $description, $value, &attributes);
+        metric!(i64, up_down_counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(i64, up_down_counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(i64, up_down_counter, add, $name, $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(i64, up_down_counter, add, $name, $description, $value, &attributes);
     };
 
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
-        metric!(i64, histogram, record, $name, $description, $value, $attrs);
+        metric!(i64, up_down_counter, add, $name, $description, $value, $attrs);
     };
 
     ($name:literal, $description:literal, $value: expr) => {
-        metric!(i64, histogram, record, $name, $description, $value, &[]);
+        metric!(i64, up_down_counter, add, $name, $description, $value, &[]);
     };
 }
 
@@ -440,17 +484,32 @@ macro_rules! i64_up_down_counter {
 /// New metrics should be added using these macros.
 #[allow(unused_macros)]
 macro_rules! f64_up_down_counter {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
-        metric!(f64, histogram, record, $name, $description, $value, &attributes);
+        metric!(f64, up_down_counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(f64, up_down_counter, add, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(f64, up_down_counter, add, $name, $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(f64, up_down_counter, add, $name, $description, $value, &attributes);
     };
 
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
-        metric!(f64, histogram, record, $name, $description, $value, $attrs);
+        metric!(f64, up_down_counter, add, $name, $description, $value, $attrs);
     };
 
     ($name:literal, $description:literal, $value: expr) => {
-        metric!(f64, histogram, record, $name, $description, $value, &[]);
+        metric!(f64, up_down_counter, add, $name, $description, $value, &[]);
     };
 }
 
@@ -464,11 +523,26 @@ macro_rules! f64_up_down_counter {
 /// New metrics should be added using these macros.
 #[allow(unused_macros)]
 macro_rules! f64_histogram {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(f64, histogram, record, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(f64, histogram, record, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
         metric!(f64, histogram, record, $name, $description, $value, &attributes);
     };
 
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(f64, histogram, record, $name, $description, $value, &attributes);
+    };
+
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
         metric!(f64, histogram, record, $name, $description, $value, $attrs);
     };
@@ -488,11 +562,26 @@ macro_rules! f64_histogram {
 /// New metrics should be added using these macros.
 #[allow(unused_macros)]
 macro_rules! u64_histogram {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(u64, histogram, record, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(u64, histogram, record, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
         metric!(u64, histogram, record, $name, $description, $value, &attributes);
     };
 
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(u64, histogram, record, $name, $description, $value, &attributes);
+    };
+
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
         metric!(u64, histogram, record, $name, $description, $value, $attrs);
     };
@@ -512,11 +601,26 @@ macro_rules! u64_histogram {
 /// New metrics should be added using these macros.
 #[allow(unused_macros)]
 macro_rules! i64_histogram {
-    ($name:literal, $description:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        metric!(i64, histogram, record, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($($name:ident).+, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(i64, histogram, record, stringify!($($name).+), $description, $value, &attributes);
+    };
+
+    ($name:literal, $description:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
         metric!(i64, histogram, record, $name, $description, $value, &attributes);
     };
 
+    ($name:literal, $description:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        metric!(i64, histogram, record, $name, $description, $value, &attributes);
+    };
+
     ($name:literal, $description:literal, $value: expr, $attrs: expr) => {
         metric!(i64, histogram, record, $name, $description, $value, $attrs);
     };
@@ -532,7 +636,7 @@ thread_local! {
     pub(crate) static CACHE_CALLSITE: std::sync::atomic::AtomicBool = const {std::sync::atomic::AtomicBool::new(false)};
 }
 macro_rules! metric {
-    ($ty:ident, $instrument:ident, $mutation:ident, $name:literal, $description:literal, $value: expr, $attrs: expr) => {
+    ($ty:ident, $instrument:ident, $mutation:ident, $name:expr, $description:literal, $value: expr, $attrs: expr) => {
 
         // The way this works is that we have a static at each call site that holds a weak reference to the instrument.
         // We make a call we try to upgrade the weak reference. If it succeeds we use the instrument.
@@ -591,10 +695,26 @@ macro_rules! metric {
 
 #[cfg(test)]
 macro_rules! assert_metric {
-    ($name:literal, $value: expr, $($attr_key:expr => $attr_value:expr),+) => {
+    ($($name:ident).+, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
         let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        crate::metrics::collect_metrics().assert(stringify!($($name).+), $value, &attributes);
+    };
+
+    ($($name:ident).+, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
+        crate::metrics::collect_metrics().assert(stringify!($($name).+), $value, &attributes);
+    };
+
+    ($name:literal, $value: expr, $($attr_key:literal = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new($attr_key, $attr_value)),+];
+        crate::metrics::collect_metrics().assert($name, $value, &attributes);
+    };
+
+    ($name:literal, $value: expr, $($($attr_key:ident).+ = $attr_value:expr),+) => {
+        let attributes = vec![$(opentelemetry::KeyValue::new(stringify!($($attr_key).+), $attr_value)),+];
         crate::metrics::collect_metrics().assert($name, $value, &attributes);
     };
+
     ($name:literal, $value: expr) => {
         crate::metrics::collect_metrics().assert($name, $value, &[]);
     };
@@ -662,35 +782,35 @@ mod test {
     fn test_dynamic_attributes() {
         let attributes = vec![KeyValue::new("attr", "val")];
         u64_counter!("test", "test description", 1, attributes);
-        assert_metric!("test", 1, "attr" => "val");
+        assert_metric!("test", 1, "attr" = "val");
     }
 
     #[test]
     fn test_multiple_calls() {
         fn my_method(val: &'static str) {
-            u64_counter!("test", "test description", 1, "attr" => val);
+            u64_counter!("test", "test description", 1, "attr" = val);
         }
 
         my_method("jill");
         my_method("jill");
         my_method("bob");
-        assert_metric!("test", 2, "attr" => "jill");
-        assert_metric!("test", 1, "attr" => "bob");
+        assert_metric!("test", 2, "attr" = "jill");
+        assert_metric!("test", 1, "attr" = "bob");
     }
 
     #[test]
     fn test_non_async() {
         // Each test is run in a separate thread, metrics are stored in a thread local.
-        u64_counter!("test", "test description", 1, "attr" => "val");
-        assert_metric!("test", 1, "attr" => "val");
+        u64_counter!("test", "test description", 1, "attr" = "val");
+        assert_metric!("test", 1, "attr" = "val");
     }
 
     #[tokio::test(flavor = "multi_thread")]
     async fn test_async_multi() {
         // Multi-threaded runtime needs to use a tokio task local to avoid tests interfering with each other
         async {
-            u64_counter!("test", "test description", 1, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            u64_counter!("test", "test description", 1, "attr" = "val");
+            assert_metric!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -700,8 +820,8 @@ mod test {
     async fn test_async_single() {
         async {
             // It's a single threaded tokio runtime, so we can still use a thread local
-            u64_counter!("test", "test description", 1, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            u64_counter!("test", "test description", 1, "attr" = "val");
+            assert_metric!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -710,8 +830,26 @@ mod test {
     #[tokio::test]
     async fn test_u64_counter() {
         async {
-            u64_counter!("test", "test description", 1, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            u64_counter!("test", "test description", 1, attr = "val");
+            u64_counter!("test", "test description", 1, attr.test = "val");
+            u64_counter!("test", "test description", 1, attr.test_underscore = "val");
+            u64_counter!(
+                test.dot,
+                "test description",
+                1,
+                "attr.test_underscore" = "val"
+            );
+            u64_counter!(
+                test.dot,
+                "test description",
+                1,
+                attr.test_underscore = "val"
+            );
+            assert_metric!("test", 1, "attr" = "val");
+            assert_metric!("test", 1, "attr.test" = "val");
+            assert_metric!("test", 1, attr.test_underscore = "val");
+            assert_metric!(test.dot, 2, attr.test_underscore = "val");
+            assert_metric!(test.dot, 2, "attr.test_underscore" = "val");
         }
         .with_metrics()
         .await;
@@ -720,8 +858,8 @@ mod test {
     #[tokio::test]
     async fn test_f64_counter() {
         async {
-            f64_counter!("test", "test description", 1.5, "attr" => "val");
-            assert_metric!("test", 1.5, "attr" => "val");
+            f64_counter!("test", "test description", 1.5, "attr" = "val");
+            assert_metric!("test", 1.5, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -730,8 +868,8 @@ mod test {
     #[tokio::test]
     async fn test_i64_up_down_counter() {
         async {
-            i64_up_down_counter!("test", "test description", 1, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            i64_up_down_counter!("test", "test description", 1, "attr" = "val");
+            assert_metric!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -740,8 +878,8 @@ mod test {
     #[tokio::test]
     async fn test_f64_up_down_counter() {
         async {
-            f64_up_down_counter!("test", "test description", 1.5, "attr" => "val");
-            assert_metric!("test", 1.5, "attr" => "val");
+            f64_up_down_counter!("test", "test description", 1.5, "attr" = "val");
+            assert_metric!("test", 1.5, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -750,8 +888,8 @@ mod test {
     #[tokio::test]
     async fn test_u64_histogram() {
         async {
-            u64_histogram!("test", "test description", 1, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            u64_histogram!("test", "test description", 1, "attr" = "val");
+            assert_metric!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -760,8 +898,8 @@ mod test {
     #[tokio::test]
     async fn test_i64_histogram() {
         async {
-            i64_histogram!("test", "test description", 1, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            i64_histogram!("test", "test description", 1, "attr" = "val");
+            assert_metric!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -770,8 +908,8 @@ mod test {
     #[tokio::test]
     async fn test_f64_histogram() {
         async {
-            f64_histogram!("test", "test description", 1.0, "attr" => "val");
-            assert_metric!("test", 1, "attr" => "val");
+            f64_histogram!("test", "test description", 1.0, "attr" = "val");
+            assert_metric!("test", 1, "attr" = "val");
         }
         .with_metrics()
         .await;
@@ -785,7 +923,7 @@ mod test {
         super::CACHE_CALLSITE.with(|cell| cell.store(true, std::sync::atomic::Ordering::SeqCst));
         fn test() {
             // This is a single callsite so should only have one metric
-            u64_counter!("test", "test description", 1, "attr" => "val");
+            u64_counter!("test", "test description", 1, "attr" = "val");
         }
 
         // Callsite hasn't been used yet, so there should be no metrics
@@ -793,12 +931,12 @@ mod test {
 
         // Call the metrics, it will be registered
         test();
-        assert_metric!("test", 1, "attr" => "val");
+        assert_metric!("test", 1, "attr" = "val");
         assert_eq!(meter_provider().registered_instruments(), 1);
 
         // Call the metrics again, but the second call will not register a new metric because it will have be retrieved from the static
         test();
-        assert_metric!("test", 2, "attr" => "val");
+        assert_metric!("test", 2, "attr" = "val");
         assert_eq!(meter_provider().registered_instruments(), 1);
 
         // Force invalidation of instruments
diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs
index ea1456c657..403142e584 100644
--- a/apollo-router/src/plugins/telemetry/mod.rs
+++ b/apollo-router/src/plugins/telemetry/mod.rs
@@ -837,7 +837,12 @@ impl Telemetry {
                 if !parts.status.is_success() {
                     metric_attrs.push(KeyValue::new("error", parts.status.to_string()));
                 }
-                u64_counter!("apollo.router.operations", "The number of graphql operations performed by the Router", 1, "http.response.status_code" => parts.status.as_u16() as i64);
+                u64_counter!(
+                    "apollo.router.operations",
+                    "The number of graphql operations performed by the Router",
+                    1,
+                    "http.response.status_code" = parts.status.as_u16() as i64
+                );
                 let response = http::Response::from_parts(
                     parts,
                     once(ready(first_response.unwrap_or_default()))
@@ -849,7 +854,12 @@ impl Telemetry {
             }
             Err(err) => {
                 metric_attrs.push(KeyValue::new("status", "500"));
-                u64_counter!("apollo.router.operations", "The number of graphql operations performed by the Router", 1, "http.response.status_code" => 500);
+                u64_counter!(
+                    "apollo.router.operations",
+                    "The number of graphql operations performed by the Router",
+                    1,
+                    "http.response.status_code" = 500
+                );
                 Err(err)
             }
         };
@@ -2180,32 +2190,53 @@ mod tests {
     async fn test_supergraph_metrics_ok() {
         async {
             let plugin =
-                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml")).await;
+                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml"))
+                    .await;
             make_supergraph_request(plugin.as_ref()).await;
 
-            assert_metric!("apollo_router_http_requests_total", 1, "another_test" => "my_default_value", "my_value" => 2, "myname" => "label_value", "renamed_value" => "my_value_set", "status" => "200", "x-custom" => "coming_from_header");
-            assert_metric!("apollo_router_http_request_duration_seconds", 1, "another_test" => "my_default_value", "my_value" => 2, "myname" => "label_value", "renamed_value" => "my_value_set", "status" => "200", "x-custom" => "coming_from_header");
-        }.with_metrics().await;
+            assert_metric!(
+                "apollo_router_http_requests_total",
+                1,
+                "another_test" = "my_default_value",
+                "my_value" = 2,
+                "myname" = "label_value",
+                "renamed_value" = "my_value_set",
+                "status" = "200",
+                "x-custom" = "coming_from_header"
+            );
+            assert_metric!(
+                "apollo_router_http_request_duration_seconds",
+                1,
+                "another_test" = "my_default_value",
+                "my_value" = 2,
+                "myname" = "label_value",
+                "renamed_value" = "my_value_set",
+                "status" = "200",
+                "x-custom" = "coming_from_header"
+            );
+        }
+        .with_metrics()
+        .await;
     }
 
     #[tokio::test]
     async fn test_supergraph_metrics_bad_request() {
         async {
             let plugin =
-                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml")).await;
+                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml"))
+                    .await;
 
             let mut mock_bad_request_service = MockSupergraphService::new();
-            mock_bad_request_service
-                .expect_call()
-                .times(1)
-                .returning(move |req: SupergraphRequest| {
+            mock_bad_request_service.expect_call().times(1).returning(
+                move |req: SupergraphRequest| {
                     Ok(SupergraphResponse::fake_builder()
                         .context(req.context)
                         .status_code(StatusCode::BAD_REQUEST)
                         .data(json!({"errors": [{"message": "nope"}]}))
                         .build()
                         .unwrap())
-                });
+                },
+            );
             let mut bad_request_supergraph_service =
                 plugin.supergraph_service(BoxService::new(mock_bad_request_service));
             let router_req = SupergraphRequest::fake_builder().header("test", "my_value_set");
@@ -2220,15 +2251,26 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_metric!("apollo_router_http_requests_total", 1, "another_test" => "my_default_value", "error" => "400 Bad Request", "myname" => "label_value", "renamed_value" => "my_value_set", "status" => "400");
-        }.with_metrics().await;
+            assert_metric!(
+                "apollo_router_http_requests_total",
+                1,
+                "another_test" = "my_default_value",
+                "error" = "400 Bad Request",
+                "myname" = "label_value",
+                "renamed_value" = "my_value_set",
+                "status" = "400"
+            );
+        }
+        .with_metrics()
+        .await;
     }
 
     #[tokio::test]
     async fn test_subgraph_metrics_ok() {
         async {
             let plugin =
-                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml")).await;
+                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml"))
+                    .await;
 
             let mut mock_subgraph_service = MockSubgraphService::new();
             mock_subgraph_service
@@ -2281,15 +2323,27 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_metric!("apollo_router_http_requests_total", 1, "error" => "custom_error_for_propagation", "my_key" => "my_custom_attribute_from_context", "query_from_request" => "query { test }", "status" => "200", "subgraph" => "my_subgraph_name", "unknown_data" => "default_value");
-        }.with_metrics().await;
+            assert_metric!(
+                "apollo_router_http_requests_total",
+                1,
+                "error" = "custom_error_for_propagation",
+                "my_key" = "my_custom_attribute_from_context",
+                "query_from_request" = "query { test }",
+                "status" = "200",
+                "subgraph" = "my_subgraph_name",
+                "unknown_data" = "default_value"
+            );
+        }
+        .with_metrics()
+        .await;
     }
 
     #[tokio::test]
     async fn test_subgraph_metrics_http_error() {
         async {
             let plugin =
-                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml")).await;
+                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml"))
+                    .await;
 
             let mut mock_subgraph_service_in_error = MockSubgraphService::new();
             mock_subgraph_service_in_error
@@ -2329,28 +2383,37 @@ mod tests {
                 .await
                 .expect_err("should be an error");
 
-            assert_metric!("apollo_router_http_requests_total", 1, "message" => "cannot contact the subgraph", "status" => "500", "subgraph" => "my_subgraph_name_error", "subgraph_error_extended_code" => "SUBREQUEST_HTTP_ERROR");
-        }.with_metrics().await;
+            assert_metric!(
+                "apollo_router_http_requests_total",
+                1,
+                "message" = "cannot contact the subgraph",
+                "status" = "500",
+                "subgraph" = "my_subgraph_name_error",
+                "subgraph_error_extended_code" = "SUBREQUEST_HTTP_ERROR"
+            );
+        }
+        .with_metrics()
+        .await;
     }
 
     #[tokio::test]
     async fn test_subgraph_metrics_bad_request() {
         async {
             let plugin =
-                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml")).await;
+                create_plugin_with_config(include_str!("testdata/custom_attributes.router.yaml"))
+                    .await;
 
             let mut mock_bad_request_service = MockSupergraphService::new();
-            mock_bad_request_service
-                .expect_call()
-                .times(1)
-                .returning(move |req: SupergraphRequest| {
+            mock_bad_request_service.expect_call().times(1).returning(
+                move |req: SupergraphRequest| {
                     Ok(SupergraphResponse::fake_builder()
                         .context(req.context)
                         .status_code(StatusCode::BAD_REQUEST)
                         .data(json!({"errors": [{"message": "nope"}]}))
                         .build()
                         .unwrap())
-                });
+                },
+            );
 
             let mut bad_request_supergraph_service =
                 plugin.supergraph_service(BoxService::new(mock_bad_request_service));
@@ -2368,10 +2431,27 @@ mod tests {
                 .await
                 .unwrap();
 
-            assert_metric!("apollo_router_http_requests_total", 1, "another_test" => "my_default_value", "error" => "400 Bad Request", "myname" => "label_value", "renamed_value" => "my_value_set", "status" => "400");
-            assert_metric!("apollo_router_http_request_duration_seconds", 1, "another_test" => "my_default_value", "error" => "400 Bad Request", "myname" => "label_value", "renamed_value" => "my_value_set", "status" => "400");
-
-        }.with_metrics().await;
+            assert_metric!(
+                "apollo_router_http_requests_total",
+                1,
+                "another_test" = "my_default_value",
+                "error" = "400 Bad Request",
+                "myname" = "label_value",
+                "renamed_value" = "my_value_set",
+                "status" = "400"
+            );
+            assert_metric!(
+                "apollo_router_http_request_duration_seconds",
+                1,
+                "another_test" = "my_default_value",
+                "error" = "400 Bad Request",
+                "myname" = "label_value",
+                "renamed_value" = "my_value_set",
+                "status" = "400"
+            );
+        }
+        .with_metrics()
+        .await;
     }
 
     #[tokio::test]