Ensure metrics are not evicted for active routes (#124)

It was possible for a metrics scope to be deregistered for active
routes. This could cause metrics to disappear and never be recorded in
some situations.

This change ensure that metrics are only evicted for scopes that are not
active (i.e. in a router, load balancer, etc).

src/proxy/http/metrics/mod.rs

@@ -74,8 +74,12 @@ where
     T: Hash + Eq,
     C: Hash + Eq,
 {
+    /// Retains metrics for all targets that (1) no longer have an active
+    /// reference to the `Metrics` structure and (2) have not been updated since `epoch`.
     fn retain_since(&mut self, epoch: Instant) {
-        self.by_target.retain(|_, m| m.lock().map(|m| m.last_update >= epoch).unwrap_or(false))
+        self.by_target.retain(|_, m| {
+            Arc::strong_count(&m) > 1 || m.lock().map(|m| m.last_update >= epoch).unwrap_or(false)
+        })
     }
 }
 
@@ -103,3 +107,76 @@ where
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn expiry() {
+        use std::fmt;
+        use std::time::Duration;
+        use tokio_timer::clock;
+
+        use metrics::FmtLabels;
+
+        #[derive(Clone, Debug, Hash, Eq, PartialEq)]
+        struct Target(usize);
+        impl FmtLabels for Target {
+            fn fmt_labels(&self, f: &mut fmt::Formatter) -> fmt::Result {
+                write!(f, "n=\"{}\"", self.0)
+            }
+        }
+
+        #[allow(dead_code)]
+        #[derive(Clone, Debug, Hash, Eq, PartialEq)]
+        enum Class {
+            Good,
+            Bad,
+        };
+        impl FmtLabels for Class {
+            fn fmt_labels(&self, f: &mut fmt::Formatter) -> fmt::Result {
+                use std::fmt::Display;
+                match self {
+                    Class::Good => "class=\"good\"".fmt(f),
+                    Class::Bad => "class=\"bad\"".fmt(f),
+                }
+            }
+        }
+
+        let retain_idle_for = Duration::from_secs(1);
+        let (r, report) = super::new::<Target, Class>(retain_idle_for);
+        let mut registry = r.lock().unwrap();
+
+        let before_update = clock::now();
+        let metrics = registry
+            .by_target
+            .entry(Target(123))
+            .or_insert_with(|| Default::default())
+            .clone();
+        assert_eq!(registry.by_target.len(), 1, "target should be registered");
+        let after_update = clock::now();
+
+        registry.retain_since(after_update);
+        assert_eq!(
+            registry.by_target.len(),
+            1,
+            "target should not be evicted by time alone"
+        );
+
+        drop(metrics);
+        registry.retain_since(before_update);
+        assert_eq!(
+            registry.by_target.len(),
+            1,
+            "target should not be evicted by availability alone"
+        );
+
+        registry.retain_since(after_update);
+        assert_eq!(
+            registry.by_target.len(),
+            0,
+            "target should be evicted by time once the handle is dropped"
+        );
+
+        drop((registry, report));
+    }
+}