diff --git a/CHANGELOG.md b/CHANGELOG.md index 32de9b95..66eb579d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [PR 216]: https://github.com/prometheus/client_rust/pull/216 [PR 217]: https://github.com/prometheus/client_rust/pull/217 +### Fixed + +- Don't prepend `,` when encoding empty family label set. + See [PR 175]. + +[PR 175]: https://github.com/prometheus/client_rust/pull/175 + ## [0.22.3] ### Added diff --git a/examples/custom-metric.rs b/examples/custom-metric.rs index 9ad17a5a..a61ff8a7 100644 --- a/examples/custom-metric.rs +++ b/examples/custom-metric.rs @@ -1,4 +1,4 @@ -use prometheus_client::encoding::{text::encode, EncodeMetric, MetricEncoder}; +use prometheus_client::encoding::{text::encode, EncodeMetric, MetricEncoder, NoLabelSet}; use prometheus_client::metrics::MetricType; use prometheus_client::registry::Registry; @@ -20,7 +20,7 @@ impl EncodeMetric for MyCustomMetric { // E.g. every CPU cycle spend in this method delays the response send to // the Prometheus server. - encoder.encode_counter::<(), _, u64>(&rand::random::(), None) + encoder.encode_counter::(&rand::random::(), None) } fn metric_type(&self) -> prometheus_client::metrics::MetricType { diff --git a/src/encoding.rs b/src/encoding.rs index c9c9a838..c644f82b 100644 --- a/src/encoding.rs +++ b/src/encoding.rs @@ -247,6 +247,10 @@ pub trait EncodeLabel { #[derive(Debug)] pub struct LabelEncoder<'a>(LabelEncoderInner<'a>); +/// Uninhabited type to represent the lack of a label set for a metric +#[derive(Debug)] +pub enum NoLabelSet {} + #[derive(Debug)] enum LabelEncoderInner<'a> { Text(text::LabelEncoder<'a>), @@ -352,7 +356,7 @@ impl EncodeLabelSet for Vec { } } -impl EncodeLabelSet for () { +impl EncodeLabelSet for NoLabelSet { fn encode(&self, _encoder: LabelSetEncoder) -> Result<(), std::fmt::Error> { Ok(()) } diff --git a/src/encoding/text.rs b/src/encoding/text.rs index 955396a6..4946fe35 100644 --- a/src/encoding/text.rs +++ b/src/encoding/text.rs @@ -37,7 +37,7 @@ //! assert_eq!(expected_msg, buffer); //! ``` -use crate::encoding::{EncodeExemplarValue, EncodeLabelSet}; +use crate::encoding::{EncodeExemplarValue, EncodeLabelSet, NoLabelSet}; use crate::metrics::exemplar::Exemplar; use crate::metrics::MetricType; use crate::registry::{Prefix, Registry, Unit}; @@ -324,7 +324,7 @@ impl<'a> MetricEncoder<'a> { self.write_suffix("total")?; - self.encode_labels::<()>(None)?; + self.encode_labels::(None)?; v.encode( &mut CounterValueEncoder { @@ -348,7 +348,7 @@ impl<'a> MetricEncoder<'a> { ) -> Result<(), std::fmt::Error> { self.write_prefix_name_unit()?; - self.encode_labels::<()>(None)?; + self.encode_labels::(None)?; v.encode( &mut GaugeValueEncoder { @@ -404,14 +404,14 @@ impl<'a> MetricEncoder<'a> { ) -> Result<(), std::fmt::Error> { self.write_prefix_name_unit()?; self.write_suffix("sum")?; - self.encode_labels::<()>(None)?; + self.encode_labels::(None)?; self.writer.write_str(" ")?; self.writer.write_str(dtoa::Buffer::new().format(sum))?; self.newline()?; self.write_prefix_name_unit()?; self.write_suffix("count")?; - self.encode_labels::<()>(None)?; + self.encode_labels::(None)?; self.writer.write_str(" ")?; self.writer.write_str(itoa::Buffer::new().format(count))?; self.newline()?; @@ -512,12 +512,37 @@ impl<'a> MetricEncoder<'a> { additional_labels.encode(LabelSetEncoder::new(self.writer).into())?; } - if let Some(labels) = &self.family_labels { - if !self.const_labels.is_empty() || additional_labels.is_some() { - self.writer.write_str(",")?; + /// Writer impl which prepends a comma on the first call to write output to the wrapped writer + struct CommaPrependingWriter<'a> { + writer: &'a mut dyn Write, + should_prepend: bool, + } + + impl Write for CommaPrependingWriter<'_> { + fn write_str(&mut self, s: &str) -> std::fmt::Result { + if self.should_prepend { + self.writer.write_char(',')?; + self.should_prepend = false; + } + self.writer.write_str(s) } + } - labels.encode(LabelSetEncoder::new(self.writer).into())?; + if let Some(labels) = self.family_labels { + // if const labels or additional labels have been written, a comma must be prepended before writing the family labels. + // However, it could be the case that the family labels are `Some` and yet empty, so the comma should _only_ + // be prepended if one of the `Write` methods are actually called when attempting to write the family labels. + // Therefore, wrap the writer on `Self` with a CommaPrependingWriter if other labels have been written and + // there may be a need to prepend an extra comma before writing additional labels. + if !self.const_labels.is_empty() || additional_labels.is_some() { + let mut writer = CommaPrependingWriter { + writer: self.writer, + should_prepend: true, + }; + labels.encode(LabelSetEncoder::new(&mut writer).into())?; + } else { + labels.encode(LabelSetEncoder::new(self.writer).into())?; + }; } self.writer.write_str("}")?; @@ -709,6 +734,7 @@ mod tests { use crate::metrics::{counter::Counter, exemplar::CounterWithExemplar}; use pyo3::{prelude::*, types::PyModule}; use std::borrow::Cow; + use std::fmt::Error; use std::sync::atomic::{AtomicI32, AtomicU32}; #[test] @@ -899,6 +925,31 @@ mod tests { parse_with_python_client(encoded); } + #[test] + fn encode_histogram_family_with_empty_struct_family_labels() { + let mut registry = Registry::default(); + let family = + Family::new_with_constructor(|| Histogram::new(exponential_buckets(1.0, 2.0, 10))); + registry.register("my_histogram", "My histogram", family.clone()); + + #[derive(Eq, PartialEq, Hash, Debug, Clone)] + struct EmptyLabels {} + + impl EncodeLabelSet for EmptyLabels { + fn encode(&self, _encoder: crate::encoding::LabelSetEncoder) -> Result<(), Error> { + Ok(()) + } + } + + family.get_or_create(&EmptyLabels {}).observe(1.0); + + let mut encoded = String::new(); + + encode(&mut encoded, ®istry).unwrap(); + + parse_with_python_client(encoded); + } + #[test] fn encode_histogram_with_exemplars() { let mut registry = Registry::default(); diff --git a/src/metrics/counter.rs b/src/metrics/counter.rs index d1fee30f..7016c361 100644 --- a/src/metrics/counter.rs +++ b/src/metrics/counter.rs @@ -2,7 +2,7 @@ //! //! See [`Counter`] for details. -use crate::encoding::{EncodeMetric, MetricEncoder}; +use crate::encoding::{EncodeMetric, MetricEncoder, NoLabelSet}; use super::{MetricType, TypedMetric}; use std::marker::PhantomData; @@ -204,7 +204,7 @@ where A: Atomic, { fn encode(&self, mut encoder: MetricEncoder) -> Result<(), std::fmt::Error> { - encoder.encode_counter::<(), _, u64>(&self.get(), None) + encoder.encode_counter::(&self.get(), None) } fn metric_type(&self) -> MetricType { @@ -236,7 +236,7 @@ where N: crate::encoding::EncodeCounterValue, { fn encode(&self, mut encoder: MetricEncoder) -> Result<(), std::fmt::Error> { - encoder.encode_counter::<(), _, u64>(&self.value, None) + encoder.encode_counter::(&self.value, None) } fn metric_type(&self) -> MetricType { diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index 66f4496d..1e418ad9 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -2,7 +2,7 @@ //! //! See [`Histogram`] for details. -use crate::encoding::{EncodeMetric, MetricEncoder}; +use crate::encoding::{EncodeMetric, MetricEncoder, NoLabelSet}; use super::{MetricType, TypedMetric}; use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard}; @@ -133,7 +133,7 @@ pub fn linear_buckets(start: f64, width: f64, length: u16) -> impl Iterator Result<(), std::fmt::Error> { let (sum, count, buckets) = self.get(); - encoder.encode_histogram::<()>(sum, count, &buckets, None) + encoder.encode_histogram::(sum, count, &buckets, None) } fn metric_type(&self) -> MetricType {