proxy: Make each metric type responsible for formatting (#878)

In order to set up for a refactor that removes the `Metric` type, the
`FmtMetric` trait--implemented by `Counter`, `Gauge`, and
`Histogram`--is introduced to push prometheus formatting down into each
type.

With this change, the `Histogram` type now relies on `Counter` (and its
metric formatting) more heavily.

proxy/src/telemetry/metrics/counter.rs

@@ -1,5 +1,8 @@
-use std::{fmt, ops};
+use std::fmt::{self, Display};
 use std::num::Wrapping;
+use std::ops;
+
+use super::FmtMetric;
 
 /// A Prometheus counter is represented by a `Wrapping` unsigned 64-bit int.
 ///
@@ -53,8 +56,26 @@ impl ops::AddAssign<u64> for Counter {
     }
 }
 
-impl fmt::Display for Counter {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.0.fmt(f)
+impl ops::AddAssign<Self> for Counter {
+    fn add_assign(&mut self, Counter(rhs): Self) {
+        (*self).0 += rhs
+    }
+}
+
+impl FmtMetric for Counter {
+    fn fmt_metric<N: Display>(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result {
+        writeln!(f, "{} {}", name, self.0)
+    }
+
+    fn fmt_metric_labeled<N, L>(&self, f: &mut fmt::Formatter, name: N, labels: L) -> fmt::Result
+    where
+        L: Display,
+        N: Display,
+    {
+        writeln!(f, "{name}{{{labels}}} {value}",
+            name = name,
+            labels = labels,
+            value = self.0,
+        )
     }
 }

proxy/src/telemetry/metrics/gauge.rs

@@ -1,4 +1,6 @@
-use std::fmt;
+use std::fmt::{self, Display};
+
+use super::FmtMetric;
 
 /// An instaneous metric value.
 #[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
@@ -36,8 +38,20 @@ impl Into<u64> for Gauge {
     }
 }
 
-impl fmt::Display for Gauge {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.0.fmt(f)
+impl FmtMetric for Gauge {
+    fn fmt_metric<N: Display>(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result {
+        writeln!(f, "{} {}", name, self.0)
+    }
+
+    fn fmt_metric_labeled<N, L>(&self, f: &mut fmt::Formatter, name: N, labels: L) -> fmt::Result
+    where
+        N: Display,
+        L: Display,
+    {
+        writeln!(f, "{name}{{{labels}}} {value}",
+            name = name,
+            labels = labels,
+            value = self.0,
+        )
     }
 }

proxy/src/telemetry/metrics/histogram.rs

@@ -1,8 +1,8 @@
-use std::{cmp, fmt, iter, slice};
-use std::num::Wrapping;
+use std::{cmp, iter, slice};
+use std::fmt::{self, Display};
 use std::marker::PhantomData;
 
-use super::Counter;
+use super::{Counter, FmtMetric};
 
 /// A series of latency values and counts.
 #[derive(Debug, Clone)]
@@ -30,7 +30,7 @@ pub struct Histogram<V: Into<u64>> {
     // TODO: Implement Prometheus reset semantics correctly, taking into consideration
     //       that Prometheus represents this as `f64` and so there are only 52 significant
     //       bits.
-    pub sum: Wrapping<u64>,
+    sum: Counter,
 
     _p: PhantomData<V>,
 }
@@ -45,6 +45,15 @@ pub enum Bucket {
 #[derive(Debug)]
 pub struct Bounds(pub &'static [Bucket]);
 
+/// Helper that lazily formats metric keys as {0}_{1}.
+struct Key<A: Display, B: Display>(A, B);
+
+/// Helper that lazily formats comma-separated labels `A,B`.
+struct Labels<A: Display, B: Display>(A, B);
+
+/// Helper that lazily formats an `{K}="{V}"`" label.
+struct Label<K: Display, V: Display>(K, V);
+
 // ===== impl Histogram =====
 
 impl<V: Into<u64>> Histogram<V> {
@@ -60,7 +69,7 @@ impl<V: Into<u64>> Histogram<V> {
         Self {
             bounds,
             buckets: buckets.into_boxed_slice(),
-            sum: Wrapping(0),
+            sum: Counter::default(),
             _p: PhantomData,
         }
     }
@@ -77,11 +86,7 @@ impl<V: Into<u64>> Histogram<V> {
             .expect("all values must fit into a bucket");
 
         self.buckets[idx].incr();
-        self.sum += Wrapping(value);
-    }
-
-    pub fn sum(&self) -> u64 {
-        self.sum.0
+        self.sum += value;
     }
 }
 
@@ -97,6 +102,60 @@ impl<'a, V: Into<u64>> IntoIterator for &'a Histogram<V> {
     }
 }
 
+impl<V: Into<u64>> FmtMetric for Histogram<V> {
+    fn fmt_metric<N: Display>(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result {
+        let mut total = Counter::default();
+        for (le, count) in self {
+            total += *count;
+            total.fmt_metric_labeled(f, Key(&name, "bucket"), Label("le", le))?;
+        }
+        total.fmt_metric(f, Key(&name, "count"))?;
+        self.sum.fmt_metric(f, Key(&name, "sum"))?;
+
+        Ok(())
+    }
+
+    fn fmt_metric_labeled<N, L>(&self, f: &mut fmt::Formatter, name: N, labels: L) -> fmt::Result
+    where
+        N: Display,
+        L: Display,
+    {
+        let mut total = Counter::default();
+        for (le, count) in self {
+            total += *count;
+            total.fmt_metric_labeled(f, Key(&name, "bucket"), Labels(&labels, Label("le", le)))?;
+        }
+        total.fmt_metric_labeled(f, Key(&name, "count"), &labels)?;
+        self.sum.fmt_metric_labeled(f, Key(&name, "sum"), &labels)?;
+
+        Ok(())
+    }
+}
+
+// ===== impl Key =====
+
+impl<A: Display, B: Display> fmt::Display for Key<A, B> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}_{}", self.0, self.1)
+    }
+}
+
+// ===== impl Label =====
+
+impl<K: Display, V: Display> fmt::Display for Label<K, V> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}=\"{}\"", self.0, self.1)
+    }
+}
+
+// ===== impl Labels =====
+
+impl<A: Display, B: Display> fmt::Display for Labels<A, B> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{},{}", self.0, self.1)
+    }
+}
+
 // ===== impl Bucket =====
 
 impl fmt::Display for Bucket {
@@ -129,8 +188,8 @@ impl cmp::Ord for Bucket {
 mod tests {
     use super::*;
 
-    use std::collections::HashMap;
     use std::u64;
+    use std::collections::HashMap;
 
     const NUM_BUCKETS: usize = 47;
     static BOUNDS: &'static Bounds = &Bounds(&[
@@ -204,9 +263,9 @@ mod tests {
         fn sum_equals_total_of_observations(observations: Vec<u64>) -> bool {
             let mut hist = Histogram::<u64>::new(&BOUNDS);
 
-            let mut expected_sum = Wrapping(0u64);
+            let mut expected_sum = Counter::default();
             for obs in observations {
-                expected_sum += Wrapping(obs);
+                expected_sum += obs;
                 hist.add(obs);
             }
 

proxy/src/telemetry/metrics/mod.rs

@@ -27,9 +27,10 @@
 //! to worry about missing commas, double commas, or trailing commas at the
 //! end of the label set (all of which will make Prometheus angry).
 use std::default::Default;
-use std::{fmt, time};
+use std::fmt::{self, Display};
 use std::hash::Hash;
 use std::sync::{Arc, Mutex};
+use std::time;
 
 use indexmap::IndexMap;
 
@@ -56,6 +57,22 @@ pub use self::labels::DstLabels;
 pub use self::record::Record;
 pub use self::serve::Serve;
 
+/// Writes a metric in prometheus-formatted output.
+///
+/// This trait is implemented by `Counter`, `Gauge`, and `Histogram` to account for the
+/// differences in formatting each type of metric. Specifically, `Histogram` formats a
+/// counter for each bucket, as well as a count and total sum.
+trait FmtMetric {
+    /// Writes a metric with the given name and no labels.
+    fn fmt_metric<N: Display>(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result;
+
+    /// Writes a metric with the given name and labels.
+    fn fmt_metric_labeled<N, L>(&self, f: &mut fmt::Formatter, name: N, labels: L) -> fmt::Result
+    where
+        N: Display,
+        L: Display;
+}
+
 #[derive(Debug, Clone)]
 struct Metrics {
     request_total: Metric<Counter, RequestLabels>,
@@ -300,11 +317,7 @@ where
         )?;
 
         for (labels, value) in &self.values {
-            write!(f, "{name}{{{labels}}} {value}\n",
-                name = self.name,
-                labels = labels,
-                value = value,
-            )?;
+            value.fmt_metric_labeled(f, self.name, labels)?;
         }
 
         Ok(())
@@ -324,11 +337,7 @@ where
         )?;
 
         for (labels, value) in &self.values {
-            write!(f, "{name}{{{labels}}} {value}\n",
-                name = self.name,
-                labels = labels,
-                value = value,
-            )?;
+            value.fmt_metric_labeled(f, self.name, labels)?;
         }
 
         Ok(())
@@ -348,31 +357,7 @@ impl<L, V> fmt::Display for Metric<Histogram<V>, L> where
         )?;
 
         for (labels, histogram) in &self.values {
-            // Since Prometheus expects each bucket's value to be the sum of the number of
-            // values in this bucket and all lower buckets, track the total count here.
-            let mut total_count = 0u64;
-            for (le, count) in histogram.into_iter() {
-                // Add this bucket's count to the total count.
-                let c: u64 = (*count).into();
-                total_count += c;
-                write!(f, "{name}_bucket{{{labels},le=\"{le}\"}} {count}\n",
-                    name = self.name,
-                    labels = labels,
-                    le = le,
-                    // Print the total count *as of this iteration*.
-                    count = total_count,
-                )?;
-            }
-
-            // Print the total count and histogram sum stats.
-            write!(f,
-                "{name}_count{{{labels}}} {count}\n\
-                 {name}_sum{{{labels}}} {sum}\n",
-                name = self.name,
-                labels = labels,
-                count = total_count,
-                sum = histogram.sum(),
-            )?;
+            histogram.fmt_metric_labeled(f, self.name, labels)?;
         }
 
         Ok(())