Extract metrics::Scopes into its own module (#55)

The metrics::Scopes type exposes its internal implementation to many of
its uses.

By extracting the type into its own module, we are forced to provide an
explicit public interface, hiding its IndexMap implementation details.

src/telemetry/metrics/http.rs

@@ -36,7 +36,7 @@ impl RequestScopes {
 
 impl fmt::Display for RequestScopes {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if self.scopes.is_empty() {
+        if self.is_empty() {
             return Ok(());
         }
 
@@ -74,7 +74,7 @@ impl ResponseScopes {
 
 impl fmt::Display for ResponseScopes {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if self.scopes.is_empty() {
+        if self.is_empty() {
             return Ok(());
         }
 

src/telemetry/metrics/mod.rs

@@ -33,8 +33,6 @@ use std::marker::PhantomData;
 use std::sync::{Arc, Mutex};
 use std::time::{Duration, Instant};
 
-use indexmap::IndexMap;
-
 use ctx;
 mod counter;
 mod gauge;
@@ -43,6 +41,7 @@ mod http;
 mod labels;
 pub mod latency;
 mod record;
+mod scopes;
 mod serve;
 mod transport;
 
@@ -57,6 +56,7 @@ use self::labels::{
 };
 pub use self::labels::DstLabels;
 pub use self::record::Record;
+pub use self::scopes::Scopes;
 pub use self::serve::Serve;
 use super::process;
 use super::tls_config_reload;
@@ -100,15 +100,6 @@ struct Root {
     process: process::Report,
 }
 
-
-/// Holds an `S`-typed scope for each `L`-typed label set.
-///
-/// An `S` type typically holds one or more metrics.
-#[derive(Debug)]
-pub struct Scopes<L: Display + Hash + Eq, S> {
-    scopes: IndexMap<L, S>,
-}
-
 #[derive(Debug)]
 struct Stamped<T> {
     stamp: Instant,
@@ -150,7 +141,7 @@ impl<'a, M: FmtMetric> Metric<'a, M> {
         scopes: &Scopes<L, S>,
         to_metric: F
     )-> fmt::Result {
-        for (labels, scope) in &scopes.scopes {
+        for (labels, scope) in scopes {
             to_metric(scope).fmt_metric_labeled(f, self.name, labels)?;
         }
 
@@ -170,30 +161,25 @@ impl Root {
     }
 
     fn request(&mut self, labels: RequestLabels) -> &mut http::RequestMetrics {
-        self.requests.scopes.entry(labels)
-            .or_insert_with(|| http::RequestMetrics::default().into())
-            .stamped()
+        self.requests.get_or_default(labels).stamped()
+
     }
 
     fn response(&mut self, labels: ResponseLabels) -> &mut http::ResponseMetrics {
-        self.responses.scopes.entry(labels)
-            .or_insert_with(|| http::ResponseMetrics::default().into())
-            .stamped()
+        self.responses.get_or_default(labels).stamped()
     }
 
     fn transport(&mut self, labels: TransportLabels) -> &mut transport::OpenMetrics {
-        self.transports.scopes.entry(labels)
-            .or_insert_with(|| transport::OpenMetrics::default())
+        self.transports.get_or_default(labels)
     }
 
     fn transport_close(&mut self, labels: TransportCloseLabels) -> &mut transport::CloseMetrics {
-        self.transport_closes.scopes.entry(labels)
-            .or_insert_with(|| transport::CloseMetrics::default())
+        self.transport_closes.get_or_default(labels)
     }
 
     fn retain_since(&mut self, epoch: Instant) {
-        self.requests.retain_since(epoch);
-        self.responses.retain_since(epoch);
+        self.requests.retain(|_, v| v.stamp >= epoch);
+        self.responses.retain(|_, v| v.stamp >= epoch);
     }
 }
 
@@ -219,6 +205,12 @@ impl<T> Stamped<T> {
     }
 }
 
+impl<T: Default> Default for Stamped<T> {
+    fn default() -> Self {
+        T::default().into()
+    }
+}
+
 impl<T> From<T> for Stamped<T> {
     fn from(inner: T) -> Self {
         Self {
@@ -235,32 +227,6 @@ impl<T> ::std::ops::Deref for Stamped<T> {
     }
 }
 
-// ===== impl Scopes =====
-
-impl<L: Display + Hash + Eq, S> Default for Scopes<L, S> {
-    fn default() -> Self {
-        Scopes { scopes: IndexMap::default(), }
-    }
-}
-
-impl<L: Display + Hash + Eq, S> Scopes<L, S> {
-    pub fn is_empty(&self) -> bool {
-        self.scopes.is_empty()
-    }
-}
-
-impl<L: Display + Hash + Eq, S: Default> Scopes<L, S> {
-    pub fn get_or_default(&mut self, key: L) -> &mut S {
-        self.scopes.entry(key).or_insert_with(|| S::default())
-    }
-}
-
-impl<L: Display + Hash + Eq, S> Scopes<L, Stamped<S>> {
-    fn retain_since(&mut self, epoch: Instant) {
-        self.scopes.retain(|_, v| v.stamp >= epoch);
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use ctx::test_util::*;
@@ -318,27 +284,27 @@ mod tests {
         mock_route(&mut root, &proxy, &server, "sixers");
         let t2 = Instant::now();
 
-        assert_eq!(root.requests.scopes.len(), 2);
-        assert_eq!(root.responses.scopes.len(), 2);
-        assert_eq!(root.transports.scopes.len(), 2);
-        assert_eq!(root.transport_closes.scopes.len(), 1);
+        assert_eq!(root.requests.len(), 2);
+        assert_eq!(root.responses.len(), 2);
+        assert_eq!(root.transports.len(), 2);
+        assert_eq!(root.transport_closes.len(), 1);
 
         root.retain_since(t0);
-        assert_eq!(root.requests.scopes.len(), 2);
-        assert_eq!(root.responses.scopes.len(), 2);
-        assert_eq!(root.transports.scopes.len(), 2);
-        assert_eq!(root.transport_closes.scopes.len(), 1);
+        assert_eq!(root.requests.len(), 2);
+        assert_eq!(root.responses.len(), 2);
+        assert_eq!(root.transports.len(), 2);
+        assert_eq!(root.transport_closes.len(), 1);
 
         root.retain_since(t1);
-        assert_eq!(root.requests.scopes.len(), 1);
-        assert_eq!(root.responses.scopes.len(), 1);
-        assert_eq!(root.transports.scopes.len(), 2);
-        assert_eq!(root.transport_closes.scopes.len(), 1);
+        assert_eq!(root.requests.len(), 1);
+        assert_eq!(root.responses.len(), 1);
+        assert_eq!(root.transports.len(), 2);
+        assert_eq!(root.transport_closes.len(), 1);
 
         root.retain_since(t2);
-        assert_eq!(root.requests.scopes.len(), 0);
-        assert_eq!(root.responses.scopes.len(), 0);
-        assert_eq!(root.transports.scopes.len(), 2);
-        assert_eq!(root.transport_closes.scopes.len(), 1);
+        assert_eq!(root.requests.len(), 0);
+        assert_eq!(root.responses.len(), 0);
+        assert_eq!(root.transports.len(), 2);
+        assert_eq!(root.transport_closes.len(), 1);
     }
 }

src/telemetry/metrics/record.rs

@@ -137,7 +137,7 @@ mod test {
 
         assert!(r.metrics.lock()
             .expect("lock")
-            .responses.scopes
+            .responses
             .get(&labels)
             .is_none()
         );
@@ -146,7 +146,8 @@ mod test {
         {
             let lock = r.metrics.lock()
                 .expect("lock");
-            let scope = lock.responses.scopes
+            let scope = lock
+                .responses
                 .get(&labels)
                 .expect("scope should be some after event");
 
@@ -249,12 +250,12 @@ mod test {
         {
             let lock = r.metrics.lock()
                 .expect("lock");
-            assert!(lock.requests.scopes.get(&req_labels).is_none());
-            assert!(lock.responses.scopes.get(&rsp_labels).is_none());
-            assert!(lock.transports.scopes.get(&srv_open_labels).is_none());
-            assert!(lock.transports.scopes.get(&client_open_labels).is_none());
-            assert!(lock.transport_closes.scopes.get(&srv_close_labels).is_none());
-            assert!(lock.transport_closes.scopes.get(&client_close_labels).is_none());
+            assert!(lock.requests.get(&req_labels).is_none());
+            assert!(lock.responses.get(&rsp_labels).is_none());
+            assert!(lock.transports.get(&srv_open_labels).is_none());
+            assert!(lock.transports.get(&client_open_labels).is_none());
+            assert!(lock.transport_closes.get(&srv_close_labels).is_none());
+            assert!(lock.transport_closes.get(&client_close_labels).is_none());
         }
 
         for e in &events {
@@ -267,7 +268,7 @@ mod test {
 
             // === request scope ====================================
             assert_eq!(
-                lock.requests.scopes
+                lock.requests
                     .get(&req_labels)
                     .map(|scope| scope.total()),
                 Some(1)
@@ -275,7 +276,7 @@ mod test {
 
             // === response scope ===================================
             let response_scope = lock
-                .responses.scopes
+                .responses
                 .get(&rsp_labels)
                 .expect("response scope missing");
             assert_eq!(response_scope.total(), 1);
@@ -287,7 +288,7 @@ mod test {
 
             // === server transport open scope ======================
             let srv_transport_scope = lock
-                .transports.scopes
+                .transports
                 .get(&srv_open_labels)
                 .expect("server transport scope missing");
             assert_eq!(srv_transport_scope.open_total(), 1);
@@ -296,7 +297,7 @@ mod test {
 
             // === client transport open scope ======================
             let client_transport_scope = lock
-                .transports.scopes
+                .transports
                 .get(&client_open_labels)
                 .expect("client transport scope missing");
             assert_eq!(client_transport_scope.open_total(), 1);
@@ -307,7 +308,7 @@ mod test {
 
             // === server transport close scope =====================
             let srv_transport_close_scope = lock
-                .transport_closes.scopes
+                .transport_closes
                 .get(&srv_close_labels)
                 .expect("server transport close scope missing");
             assert_eq!(srv_transport_close_scope.close_total(), 1);
@@ -318,7 +319,7 @@ mod test {
 
             // === client transport close scope =====================
             let client_transport_close_scope = lock
-                .transport_closes.scopes
+                .transport_closes
                 .get(&client_close_labels)
                 .expect("client transport close scope missing");
             assert_eq!(client_transport_close_scope.close_total(), 1);

src/telemetry/metrics/scopes.rs

@@ -0,0 +1,50 @@
+use indexmap::IndexMap;
+use std::{fmt::Display, hash::Hash};
+
+/// Holds an `S`-typed scope for each `L`-typed label set.
+///
+/// An `S` type typically holds one or more metrics.
+#[derive(Debug)]
+pub struct Scopes<L: Display + Hash + Eq, S>(IndexMap<L, S>);
+
+impl<L: Display + Hash + Eq, S> Default for Scopes<L, S> {
+    fn default() -> Self {
+        Scopes(IndexMap::default())
+    }
+}
+
+impl<L: Display + Hash + Eq, S> Scopes<L, S> {
+    pub fn get(&self, key: &L) -> Option<&S> {
+        self.0.get(key)
+    }
+
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
+
+    pub fn len(&self) -> usize {
+        self.0.len()
+    }
+
+    pub fn retain<F>(&mut self, f: F)
+    where
+        F: FnMut(&L, &mut S) -> bool,
+    {
+        self.0.retain(f)
+    }
+}
+
+impl<L: Display + Hash + Eq, S: Default> Scopes<L, S> {
+    pub fn get_or_default(&mut self, key: L) -> &mut S {
+        self.0.entry(key).or_insert_with(|| S::default())
+    }
+}
+
+impl<'a, L: Display + Hash + Eq, S> IntoIterator for &'a Scopes<L, S> {
+    type Item = <&'a IndexMap<L, S> as IntoIterator>::Item;
+    type IntoIter = <&'a IndexMap<L, S> as IntoIterator>::IntoIter;
+
+    fn into_iter(self) -> Self::IntoIter {
+        self.0.iter()
+    }
+}

src/telemetry/metrics/transport.rs

@@ -42,7 +42,7 @@ impl OpenScopes {
 
 impl fmt::Display for OpenScopes {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if self.scopes.is_empty() {
+        if self.is_empty() {
             return Ok(());
         }
 
@@ -103,7 +103,7 @@ impl CloseScopes {
 
 impl fmt::Display for CloseScopes {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        if self.scopes.is_empty() {
+        if self.is_empty() {
             return Ok(());
         }