From 4881cd50b51a563764e35502c0fd68a56ca3360b Mon Sep 17 00:00:00 2001 From: Sidhant Kohli Date: Fri, 11 Oct 2024 15:21:35 -0700 Subject: [PATCH 1/3] feat: add range based exponential buckets in histogram Signed-off-by: Sidhant Kohli --- src/metrics/histogram.rs | 29 +++++++++++++++++++++++++++++ src/registry.rs | 2 +- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index 1e418ad9..78206345 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -122,6 +122,27 @@ pub fn exponential_buckets(start: f64, factor: f64, length: u16) -> impl Iterato .take(length.into()) } +/// Exponential bucket distribution within a range +/// /// Creates `length` buckets, where the lowest bucket is `min` and the highest bucket is `max`. +/// /// The final +Inf bucket is not counted and not included in the returned iterator. +/// /// The function panics if `length` is 0 or negative, or if `min` is 0 or negative. +fn exponential_buckets_range(min: f64, max: f64, length: u16) -> impl Iterator { + if length < 1 { + panic!("ExponentialBucketsRange length needs a positive length"); + } + if min <= 0.0 { + panic!("ExponentialBucketsRange min needs to be greater than 0"); + } + + // We know max/min and highest bucket. Solve for growth_factor. + let growth_factor = (max / min).powf(1.0 / (length as f64 - 1.0)); + + iter::repeat(()) + .enumerate() + .map(move |(i, _)| min * growth_factor.powf(i as f64)) + .take(length.into()) +} + /// Linear bucket distribution. pub fn linear_buckets(start: f64, width: f64, length: u16) -> impl Iterator { iter::repeat(()) @@ -166,4 +187,12 @@ mod tests { linear_buckets(0.0, 1.0, 10).collect::>() ); } + + #[test] + fn exponential_range() { + assert_eq!( + vec![1.0, 2.0, 4.0, 8.0, 16.0, 32.0], + exponential_buckets_range(1.0, 32.0, 6).collect::>() + ); + } } diff --git a/src/registry.rs b/src/registry.rs index c7dec3ba..53aab234 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -234,7 +234,7 @@ impl Registry { /// let subsystem_b_counter_1: Counter = Counter::default(); /// let subsystem_b_counter_2: Counter = Counter::default(); /// - /// let subsystem_a_registry = registry.sub_registry_with_prefix("subsystem_b"); + /// let subsystem_b_registry = registry.sub_registry_with_prefix("subsystem_b"); /// registry.register("counter_1", "", subsystem_b_counter_1.clone()); /// registry.register("counter_2", "", subsystem_b_counter_2.clone()); /// ``` From fdd98603504518c88c829f745780a5595310e651 Mon Sep 17 00:00:00 2001 From: Sidhant Kohli Date: Tue, 29 Oct 2024 10:16:24 -0400 Subject: [PATCH 2/3] chore: comments Signed-off-by: Sidhant Kohli --- Cargo.toml | 1 + src/metrics/histogram.rs | 25 ++++++++++++++++--------- src/registry.rs | 2 +- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5514ab68..5fae8d0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ parking_lot = "0.12" prometheus-client-derive-encode = { version = "0.4.1", path = "derive-encode" } prost = { version = "0.12.0", optional = true } prost-types = { version = "0.12.0", optional = true } +log = "0.4.22" [dev-dependencies] async-std = { version = "1", features = ["attributes"] } diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index 78206345..c0875852 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -5,6 +5,7 @@ use crate::encoding::{EncodeMetric, MetricEncoder, NoLabelSet}; use super::{MetricType, TypedMetric}; +use log::warn; use parking_lot::{MappedRwLockReadGuard, RwLock, RwLockReadGuard}; use std::iter::{self, once}; use std::sync::Arc; @@ -123,24 +124,30 @@ pub fn exponential_buckets(start: f64, factor: f64, length: u16) -> impl Iterato } /// Exponential bucket distribution within a range -/// /// Creates `length` buckets, where the lowest bucket is `min` and the highest bucket is `max`. -/// /// The final +Inf bucket is not counted and not included in the returned iterator. -/// /// The function panics if `length` is 0 or negative, or if `min` is 0 or negative. -fn exponential_buckets_range(min: f64, max: f64, length: u16) -> impl Iterator { +/// +/// Creates `length` buckets, where the lowest bucket is `min` and the highest bucket is `max`. +/// +/// The function defaults to `length` = 2 if `length` is 0 or negative, +/// and defaults to `min` = 1.0 if `min` is 0 or negative. +pub fn exponential_buckets_range(min: f64, max: f64, length: u16) -> impl Iterator { + let mut len_observed = length; + let mut min_bucket = min; if length < 1 { - panic!("ExponentialBucketsRange length needs a positive length"); + warn!("exponential_buckets_range length needs a positive length, defaulting to 1"); + len_observed = 1; } if min <= 0.0 { - panic!("ExponentialBucketsRange min needs to be greater than 0"); + warn!("exponential_buckets_range min needs to be greater than 0, defaulting to 1.0"); + min_bucket = 1.0; } // We know max/min and highest bucket. Solve for growth_factor. - let growth_factor = (max / min).powf(1.0 / (length as f64 - 1.0)); + let growth_factor = (max / min_bucket).powf(1.0 / (len_observed as f64 - 1.0)); iter::repeat(()) .enumerate() - .map(move |(i, _)| min * growth_factor.powf(i as f64)) - .take(length.into()) + .map(move |(i, _)| min_bucket * growth_factor.powf(i as f64)) + .take(len_observed.into()) } /// Linear bucket distribution. diff --git a/src/registry.rs b/src/registry.rs index 53aab234..c7dec3ba 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -234,7 +234,7 @@ impl Registry { /// let subsystem_b_counter_1: Counter = Counter::default(); /// let subsystem_b_counter_2: Counter = Counter::default(); /// - /// let subsystem_b_registry = registry.sub_registry_with_prefix("subsystem_b"); + /// let subsystem_a_registry = registry.sub_registry_with_prefix("subsystem_b"); /// registry.register("counter_1", "", subsystem_b_counter_1.clone()); /// registry.register("counter_2", "", subsystem_b_counter_2.clone()); /// ``` From 5107c4c15f257da7b38cc0939b9cb9e4bb9e0167 Mon Sep 17 00:00:00 2001 From: Sidhant Kohli Date: Tue, 29 Oct 2024 10:19:57 -0400 Subject: [PATCH 3/3] chore: comments Signed-off-by: Sidhant Kohli --- src/metrics/histogram.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index c0875852..e2bd0f88 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -127,7 +127,7 @@ pub fn exponential_buckets(start: f64, factor: f64, length: u16) -> impl Iterato /// /// Creates `length` buckets, where the lowest bucket is `min` and the highest bucket is `max`. /// -/// The function defaults to `length` = 2 if `length` is 0 or negative, +/// The function defaults to `length` = 1 if `length` is 0 or negative, /// and defaults to `min` = 1.0 if `min` is 0 or negative. pub fn exponential_buckets_range(min: f64, max: f64, length: u16) -> impl Iterator { let mut len_observed = length;