[Metrics-API] Update asynchronous instruments to not allow calling observe outside of a callback (#2210)

Co-authored-by: Cijo Thomas <[email protected]>

Author: utpilla

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<T>(
@@ -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<T>(
@@ -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<T>(

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<T> SyncInstrument<T> 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<T> AsyncInstrument<T> for NoopAsyncInstrument {
-    fn observe(&self, _value: T, _attributes: &[KeyValue]) {
-        // Ignored
-    }
-}

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

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;
 
@@ -33,12 +33,17 @@ impl<T> Counter<T> {
 /// An async instrument that records increasing values.
 #[derive(Clone)]
 #[non_exhaustive]
-pub struct ObservableCounter<T>(Arc<dyn AsyncInstrument<T>>);
+pub struct ObservableCounter<T> {
+    _marker: std::marker::PhantomData<T>,
+}
 
 impl<T> ObservableCounter<T> {
     /// Create a new observable counter.
-    pub fn new(inner: Arc<dyn AsyncInstrument<T>>) -> Self {
-        ObservableCounter(inner)
+    #[allow(clippy::new_without_default)]
+    pub fn new() -> Self {
+        ObservableCounter {
+            _marker: std::marker::PhantomData,
+        }
     }
 }
 
@@ -50,9 +55,3 @@ impl<T> fmt::Debug for ObservableCounter<T> {
         ))
     }
 }
-
-impl<T> AsyncInstrument<T> for ObservableCounter<T> {
-    fn observe(&self, measurement: T, attributes: &[KeyValue]) {
-        self.0.observe(measurement, attributes)
-    }
-}

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;
 
@@ -33,7 +33,9 @@ impl<T> Gauge<T> {
 /// An async instrument that records independent readings.
 #[derive(Clone)]
 #[non_exhaustive]
-pub struct ObservableGauge<T>(Arc<dyn AsyncInstrument<T>>);
+pub struct ObservableGauge<T> {
+    _marker: std::marker::PhantomData<T>,
+}
 
 impl<T> fmt::Debug for ObservableGauge<T>
 where
@@ -47,15 +49,12 @@ where
     }
 }
 
-impl<M> AsyncInstrument<M> for ObservableGauge<M> {
-    fn observe(&self, measurement: M, attributes: &[KeyValue]) {
-        self.0.observe(measurement, attributes)
-    }
-}
-
 impl<T> ObservableGauge<T> {
     /// Create a new gauge
-    pub fn new(inner: Arc<dyn AsyncInstrument<T>>) -> Self {
-        ObservableGauge(inner)
+    #[allow(clippy::new_without_default)]
+    pub fn new() -> Self {
+        ObservableGauge {
+            _marker: std::marker::PhantomData,
+        }
     }
 }

opentelemetry/src/metrics/instruments/mod.rs

@@ -241,10 +241,7 @@ pub type Callback<T> = Box<dyn Fn(&dyn AsyncInstrument<T>) + 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<M>,
-{
+pub struct AsyncInstrumentBuilder<'a, I, M> {
     /// Instrument provider is used to create the instrument.
     pub instrument_provider: &'a dyn InstrumentProvider,
 
@@ -263,10 +260,7 @@ where
     _inst: marker::PhantomData<I>,
 }
 
-impl<'a, I, M> AsyncInstrumentBuilder<'a, I, M>
-where
-    I: AsyncInstrument<M>,
-{
+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 {

opentelemetry/src/metrics/instruments/up_down_counter.rs

@@ -2,7 +2,7 @@ use crate::KeyValue;
 use core::fmt;
 use std::sync::Arc;
 
-use super::{AsyncInstrument, SyncInstrument};
+use super::SyncInstrument;
 
 /// An instrument that records increasing or decreasing values.
 #[derive(Clone)]
@@ -36,7 +36,9 @@ impl<T> UpDownCounter<T> {
 /// An async instrument that records increasing or decreasing values.
 #[derive(Clone)]
 #[non_exhaustive]
-pub struct ObservableUpDownCounter<T>(Arc<dyn AsyncInstrument<T>>);
+pub struct ObservableUpDownCounter<T> {
+    _marker: std::marker::PhantomData<T>,
+}
 
 impl<T> fmt::Debug for ObservableUpDownCounter<T>
 where
@@ -52,13 +54,10 @@ where
 
 impl<T> ObservableUpDownCounter<T> {
     /// Create a new observable up down counter.
-    pub fn new(inner: Arc<dyn AsyncInstrument<T>>) -> Self {
-        ObservableUpDownCounter(inner)
-    }
-}
-
-impl<T> AsyncInstrument<T> for ObservableUpDownCounter<T> {
-    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,
+        }
     }
 }

opentelemetry/src/metrics/mod.rs

@@ -123,19 +123,15 @@ pub trait InstrumentProvider {
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableCounter<u64>, u64>,
     ) -> Result<ObservableCounter<u64>> {
-        Ok(ObservableCounter::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableCounter::new())
     }
 
     /// creates an instrument for recording increasing values via callback.
     fn f64_observable_counter(
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableCounter<f64>, f64>,
     ) -> Result<ObservableCounter<f64>> {
-        Ok(ObservableCounter::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableCounter::new())
     }
 
     /// creates an instrument for recording changes of a value.
@@ -163,19 +159,15 @@ pub trait InstrumentProvider {
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableUpDownCounter<i64>, i64>,
     ) -> Result<ObservableUpDownCounter<i64>> {
-        Ok(ObservableUpDownCounter::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableUpDownCounter::new())
     }
 
     /// creates an instrument for recording changes of a value via callback.
     fn f64_observable_up_down_counter(
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableUpDownCounter<f64>, f64>,
     ) -> Result<ObservableUpDownCounter<f64>> {
-        Ok(ObservableUpDownCounter::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableUpDownCounter::new())
     }
 
     /// creates an instrument for recording independent values.
@@ -198,29 +190,23 @@ pub trait InstrumentProvider {
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableGauge<u64>, u64>,
     ) -> Result<ObservableGauge<u64>> {
-        Ok(ObservableGauge::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableGauge::new())
     }
 
     /// creates an instrument for recording the current value via callback.
     fn i64_observable_gauge(
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableGauge<i64>, i64>,
     ) -> Result<ObservableGauge<i64>> {
-        Ok(ObservableGauge::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableGauge::new())
     }
 
     /// creates an instrument for recording the current value via callback.
     fn f64_observable_gauge(
         &self,
         _builder: AsyncInstrumentBuilder<'_, ObservableGauge<f64>, f64>,
     ) -> Result<ObservableGauge<f64>> {
-        Ok(ObservableGauge::new(Arc::new(
-            noop::NoopAsyncInstrument::new(),
-        )))
+        Ok(ObservableGauge::new())
     }
 
     /// creates an instrument for recording a distribution of values.

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<T> SyncInstrument<T> 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<T> AsyncInstrument<T> for NoopAsyncInstrument {
-    fn observe(&self, _value: T, _attributes: &[KeyValue]) {
-        // Ignored
-    }
-}