Prometheus: Add setting for recommended metric names (#596)

Author: michelhe

metrics-exporter-prometheus/src/exporter/builder.rs

@@ -48,6 +48,8 @@ pub struct PrometheusBuilder {
     upkeep_timeout: Duration,
     recency_mask: MetricKindMask,
     global_labels: Option<IndexMap<String, String>>,
+    enable_recommended_naming: bool,
+    /// TODO Remove this field in next version and merge with `enable_recommended_naming`
     enable_unit_suffix: bool,
 }
 
@@ -81,6 +83,7 @@ impl PrometheusBuilder {
             upkeep_timeout,
             recency_mask: MetricKindMask::NONE,
             global_labels: None,
+            enable_recommended_naming: false,
             enable_unit_suffix: false,
         }
     }
@@ -291,11 +294,27 @@ impl PrometheusBuilder {
     ///
     /// Defaults to false.
     #[must_use]
+    #[deprecated(
+        since = "0.18.0",
+        note = "users should prefer `with_recommended_naming` which automatically enables unit suffixes"
+    )]
     pub fn set_enable_unit_suffix(mut self, enabled: bool) -> Self {
         self.enable_unit_suffix = enabled;
         self
     }
 
+    /// Enables Prometheus naming best practices for metrics.
+    ///
+    /// When set to `true`, counter names are suffixed with `_total` and unit suffixes are appended to metric names,
+    /// following [Prometheus Best Practices](https://prometheus.io/docs/practices/naming/).
+    ///
+    /// Defaults to `false`.
+    #[must_use]
+    pub fn with_recommended_naming(mut self, enabled: bool) -> Self {
+        self.enable_recommended_naming = enabled;
+        self
+    }
+
     /// Sets the bucket for a specific pattern.
     ///
     /// The match pattern can be a full match (equality), prefix match, or suffix match.  The matchers are applied in
@@ -538,7 +557,8 @@ impl PrometheusBuilder {
             ),
             descriptions: RwLock::new(HashMap::new()),
             global_labels: self.global_labels.unwrap_or_default(),
-            enable_unit_suffix: self.enable_unit_suffix,
+            enable_unit_suffix: self.enable_recommended_naming || self.enable_unit_suffix,
+            counter_suffix: self.enable_recommended_naming.then_some("total"),
         };
 
         PrometheusRecorder::from(inner)
@@ -558,7 +578,7 @@ mod tests {
 
     use quanta::Clock;
 
-    use metrics::{Key, KeyName, Label, Recorder};
+    use metrics::{Key, KeyName, Label, Recorder, Unit};
     use metrics_util::MetricKindMask;
 
     use super::{Matcher, PrometheusBuilder};
@@ -610,6 +630,43 @@ mod tests {
         assert_eq!(rendered, expected_histogram);
     }
 
+    #[test]
+    fn test_render_with_recommended_naming() {
+        // test 1 - no unit or description
+        let recorder = PrometheusBuilder::new().with_recommended_naming(true).build_recorder();
+
+        let key = Key::from_name("basic_counter");
+        let counter1 = recorder.register_counter(&key, &METADATA);
+        counter1.increment(42);
+
+        let handle = recorder.handle();
+        let rendered = handle.render();
+        let expected_counter = "# TYPE basic_counter counter\nbasic_counter_total 42\n\n";
+
+        assert_eq!(rendered, expected_counter);
+
+        // test 2 - with unit and description
+        // Note: we need to create a new recorder, as the render order is not deterministic
+        let recorder = PrometheusBuilder::new().with_recommended_naming(true).build_recorder();
+
+        let key_name = KeyName::from_const_str("counter_with_unit");
+        let key = Key::from_name(key_name.clone());
+        recorder.describe_counter(key_name, Some(Unit::Bytes), "A counter with a unit".into());
+        let counter2 = recorder.register_counter(&key, &METADATA);
+        counter2.increment(42);
+
+        let handle = recorder.handle();
+        let rendered = handle.render();
+        let expected_counter = concat!(
+            "# HELP counter_with_unit_bytes A counter with a unit\n",
+            "# TYPE counter_with_unit_bytes counter\n",
+            "counter_with_unit_bytes_total 42\n",
+            "\n",
+        );
+
+        assert_eq!(rendered, expected_counter);
+    }
+
     #[test]
     fn test_buckets() {
         const DEFAULT_VALUES: [f64; 3] = [10.0, 100.0, 1000.0];

metrics-exporter-prometheus/src/formatting.rs

@@ -29,9 +29,13 @@ pub fn key_to_parts(
 /// Writes a help (description) line in the Prometheus [exposition format].
 ///
 /// [exposition format]: https://github.com/prometheus/docs/blob/main/content/docs/instrumenting/exposition_formats.md#text-format-details
-pub fn write_help_line(buffer: &mut String, name: &str, desc: &str) {
+pub fn write_help_line(buffer: &mut String, name: &str, unit: Option<Unit>, desc: &str) {
     buffer.push_str("# HELP ");
     buffer.push_str(name);
+    if let Some(unit) = unit {
+        buffer.push('_');
+        buffer.push_str(unit.as_str());
+    }
     buffer.push(' ');
     let desc = sanitize_description(desc);
     buffer.push_str(&desc);
@@ -41,9 +45,13 @@ pub fn write_help_line(buffer: &mut String, name: &str, desc: &str) {
 /// Writes a metric type line in the Prometheus [exposition format].
 ///
 /// [exposition format]: https://github.com/prometheus/docs/blob/main/content/docs/instrumenting/exposition_formats.md#text-format-details
-pub fn write_type_line(buffer: &mut String, name: &str, metric_type: &str) {
+pub fn write_type_line(buffer: &mut String, name: &str, unit: Option<Unit>, metric_type: &str) {
     buffer.push_str("# TYPE ");
     buffer.push_str(name);
+    if let Some(unit) = unit {
+        buffer.push('_');
+        buffer.push_str(unit.as_str());
+    }
     buffer.push(' ');
     buffer.push_str(metric_type);
     buffer.push('\n');
@@ -70,17 +78,18 @@ pub fn write_metric_line<T, T2>(
     T2: std::fmt::Display,
 {
     buffer.push_str(name);
-    if let Some(suffix) = suffix {
-        buffer.push('_');
-        buffer.push_str(suffix);
-    }
 
     match unit {
         Some(Unit::Count) | None => {}
         Some(Unit::Percent) => add_unit(buffer, "ratio"),
         Some(unit) => add_unit(buffer, unit.as_str()),
     }
 
+    if let Some(suffix) = suffix {
+        buffer.push('_');
+        buffer.push_str(suffix);
+    }
+
     if !labels.is_empty() || additional_label.is_some() {
         buffer.push('{');
 

metrics-exporter-prometheus/src/recorder.rs

@@ -24,6 +24,7 @@ pub(crate) struct Inner {
     pub descriptions: RwLock<HashMap<String, (SharedString, Option<Unit>)>>,
     pub global_labels: IndexMap<String, String>,
     pub enable_unit_suffix: bool,
+    pub counter_suffix: Option<&'static str>,
 }
 
 impl Inner {
@@ -118,32 +119,34 @@ impl Inner {
 
         for (name, mut by_labels) in counters.drain() {
             let unit = descriptions.get(name.as_str()).and_then(|(desc, unit)| {
-                write_help_line(&mut output, name.as_str(), desc);
-                *unit
+                let unit = unit.filter(|_| self.enable_unit_suffix);
+                write_help_line(&mut output, name.as_str(), unit, desc);
+                unit
             });
 
-            write_type_line(&mut output, name.as_str(), "counter");
+            write_type_line(&mut output, name.as_str(), unit, "counter");
             for (labels, value) in by_labels.drain() {
                 write_metric_line::<&str, u64>(
                     &mut output,
                     &name,
-                    None,
+                    self.counter_suffix,
                     &labels,
                     None,
                     value,
-                    unit.filter(|_| self.enable_unit_suffix),
+                    unit,
                 );
             }
             output.push('\n');
         }
 
         for (name, mut by_labels) in gauges.drain() {
             let unit = descriptions.get(name.as_str()).and_then(|(desc, unit)| {
-                write_help_line(&mut output, name.as_str(), desc);
-                *unit
+                let unit = unit.filter(|_| self.enable_unit_suffix);
+                write_help_line(&mut output, name.as_str(), unit, desc);
+                unit
             });
 
-            write_type_line(&mut output, name.as_str(), "gauge");
+            write_type_line(&mut output, name.as_str(), unit, "gauge");
             for (labels, value) in by_labels.drain() {
                 write_metric_line::<&str, f64>(
                     &mut output,
@@ -152,20 +155,21 @@ impl Inner {
                     &labels,
                     None,
                     value,
-                    unit.filter(|_| self.enable_unit_suffix),
+                    unit,
                 );
             }
             output.push('\n');
         }
 
         for (name, mut by_labels) in distributions.drain() {
             let unit = descriptions.get(name.as_str()).and_then(|(desc, unit)| {
-                write_help_line(&mut output, name.as_str(), desc);
-                *unit
+                let unit = unit.filter(|_| self.enable_unit_suffix);
+                write_help_line(&mut output, name.as_str(), unit, desc);
+                unit
             });
 
             let distribution_type = self.distribution_builder.get_distribution_type(name.as_str());
-            write_type_line(&mut output, name.as_str(), distribution_type);
+            write_type_line(&mut output, name.as_str(), unit, distribution_type);
             for (labels, distribution) in by_labels.drain(..) {
                 let (sum, count) = match distribution {
                     Distribution::Summary(summary, quantiles, sum) => {
@@ -179,7 +183,7 @@ impl Inner {
                                 &labels,
                                 Some(("quantile", quantile.value())),
                                 value,
-                                unit.filter(|_| self.enable_unit_suffix),
+                                unit,
                             );
                         }
 
@@ -194,7 +198,7 @@ impl Inner {
                                 &labels,
                                 Some(("le", le)),
                                 count,
-                                unit.filter(|_| self.enable_unit_suffix),
+                                unit,
                             );
                         }
                         write_metric_line(
@@ -204,7 +208,7 @@ impl Inner {
                             &labels,
                             Some(("le", "+Inf")),
                             histogram.count(),
-                            unit.filter(|_| self.enable_unit_suffix),
+                            unit,
                         );
 
                         (histogram.sum(), histogram.count())
@@ -218,7 +222,7 @@ impl Inner {
                     &labels,
                     None,
                     sum,
-                    unit.filter(|_| self.enable_unit_suffix),
+                    unit,
                 );
                 write_metric_line::<&str, u64>(
                     &mut output,
@@ -227,7 +231,7 @@ impl Inner {
                     &labels,
                     None,
                     count,
-                    unit.filter(|_| self.enable_unit_suffix),
+                    unit,
                 );
             }