From c63f0a19760a3ae6fb0baa6957111cb4764adcd5 Mon Sep 17 00:00:00 2001 From: Oliver Gould Date: Mon, 30 Apr 2018 13:00:21 -0700 Subject: [PATCH] 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 | 29 ++++++-- proxy/src/telemetry/metrics/gauge.rs | 22 ++++-- proxy/src/telemetry/metrics/histogram.rs | 85 ++++++++++++++++++++---- proxy/src/telemetry/metrics/mod.rs | 57 ++++++---------- 4 files changed, 136 insertions(+), 57 deletions(-) diff --git a/proxy/src/telemetry/metrics/counter.rs b/proxy/src/telemetry/metrics/counter.rs index b9d086fce..808642831 100644 --- a/proxy/src/telemetry/metrics/counter.rs +++ b/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 for Counter { } } -impl fmt::Display for Counter { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - self.0.fmt(f) +impl ops::AddAssign for Counter { + fn add_assign(&mut self, Counter(rhs): Self) { + (*self).0 += rhs + } +} + +impl FmtMetric for Counter { + fn fmt_metric(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result { + writeln!(f, "{} {}", name, self.0) + } + + fn fmt_metric_labeled(&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, + ) } } diff --git a/proxy/src/telemetry/metrics/gauge.rs b/proxy/src/telemetry/metrics/gauge.rs index 150dacff2..768db2ebe 100644 --- a/proxy/src/telemetry/metrics/gauge.rs +++ b/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 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(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result { + writeln!(f, "{} {}", name, self.0) + } + + fn fmt_metric_labeled(&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, + ) } } diff --git a/proxy/src/telemetry/metrics/histogram.rs b/proxy/src/telemetry/metrics/histogram.rs index c3bb22f3f..ebe6aedf8 100644 --- a/proxy/src/telemetry/metrics/histogram.rs +++ b/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> { // 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, + sum: Counter, _p: PhantomData, } @@ -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, B); + +/// Helper that lazily formats comma-separated labels `A,B`. +struct Labels(A, B); + +/// Helper that lazily formats an `{K}="{V}"`" label. +struct Label(K, V); + // ===== impl Histogram ===== impl> Histogram { @@ -60,7 +69,7 @@ impl> Histogram { Self { bounds, buckets: buckets.into_boxed_slice(), - sum: Wrapping(0), + sum: Counter::default(), _p: PhantomData, } } @@ -77,11 +86,7 @@ impl> Histogram { .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> IntoIterator for &'a Histogram { } } +impl> FmtMetric for Histogram { + fn fmt_metric(&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(&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 fmt::Display for Key { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}_{}", self.0, self.1) + } +} + +// ===== impl Label ===== + +impl fmt::Display for Label { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}=\"{}\"", self.0, self.1) + } +} + +// ===== impl Labels ===== + +impl fmt::Display for Labels { + 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) -> bool { let mut hist = Histogram::::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); } diff --git a/proxy/src/telemetry/metrics/mod.rs b/proxy/src/telemetry/metrics/mod.rs index 996cc119e..69479b0ae 100644 --- a/proxy/src/telemetry/metrics/mod.rs +++ b/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(&self, f: &mut fmt::Formatter, name: N) -> fmt::Result; + + /// Writes a metric with the given name and labels. + fn fmt_metric_labeled(&self, f: &mut fmt::Formatter, name: N, labels: L) -> fmt::Result + where + N: Display, + L: Display; +} + #[derive(Debug, Clone)] struct Metrics { request_total: Metric, @@ -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 fmt::Display for Metric, 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(())