massive rework to make metric description more succinct (#260)

Signed-off-by: Toby Lawrence <[email protected]>

Author: tobz

metrics-benchmark/src/main.rs

@@ -4,7 +4,7 @@ use hdrhistogram::Histogram as HdrHistogram;
 use log::{error, info};
 use metrics::{
     gauge, histogram, increment_counter, register_counter, register_gauge, register_histogram,
-    Counter, Gauge, Histogram, Key, Recorder, Unit,
+    Counter, Gauge, Histogram, Key, KeyName, Recorder, Unit,
 };
 use metrics_util::{Registry, StandardPrimitives};
 use quanta::{Clock, Instant as QuantaInstant};
@@ -47,16 +47,12 @@ pub struct BenchmarkingRecorder {
 impl BenchmarkingRecorder {
     /// Creates a new `BenchmarkingRecorder`.
     pub fn new() -> BenchmarkingRecorder {
-        BenchmarkingRecorder {
-            registry: Arc::new(Registry::new()),
-        }
+        BenchmarkingRecorder { registry: Arc::new(Registry::new()) }
     }
 
     /// Gets a `Controller` attached to this recorder.
     pub fn controller(&self) -> Controller {
-        Controller {
-            registry: self.registry.clone(),
-        }
+        Controller { registry: self.registry.clone() }
     }
 
     /// Installs this recorder as the global recorder.
@@ -66,31 +62,22 @@ impl BenchmarkingRecorder {
 }
 
 impl Recorder for BenchmarkingRecorder {
-    fn describe_counter(&self, key: &Key, _: Option<Unit>, _: Option<&'static str>) {
-        self.registry.get_or_create_counter(key, |_| {})
-    }
+    fn describe_counter(&self, _: KeyName, _: Option<Unit>, _: &'static str) {}
 
-    fn describe_gauge(&self, key: &Key, _: Option<Unit>, _: Option<&'static str>) {
-        self.registry.get_or_create_gauge(key, |_| {})
-    }
+    fn describe_gauge(&self, _: KeyName, _: Option<Unit>, _: &'static str) {}
 
-    fn describe_histogram(&self, key: &Key, _: Option<Unit>, _: Option<&'static str>) {
-        self.registry.get_or_create_histogram(key, |_| {})
-    }
+    fn describe_histogram(&self, _: KeyName, _: Option<Unit>, _: &'static str) {}
 
     fn register_counter(&self, key: &Key) -> Counter {
-        self.registry
-            .get_or_create_counter(key, |c| Counter::from_arc(c.clone()))
+        self.registry.get_or_create_counter(key, |c| Counter::from_arc(c.clone()))
     }
 
     fn register_gauge(&self, key: &Key) -> Gauge {
-        self.registry
-            .get_or_create_gauge(key, |g| Gauge::from_arc(g.clone()))
+        self.registry.get_or_create_gauge(key, |g| Gauge::from_arc(g.clone()))
     }
 
     fn register_histogram(&self, key: &Key) -> Histogram {
-        self.registry
-            .get_or_create_histogram(key, |h| Histogram::from_arc(h.clone()))
+        self.registry.get_or_create_histogram(key, |h| Histogram::from_arc(h.clone()))
     }
 }
 
@@ -131,11 +118,7 @@ impl Generator {
             let t1 = clock.recent();
 
             if let Some(t0) = self.t0 {
-                let start = if loop_counter % LOOP_SAMPLE == 0 {
-                    Some(clock.now())
-                } else {
-                    None
-                };
+                let start = if loop_counter % LOOP_SAMPLE == 0 { Some(clock.now()) } else { None };
 
                 increment_counter!("ok");
                 gauge!("total", self.gauge as f64);
@@ -146,8 +129,7 @@ impl Generator {
                     self.hist.saturating_record(delta.as_nanos() as u64);
 
                     // We also increment our global counter for the sample rate here.
-                    self.rate_counter
-                        .fetch_add(LOOP_SAMPLE * 3, Ordering::AcqRel);
+                    self.rate_counter.fetch_add(LOOP_SAMPLE * 3, Ordering::AcqRel);
 
                     if self.done.load(Ordering::Relaxed) {
                         break;
@@ -175,11 +157,7 @@ impl Generator {
             let t1 = clock.recent();
 
             if let Some(t0) = self.t0 {
-                let start = if loop_counter % LOOP_SAMPLE == 0 {
-                    Some(clock.now())
-                } else {
-                    None
-                };
+                let start = if loop_counter % LOOP_SAMPLE == 0 { Some(clock.now()) } else { None };
 
                 counter.increment(1);
                 gauge.set(self.gauge as f64);
@@ -190,8 +168,7 @@ impl Generator {
                     self.hist.saturating_record(delta.as_nanos() as u64);
 
                     // We also increment our global counter for the sample rate here.
-                    self.rate_counter
-                        .fetch_add(LOOP_SAMPLE * 3, Ordering::AcqRel);
+                    self.rate_counter.fetch_add(LOOP_SAMPLE * 3, Ordering::AcqRel);
 
                     if self.done.load(Ordering::Relaxed) {
                         break;
@@ -226,12 +203,7 @@ fn print_usage(program: &str, opts: &Options) {
 pub fn opts() -> Options {
     let mut opts = Options::new();
 
-    opts.optopt(
-        "d",
-        "duration",
-        "number of seconds to run the benchmark",
-        "INTEGER",
-    );
+    opts.optopt("d", "duration", "number of seconds to run the benchmark", "INTEGER");
     opts.optopt(
         "m",
         "mode",
@@ -267,25 +239,11 @@ fn main() {
     info!("metrics benchmark");
 
     // Build our sink and configure the facets.
-    let seconds = matches
-        .opt_str("duration")
-        .unwrap_or_else(|| "60".to_owned())
-        .parse()
-        .unwrap();
-    let producers = matches
-        .opt_str("producers")
-        .unwrap_or_else(|| "1".to_owned())
-        .parse()
-        .unwrap();
+    let seconds = matches.opt_str("duration").unwrap_or_else(|| "60".to_owned()).parse().unwrap();
+    let producers = matches.opt_str("producers").unwrap_or_else(|| "1".to_owned()).parse().unwrap();
     let mode = matches
         .opt_str("mode")
-        .map(|s| {
-            if s.to_ascii_lowercase() == "fast" {
-                "fast"
-            } else {
-                "slow"
-            }
-        })
+        .map(|s| if s.to_ascii_lowercase() == "fast" { "fast" } else { "slow" })
         .unwrap_or_else(|| "slow")
         .to_owned();
 

metrics-exporter-prometheus/examples/prometheus_push_gateway.rs

@@ -21,10 +21,7 @@ fn main() {
     tracing_subscriber::fmt::init();
 
     PrometheusBuilder::new()
-        .with_push_gateway(
-            "http://127.0.0.1:9091/metrics/job/example",
-            Duration::from_secs(10),
-        )
+        .with_push_gateway("http://127.0.0.1:9091/metrics/job/example", Duration::from_secs(10))
         .expect("push gateway endpoint should be valid")
         .idle_timeout(
             MetricKindMask::COUNTER | MetricKindMask::HISTOGRAM,
@@ -39,10 +36,7 @@ fn main() {
     //
     // Registering metrics ahead of using them is not required, but is the only way to specify the
     // description of a metric.
-    describe_counter!(
-        "tcp_server_loops",
-        "The iterations of the TCP server event loop so far."
-    );
+    describe_counter!("tcp_server_loops", "The iterations of the TCP server event loop so far.");
     describe_histogram!(
         "tcp_server_loop_delta_secs",
         "The time taken for iterations of the TCP server event loop."

metrics-exporter-prometheus/examples/prometheus_server.rs

@@ -29,10 +29,7 @@ fn main() {
     //
     // Registering metrics ahead of using them is not required, but is the only way to specify the
     // description of a metric.
-    describe_counter!(
-        "tcp_server_loops",
-        "The iterations of the TCP server event loop so far."
-    );
+    describe_counter!("tcp_server_loops", "The iterations of the TCP server event loop so far.");
     describe_histogram!(
         "tcp_server_loop_delta_secs",
         "The time taken for iterations of the TCP server event loop."

metrics-exporter-prometheus/src/builder.rs

@@ -64,10 +64,7 @@ enum ExporterConfig {
 }
 
 impl ExporterConfig {
-    #[cfg_attr(
-        not(any(feature = "http-listener", feature = "push-gateway")),
-        allow(dead_code)
-    )]
+    #[cfg_attr(not(any(feature = "http-listener", feature = "push-gateway")), allow(dead_code))]
     fn as_type_str(&self) -> &'static str {
         match self {
             #[cfg(feature = "http-listener")]
@@ -81,10 +78,7 @@ impl ExporterConfig {
 
 /// Builder for creating and installing a Prometheus recorder/exporter.
 pub struct PrometheusBuilder {
-    #[cfg_attr(
-        not(any(feature = "http-listener", feature = "push-gateway")),
-        allow(dead_code)
-    )]
+    #[cfg_attr(not(any(feature = "http-listener", feature = "push-gateway")), allow(dead_code))]
     exporter_config: ExporterConfig,
     #[cfg(feature = "http-listener")]
     allowed_addresses: Option<Vec<IpNet>>,
@@ -134,9 +128,7 @@ impl PrometheusBuilder {
     #[cfg(feature = "http-listener")]
     #[cfg_attr(docsrs, doc(cfg(feature = "http-listener")))]
     pub fn with_http_listener(mut self, addr: impl Into<SocketAddr>) -> Self {
-        self.exporter_config = ExporterConfig::HttpListener {
-            listen_address: addr.into(),
-        };
+        self.exporter_config = ExporterConfig::HttpListener { listen_address: addr.into() };
         self
     }
 
@@ -292,11 +284,7 @@ impl PrometheusBuilder {
     /// information on defining a metric kind mask.
     pub fn idle_timeout(mut self, mask: MetricKindMask, timeout: Option<Duration>) -> Self {
         self.idle_timeout = timeout;
-        self.recency_mask = if self.idle_timeout.is_none() {
-            MetricKindMask::NONE
-        } else {
-            mask
-        };
+        self.recency_mask = if self.idle_timeout.is_none() { MetricKindMask::NONE } else { mask };
         self
     }
 
@@ -326,10 +314,7 @@ impl PrometheusBuilder {
     /// If there is an error while either building the recorder and exporter, or installing the
     /// recorder and exporter, an error variant will be returned describing the error.
     #[cfg(any(feature = "http-listener", feature = "push-gateway"))]
-    #[cfg_attr(
-        docsrs,
-        doc(cfg(any(feature = "http-listener", feature = "push-gateway")))
-    )]
+    #[cfg_attr(docsrs, doc(cfg(any(feature = "http-listener", feature = "push-gateway"))))]
     pub fn install(self) -> Result<(), BuildError> {
         let recorder = if let Ok(handle) = runtime::Handle::try_current() {
             let (recorder, exporter) = {
@@ -341,10 +326,8 @@ impl PrometheusBuilder {
 
             recorder
         } else {
-            let thread_name = format!(
-                "metrics-exporter-prometheus-{}",
-                self.exporter_config.as_type_str()
-            );
+            let thread_name =
+                format!("metrics-exporter-prometheus-{}", self.exporter_config.as_type_str());
 
             let runtime = runtime::Builder::new_current_thread()
                 .enable_all()
@@ -402,10 +385,7 @@ impl PrometheusBuilder {
     /// If there is an error while building the recorder and exporter, an error variant will be
     /// returned describing the error.
     #[cfg(any(feature = "http-listener", feature = "push-gateway"))]
-    #[cfg_attr(
-        docsrs,
-        doc(cfg(any(feature = "http-listener", feature = "push-gateway")))
-    )]
+    #[cfg_attr(docsrs, doc(cfg(any(feature = "http-listener", feature = "push-gateway"))))]
     #[cfg_attr(not(feature = "http-listener"), allow(unused_mut))]
     pub fn build(mut self) -> Result<(PrometheusRecorder, ExporterFuture), BuildError> {
         #[cfg(feature = "http-listener")]
@@ -428,9 +408,7 @@ impl PrometheusBuilder {
                         // If the allowlist is empty, the request is allowed.  Otherwise, it must
                         // match one of the entries in the allowlist or it will be denied.
                         let is_allowed = allowed_addresses.as_ref().map_or(true, |addresses| {
-                            addresses
-                                .iter()
-                                .any(|address| address.contains(&remote_addr))
+                            addresses.iter().any(|address| address.contains(&remote_addr))
                         });
 
                         let handle = handle.clone();
@@ -551,7 +529,7 @@ mod tests {
 
     use quanta::Clock;
 
-    use metrics::{Key, Label, Recorder};
+    use metrics::{Key, KeyName, Label, Recorder};
     use metrics_util::MetricKindMask;
 
     use super::{Matcher, PrometheusBuilder};
@@ -751,9 +729,7 @@ mod tests {
 
     #[test]
     pub fn test_global_labels_overrides() {
-        let recorder = PrometheusBuilder::new()
-            .add_global_label("foo", "foo")
-            .build_recorder();
+        let recorder = PrometheusBuilder::new().add_global_label("foo", "foo").build_recorder();
 
         let key =
             Key::from_name("overridden").with_extra_labels(vec![Label::new("foo", "overridden")]);
@@ -769,13 +745,12 @@ mod tests {
 
     #[test]
     pub fn test_sanitized_render() {
-        let recorder = PrometheusBuilder::new()
-            .add_global_label("foo:", "foo")
-            .build_recorder();
+        let recorder = PrometheusBuilder::new().add_global_label("foo:", "foo").build_recorder();
 
-        let key = Key::from_name("yee_haw:lets go")
+        let key_name = KeyName::from("yee_haw:lets go");
+        let key = Key::from_name(key_name.clone())
             .with_extra_labels(vec![Label::new("øhno", "\"yeet\nies\\\"")]);
-        recorder.describe_counter(&key, None, Some("\"Simplë stuff.\nRëally.\""));
+        recorder.describe_counter(key_name, None, "\"Simplë stuff.\nRëally.\"");
         let counter1 = recorder.register_counter(&key);
         counter1.increment(1);
 

metrics-exporter-prometheus/src/common.rs

@@ -185,12 +185,7 @@ mod tests {
 
     #[test]
     fn test_sanitize_metric_name_known_cases() {
-        let cases = &[
-            ("*", "_"),
-            ("\"", "_"),
-            ("foo_bar", "foo_bar"),
-            ("1foobar", "_foobar"),
-        ];
+        let cases = &[("*", "_"), ("\"", "_"), ("foo_bar", "foo_bar"), ("1foobar", "_foobar")];
 
         for (input, expected) in cases {
             let result = sanitize_metric_name(input);