From 55be424e6fcea85b8f472ad85d916585fe04b590 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 14 Oct 2024 02:38:45 +0000 Subject: [PATCH 1/3] Update asynchronous instruments --- opentelemetry-sdk/src/metrics/meter.rs | 24 +++++++--------- .../src/metrics/instruments/counter.rs | 19 ++++++------- .../src/metrics/instruments/gauge.rs | 19 ++++++------- opentelemetry/src/metrics/instruments/mod.rs | 10 ++----- .../metrics/instruments/up_down_counter.rs | 19 ++++++------- opentelemetry/src/metrics/mod.rs | 28 +++++-------------- 6 files changed, 45 insertions(+), 74 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/meter.rs b/opentelemetry-sdk/src/metrics/meter.rs index 26169f9616..0234df35b7 100644 --- a/opentelemetry-sdk/src/metrics/meter.rs +++ b/opentelemetry-sdk/src/metrics/meter.rs @@ -17,7 +17,7 @@ use crate::metrics::{ pipeline::{Pipelines, Resolver}, }; -use super::noop::{NoopAsyncInstrument, NoopSyncInstrument}; +use super::noop::NoopSyncInstrument; // maximum length of instrument name const INSTRUMENT_NAME_MAX_LENGTH: usize = 255; @@ -108,7 +108,7 @@ impl SdkMeter { let validation_result = validate_instrument_config(builder.name.as_ref(), &builder.unit); if let Err(err) = validation_result { global::handle_error(err); - return Ok(ObservableCounter::new(Arc::new(NoopAsyncInstrument::new()))); + return Ok(ObservableCounter::new()); } let ms = resolver.measures( @@ -120,7 +120,7 @@ impl SdkMeter { )?; if ms.is_empty() { - return Ok(ObservableCounter::new(Arc::new(NoopAsyncInstrument::new()))); + return Ok(ObservableCounter::new()); } let observable = Arc::new(Observable::new(ms)); @@ -131,7 +131,7 @@ impl SdkMeter { .register_callback(move || callback(cb_inst.as_ref())); } - Ok(ObservableCounter::new(observable)) + Ok(ObservableCounter::new()) } fn create_observable_updown_counter( @@ -145,9 +145,7 @@ impl SdkMeter { let validation_result = validate_instrument_config(builder.name.as_ref(), &builder.unit); if let Err(err) = validation_result { global::handle_error(err); - return Ok(ObservableUpDownCounter::new(Arc::new( - NoopAsyncInstrument::new(), - ))); + return Ok(ObservableUpDownCounter::new()); } let ms = resolver.measures( @@ -159,9 +157,7 @@ impl SdkMeter { )?; if ms.is_empty() { - return Ok(ObservableUpDownCounter::new(Arc::new( - NoopAsyncInstrument::new(), - ))); + return Ok(ObservableUpDownCounter::new()); } let observable = Arc::new(Observable::new(ms)); @@ -172,7 +168,7 @@ impl SdkMeter { .register_callback(move || callback(cb_inst.as_ref())); } - Ok(ObservableUpDownCounter::new(observable)) + Ok(ObservableUpDownCounter::new()) } fn create_observable_gauge( @@ -186,7 +182,7 @@ impl SdkMeter { let validation_result = validate_instrument_config(builder.name.as_ref(), &builder.unit); if let Err(err) = validation_result { global::handle_error(err); - return Ok(ObservableGauge::new(Arc::new(NoopAsyncInstrument::new()))); + return Ok(ObservableGauge::new()); } let ms = resolver.measures( @@ -198,7 +194,7 @@ impl SdkMeter { )?; if ms.is_empty() { - return Ok(ObservableGauge::new(Arc::new(NoopAsyncInstrument::new()))); + return Ok(ObservableGauge::new()); } let observable = Arc::new(Observable::new(ms)); @@ -209,7 +205,7 @@ impl SdkMeter { .register_callback(move || callback(cb_inst.as_ref())); } - Ok(ObservableGauge::new(observable)) + Ok(ObservableGauge::new()) } fn create_updown_counter( diff --git a/opentelemetry/src/metrics/instruments/counter.rs b/opentelemetry/src/metrics/instruments/counter.rs index a03618a0f8..d8f1e0e4c3 100644 --- a/opentelemetry/src/metrics/instruments/counter.rs +++ b/opentelemetry/src/metrics/instruments/counter.rs @@ -1,4 +1,4 @@ -use crate::{metrics::AsyncInstrument, KeyValue}; +use crate::KeyValue; use core::fmt; use std::sync::Arc; @@ -37,12 +37,17 @@ impl Counter { /// An async instrument that records increasing values. #[derive(Clone)] #[non_exhaustive] -pub struct ObservableCounter(Arc>); +pub struct ObservableCounter { + _marker: std::marker::PhantomData, +} impl ObservableCounter { /// Create a new observable counter. - pub fn new(inner: Arc>) -> Self { - ObservableCounter(inner) + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + ObservableCounter { + _marker: std::marker::PhantomData, + } } } @@ -54,9 +59,3 @@ impl fmt::Debug for ObservableCounter { )) } } - -impl AsyncInstrument for ObservableCounter { - fn observe(&self, measurement: T, attributes: &[KeyValue]) { - self.0.observe(measurement, attributes) - } -} diff --git a/opentelemetry/src/metrics/instruments/gauge.rs b/opentelemetry/src/metrics/instruments/gauge.rs index b1c14e1f04..a99f663526 100644 --- a/opentelemetry/src/metrics/instruments/gauge.rs +++ b/opentelemetry/src/metrics/instruments/gauge.rs @@ -1,4 +1,4 @@ -use crate::{metrics::AsyncInstrument, KeyValue}; +use crate::KeyValue; use core::fmt; use std::sync::Arc; @@ -37,7 +37,9 @@ impl Gauge { /// An async instrument that records independent readings. #[derive(Clone)] #[non_exhaustive] -pub struct ObservableGauge(Arc>); +pub struct ObservableGauge { + _marker: std::marker::PhantomData, +} impl fmt::Debug for ObservableGauge where @@ -51,15 +53,12 @@ where } } -impl AsyncInstrument for ObservableGauge { - fn observe(&self, measurement: M, attributes: &[KeyValue]) { - self.0.observe(measurement, attributes) - } -} - impl ObservableGauge { /// Create a new gauge - pub fn new(inner: Arc>) -> Self { - ObservableGauge(inner) + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + ObservableGauge { + _marker: std::marker::PhantomData, + } } } diff --git a/opentelemetry/src/metrics/instruments/mod.rs b/opentelemetry/src/metrics/instruments/mod.rs index cbe08c7719..8af4a8aa61 100644 --- a/opentelemetry/src/metrics/instruments/mod.rs +++ b/opentelemetry/src/metrics/instruments/mod.rs @@ -235,10 +235,7 @@ pub type Callback = Box) + Send + Sync>; /// Configuration for building an async instrument. #[non_exhaustive] // We expect to add more configuration fields in the future -pub struct AsyncInstrumentBuilder<'a, I, M> -where - I: AsyncInstrument, -{ +pub struct AsyncInstrumentBuilder<'a, I, M> { /// Instrument provider is used to create the instrument. pub instrument_provider: &'a dyn InstrumentProvider, @@ -257,10 +254,7 @@ where _inst: marker::PhantomData, } -impl<'a, I, M> AsyncInstrumentBuilder<'a, I, M> -where - I: AsyncInstrument, -{ +impl<'a, I, M> AsyncInstrumentBuilder<'a, I, M> { /// Create a new instrument builder pub(crate) fn new(meter: &'a Meter, name: Cow<'static, str>) -> Self { AsyncInstrumentBuilder { diff --git a/opentelemetry/src/metrics/instruments/up_down_counter.rs b/opentelemetry/src/metrics/instruments/up_down_counter.rs index 2fb3dcaaf0..e4346fa077 100644 --- a/opentelemetry/src/metrics/instruments/up_down_counter.rs +++ b/opentelemetry/src/metrics/instruments/up_down_counter.rs @@ -2,8 +2,6 @@ use crate::KeyValue; use core::fmt; use std::sync::Arc; -use super::AsyncInstrument; - /// An SDK implemented instrument that records increasing or decreasing values. pub trait SyncUpDownCounter { /// Records an increment or decrement to the counter. @@ -42,7 +40,9 @@ impl UpDownCounter { /// An async instrument that records increasing or decreasing values. #[derive(Clone)] #[non_exhaustive] -pub struct ObservableUpDownCounter(Arc>); +pub struct ObservableUpDownCounter { + _marker: std::marker::PhantomData, +} impl fmt::Debug for ObservableUpDownCounter where @@ -58,13 +58,10 @@ where impl ObservableUpDownCounter { /// Create a new observable up down counter. - pub fn new(inner: Arc>) -> Self { - ObservableUpDownCounter(inner) - } -} - -impl AsyncInstrument for ObservableUpDownCounter { - fn observe(&self, measurement: T, attributes: &[KeyValue]) { - self.0.observe(measurement, attributes) + #[allow(clippy::new_without_default)] + pub fn new() -> Self { + ObservableUpDownCounter { + _marker: std::marker::PhantomData, + } } } diff --git a/opentelemetry/src/metrics/mod.rs b/opentelemetry/src/metrics/mod.rs index 4b89765d41..7584e9cd0b 100644 --- a/opentelemetry/src/metrics/mod.rs +++ b/opentelemetry/src/metrics/mod.rs @@ -122,9 +122,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableCounter, u64>, ) -> Result> { - Ok(ObservableCounter::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableCounter::new()) } /// creates an instrument for recording increasing values via callback. @@ -132,9 +130,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableCounter, f64>, ) -> Result> { - Ok(ObservableCounter::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableCounter::new()) } /// creates an instrument for recording changes of a value. @@ -162,9 +158,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableUpDownCounter, i64>, ) -> Result> { - Ok(ObservableUpDownCounter::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableUpDownCounter::new()) } /// creates an instrument for recording changes of a value via callback. @@ -172,9 +166,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableUpDownCounter, f64>, ) -> Result> { - Ok(ObservableUpDownCounter::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableUpDownCounter::new()) } /// creates an instrument for recording independent values. @@ -197,9 +189,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableGauge, u64>, ) -> Result> { - Ok(ObservableGauge::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableGauge::new()) } /// creates an instrument for recording the current value via callback. @@ -207,9 +197,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableGauge, i64>, ) -> Result> { - Ok(ObservableGauge::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableGauge::new()) } /// creates an instrument for recording the current value via callback. @@ -217,9 +205,7 @@ pub trait InstrumentProvider { &self, _builder: AsyncInstrumentBuilder<'_, ObservableGauge, f64>, ) -> Result> { - Ok(ObservableGauge::new(Arc::new( - noop::NoopAsyncInstrument::new(), - ))) + Ok(ObservableGauge::new()) } /// creates an instrument for recording a distribution of values. From 62297c03ae7f615e52391290ab50442a90823116 Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 14 Oct 2024 03:32:57 +0000 Subject: [PATCH 2/3] Remove unsed code --- opentelemetry-sdk/src/metrics/noop.rs | 21 +-------------------- opentelemetry/src/metrics/noop.rs | 21 +-------------------- 2 files changed, 2 insertions(+), 40 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/noop.rs b/opentelemetry-sdk/src/metrics/noop.rs index 447c89432c..1a490698ad 100644 --- a/opentelemetry-sdk/src/metrics/noop.rs +++ b/opentelemetry-sdk/src/metrics/noop.rs @@ -1,5 +1,5 @@ use opentelemetry::{ - metrics::{AsyncInstrument, InstrumentProvider, SyncInstrument}, + metrics::{InstrumentProvider, SyncInstrument}, KeyValue, }; @@ -36,22 +36,3 @@ impl SyncInstrument for NoopSyncInstrument { // Ignored } } - -/// A no-op async instrument. -#[derive(Debug, Default)] -pub(crate) struct NoopAsyncInstrument { - _private: (), -} - -impl NoopAsyncInstrument { - /// Create a new no-op async instrument - pub(crate) fn new() -> Self { - NoopAsyncInstrument { _private: () } - } -} - -impl AsyncInstrument for NoopAsyncInstrument { - fn observe(&self, _value: T, _attributes: &[KeyValue]) { - // Ignored - } -} diff --git a/opentelemetry/src/metrics/noop.rs b/opentelemetry/src/metrics/noop.rs index 8e388641d2..dbe0726515 100644 --- a/opentelemetry/src/metrics/noop.rs +++ b/opentelemetry/src/metrics/noop.rs @@ -4,7 +4,7 @@ //! has been set. It is expected to have minimal resource utilization and //! runtime impact. use crate::{ - metrics::{AsyncInstrument, InstrumentProvider, Meter, MeterProvider}, + metrics::{InstrumentProvider, Meter, MeterProvider}, KeyValue, }; use std::sync::Arc; @@ -69,22 +69,3 @@ impl SyncInstrument for NoopSyncInstrument { // Ignored } } - -/// A no-op async instrument. -#[derive(Debug, Default)] -pub(crate) struct NoopAsyncInstrument { - _private: (), -} - -impl NoopAsyncInstrument { - /// Create a new no-op async instrument - pub(crate) fn new() -> Self { - NoopAsyncInstrument { _private: () } - } -} - -impl AsyncInstrument for NoopAsyncInstrument { - fn observe(&self, _value: T, _attributes: &[KeyValue]) { - // Ignored - } -} From aaea1026796b02e248fb264e37f83db8f42ef90c Mon Sep 17 00:00:00 2001 From: Utkarsh Umesan Pillai <66651184+utpilla@users.noreply.github.com> Date: Mon, 14 Oct 2024 23:38:57 +0000 Subject: [PATCH 3/3] Update CHANGLOG --- opentelemetry/CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md index 18b70af0c9..cbe3269d3b 100644 --- a/opentelemetry/CHANGELOG.md +++ b/opentelemetry/CHANGELOG.md @@ -4,8 +4,9 @@ - Bump MSRV to 1.70 [#2179](https://github.com/open-telemetry/opentelemetry-rust/pull/2179) - Add `LogRecord::set_trace_context`; an optional method conditional on the `trace` feature for setting trace context on a log record. -- Removed unnecessary public methods named `as_any` from `AsyncInstrument` trait and the implementing instruments: `ObservableCounter`, `ObservableGauge`, and `ObservableUpDownCounter` [#2187](https://github.com/open-telemetry/opentelemetry-rust/issues/2187) -- Introduced `SyncInstrument` trait to replace the individual synchronous instrument traits (`SyncCounter`, `SyncGauge`, `SyncHistogram`, `SyncUpDownCounter`) which are meant for SDK implementation. [#2207](https://github.com/open-telemetry/opentelemetry-rust/issues/2207) +- Removed unnecessary public methods named `as_any` from `AsyncInstrument` trait and the implementing instruments: `ObservableCounter`, `ObservableGauge`, and `ObservableUpDownCounter` [#2187](https://github.com/open-telemetry/opentelemetry-rust/pull/2187) +- Introduced `SyncInstrument` trait to replace the individual synchronous instrument traits (`SyncCounter`, `SyncGauge`, `SyncHistogram`, `SyncUpDownCounter`) which are meant for SDK implementation. [#2207](https://github.com/open-telemetry/opentelemetry-rust/pull/2207) +- Ensured that `observe` method on asynchronous instruments can only be called inside a callback. This was done by removing the implementation of `AsyncInstrument` trait for each of the asynchronous instruments. [#2210](https://github.com/open-telemetry/opentelemetry-rust/pull/2210) ## v0.26.0 Released 2024-Sep-30