Update ResponseBody metrics to use a histogram

Signed-off-by: Oliver Gould <[email protected]>

Author: olix0r

linkerd/http/prom/src/body_data/body.rs

@@ -41,10 +41,7 @@ where
     ) -> Poll<Option<Result<Self::Data, Self::Error>>> {
         let this = self.project();
         let inner = this.inner;
-        let BodyDataMetrics {
-            frames_total,
-            frames_bytes,
-        } = this.metrics;
+        let BodyDataMetrics { frame_size } = this.metrics;
 
         let data = std::task::ready!(inner.poll_data(cx));
 
@@ -53,11 +50,8 @@ where
             //
             // NB: We're careful to call `remaining()` rather than `chunk()`, which
             // "can return a shorter slice (this allows non-continuous internal representation)."
-            let bytes = <B::Data as bytes::Buf>::remaining(data)
-                .try_into()
-                .unwrap_or(u64::MAX);
-            frames_bytes.inc_by(bytes);
-            frames_total.inc();
+            let bytes = bytes::Buf::remaining(data);
+            frame_size.observe(linkerd_metrics::to_f64(bytes as u64));
         }
 
         Poll::Ready(data)

linkerd/http/prom/src/body_data/metrics.rs

@@ -1,23 +1,30 @@
 //! Prometheus counters for request and response bodies.
 
-use linkerd_metrics::prom::{self, Counter, Family, Registry};
+use linkerd_metrics::prom::{
+    self, metrics::family::MetricConstructor, Family, Histogram, Registry, Unit,
+};
 
 /// Counters for response body frames.
 #[derive(Clone, Debug)]
 pub struct ResponseBodyFamilies<L> {
-    /// Counts the number of response body frames.
-    resp_body_frames_total: Family<L, Counter>,
-    /// Counts the total number of bytes in response body frames.
-    resp_body_frames_bytes: Family<L, Counter>,
+    /// Counts the number of response body frames by size.
+    frame_sizes: Family<L, Histogram, NewHisto>,
 }
 
 /// Counters to instrument a request or response body.
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Debug)]
 pub struct BodyDataMetrics {
     /// Counts the number of request body frames.
-    pub frames_total: Counter,
-    /// Counts the total number of bytes in request body frames.
-    pub frames_bytes: Counter,
+    pub frame_size: Histogram,
+}
+
+#[derive(Clone, Copy)]
+struct NewHisto;
+
+impl MetricConstructor<Histogram> for NewHisto {
+    fn new_metric(&self) -> Histogram {
+        Histogram::new([128.0, 1024.0, 10240.0].into_iter())
+    }
 }
 
 // === impl ResponseBodyFamilies ===
@@ -28,8 +35,7 @@ where
 {
     fn default() -> Self {
         Self {
-            resp_body_frames_total: Default::default(),
-            resp_body_frames_bytes: Default::default(),
+            frame_sizes: Family::new_with_constructor(NewHisto),
         }
     }
 }
@@ -45,50 +51,25 @@ where
         + Sync
         + 'static,
 {
-    const RESP_BODY_FRAMES_TOTAL_NAME: &'static str = "resp_body_frames_total";
-    const RESP_BODY_FRAMES_TOTAL_HELP: &'static str =
-        "Counts the number of frames in response bodies.";
-
-    const RESP_BODY_FRAMES_BYTES_NAME: &'static str = "resp_body_frames_bytes";
-    const RESP_BODY_FRAMES_BYTES_HELP: &'static str =
-        "Counts the total number of bytes in response bodies.";
-
     /// Registers and returns a new family of body data metrics.
     pub fn register(registry: &mut Registry) -> Self {
-        let resp_body_frames_total = Family::default();
-        registry.register(
-            Self::RESP_BODY_FRAMES_TOTAL_NAME,
-            Self::RESP_BODY_FRAMES_TOTAL_HELP,
-            resp_body_frames_total.clone(),
-        );
-
-        let resp_body_frames_bytes = Family::default();
+        let frame_sizes = Family::new_with_constructor(NewHisto);
         registry.register_with_unit(
-            Self::RESP_BODY_FRAMES_BYTES_NAME,
-            Self::RESP_BODY_FRAMES_BYTES_HELP,
-            prom::Unit::Bytes,
-            resp_body_frames_bytes.clone(),
+            "response_frame_size",
+            "Response data frame sizes",
+            Unit::Bytes,
+            frame_sizes.clone(),
         );
 
-        Self {
-            resp_body_frames_total,
-            resp_body_frames_bytes,
-        }
+        Self { frame_sizes }
     }
 
     /// Returns the [`BodyDataMetrics`] for the given label set.
     pub fn metrics(&self, labels: &L) -> BodyDataMetrics {
-        let Self {
-            resp_body_frames_total,
-            resp_body_frames_bytes,
-        } = self;
+        let Self { frame_sizes } = self;
 
-        let frames_total = resp_body_frames_total.get_or_create(labels).clone();
-        let frames_bytes = resp_body_frames_bytes.get_or_create(labels).clone();
+        let frame_size = frame_sizes.get_or_create(labels).clone();
 
-        BodyDataMetrics {
-            frames_total,
-            frames_bytes,
-        }
+        BodyDataMetrics { frame_size }
     }
 }

linkerd/metrics/src/lib.rs

@@ -85,7 +85,12 @@ pub trait Factor {
 const MAX_PRECISE_UINT64: u64 = 0x20_0000_0000_0000;
 
 impl Factor for () {
+    #[inline]
     fn factor(n: u64) -> f64 {
-        n.wrapping_rem(MAX_PRECISE_UINT64 + 1) as f64
+        to_f64(n)
     }
 }
+
+pub fn to_f64(n: u64) -> f64 {
+    n.wrapping_rem(MAX_PRECISE_UINT64 + 1) as f64
+}