Fix intermittent issue with metrics tests.

awslabs · Dec 4, 2023 · 60782e9 · 60782e9
1 parent 27bac02
commit 60782e9
Showing 1 changed file with 34 additions and 25 deletions.
diff --git a/mountpoint-s3/src/metrics.rs b/mountpoint-s3/src/metrics.rs
@@ -188,36 +188,45 @@ mod tests {
 
     use super::*;
 
+    const TEST_COUNTER: &str = "test_counter";
+    const TEST_GAUGE: &str = "test_gauge";
+    const TEST_HISTOGRAM: &str = "test_histogram";
+    const TEST_METRICS: [&str; 3] = [TEST_COUNTER, TEST_GAUGE, TEST_HISTOGRAM];
+
     #[test]
     fn basic_metrics() {
         let sink = Arc::new(MetricsSink::new());
         let recorder = MetricsRecorder { sink: sink.clone() };
         metrics::set_boxed_recorder(Box::new(recorder)).unwrap();
+        // Helps with filtering only the metrics used by this test. Since `metric` crate operates
+        // on global recorders, we need to make sure we're evaluating against the metrics emitted
+        // by this test and not others.
+        let is_test_metric = |name: &str| TEST_METRICS.contains(&name);
 
         // Run twice to check reset works
         for _ in 0..2 {
-            metrics::counter!("test_counter", 1, "type" => "get");
-            metrics::counter!("test_counter", 1, "type" => "put");
-            metrics::counter!("test_counter", 2, "type" => "get");
-            metrics::counter!("test_counter", 2, "type" => "put");
-            metrics::counter!("test_counter", 3, "type" => "get");
-            metrics::counter!("test_counter", 4, "type" => "put");
-
-            metrics::gauge!("test_gauge", 5.0, "type" => "processing");
-            metrics::gauge!("test_gauge", 5.0, "type" => "in_queue");
-            metrics::gauge!("test_gauge", 2.0, "type" => "processing");
-            metrics::gauge!("test_gauge", 3.0, "type" => "in_queue");
-
-            metrics::histogram!("test_histogram", 3.0, "type" => "get");
-            metrics::histogram!("test_histogram", 4.0, "type" => "put");
-            metrics::histogram!("test_histogram", 4.0, "type" => "put");
-
-            for mut entry in sink.metrics.iter_mut() {
+            metrics::counter!(TEST_COUNTER, 1, "type" => "get");
+            metrics::counter!(TEST_COUNTER, 1, "type" => "put");
+            metrics::counter!(TEST_COUNTER, 2, "type" => "get");
+            metrics::counter!(TEST_COUNTER, 2, "type" => "put");
+            metrics::counter!(TEST_COUNTER, 3, "type" => "get");
+            metrics::counter!(TEST_COUNTER, 4, "type" => "put");
+
+            metrics::gauge!(TEST_GAUGE, 5.0, "type" => "processing");
+            metrics::gauge!(TEST_GAUGE, 5.0, "type" => "in_queue");
+            metrics::gauge!(TEST_GAUGE, 2.0, "type" => "processing");
+            metrics::gauge!(TEST_GAUGE, 3.0, "type" => "in_queue");
+
+            metrics::histogram!(TEST_HISTOGRAM, 3.0, "type" => "get");
+            metrics::histogram!(TEST_HISTOGRAM, 4.0, "type" => "put");
+            metrics::histogram!(TEST_HISTOGRAM, 4.0, "type" => "put");
+
+            for mut entry in sink.metrics.iter_mut().filter(|m| is_test_metric(m.key().name())) {
                 let (key, metric) = entry.pair_mut();
-                assert_eq!(key.labels().count(), 1);
+                assert_eq!(key.labels().count(), 1, "{} has no labels", key);
                 match metric {
                     Metric::Counter(inner) => {
-                        assert_eq!(key.name(), "test_counter");
+                        assert_eq!(key.name(), TEST_COUNTER);
                         let (sum, n) = inner.load_and_reset().expect("should have a value");
                         assert_eq!(n, 3);
                         let label = key.labels().next().unwrap();
@@ -230,7 +239,7 @@ mod tests {
                         }
                     }
                     Metric::Gauge(inner) => {
-                        assert_eq!(key.name(), "test_gauge");
+                        assert_eq!(key.name(), TEST_GAUGE);
                         let value = inner.load();
                         let label = key.labels().next().unwrap();
                         if label == &Label::new("type", "processing") {
@@ -242,7 +251,7 @@ mod tests {
                         }
                     }
                     Metric::Histogram(inner) => {
-                        assert_eq!(key.name(), "test_histogram");
+                        assert_eq!(key.name(), TEST_HISTOGRAM);
                         let label = key.labels().next().unwrap();
                         inner.run_and_reset(|histogram| {
                             if label == &Label::new("type", "get") {
@@ -259,7 +268,7 @@ mod tests {
         }
 
         // Check that each metric is zeroed (returns None) after the end of the loop reset it
-        for mut entry in sink.metrics.iter_mut() {
+        for mut entry in sink.metrics.iter_mut().filter(|m| is_test_metric(m.key().name())) {
             let metric = entry.value_mut();
             match metric {
                 Metric::Counter(inner) => assert!(inner.load_and_reset().is_none()),
@@ -270,9 +279,9 @@ mod tests {
         }
 
         // Set the gauges to zero and check they're no longer emitted
-        metrics::gauge!("test_gauge", 0.0, "type" => "processing");
-        metrics::gauge!("test_gauge", 0.0, "type" => "in_queue");
-        for mut entry in sink.metrics.iter_mut() {
+        metrics::gauge!(TEST_GAUGE, 0.0, "type" => "processing");
+        metrics::gauge!(TEST_GAUGE, 0.0, "type" => "in_queue");
+        for mut entry in sink.metrics.iter_mut().filter(|m| is_test_metric(m.key().name())) {
             let metric = entry.value_mut();
             let Metric::Gauge(inner) = metric else {
                 continue;