Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src/metrics/histogram: Use generic-array for buckets #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ keywords = ["openmetrics", "prometheus", "metrics", "instrumentation", "monitori
owning_ref = "0.4"
itoa = "0.4"
dtoa = "0.4"
generic-array = "0.14"

[dev-dependencies]
pyo3 = "0.13"
Expand Down
5 changes: 3 additions & 2 deletions benches/encoding/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use open_metrics_client::metrics::histogram::{exponential_series, Histogram};
use open_metrics_client::registry::Registry;
use std::io::Write;
use std::sync::atomic::AtomicU64;
use generic_array::typenum::U10;

pub fn text(c: &mut Criterion) {
c.bench_function("encode", |b| {
Expand Down Expand Up @@ -59,8 +60,8 @@ pub fn text(c: &mut Criterion) {

for i in 0..100 {
let counter_family = Family::<Labels, Counter<AtomicU64>>::default();
let histogram_family = Family::<Labels, Histogram>::new_with_constructor(|| {
Histogram::new(exponential_series(1.0, 2.0, 10))
let histogram_family = Family::<Labels, Histogram<U10>>::new_with_constructor(|| {
Histogram::new(exponential_series(1.0, 2.0))
});

registry.register(
Expand Down
6 changes: 4 additions & 2 deletions src/encoding/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ use crate::metrics::histogram::Histogram;
use crate::metrics::{MetricType, TypedMetric};
use crate::registry::{Registry, Unit};

use generic_array::ArrayLength;
use std::io::Write;
use std::ops::Deref;

Expand Down Expand Up @@ -360,7 +361,7 @@ where
}
}

impl EncodeMetric for Histogram {
impl<NumBuckets: ArrayLength<(f64, u64)>> EncodeMetric for Histogram<NumBuckets> {
fn encode(&self, mut encoder: Encoder) -> Result<(), std::io::Error> {
let (sum, count, buckets) = self.get();
encoder
Expand Down Expand Up @@ -395,6 +396,7 @@ mod tests {
use crate::metrics::histogram::exponential_series;
use pyo3::{prelude::*, types::PyModule};
use std::sync::atomic::AtomicU64;
use generic_array::typenum::U10;

#[test]
fn encode_counter() {
Expand Down Expand Up @@ -461,7 +463,7 @@ mod tests {
#[test]
fn encode_histogram() {
let mut registry = Registry::default();
let histogram = Histogram::new(exponential_series(1.0, 2.0, 10));
let histogram = Histogram::<U10>::new(exponential_series(1.0, 2.0));
registry.register("my_histogram", "My histogram", histogram.clone());
histogram.observe(1.0);

Expand Down
4 changes: 2 additions & 2 deletions src/metrics/family.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl<S: Clone + std::hash::Hash + Eq, M> Family<S, M> {
/// # use open_metrics_client::metrics::family::Family;
/// # use open_metrics_client::metrics::histogram::{exponential_series, Histogram};
/// Family::<Vec<(String, String)>, Histogram>::new_with_constructor(|| {
/// Histogram::new(exponential_series(1.0, 2.0, 10))
/// Histogram::new(exponential_series(1.0, 2.0))
/// });
/// ```
pub fn new_with_constructor(constructor: fn() -> M) -> Self {
Expand Down Expand Up @@ -245,7 +245,7 @@ mod tests {
#[test]
fn histogram_family() {
Family::<(), Histogram>::new_with_constructor(|| {
Histogram::new(exponential_series(1.0, 2.0, 10))
Histogram::new(exponential_series(1.0, 2.0))
});
}
}
47 changes: 22 additions & 25 deletions src/metrics/histogram.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,50 +3,48 @@
//! See [`Histogram`] for details.

use super::{MetricType, TypedMetric};
use generic_array::{ArrayLength, GenericArray};
use generic_array::sequence::GenericSequence;
use generic_array::typenum::U10;
use owning_ref::OwningRef;
use std::iter::once;
use std::sync::{Arc, Mutex, MutexGuard};

/// Open Metrics [`Histogram`] to measure distributions of discrete events.
///
/// ```
/// # use open_metrics_client::metrics::histogram::{Histogram, exponential_series};
/// let histogram = Histogram::new(exponential_series(1.0, 2.0, 10));
/// let histogram = Histogram::new(exponential_series(1.0, 2.0));
/// histogram.observe(4.2);
/// ```
// TODO: Consider using atomics. See
// https://github.com/tikv/rust-prometheus/pull/314.
pub struct Histogram {
inner: Arc<Mutex<Inner>>,
pub struct Histogram<NumBuckets: ArrayLength<(f64, u64)> = U10> {
inner: Arc<Mutex<Inner<NumBuckets>>>,
}

impl Clone for Histogram {
impl<NumBuckets: ArrayLength<(f64, u64)>> Clone for Histogram<NumBuckets> {
fn clone(&self) -> Self {
Histogram {
inner: self.inner.clone(),
}
}
}

pub(crate) struct Inner {
pub(crate) struct Inner<NumBuckets: ArrayLength<(f64, u64)>> {
// TODO: Consider allowing integer observe values.
sum: f64,
count: u64,
// TODO: Consider being generic over the bucket length.
buckets: Vec<(f64, u64)>,
buckets: GenericArray<(f64, u64), NumBuckets>,
}

impl Histogram {
pub fn new(buckets: impl Iterator<Item = f64>) -> Self {
impl<NumBuckets: ArrayLength<(f64, u64)>> Histogram<NumBuckets> {
pub fn new(buckets: GenericArray<(f64, u64), NumBuckets>) -> Self {
Self {
inner: Arc::new(Mutex::new(Inner {
sum: Default::default(),
count: Default::default(),
buckets: buckets
.into_iter()
.chain(once(f64::MAX))
.map(|upper_bound| (upper_bound, 0))
.collect(),
buckets,
})),
}
}
Expand All @@ -62,7 +60,7 @@ impl Histogram {
}
}

pub(crate) fn get(&self) -> (f64, u64, MutexGuardedBuckets) {
pub(crate) fn get(&self) -> (f64, u64, MutexGuardedBuckets<NumBuckets>) {
let inner = self.inner.lock().unwrap();
let sum = inner.sum;
let count = inner.count;
Expand All @@ -71,20 +69,19 @@ impl Histogram {
}
}

type MutexGuardedBuckets<'a> = OwningRef<MutexGuard<'a, Inner>, Vec<(f64, u64)>>;
type MutexGuardedBuckets<'a, NumBuckets> =
OwningRef<MutexGuard<'a, Inner<NumBuckets>>, GenericArray<(f64, u64), NumBuckets>>;

impl TypedMetric for Histogram {
impl<NumBuckets: ArrayLength<(f64, u64)>> TypedMetric for Histogram<NumBuckets> {
const TYPE: MetricType = MetricType::Histogram;
}

// TODO: consider renaming to exponential_buckets
pub fn exponential_series(start: f64, factor: f64, length: u16) -> impl Iterator<Item = f64> {
let mut current = start;
(0..length).map(move |_| {
let to_return = current;
current *= factor;
to_return
})
pub fn exponential_series<NumBuckets: ArrayLength<(f64, u64)>>(
start: f64,
factor: f64,
) -> GenericArray<(f64, u64), NumBuckets> {
GenericArray::generate(|i: usize| (start * factor.powf(i as f64), 0))
}

#[cfg(test)]
Expand All @@ -93,7 +90,7 @@ mod tests {

#[test]
fn histogram() {
let histogram = Histogram::new(exponential_series(1.0, 2.0, 10));
let histogram = Histogram::<U10>::new(exponential_series(1.0, 2.0));
histogram.observe(1.0);
}
}