From 5bb21d25bdaf99ef5755558225938bfe419dd565 Mon Sep 17 00:00:00 2001
From: Nan Jiang <njiang028@gmail.com>
Date: Tue, 2 May 2023 16:22:43 -0400
Subject: [PATCH] feat: Add a metric to measure the tile count of adm response
 (#533)

---
 src/adm/tiles.rs |  6 ++++++
 src/metrics.rs   | 34 ++++++++++++----------------------
 src/tags.rs      | 22 ++++++++++++++++++++++
 src/web/test.rs  | 11 ++++++++---
 4 files changed, 48 insertions(+), 25 deletions(-)

diff --git a/src/adm/tiles.rs b/src/adm/tiles.rs
index 7db0831b..60172079 100644
--- a/src/adm/tiles.rs
+++ b/src/adm/tiles.rs
@@ -245,6 +245,12 @@ pub async fn get_tiles(
                 })?
         }
     };
+    metrics.count_with_tags(
+        "tiles.adm.response.tiles_count",
+        response.tiles.len() as i64,
+        Some(tags),
+    );
+    // Keep this counter to facilitate the "zero" ad fill rate monitoring
     if response.tiles.is_empty() {
         warn!("adm::get_tiles empty response {}", adm_url);
         metrics.incr_with_tags("filter.adm.empty_response", Some(tags));
diff --git a/src/metrics.rs b/src/metrics.rs
index 060043ab..383bb08f 100644
--- a/src/metrics.rs
+++ b/src/metrics.rs
@@ -118,9 +118,7 @@ impl Metrics {
     /// Duration is calculated when this timer is dropped.
     pub fn start_timer(&mut self, label: &str, tags: Option<Tags>) {
         let mut mtags = self.tags.clone().unwrap_or_default();
-        if let Some(t) = tags {
-            mtags.extend(t)
-        }
+        Extend::extend(&mut mtags, tags.into_iter());
 
         trace!("⌚ Starting timer... {:?}", &label; &mtags);
         self.timer = Some(MetricTimer {
@@ -140,13 +138,10 @@ impl Metrics {
         if let Some(client) = self.client.as_ref() {
             let mut tagged = client.incr_with_tags(label);
             let mut mtags = self.tags.clone().unwrap_or_default();
-            if let Some(tags) = tags {
-                mtags.extend(tags.clone());
-            }
-            for key in mtags.tags.keys().clone() {
-                if let Some(val) = mtags.tags.get(key) {
-                    tagged = tagged.with_tag(key, val.as_ref());
-                }
+            Extend::extend(&mut mtags, tags.into_iter());
+
+            for (key, val) in mtags.tags.iter() {
+                tagged = tagged.with_tag(key, val);
             }
             // Include any "hard coded" tags.
             // incr = incr.with_tag("version", env!("CARGO_PKG_VERSION"));
@@ -166,23 +161,18 @@ impl Metrics {
     }
 
     /// increment by count with [crate::tags::Tags] information
-    pub fn count_with_tags(&self, label: &str, count: i64, tags: Option<Tags>) {
+    pub fn count_with_tags(&self, label: &str, count: i64, tags: Option<&Tags>) {
         if let Some(client) = self.client.as_ref() {
             let mut tagged = client.count_with_tags(label, count);
             let mut mtags = self.tags.clone().unwrap_or_default();
-            if let Some(tags) = tags {
-                mtags.extend(tags);
-            }
-            for key in mtags.tags.keys().clone() {
-                if let Some(val) = mtags.tags.get(key) {
-                    tagged = tagged.with_tag(key, val.as_ref());
-                }
+            Extend::extend(&mut mtags, tags.into_iter());
+
+            for (key, val) in mtags.tags.iter() {
+                tagged = tagged.with_tag(key, val);
             }
             // mix in the metric only tags.
-            for key in mtags.metric.keys().clone() {
-                if let Some(val) = mtags.metric.get(key) {
-                    tagged = tagged.with_tag(key, val.as_ref())
-                }
+            for (key, val) in mtags.metric.iter() {
+                tagged = tagged.with_tag(key, val);
             }
             // Include any "hard coded" tags.
             // incr = incr.with_tag("version", env!("CARGO_PKG_VERSION"));
diff --git a/src/tags.rs b/src/tags.rs
index 08a6201c..99b4b119 100644
--- a/src/tags.rs
+++ b/src/tags.rs
@@ -210,6 +210,28 @@ impl Tags {
     }
 }
 
+impl Extend<Tags> for Tags {
+    fn extend<T>(&mut self, iter: T)
+    where
+        T: IntoIterator<Item = Tags>,
+    {
+        for tag in iter {
+            self.extend(tag);
+        }
+    }
+}
+
+impl<'a> Extend<&'a Tags> for Tags {
+    fn extend<T>(&mut self, iter: T)
+    where
+        T: IntoIterator<Item = &'a Tags>,
+    {
+        for tag in iter {
+            self.extend(tag.clone());
+        }
+    }
+}
+
 impl FromRequest for Tags {
     type Error = Error;
     type Future = Ready<Result<Self, Self::Error>>;
diff --git a/src/web/test.rs b/src/web/test.rs
index 30930e30..6eacde16 100644
--- a/src/web/test.rs
+++ b/src/web/test.rs
@@ -915,9 +915,13 @@ async fn metrics() {
     let resp = test::call_service(&app, req).await;
     assert_eq!(resp.status(), StatusCode::OK);
 
-    let prefixes = &["contile.tiles.get:1", "contile.tiles.adm.request:1"];
+    let prefixes = &[
+        "contile.tiles.get:1",
+        "contile.tiles.adm.request:1",
+        "contile.tiles.adm.response.tiles_count:4",
+    ];
     let metrics = find_metrics(&spy, prefixes);
-    assert_eq!(metrics.len(), 2);
+    assert_eq!(metrics.len(), 3);
     let get_metric = &metrics[0];
     assert!(get_metric.contains("ua.form_factor:desktop"));
     assert!(get_metric.contains("ua.os.family:windows"));
@@ -932,7 +936,7 @@ async fn metrics() {
     assert_eq!(resp.status(), StatusCode::OK);
 
     let metrics = find_metrics(&spy, prefixes);
-    assert_eq!(metrics.len(), 2);
+    assert_eq!(metrics.len(), 3);
     let get_metric = &metrics[0];
     assert!(get_metric.contains("ua.form_factor:phone"));
     assert!(get_metric.contains("ua.os.family:ios"));
@@ -1118,6 +1122,7 @@ async fn fallback_on_error() {
         vec![
             "contile.tiles.get:1",
             "contile.tiles.adm.request:1",
+            "contile.tiles.adm.response.tiles_count:4",
             "contile.tiles.get:1",
             "contile.tiles.adm.request:1",
             "contile.tiles.get.error:1"