From ea37871ff9c41c577a8690ea36f9b13636d7673d Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 11 Sep 2024 16:19:19 -0700 Subject: [PATCH 1/4] Remove MetricProducer --- examples/metrics-basic/src/main.rs | 103 +----------------- opentelemetry-sdk/CHANGELOG.md | 2 + .../src/metrics/manual_reader.rs | 39 +------ .../src/metrics/periodic_reader.rs | 32 +----- opentelemetry-sdk/src/metrics/reader.rs | 8 +- 5 files changed, 10 insertions(+), 174 deletions(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index ecd5083d7c..4a0fbdd01b 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -34,109 +34,8 @@ async fn main() -> Result<(), Box> { let counter = meter.u64_counter("my_counter").init(); // Record measurements using the Counter instrument. - counter.add( - 10, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ); + counter.add(10, &[]); - // Create a ObservableCounter instrument and register a callback that reports the measurement. - let _observable_counter = meter - .u64_observable_counter("my_observable_counter") - .with_description("My observable counter example description") - .with_unit("myunit") - .with_callback(|observer| { - observer.observe( - 100, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ) - }) - .init(); - - // Create a UpCounter Instrument. - let updown_counter = meter.i64_up_down_counter("my_updown_counter").init(); - - // Record measurements using the UpCounter instrument. - updown_counter.add( - -10, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ); - - // Create a Observable UpDownCounter instrument and register a callback that reports the measurement. - let _observable_up_down_counter = meter - .i64_observable_up_down_counter("my_observable_updown_counter") - .with_description("My observable updown counter example description") - .with_unit("myunit") - .with_callback(|observer| { - observer.observe( - 100, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ) - }) - .init(); - - // Create a Histogram Instrument. - let histogram = meter - .f64_histogram("my_histogram") - .with_description("My histogram example description") - .init(); - - // Record measurements using the histogram instrument. - histogram.record( - 10.5, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ); - - // Note that there is no ObservableHistogram instrument. - - // Create a Gauge Instrument. - let gauge = meter - .f64_gauge("my_gauge") - .with_description("A gauge set to 1.0") - .with_unit("myunit") - .init(); - - gauge.record( - 1.0, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ); - - // Create a ObservableGauge instrument and register a callback that reports the measurement. - let _observable_gauge = meter - .f64_observable_gauge("my_observable_gauge") - .with_description("An observable gauge set to 1.0") - .with_unit("myunit") - .with_callback(|observer| { - observer.observe( - 1.0, - &[ - KeyValue::new("mykey1", "myvalue1"), - KeyValue::new("mykey2", "myvalue2"), - ], - ) - }) - .init(); - - // Metrics are exported by default every 30 seconds when using stdout exporter, - // however shutting down the MeterProvider here instantly flushes - // the metrics, instead of waiting for the 30 sec interval. meter_provider.shutdown()?; Ok(()) } diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 0b5eab535b..628f8ae39d 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -3,6 +3,8 @@ ## vNext - Update `async-std` dependency version to 1.13 +- *Breaking* - Remove support for `MetricProducer` which allowed metrics from + external sources to be sent through OpenTelemetry. ## v0.25.0 diff --git a/opentelemetry-sdk/src/metrics/manual_reader.rs b/opentelemetry-sdk/src/metrics/manual_reader.rs index 0d1000ad35..1365396102 100644 --- a/opentelemetry-sdk/src/metrics/manual_reader.rs +++ b/opentelemetry-sdk/src/metrics/manual_reader.rs @@ -12,9 +12,7 @@ use super::{ data::{ResourceMetrics, Temporality}, instrument::InstrumentKind, pipeline::Pipeline, - reader::{ - DefaultTemporalitySelector, MetricProducer, MetricReader, SdkProducer, TemporalitySelector, - }, + reader::{DefaultTemporalitySelector, MetricReader, SdkProducer, TemporalitySelector}, }; /// A simple [MetricReader] that allows an application to read metrics on demand. @@ -51,7 +49,6 @@ impl fmt::Debug for ManualReader { struct ManualReaderInner { sdk_producer: Option>, is_shutdown: bool, - external_producers: Vec>, } impl ManualReader { @@ -61,15 +58,11 @@ impl ManualReader { } /// A [MetricReader] which is directly called to collect metrics. - pub(crate) fn new( - temporality_selector: Box, - producers: Vec>, - ) -> Self { + pub(crate) fn new(temporality_selector: Box) -> Self { ManualReader { inner: Box::new(Mutex::new(ManualReaderInner { sdk_producer: None, is_shutdown: false, - external_producers: producers, })), temporality_selector, } @@ -113,19 +106,7 @@ impl MetricReader for ManualReader { } }; - let mut errs = vec![]; - for producer in &inner.external_producers { - match producer.produce() { - Ok(metrics) => rm.scope_metrics.push(metrics), - Err(err) => errs.push(err), - } - } - - if errs.is_empty() { - Ok(()) - } else { - Err(MetricsError::Other(format!("{:?}", errs))) - } + Ok(()) } /// ForceFlush is a no-op, it always returns nil. @@ -140,7 +121,6 @@ impl MetricReader for ManualReader { // Any future call to collect will now return an error. inner.sdk_producer = None; inner.is_shutdown = true; - inner.external_producers = Vec::new(); Ok(()) } @@ -149,7 +129,6 @@ impl MetricReader for ManualReader { /// Configuration for a [ManualReader] pub struct ManualReaderBuilder { temporality_selector: Box, - producers: Vec>, } impl fmt::Debug for ManualReaderBuilder { @@ -162,7 +141,6 @@ impl Default for ManualReaderBuilder { fn default() -> Self { ManualReaderBuilder { temporality_selector: Box::new(DefaultTemporalitySelector { _private: () }), - producers: vec![], } } } @@ -184,17 +162,8 @@ impl ManualReaderBuilder { self } - /// Registers a an external [MetricProducer] with this reader. - /// - /// The producer is used as a source of aggregated metric data which is - /// incorporated into metrics collected from the SDK. - pub fn with_producer(mut self, producer: impl MetricProducer + 'static) -> Self { - self.producers.push(Box::new(producer)); - self - } - /// Create a new [ManualReader] from this configuration. pub fn build(self) -> ManualReader { - ManualReader::new(self.temporality_selector, self.producers) + ManualReader::new(self.temporality_selector) } } diff --git a/opentelemetry-sdk/src/metrics/periodic_reader.rs b/opentelemetry-sdk/src/metrics/periodic_reader.rs index b664f4014e..7821786675 100644 --- a/opentelemetry-sdk/src/metrics/periodic_reader.rs +++ b/opentelemetry-sdk/src/metrics/periodic_reader.rs @@ -18,10 +18,7 @@ use opentelemetry::{ use crate::runtime::Runtime; use crate::{ - metrics::{ - exporter::PushMetricsExporter, - reader::{MetricProducer, SdkProducer}, - }, + metrics::{exporter::PushMetricsExporter, reader::SdkProducer}, Resource, }; @@ -57,7 +54,6 @@ pub struct PeriodicReaderBuilder { interval: Duration, timeout: Duration, exporter: E, - producers: Vec>, runtime: RT, } @@ -79,7 +75,6 @@ where PeriodicReaderBuilder { interval, timeout, - producers: vec![], exporter, runtime, } @@ -114,15 +109,6 @@ where self } - /// Registers a an external [MetricProducer] with this reader. - /// - /// The producer is used as a source of aggregated metric data which is - /// incorporated into metrics collected from the SDK. - pub fn with_producer(mut self, producer: impl MetricProducer + 'static) -> Self { - self.producers.push(Box::new(producer)); - self - } - /// Create a [PeriodicReader] with the given config. pub fn build(self) -> PeriodicReader { let (message_sender, message_receiver) = mpsc::channel(256); @@ -156,7 +142,6 @@ where inner: Arc::new(Mutex::new(PeriodicReaderInner { message_sender, is_shutdown: false, - external_producers: self.producers, sdk_producer_or_worker: ProducerOrWorker::Worker(Box::new(worker)), })), } @@ -226,7 +211,6 @@ impl fmt::Debug for PeriodicReader { struct PeriodicReaderInner { message_sender: mpsc::Sender, is_shutdown: bool, - external_producers: Vec>, sdk_producer_or_worker: ProducerOrWorker, } @@ -342,19 +326,7 @@ impl MetricReader for PeriodicReader { return Err(MetricsError::Other("reader is not registered".into())); } - let mut errs = vec![]; - for producer in &inner.external_producers { - match producer.produce() { - Ok(metrics) => rm.scope_metrics.push(metrics), - Err(err) => errs.push(err), - } - } - - if errs.is_empty() { - Ok(()) - } else { - Err(MetricsError::Other(format!("{:?}", errs))) - } + Ok(()) } fn force_flush(&self) -> Result<()> { diff --git a/opentelemetry-sdk/src/metrics/reader.rs b/opentelemetry-sdk/src/metrics/reader.rs index ec54affe3d..0a6dbc1b4e 100644 --- a/opentelemetry-sdk/src/metrics/reader.rs +++ b/opentelemetry-sdk/src/metrics/reader.rs @@ -4,7 +4,7 @@ use std::{fmt, sync::Weak}; use opentelemetry::metrics::Result; use super::{ - data::{ResourceMetrics, ScopeMetrics, Temporality}, + data::{ResourceMetrics, Temporality}, instrument::InstrumentKind, pipeline::Pipeline, }; @@ -59,12 +59,6 @@ pub(crate) trait SdkProducer: fmt::Debug + Send + Sync { fn produce(&self, rm: &mut ResourceMetrics) -> Result<()>; } -/// Produces metrics for a [MetricReader] from an external source. -pub trait MetricProducer: fmt::Debug + Send + Sync { - /// Returns aggregated metrics from an external source. - fn produce(&self) -> Result; -} - /// An interface for selecting the temporality for an [InstrumentKind]. pub trait TemporalitySelector: Send + Sync { /// Selects the temporality to use based on the [InstrumentKind]. From 0356c0941dd567268360874d0c7dbe06a8a7fce6 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 11 Sep 2024 16:22:49 -0700 Subject: [PATCH 2/4] revert exampe --- examples/metrics-basic/src/main.rs | 103 ++++++++++++++++++++++++++++- 1 file changed, 102 insertions(+), 1 deletion(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index 4a0fbdd01b..ecd5083d7c 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -34,8 +34,109 @@ async fn main() -> Result<(), Box> { let counter = meter.u64_counter("my_counter").init(); // Record measurements using the Counter instrument. - counter.add(10, &[]); + counter.add( + 10, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ); + // Create a ObservableCounter instrument and register a callback that reports the measurement. + let _observable_counter = meter + .u64_observable_counter("my_observable_counter") + .with_description("My observable counter example description") + .with_unit("myunit") + .with_callback(|observer| { + observer.observe( + 100, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ) + }) + .init(); + + // Create a UpCounter Instrument. + let updown_counter = meter.i64_up_down_counter("my_updown_counter").init(); + + // Record measurements using the UpCounter instrument. + updown_counter.add( + -10, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ); + + // Create a Observable UpDownCounter instrument and register a callback that reports the measurement. + let _observable_up_down_counter = meter + .i64_observable_up_down_counter("my_observable_updown_counter") + .with_description("My observable updown counter example description") + .with_unit("myunit") + .with_callback(|observer| { + observer.observe( + 100, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ) + }) + .init(); + + // Create a Histogram Instrument. + let histogram = meter + .f64_histogram("my_histogram") + .with_description("My histogram example description") + .init(); + + // Record measurements using the histogram instrument. + histogram.record( + 10.5, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ); + + // Note that there is no ObservableHistogram instrument. + + // Create a Gauge Instrument. + let gauge = meter + .f64_gauge("my_gauge") + .with_description("A gauge set to 1.0") + .with_unit("myunit") + .init(); + + gauge.record( + 1.0, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ); + + // Create a ObservableGauge instrument and register a callback that reports the measurement. + let _observable_gauge = meter + .f64_observable_gauge("my_observable_gauge") + .with_description("An observable gauge set to 1.0") + .with_unit("myunit") + .with_callback(|observer| { + observer.observe( + 1.0, + &[ + KeyValue::new("mykey1", "myvalue1"), + KeyValue::new("mykey2", "myvalue2"), + ], + ) + }) + .init(); + + // Metrics are exported by default every 30 seconds when using stdout exporter, + // however shutting down the MeterProvider here instantly flushes + // the metrics, instead of waiting for the 30 sec interval. meter_provider.shutdown()?; Ok(()) } From b479a0f96f074f3c23dec55458fe232bea8a163b Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 11 Sep 2024 16:23:38 -0700 Subject: [PATCH 3/4] changelog --- opentelemetry-sdk/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 628f8ae39d..876e1a973c 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -5,6 +5,7 @@ - Update `async-std` dependency version to 1.13 - *Breaking* - Remove support for `MetricProducer` which allowed metrics from external sources to be sent through OpenTelemetry. + [#2105](https://github.com/open-telemetry/opentelemetry-rust/pull/2105) ## v0.25.0 From 225905f44922e295d9a89cb91a3e4e0e9d35181c Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Wed, 11 Sep 2024 16:25:56 -0700 Subject: [PATCH 4/4] fix doc --- opentelemetry-sdk/src/metrics/manual_reader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/src/metrics/manual_reader.rs b/opentelemetry-sdk/src/metrics/manual_reader.rs index 1365396102..c3e7860e64 100644 --- a/opentelemetry-sdk/src/metrics/manual_reader.rs +++ b/opentelemetry-sdk/src/metrics/manual_reader.rs @@ -91,7 +91,7 @@ impl MetricReader for ManualReader { }); } - /// Gathers all metrics from the SDK and other [MetricProducer]s, calling any + /// Gathers all metrics from the SDK, calling any /// callbacks necessary and returning the results. /// /// Returns an error if called after shutdown.