From 1ef219b50d91fe6b32bc34e86d70a23d84c31a93 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 15:56:44 -0400 Subject: [PATCH 1/8] fix: enable exponential histograms --- .../src/custom_aggregation_selector.rs | 29 +++++++++++++++++++ crates/metrics/src/lib.rs | 5 +++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 crates/metrics/src/custom_aggregation_selector.rs diff --git a/crates/metrics/src/custom_aggregation_selector.rs b/crates/metrics/src/custom_aggregation_selector.rs new file mode 100644 index 0000000..2e5f906 --- /dev/null +++ b/crates/metrics/src/custom_aggregation_selector.rs @@ -0,0 +1,29 @@ +use opentelemetry_sdk::metrics::{ + reader::{AggregationSelector, DefaultAggregationSelector}, + Aggregation, InstrumentKind, +}; + +#[derive(Clone, Default, Debug)] +pub struct CustomAggregationSelector { + default_aggregation_selector: DefaultAggregationSelector, +} + +impl CustomAggregationSelector { + /// Create a new default aggregation selector. + pub fn new() -> Self { + Self::default() + } +} + +impl AggregationSelector for CustomAggregationSelector { + fn aggregation(&self, kind: InstrumentKind) -> Aggregation { + match kind { + InstrumentKind::Histogram => Aggregation::Base2ExponentialHistogram { + max_size: 160, + max_scale: 20, + record_min_max: true, + }, + x => self.default_aggregation_selector.aggregation(x), + } + } +} diff --git a/crates/metrics/src/lib.rs b/crates/metrics/src/lib.rs index e28ee58..da08ff7 100644 --- a/crates/metrics/src/lib.rs +++ b/crates/metrics/src/lib.rs @@ -1,5 +1,5 @@ -pub use {future::*, once_cell::sync::Lazy, opentelemetry as otel, task::*}; use { + custom_aggregation_selector::CustomAggregationSelector, opentelemetry_sdk::metrics::SdkMeterProvider, otel::metrics::{Meter, MeterProvider}, prometheus::{Error as PrometheusError, Registry, TextEncoder}, @@ -8,7 +8,9 @@ use { time::Duration, }, }; +pub use {future::*, once_cell::sync::Lazy, opentelemetry as otel, task::*}; +pub mod custom_aggregation_selector; pub mod future; pub mod macros; pub mod task; @@ -22,6 +24,7 @@ static METRICS_CORE: Lazy> = Lazy::new(|| { let registry = Registry::new(); let prometheus_exporter = opentelemetry_prometheus::exporter() + .with_aggregation_selector(CustomAggregationSelector::new()) .with_registry(registry.clone()) .build() .unwrap(); From 292436e1d8bb5882faa8e0c592498b582483f3d0 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 15:59:34 -0400 Subject: [PATCH 2/8] chore: comment --- crates/metrics/src/custom_aggregation_selector.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/metrics/src/custom_aggregation_selector.rs b/crates/metrics/src/custom_aggregation_selector.rs index 2e5f906..efbf749 100644 --- a/crates/metrics/src/custom_aggregation_selector.rs +++ b/crates/metrics/src/custom_aggregation_selector.rs @@ -19,8 +19,8 @@ impl AggregationSelector for CustomAggregationSelector { fn aggregation(&self, kind: InstrumentKind) -> Aggregation { match kind { InstrumentKind::Histogram => Aggregation::Base2ExponentialHistogram { - max_size: 160, - max_scale: 20, + max_size: 160, // default from https://opentelemetry.io/blog/2022/exponential-histograms/#why-use-exponential-bucket-histograms + max_scale: 20, // maxumum https://github.com/open-telemetry/opentelemetry-rust/blob/2286378632d498ce4b2da109e5aa131b34ae1a8f/opentelemetry-sdk/src/metrics/aggregation.rs#L75 record_min_max: true, }, x => self.default_aggregation_selector.aggregation(x), From d95dc05c3296a009d9b26cf4725cd219b37768bd Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 16:04:19 -0400 Subject: [PATCH 3/8] fix: fmt --- crates/metrics/src/custom_aggregation_selector.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/crates/metrics/src/custom_aggregation_selector.rs b/crates/metrics/src/custom_aggregation_selector.rs index efbf749..49d873a 100644 --- a/crates/metrics/src/custom_aggregation_selector.rs +++ b/crates/metrics/src/custom_aggregation_selector.rs @@ -1,6 +1,7 @@ use opentelemetry_sdk::metrics::{ reader::{AggregationSelector, DefaultAggregationSelector}, - Aggregation, InstrumentKind, + Aggregation, + InstrumentKind, }; #[derive(Clone, Default, Debug)] @@ -19,8 +20,10 @@ impl AggregationSelector for CustomAggregationSelector { fn aggregation(&self, kind: InstrumentKind) -> Aggregation { match kind { InstrumentKind::Histogram => Aggregation::Base2ExponentialHistogram { - max_size: 160, // default from https://opentelemetry.io/blog/2022/exponential-histograms/#why-use-exponential-bucket-histograms - max_scale: 20, // maxumum https://github.com/open-telemetry/opentelemetry-rust/blob/2286378632d498ce4b2da109e5aa131b34ae1a8f/opentelemetry-sdk/src/metrics/aggregation.rs#L75 + // default from https://opentelemetry.io/blog/2022/exponential-histograms/#why-use-exponential-bucket-histograms + max_size: 160, + // maxumum https://github.com/open-telemetry/opentelemetry-rust/blob/2286378632d498ce4b2da109e5aa131b34ae1a8f/opentelemetry-sdk/src/metrics/aggregation.rs#L75 + max_scale: 20, record_min_max: true, }, x => self.default_aggregation_selector.aggregation(x), From c63aaaafe070f8c10647d80fd534eafcef4ab95b Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 18:37:49 -0400 Subject: [PATCH 4/8] fix: use view instead --- crates/metrics/src/lib.rs | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/crates/metrics/src/lib.rs b/crates/metrics/src/lib.rs index da08ff7..a214bda 100644 --- a/crates/metrics/src/lib.rs +++ b/crates/metrics/src/lib.rs @@ -1,6 +1,13 @@ +pub use {future::*, once_cell::sync::Lazy, opentelemetry as otel, task::*}; use { - custom_aggregation_selector::CustomAggregationSelector, - opentelemetry_sdk::metrics::SdkMeterProvider, + opentelemetry_sdk::metrics::{ + new_view, + Aggregation, + Instrument, + InstrumentKind, + SdkMeterProvider, + Stream, + }, otel::metrics::{Meter, MeterProvider}, prometheus::{Error as PrometheusError, Registry, TextEncoder}, std::{ @@ -8,7 +15,6 @@ use { time::Duration, }, }; -pub use {future::*, once_cell::sync::Lazy, opentelemetry as otel, task::*}; pub mod custom_aggregation_selector; pub mod future; @@ -24,12 +30,27 @@ static METRICS_CORE: Lazy> = Lazy::new(|| { let registry = Registry::new(); let prometheus_exporter = opentelemetry_prometheus::exporter() - .with_aggregation_selector(CustomAggregationSelector::new()) + // .with_aggregation_selector(CustomAggregationSelector::new()) .with_registry(registry.clone()) .build() .unwrap(); let provider = SdkMeterProvider::builder() .with_reader(prometheus_exporter) + .with_view( + new_view( + { + let mut instrument = Instrument::new(); + instrument.kind = Some(InstrumentKind::Histogram); + instrument + }, + Stream::new().aggregation(Aggregation::Base2ExponentialHistogram { + max_size: 160, + max_scale: 20, + record_min_max: true, + }), + ) + .unwrap(), + ) .build(); let meter = provider.meter(service_name); From 90bf2d320bad0f2f6cc037ec0f0989e90863b33f Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 20:34:06 -0400 Subject: [PATCH 5/8] fix: allow customizing builder --- .../src/custom_aggregation_selector.rs | 32 ---------- crates/metrics/src/lib.rs | 58 ++++++++++--------- 2 files changed, 30 insertions(+), 60 deletions(-) delete mode 100644 crates/metrics/src/custom_aggregation_selector.rs diff --git a/crates/metrics/src/custom_aggregation_selector.rs b/crates/metrics/src/custom_aggregation_selector.rs deleted file mode 100644 index 49d873a..0000000 --- a/crates/metrics/src/custom_aggregation_selector.rs +++ /dev/null @@ -1,32 +0,0 @@ -use opentelemetry_sdk::metrics::{ - reader::{AggregationSelector, DefaultAggregationSelector}, - Aggregation, - InstrumentKind, -}; - -#[derive(Clone, Default, Debug)] -pub struct CustomAggregationSelector { - default_aggregation_selector: DefaultAggregationSelector, -} - -impl CustomAggregationSelector { - /// Create a new default aggregation selector. - pub fn new() -> Self { - Self::default() - } -} - -impl AggregationSelector for CustomAggregationSelector { - fn aggregation(&self, kind: InstrumentKind) -> Aggregation { - match kind { - InstrumentKind::Histogram => Aggregation::Base2ExponentialHistogram { - // default from https://opentelemetry.io/blog/2022/exponential-histograms/#why-use-exponential-bucket-histograms - max_size: 160, - // maxumum https://github.com/open-telemetry/opentelemetry-rust/blob/2286378632d498ce4b2da109e5aa131b34ae1a8f/opentelemetry-sdk/src/metrics/aggregation.rs#L75 - max_scale: 20, - record_min_max: true, - }, - x => self.default_aggregation_selector.aggregation(x), - } - } -} diff --git a/crates/metrics/src/lib.rs b/crates/metrics/src/lib.rs index a214bda..7d6e3ef 100644 --- a/crates/metrics/src/lib.rs +++ b/crates/metrics/src/lib.rs @@ -1,14 +1,10 @@ pub use {future::*, once_cell::sync::Lazy, opentelemetry as otel, task::*}; use { - opentelemetry_sdk::metrics::{ - new_view, - Aggregation, - Instrument, - InstrumentKind, - SdkMeterProvider, - Stream, + opentelemetry_sdk::metrics::{MeterProviderBuilder, SdkMeterProvider}, + otel::{ + global, + metrics::{Meter, MeterProvider}, }, - otel::metrics::{Meter, MeterProvider}, prometheus::{Error as PrometheusError, Registry, TextEncoder}, std::{ sync::{Arc, Mutex}, @@ -16,7 +12,6 @@ use { }, }; -pub mod custom_aggregation_selector; pub mod future; pub mod macros; pub mod task; @@ -25,35 +20,27 @@ const DEFAULT_SERVICE_NAME: &str = "unknown_service"; static SERVICE_NAME: Mutex> = Mutex::new(None); +type MeterProviderBuilderIntercept = fn(MeterProviderBuilder) -> MeterProviderBuilder; +static METER_PROVIDER_BUILDER: Mutex> = Mutex::new(None); + static METRICS_CORE: Lazy> = Lazy::new(|| { let service_name = SERVICE_NAME.lock().unwrap().unwrap_or(DEFAULT_SERVICE_NAME); + let meter_provider_builder = *METER_PROVIDER_BUILDER.lock().unwrap(); let registry = Registry::new(); let prometheus_exporter = opentelemetry_prometheus::exporter() - // .with_aggregation_selector(CustomAggregationSelector::new()) .with_registry(registry.clone()) .build() .unwrap(); - let provider = SdkMeterProvider::builder() - .with_reader(prometheus_exporter) - .with_view( - new_view( - { - let mut instrument = Instrument::new(); - instrument.kind = Some(InstrumentKind::Histogram); - instrument - }, - Stream::new().aggregation(Aggregation::Base2ExponentialHistogram { - max_size: 160, - max_scale: 20, - record_min_max: true, - }), - ) - .unwrap(), - ) - .build(); + let mut builder = SdkMeterProvider::builder().with_reader(prometheus_exporter); + if let Some(with_view) = meter_provider_builder { + builder = with_view(builder); + }; + let provider = builder.build(); let meter = provider.meter(service_name); + global::set_meter_provider(provider); + Arc::new(ServiceMetrics { registry, meter }) }); @@ -88,6 +75,21 @@ impl ServiceMetrics { Lazy::force(&METRICS_CORE); } + /// Initializes service metrics with the specified name. + /// + /// # Panics + /// + /// Panics if either prometheus exporter or opentelemetry meter fails to + /// initialize. + pub fn init_with_name_and_meter_provider_builder( + name: &'static str, + meter_provider_builder: MeterProviderBuilderIntercept, + ) { + *SERVICE_NAME.lock().unwrap() = Some(name); + *METER_PROVIDER_BUILDER.lock().unwrap() = Some(meter_provider_builder); + Lazy::force(&METRICS_CORE); + } + /// Generates export data in Prometheus format, serialized into string. pub fn export() -> Result { let data = Self::get().registry.gather(); From 1b8052df43005390cb2f4f567dc6bdb0d77feffa Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 20:34:58 -0400 Subject: [PATCH 6/8] fix: types and comment --- crates/metrics/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/metrics/src/lib.rs b/crates/metrics/src/lib.rs index 7d6e3ef..2f48ec2 100644 --- a/crates/metrics/src/lib.rs +++ b/crates/metrics/src/lib.rs @@ -20,7 +20,7 @@ const DEFAULT_SERVICE_NAME: &str = "unknown_service"; static SERVICE_NAME: Mutex> = Mutex::new(None); -type MeterProviderBuilderIntercept = fn(MeterProviderBuilder) -> MeterProviderBuilder; +pub type MeterProviderBuilderIntercept = fn(MeterProviderBuilder) -> MeterProviderBuilder; static METER_PROVIDER_BUILDER: Mutex> = Mutex::new(None); static METRICS_CORE: Lazy> = Lazy::new(|| { @@ -75,7 +75,7 @@ impl ServiceMetrics { Lazy::force(&METRICS_CORE); } - /// Initializes service metrics with the specified name. + /// Initializes service metrics with the specified name and meter provider builder. /// /// # Panics /// From 89ffe4b4e7777e1cda110fb49648364be26bb1cf Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Tue, 16 Apr 2024 21:01:05 -0400 Subject: [PATCH 7/8] fix: expose otel_sdk --- crates/metrics/src/lib.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/metrics/src/lib.rs b/crates/metrics/src/lib.rs index 2f48ec2..d899564 100644 --- a/crates/metrics/src/lib.rs +++ b/crates/metrics/src/lib.rs @@ -1,4 +1,10 @@ -pub use {future::*, once_cell::sync::Lazy, opentelemetry as otel, task::*}; +pub use { + future::*, + once_cell::sync::Lazy, + opentelemetry as otel, + opentelemetry_sdk as otel_sdk, + task::*, +}; use { opentelemetry_sdk::metrics::{MeterProviderBuilder, SdkMeterProvider}, otel::{ @@ -75,7 +81,8 @@ impl ServiceMetrics { Lazy::force(&METRICS_CORE); } - /// Initializes service metrics with the specified name and meter provider builder. + /// Initializes service metrics with the specified name and meter provider + /// builder. /// /// # Panics /// From bd56a395154758f1082d304b671daba926eae595 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Wed, 17 Apr 2024 12:40:42 -0400 Subject: [PATCH 8/8] chore: renames --- crates/metrics/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/metrics/src/lib.rs b/crates/metrics/src/lib.rs index d899564..154fdc6 100644 --- a/crates/metrics/src/lib.rs +++ b/crates/metrics/src/lib.rs @@ -26,12 +26,12 @@ const DEFAULT_SERVICE_NAME: &str = "unknown_service"; static SERVICE_NAME: Mutex> = Mutex::new(None); -pub type MeterProviderBuilderIntercept = fn(MeterProviderBuilder) -> MeterProviderBuilder; -static METER_PROVIDER_BUILDER: Mutex> = Mutex::new(None); +pub type MeterProviderBuilderFn = fn(MeterProviderBuilder) -> MeterProviderBuilder; +static METER_PROVIDER_BUILDER_FN: Mutex> = Mutex::new(None); static METRICS_CORE: Lazy> = Lazy::new(|| { let service_name = SERVICE_NAME.lock().unwrap().unwrap_or(DEFAULT_SERVICE_NAME); - let meter_provider_builder = *METER_PROVIDER_BUILDER.lock().unwrap(); + let meter_provider_builder = *METER_PROVIDER_BUILDER_FN.lock().unwrap(); let registry = Registry::new(); let prometheus_exporter = opentelemetry_prometheus::exporter() @@ -39,8 +39,8 @@ static METRICS_CORE: Lazy> = Lazy::new(|| { .build() .unwrap(); let mut builder = SdkMeterProvider::builder().with_reader(prometheus_exporter); - if let Some(with_view) = meter_provider_builder { - builder = with_view(builder); + if let Some(buidler_fn) = meter_provider_builder { + builder = buidler_fn(builder); }; let provider = builder.build(); let meter = provider.meter(service_name); @@ -88,12 +88,12 @@ impl ServiceMetrics { /// /// Panics if either prometheus exporter or opentelemetry meter fails to /// initialize. - pub fn init_with_name_and_meter_provider_builder( + pub fn init_with_meter_builder( name: &'static str, - meter_provider_builder: MeterProviderBuilderIntercept, + meter_provider_builder: MeterProviderBuilderFn, ) { *SERVICE_NAME.lock().unwrap() = Some(name); - *METER_PROVIDER_BUILDER.lock().unwrap() = Some(meter_provider_builder); + *METER_PROVIDER_BUILDER_FN.lock().unwrap() = Some(meter_provider_builder); Lazy::force(&METRICS_CORE); }