From 40e9ffcaf84593411625dad6ae278281a2d3e6aa Mon Sep 17 00:00:00 2001
From: Oliver Gould <ver@buoyant.io>
Date: Tue, 21 Aug 2018 10:43:47 -0700
Subject: [PATCH] Require Sensors for Bind (#78)

`Bind` was initially written so that a `Sensors` implementation is
optional. Over time, this hasn't proven to be very useful.

In preparation for further changes to HTTP telemetry, this change
simplifies the Bind and Sensors types so that an HTTP sensor is always
required to construct `Bind`.

Test-only constructors have been added to satisfy the case where Sensors
were omitted.
---
 src/bind.rs                     | 10 ++--------
 src/inbound.rs                  |  1 +
 src/lib.rs                      |  7 +++++--
 src/telemetry/metrics/record.rs |  5 +++++
 src/telemetry/sensor/mod.rs     | 27 +++++++++++++--------------
 5 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/src/bind.rs b/src/bind.rs
index 3dd0b9a4a..e9d784806 100644
--- a/src/bind.rs
+++ b/src/bind.rs
@@ -183,25 +183,19 @@ impl Error for BufferSpawnError {
 
 impl<B> Bind<(), B> {
     pub fn new(
+        sensors: telemetry::Sensors,
         transport_registry: telemetry::transport::Registry,
         tls_client_config: tls::ClientConfigWatch
     ) -> Self {
         Self {
             ctx: (),
-            sensors: telemetry::Sensors::null(),
+            sensors,
             transport_registry,
             tls_client_config,
             _p: PhantomData,
         }
     }
 
-    pub fn with_sensors(self, sensors: telemetry::Sensors) -> Self {
-        Self {
-            sensors,
-            ..self
-        }
-    }
-
     pub fn with_ctx<C>(self, ctx: C) -> Bind<C, B> {
         Bind {
             ctx,
diff --git a/src/inbound.rs b/src/inbound.rs
index b033dfb46..86e39a293 100644
--- a/src/inbound.rs
+++ b/src/inbound.rs
@@ -115,6 +115,7 @@ mod tests {
 
     fn new_inbound(default: Option<net::SocketAddr>, ctx: ctx::Proxy) -> Inbound<()> {
         let bind = Bind::new(
+            ::telemetry::Sensors::for_test(),
             ::telemetry::transport::Registry::default(),
             tls::ClientConfig::no_tls()
         );
diff --git a/src/lib.rs b/src/lib.rs
index bc0ae802f..df1865223 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -286,8 +286,11 @@ where
 
         let (drain_tx, drain_rx) = drain::channel();
 
-        let bind = Bind::new(transport_registry.clone(), tls_client_config)
-            .with_sensors(sensors.clone());
+        let bind = Bind::new(
+            sensors.clone(),
+            transport_registry.clone(),
+            tls_client_config
+        );
 
         // Setup the public listener. This will listen on a publicly accessible
         // address and listen for inbound connections that should be forwarded
diff --git a/src/telemetry/metrics/record.rs b/src/telemetry/metrics/record.rs
index ee7b470cc..7d872f1cd 100644
--- a/src/telemetry/metrics/record.rs
+++ b/src/telemetry/metrics/record.rs
@@ -20,6 +20,11 @@ impl Record {
         Self { metrics: metrics.clone() }
     }
 
+    #[cfg(test)]
+    pub fn for_test() -> Self {
+        Self { metrics: Default::default() }
+    }
+
     #[inline]
     fn update<F: Fn(&mut Root)>(&mut self, f: F) {
         let mut lock = self.metrics.lock()
diff --git a/src/telemetry/sensor/mod.rs b/src/telemetry/sensor/mod.rs
index 94b575479..85f2c45d4 100644
--- a/src/telemetry/sensor/mod.rs
+++ b/src/telemetry/sensor/mod.rs
@@ -20,40 +20,39 @@ struct Inner {
 
 /// Accepts events from sensors.
 #[derive(Clone, Debug)]
-struct Handle(Option<Inner>);
+struct Handle(Inner);
 
 /// Supports the creation of telemetry scopes.
 #[derive(Clone, Debug)]
-pub struct Sensors(Option<Inner>);
+pub struct Sensors(Inner);
 
 impl Handle {
     fn send<F>(&mut self, mk: F)
     where
         F: FnOnce() -> event::Event,
     {
-        if let Some(inner) = self.0.as_mut() {
-            let ev = mk();
-            trace!("event: {:?}", ev);
+        let ev = mk();
+        trace!("event: {:?}", ev);
 
-            if let Ok(mut taps) = inner.taps.lock() {
-                taps.inspect(&ev);
-            }
-
-            inner.metrics.record_event(&ev);
+        if let Ok(mut taps) = self.0.taps.lock() {
+            taps.inspect(&ev);
         }
+
+        self.0.metrics.record_event(&ev);
     }
 }
 
 impl Sensors {
     pub(super) fn new(metrics: metrics::Record, taps: &Arc<Mutex<tap::Taps>>) -> Self {
-        Sensors(Some(Inner {
+        Sensors(Inner {
             metrics,
             taps: taps.clone(),
-        }))
+        })
     }
 
-    pub fn null() -> Sensors {
-        Sensors(None)
+    #[cfg(test)]
+    pub fn for_test() -> Self {
+        Self::new(metrics::Record::for_test(), &Default::default())
     }
 
     pub fn http<N, A, B>(