From b3b1f2397e0a24924f64c465bce1dee9a6605925 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 15 Mar 2024 00:24:21 -0700 Subject: [PATCH 01/18] meter provider shutdown --- examples/metrics-advanced/src/main.rs | 14 +++++++------ examples/metrics-basic/src/main.rs | 14 +++++++------ .../examples/basic-otlp-http/src/main.rs | 18 ++++++++++------- .../examples/basic-otlp/src/main.rs | 20 +++++++++++-------- .../src/metrics/meter_provider.rs | 11 ++++++++++ opentelemetry-stdout/examples/basic.rs | 13 ++++++------ opentelemetry/src/global/metrics.rs | 7 ++++++- 7 files changed, 63 insertions(+), 34 deletions(-) diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index e5d03a9213..7f8e9850cf 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -1,3 +1,4 @@ +use opentelemetry::global::{meter_provider, set_meter_provider, shutdown_meter_provider}; use opentelemetry::metrics::Unit; use opentelemetry::Key; use opentelemetry::{metrics::MeterProvider as _, KeyValue}; @@ -7,7 +8,7 @@ use opentelemetry_sdk::metrics::{ use opentelemetry_sdk::{runtime, Resource}; use std::error::Error; -fn init_meter_provider() -> SdkMeterProvider { +fn init_meter_provider() { // for example 1 let my_view_rename_and_unit = |i: &Instrument| { if i.name == "my_histogram" { @@ -50,7 +51,7 @@ fn init_meter_provider() -> SdkMeterProvider { // Ok(serde_json::to_writer_pretty(writer, &data).unwrap())) .build(); let reader = PeriodicReader::builder(exporter, runtime::Tokio).build(); - SdkMeterProvider::builder() + let provider = SdkMeterProvider::builder() .with_reader(reader) .with_resource(Resource::new(vec![KeyValue::new( "service.name", @@ -59,13 +60,14 @@ fn init_meter_provider() -> SdkMeterProvider { .with_view(my_view_rename_and_unit) .with_view(my_view_drop_attributes) .with_view(my_view_change_aggregation) - .build() + .build(); + set_meter_provider(provider); } #[tokio::main] async fn main() -> Result<(), Box> { - let meter_provider = init_meter_provider(); - let meter = meter_provider.meter("mylibraryname"); + init_meter_provider(); + let meter = meter_provider().meter("mylibraryname"); // Example 1 - Rename metric using View. // This instrument will be renamed to "my_histogram_renamed", @@ -151,6 +153,6 @@ async fn main() -> Result<(), Box> { // 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()?; + shutdown_meter_provider(); Ok(()) } diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index f7828da796..4bc9e8591b 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -1,32 +1,34 @@ +use opentelemetry::global::{meter_provider, set_meter_provider, shutdown_meter_provider}; use opentelemetry::metrics::Unit; use opentelemetry::{metrics::MeterProvider as _, KeyValue}; use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider}; use opentelemetry_sdk::{runtime, Resource}; use std::error::Error; -fn init_meter_provider() -> SdkMeterProvider { +fn init_meter_provider() { let exporter = opentelemetry_stdout::MetricsExporterBuilder::default() // uncomment the below lines to pretty print output. // .with_encoder(|writer, data| // Ok(serde_json::to_writer_pretty(writer, &data).unwrap())) .build(); let reader = PeriodicReader::builder(exporter, runtime::Tokio).build(); - SdkMeterProvider::builder() + let provider = SdkMeterProvider::builder() .with_reader(reader) .with_resource(Resource::new(vec![KeyValue::new( "service.name", "metrics-basic-example", )])) - .build() + .build(); + set_meter_provider(provider); } #[tokio::main] async fn main() -> Result<(), Box> { // Initialize the MeterProvider with the stdout Exporter. - let meter_provider = init_meter_provider(); + init_meter_provider(); // Create a meter from the above MeterProvider. - let meter = meter_provider.meter("mylibraryname"); + let meter = meter_provider().meter("mylibraryname"); // Create a Counter Instrument. let counter = meter.u64_counter("my_counter").init(); @@ -146,6 +148,6 @@ async fn main() -> Result<(), Box> { // 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()?; + shutdown_meter_provider(); Ok(()) } diff --git a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs index 0f320dcb8d..4ac66ae8fe 100644 --- a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs @@ -1,13 +1,13 @@ use once_cell::sync::Lazy; use opentelemetry::{ - global, metrics, + global, + metrics::MetricsError, trace::{TraceContextExt, TraceError, Tracer}, Key, KeyValue, }; use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge; use opentelemetry_otlp::WithExportConfig; use opentelemetry_sdk::logs as sdklogs; -use opentelemetry_sdk::metrics as sdkmetrics; use opentelemetry_sdk::resource; use opentelemetry_sdk::trace as sdktrace; @@ -44,19 +44,23 @@ fn init_tracer() -> Result { .install_batch(opentelemetry_sdk::runtime::Tokio) } -fn init_metrics() -> metrics::Result { +fn init_metrics() -> Result<(), MetricsError> { let export_config = opentelemetry_otlp::ExportConfig { endpoint: "http://localhost:4318/v1/metrics".to_string(), ..opentelemetry_otlp::ExportConfig::default() }; - opentelemetry_otlp::new_pipeline() + let provider = opentelemetry_otlp::new_pipeline() .metrics(opentelemetry_sdk::runtime::Tokio) .with_exporter( opentelemetry_otlp::new_exporter() .http() .with_export_config(export_config), ) - .build() + .build(); + match provider { + Ok(provider) => Ok(()), + Err(err) => Err(err), + } } const LEMONS_KEY: Key = Key::from_static_str("ex.com/lemons"); @@ -74,7 +78,7 @@ static COMMON_ATTRIBUTES: Lazy<[KeyValue; 4]> = Lazy::new(|| { #[tokio::main] async fn main() -> Result<(), Box> { let _ = init_tracer()?; - let meter_provider = init_metrics()?; + let _ = init_metrics()?; let _ = init_logs(); let tracer = global::tracer("ex.com/basic"); @@ -108,7 +112,7 @@ async fn main() -> Result<(), Box> { global::shutdown_tracer_provider(); global::shutdown_logger_provider(); - meter_provider.shutdown()?; + global::shutdown_meter_provider(); Ok(()) } diff --git a/opentelemetry-otlp/examples/basic-otlp/src/main.rs b/opentelemetry-otlp/examples/basic-otlp/src/main.rs index 1bf5f7e421..852b8a3afa 100644 --- a/opentelemetry-otlp/examples/basic-otlp/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp/src/main.rs @@ -1,18 +1,18 @@ use log::{info, Level}; use once_cell::sync::Lazy; -use opentelemetry::global; +use opentelemetry::global::{self, shutdown_meter_provider}; use opentelemetry::global::{logger_provider, shutdown_logger_provider, shutdown_tracer_provider}; use opentelemetry::logs::LogError; +use opentelemetry::metrics::MetricsError; use opentelemetry::trace::TraceError; use opentelemetry::{ - metrics, trace::{TraceContextExt, Tracer}, Key, KeyValue, }; use opentelemetry_appender_log::OpenTelemetryLogBridge; use opentelemetry_otlp::{ExportConfig, WithExportConfig}; use opentelemetry_sdk::logs::Config; -use opentelemetry_sdk::{metrics::SdkMeterProvider, runtime, trace as sdktrace, Resource}; +use opentelemetry_sdk::{runtime, trace as sdktrace, Resource}; use std::error::Error; fn init_tracer() -> Result { @@ -32,12 +32,12 @@ fn init_tracer() -> Result { .install_batch(runtime::Tokio) } -fn init_metrics() -> metrics::Result { +fn init_metrics() -> Result<(), MetricsError> { let export_config = ExportConfig { endpoint: "http://localhost:4317".to_string(), ..ExportConfig::default() }; - opentelemetry_otlp::new_pipeline() + let provider = opentelemetry_otlp::new_pipeline() .metrics(runtime::Tokio) .with_exporter( opentelemetry_otlp::new_exporter() @@ -48,7 +48,11 @@ fn init_metrics() -> metrics::Result { opentelemetry_semantic_conventions::resource::SERVICE_NAME, "basic-otlp-metrics-example", )])) - .build() + .build(); + match provider { + Ok(_provider) => Ok(()), + Err(err) => Err(err), + } } fn init_logs() -> Result { @@ -87,7 +91,7 @@ async fn main() -> Result<(), Box> { // matches the containing block, reporting traces and metrics during the whole // execution. let _ = init_tracer()?; - let meter_provider = init_metrics()?; + let _ = init_metrics()?; // Initialize logs, which sets the global loggerprovider. let _ = init_logs(); @@ -139,7 +143,7 @@ async fn main() -> Result<(), Box> { shutdown_tracer_provider(); shutdown_logger_provider(); - meter_provider.shutdown()?; + shutdown_meter_provider(); Ok(()) } diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 13a14c1739..4a9c7569f6 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -9,6 +9,7 @@ use std::{ }; use opentelemetry::{ + global, metrics::{noop::NoopMeterCore, Meter, MeterProvider, MetricsError, Result}, KeyValue, }; @@ -113,6 +114,16 @@ impl SdkMeterProvider { } } +impl Drop for SdkMeterProvider { + fn drop(&mut self) { + if self.is_shutdown.load(Ordering::Relaxed) { + return; + } + if let Err(err) = self.shutdown() { + global::handle_error(err); + } + } +} impl MeterProvider for SdkMeterProvider { fn versioned_meter( &self, diff --git a/opentelemetry-stdout/examples/basic.rs b/opentelemetry-stdout/examples/basic.rs index 41cfa7a30f..b59a477b4b 100644 --- a/opentelemetry-stdout/examples/basic.rs +++ b/opentelemetry-stdout/examples/basic.rs @@ -2,7 +2,7 @@ #[cfg(all(feature = "metrics", feature = "trace"))] use opentelemetry::{ - metrics::MeterProvider as _, + global, trace::{Span, Tracer, TracerProvider as _}, KeyValue, }; @@ -22,17 +22,18 @@ fn init_trace() -> TracerProvider { } #[cfg(all(feature = "metrics", feature = "trace"))] -fn init_metrics() -> SdkMeterProvider { +fn init_metrics() { let exporter = opentelemetry_stdout::MetricsExporter::default(); let reader = PeriodicReader::builder(exporter, runtime::Tokio).build(); - SdkMeterProvider::builder().with_reader(reader).build() + let provider = SdkMeterProvider::builder().with_reader(reader).build(); + global::set_meter_provider(provider); } #[tokio::main] #[cfg(all(feature = "metrics", feature = "trace"))] async fn main() -> Result<(), Box> { let tracer_provider = init_trace(); - let meter_provider = init_metrics(); + init_metrics(); let tracer = tracer_provider.tracer("stdout-test"); let mut span = tracer.start("test_span"); @@ -43,11 +44,11 @@ async fn main() -> Result<(), Box> { ); span.end(); - let meter = meter_provider.meter("stdout-test"); + let meter = global::meter("stdout-test"); let c = meter.u64_counter("test_events").init(); c.add(1, &[KeyValue::new("test_key", "test_value")]); - meter_provider.shutdown()?; + global::shutdown_meter_provider(); Ok(()) } diff --git a/opentelemetry/src/global/metrics.rs b/opentelemetry/src/global/metrics.rs index c40477e5ac..24d293f154 100644 --- a/opentelemetry/src/global/metrics.rs +++ b/opentelemetry/src/global/metrics.rs @@ -1,4 +1,4 @@ -use crate::metrics::{self, Meter, MeterProvider}; +use crate::metrics::{self, noop::NoopMeterProvider, Meter, MeterProvider}; use crate::KeyValue; use core::fmt; use once_cell::sync::Lazy; @@ -116,6 +116,11 @@ pub fn meter(name: impl Into>) -> Meter { meter_provider().meter(name.into()) } +/// Shut down the current global [`MeterProvider`]. +pub fn shutdown_meter_provider() { + set_meter_provider(NoopMeterProvider::new()); +} + /// Creates a [`Meter`] with the name, version and schema url. /// /// - name SHOULD uniquely identify the instrumentation scope, such as the instrumentation library (e.g. io.opentelemetry.contrib.mongodb), package, module or class name. From cd9c072cd480b1013afd48c5874eb4a9c68f27f7 Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 15 Mar 2024 00:39:53 -0700 Subject: [PATCH 02/18] add changelog --- opentelemetry/CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md index 9613638a57..235201a7eb 100644 --- a/opentelemetry/CHANGELOG.md +++ b/opentelemetry/CHANGELOG.md @@ -2,7 +2,12 @@ ## vNext +### Added + +- [#1623](https://github.com/open-telemetry/opentelemetry-rust/pull/1623) Add global::meter_provider_shutdown + ### Removed + - Remove `urlencoding` crate dependency. [#1613](https://github.com/open-telemetry/opentelemetry-rust/pull/1613) ## v0.22.0 From 3b0d1834fee42dcf09173509ccfd83f6e194bb2b Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 09:53:07 -0700 Subject: [PATCH 03/18] Update examples/metrics-basic/src/main.rs Co-authored-by: Cijo Thomas --- examples/metrics-basic/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index 4bc9e8591b..ce3e80e9eb 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -148,6 +148,6 @@ async fn main() -> Result<(), Box> { // 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. - shutdown_meter_provider(); + global::shutdown_meter_provider(); Ok(()) } From f00638ab6dae437c326f003324249d3c208f83dc Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 09:53:24 -0700 Subject: [PATCH 04/18] Update examples/metrics-basic/src/main.rs Co-authored-by: Cijo Thomas --- examples/metrics-basic/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index ce3e80e9eb..fa77554276 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -19,7 +19,7 @@ fn init_meter_provider() { "metrics-basic-example", )])) .build(); - set_meter_provider(provider); + opentelemetry::global::set_meter_provider(provider); } #[tokio::main] From d5f3ed3adaa7694b46ecbb183c0c25f99335fbf2 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 10:09:19 -0700 Subject: [PATCH 05/18] build error --- examples/metrics-basic/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index fa77554276..8ae56a445e 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -1,4 +1,4 @@ -use opentelemetry::global::{meter_provider, set_meter_provider, shutdown_meter_provider}; +use opentelemetry::global::{self, meter_provider}; use opentelemetry::metrics::Unit; use opentelemetry::{metrics::MeterProvider as _, KeyValue}; use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider}; From 045b44051daf65defd8393b12118798c350deb72 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 12:05:26 -0700 Subject: [PATCH 06/18] add test --- opentelemetry-sdk/CHANGELOG.md | 3 +- .../src/metrics/meter_provider.rs | 34 +++++++++++++--- .../src/testing/metrics/metric_reader.rs | 39 +++++++++++++++++-- 3 files changed, 66 insertions(+), 10 deletions(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index abb71fc186..38ac3cf223 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -6,7 +6,8 @@ dependency on crossbeam-channel. [1612](https://github.com/open-telemetry/opentelemetry-rust/pull/1612/files) - [#1422](https://github.com/open-telemetry/opentelemetry-rust/pull/1422) - Fix metrics aggregation bug when using Views to drop attributes. + Fix metrics aggregation bug when using Views to drop attributes. +- [#1623](https://github.com/open-telemetry/opentelemetry-rust/pull/1623) Add Drop implementation for SdkTracerProvider ## v0.22.1 diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 4a9c7569f6..6a17d879cc 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -220,8 +220,10 @@ impl fmt::Debug for MeterProviderBuilder { } #[cfg(test)] mod tests { + use crate::metrics::reader::MetricReader; use crate::testing::metrics::metric_reader::TestMetricReader; use crate::Resource; + use opentelemetry::global; use opentelemetry::Key; use opentelemetry::KeyValue; use std::env; @@ -239,14 +241,14 @@ mod tests { expect.map(|s| s.to_string()) ); }; - let reader = TestMetricReader {}; + let reader = TestMetricReader::new(); let default_meter_provider = super::SdkMeterProvider::builder() .with_reader(reader) .build(); assert_service_name(default_meter_provider, Some("unknown_service")); // If user provided a resource, use that. - let reader2 = TestMetricReader {}; + let reader2 = TestMetricReader::new(); let custom_meter_provider = super::SdkMeterProvider::builder() .with_reader(reader2) .with_resource(Resource::new(vec![KeyValue::new( @@ -261,7 +263,7 @@ mod tests { Some("key1=value1, k2, k3=value2"), || { // If `OTEL_RESOURCE_ATTRIBUTES` is set, read them automatically - let reader3 = TestMetricReader {}; + let reader3 = TestMetricReader::new(); let env_resource_provider = super::SdkMeterProvider::builder() .with_reader(reader3) .build(); @@ -284,7 +286,7 @@ mod tests { "OTEL_RESOURCE_ATTRIBUTES", Some("my-custom-key=env-val,k2=value2"), || { - let reader4 = TestMetricReader {}; + let reader4 = TestMetricReader::new(); let user_provided_resource_config_provider = super::SdkMeterProvider::builder() .with_reader(reader4) .with_resource(Resource::default().merge(&mut Resource::new(vec![ @@ -306,7 +308,7 @@ mod tests { ); // If user provided a resource, it takes priority during collision. - let reader5 = TestMetricReader {}; + let reader5 = TestMetricReader::new(); let no_service_name = super::SdkMeterProvider::builder() .with_reader(reader5) .with_resource(Resource::empty()) @@ -314,4 +316,26 @@ mod tests { assert_service_name(no_service_name, None); } + + #[test] + fn test_meter_provider_shutdown() { + let reader = TestMetricReader::new(); + let provider = super::SdkMeterProvider::builder() + .with_reader(reader.clone()) + .build(); + global::set_meter_provider(provider.clone()); + assert!(!provider + .is_shutdown + .load(std::sync::atomic::Ordering::Relaxed)); + assert!(!reader.is_shutdown()); + // create a meter + let _meter = global::meter("test"); + let _ = reader.force_flush(); + // no need to drop a meter for meter_provider shutdown + global::shutdown_meter_provider(); + assert!(provider + .is_shutdown + .load(std::sync::atomic::Ordering::Relaxed)); + assert!(reader.is_shutdown()); + } } diff --git a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs index 1a63e2b06b..f2f949f88e 100644 --- a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs +++ b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs @@ -1,4 +1,4 @@ -use std::sync::Weak; +use std::sync::{Arc, Mutex, Weak}; use crate::metrics::{ aggregation::Aggregation, @@ -9,13 +9,30 @@ use crate::metrics::{ }; use opentelemetry::metrics::Result; -#[derive(Debug)] -pub struct TestMetricReader {} +#[derive(Debug, Clone)] +pub struct TestMetricReader { + is_shutdown: Arc>, +} + +impl TestMetricReader { + // Constructor to initialize the TestMetricReader + pub fn new() -> Self { + TestMetricReader { + is_shutdown: Arc::new(Mutex::new(false)), + } + } + + // Method to check if the reader is shutdown + pub fn is_shutdown(&self) -> bool { + *self.is_shutdown.lock().unwrap() + } +} impl MetricReader for TestMetricReader { fn register_pipeline(&self, _pipeline: Weak) {} fn collect(&self, _rm: &mut ResourceMetrics) -> Result<()> { + println!("Collect called.."); Ok(()) } @@ -24,7 +41,21 @@ impl MetricReader for TestMetricReader { } fn shutdown(&self) -> Result<()> { - self.force_flush() + println!( + "Shutdown called.. on {:?} and pointer for shutdown is {:?}", + self, self.is_shutdown + ); + let result = self.force_flush(); + { + let mut is_shutdown = self.is_shutdown.lock().unwrap(); + *is_shutdown = true; + } + println!( + "Shutdown completed.. on {:?} and pointer for shutdown is {:?}", + self, self.is_shutdown + ); + + result } } From 9fe2585aa9364cb3354c4db55dfc76c068b1f49c Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 12:10:31 -0700 Subject: [PATCH 07/18] fix --- opentelemetry-sdk/src/testing/metrics/metric_reader.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs index f2f949f88e..5a4c6f808f 100644 --- a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs +++ b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs @@ -28,6 +28,12 @@ impl TestMetricReader { } } +impl Default for TestMetricReader { + fn default() -> Self { + Self::new() + } +} + impl MetricReader for TestMetricReader { fn register_pipeline(&self, _pipeline: Weak) {} From 46c526772cd3337555c0ea2b516e7c2acd914d27 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 12:27:44 -0700 Subject: [PATCH 08/18] fix --- opentelemetry-sdk/src/testing/metrics/metric_reader.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs index 5a4c6f808f..6bea3fdd02 100644 --- a/opentelemetry-sdk/src/testing/metrics/metric_reader.rs +++ b/opentelemetry-sdk/src/testing/metrics/metric_reader.rs @@ -38,7 +38,6 @@ impl MetricReader for TestMetricReader { fn register_pipeline(&self, _pipeline: Weak) {} fn collect(&self, _rm: &mut ResourceMetrics) -> Result<()> { - println!("Collect called.."); Ok(()) } @@ -47,20 +46,11 @@ impl MetricReader for TestMetricReader { } fn shutdown(&self) -> Result<()> { - println!( - "Shutdown called.. on {:?} and pointer for shutdown is {:?}", - self, self.is_shutdown - ); let result = self.force_flush(); { let mut is_shutdown = self.is_shutdown.lock().unwrap(); *is_shutdown = true; } - println!( - "Shutdown completed.. on {:?} and pointer for shutdown is {:?}", - self, self.is_shutdown - ); - result } } From f8d30a1f51f6809ddd3ab3b70035a13a05fd5fd5 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 13:10:41 -0700 Subject: [PATCH 09/18] explicit global namespace --- examples/metrics-advanced/src/main.rs | 8 ++++---- examples/metrics-basic/src/main.rs | 4 ++-- opentelemetry-otlp/examples/basic-otlp/src/main.rs | 11 +++++------ opentelemetry-sdk/src/metrics/meter_provider.rs | 1 - 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index 7f8e9850cf..80a7b12dd3 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -1,4 +1,4 @@ -use opentelemetry::global::{meter_provider, set_meter_provider, shutdown_meter_provider}; +use opentelemetry::global; use opentelemetry::metrics::Unit; use opentelemetry::Key; use opentelemetry::{metrics::MeterProvider as _, KeyValue}; @@ -61,13 +61,13 @@ fn init_meter_provider() { .with_view(my_view_drop_attributes) .with_view(my_view_change_aggregation) .build(); - set_meter_provider(provider); + global::set_meter_provider(provider); } #[tokio::main] async fn main() -> Result<(), Box> { init_meter_provider(); - let meter = meter_provider().meter("mylibraryname"); + let meter = global::meter_provider().meter("mylibraryname"); // Example 1 - Rename metric using View. // This instrument will be renamed to "my_histogram_renamed", @@ -153,6 +153,6 @@ async fn main() -> Result<(), Box> { // 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. - shutdown_meter_provider(); + global::shutdown_meter_provider(); Ok(()) } diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index 8ae56a445e..c3b4a9fffa 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -1,4 +1,4 @@ -use opentelemetry::global::{self, meter_provider}; +use opentelemetry::global; use opentelemetry::metrics::Unit; use opentelemetry::{metrics::MeterProvider as _, KeyValue}; use opentelemetry_sdk::metrics::{PeriodicReader, SdkMeterProvider}; @@ -28,7 +28,7 @@ async fn main() -> Result<(), Box> { init_meter_provider(); // Create a meter from the above MeterProvider. - let meter = meter_provider().meter("mylibraryname"); + let meter = global::meter_provider().meter("mylibraryname"); // Create a Counter Instrument. let counter = meter.u64_counter("my_counter").init(); diff --git a/opentelemetry-otlp/examples/basic-otlp/src/main.rs b/opentelemetry-otlp/examples/basic-otlp/src/main.rs index 852b8a3afa..4f4320cab9 100644 --- a/opentelemetry-otlp/examples/basic-otlp/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp/src/main.rs @@ -1,7 +1,6 @@ use log::{info, Level}; use once_cell::sync::Lazy; -use opentelemetry::global::{self, shutdown_meter_provider}; -use opentelemetry::global::{logger_provider, shutdown_logger_provider, shutdown_tracer_provider}; +use opentelemetry::global; use opentelemetry::logs::LogError; use opentelemetry::metrics::MetricsError; use opentelemetry::trace::TraceError; @@ -97,7 +96,7 @@ async fn main() -> Result<(), Box> { let _ = init_logs(); // Retrieve the global LoggerProvider. - let logger_provider = logger_provider(); + let logger_provider = global::logger_provider(); // Create a new OpenTelemetryLogBridge using the above LoggerProvider. let otel_log_appender = OpenTelemetryLogBridge::new(&logger_provider); @@ -141,9 +140,9 @@ async fn main() -> Result<(), Box> { info!(target: "my-target", "hello from {}. My price is {}", "apple", 1.99); - shutdown_tracer_provider(); - shutdown_logger_provider(); - shutdown_meter_provider(); + global::shutdown_tracer_provider(); + global::shutdown_logger_provider(); + global::shutdown_meter_provider(); Ok(()) } diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 6a17d879cc..d0ede843b4 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -330,7 +330,6 @@ mod tests { assert!(!reader.is_shutdown()); // create a meter let _meter = global::meter("test"); - let _ = reader.force_flush(); // no need to drop a meter for meter_provider shutdown global::shutdown_meter_provider(); assert!(provider From 7a6f7c9cc87a697ef2edb3b55e01de506503fc4e Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 13:11:41 -0700 Subject: [PATCH 10/18] remove unused imports --- opentelemetry-sdk/src/metrics/meter_provider.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index d0ede843b4..fde136c738 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -220,7 +220,6 @@ impl fmt::Debug for MeterProviderBuilder { } #[cfg(test)] mod tests { - use crate::metrics::reader::MetricReader; use crate::testing::metrics::metric_reader::TestMetricReader; use crate::Resource; use opentelemetry::global; From 140a7d8579dfcb44e5a6e241ec152efb728272cf Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 13:12:39 -0700 Subject: [PATCH 11/18] Update opentelemetry-sdk/CHANGELOG.md Co-authored-by: Cijo Thomas --- opentelemetry-sdk/CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 38ac3cf223..352121aa2e 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -7,7 +7,8 @@ [1612](https://github.com/open-telemetry/opentelemetry-rust/pull/1612/files) - [#1422](https://github.com/open-telemetry/opentelemetry-rust/pull/1422) Fix metrics aggregation bug when using Views to drop attributes. -- [#1623](https://github.com/open-telemetry/opentelemetry-rust/pull/1623) Add Drop implementation for SdkTracerProvider +- [#1623](https://github.com/open-telemetry/opentelemetry-rust/pull/1623) Add Drop implementation for SdkTracerProvider, which shuts down +metricreaders, thereby allowing metrics still in memory to be flushed out. ## v0.22.1 From c14325c6d862e0528b1c45aeeaa296b462b692a0 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 13:14:09 -0700 Subject: [PATCH 12/18] remove opentelemetry prefix from opentelemetry::global:: --- examples/metrics-basic/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index c3b4a9fffa..f30606a13b 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -19,7 +19,7 @@ fn init_meter_provider() { "metrics-basic-example", )])) .build(); - opentelemetry::global::set_meter_provider(provider); + global::set_meter_provider(provider); } #[tokio::main] From df0943bb3116d2dd0ead0cb2e7b364b6514e4745 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 13:33:52 -0700 Subject: [PATCH 13/18] Add TODO for instrument cleanup --- opentelemetry-sdk/src/metrics/meter_provider.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index fde136c738..9c04735f6f 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -327,13 +327,18 @@ mod tests { .is_shutdown .load(std::sync::atomic::Ordering::Relaxed)); assert!(!reader.is_shutdown()); - // create a meter - let _meter = global::meter("test"); + // create a meter and an instrument + let meter = global::meter("test"); + let counter = meter.u64_counter("test_counter").init(); // no need to drop a meter for meter_provider shutdown global::shutdown_meter_provider(); assert!(provider .is_shutdown .load(std::sync::atomic::Ordering::Relaxed)); assert!(reader.is_shutdown()); + // TODO - assert that the instrument is no longer usable + // As of now, even after shutdown, the instrument can still be used. + // Even though reader is shutdown, and no collect will happen. + assert!(counter.add(1, &[]) == ()); } } From 46f41d5844fa797aefb4c5f0a809fc0c22e46340 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 15:29:59 -0700 Subject: [PATCH 14/18] Update opentelemetry-sdk/CHANGELOG.md Co-authored-by: Cijo Thomas --- opentelemetry-sdk/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 352121aa2e..e249982872 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -7,7 +7,7 @@ [1612](https://github.com/open-telemetry/opentelemetry-rust/pull/1612/files) - [#1422](https://github.com/open-telemetry/opentelemetry-rust/pull/1422) Fix metrics aggregation bug when using Views to drop attributes. -- [#1623](https://github.com/open-telemetry/opentelemetry-rust/pull/1623) Add Drop implementation for SdkTracerProvider, which shuts down +- [#1623](https://github.com/open-telemetry/opentelemetry-rust/pull/1623) Add Drop implementation for SdkMeterProvider, which shuts down metricreaders, thereby allowing metrics still in memory to be flushed out. ## v0.22.1 From 7d8bcf334697fb8a3f9ab0fba5d45df1933aea05 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 15:30:08 -0700 Subject: [PATCH 15/18] Update examples/metrics-advanced/src/main.rs Co-authored-by: Cijo Thomas --- examples/metrics-advanced/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/metrics-advanced/src/main.rs b/examples/metrics-advanced/src/main.rs index 80a7b12dd3..6c001c19e3 100644 --- a/examples/metrics-advanced/src/main.rs +++ b/examples/metrics-advanced/src/main.rs @@ -67,7 +67,7 @@ fn init_meter_provider() { #[tokio::main] async fn main() -> Result<(), Box> { init_meter_provider(); - let meter = global::meter_provider().meter("mylibraryname"); + let meter = global::meter("mylibraryname"); // Example 1 - Rename metric using View. // This instrument will be renamed to "my_histogram_renamed", From 4b4fdbe7fec5ae39f014f093c2e5c2cf5f880e10 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 15:30:13 -0700 Subject: [PATCH 16/18] Update examples/metrics-basic/src/main.rs Co-authored-by: Cijo Thomas --- examples/metrics-basic/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/metrics-basic/src/main.rs b/examples/metrics-basic/src/main.rs index f30606a13b..51cfbb20bb 100644 --- a/examples/metrics-basic/src/main.rs +++ b/examples/metrics-basic/src/main.rs @@ -28,7 +28,7 @@ async fn main() -> Result<(), Box> { init_meter_provider(); // Create a meter from the above MeterProvider. - let meter = global::meter_provider().meter("mylibraryname"); + let meter = global::meter("mylibraryname"); // Create a Counter Instrument. let counter = meter.u64_counter("my_counter").init(); From 1fdb432f3900b2bc81d33e98b70ca2b1193c235e Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Fri, 15 Mar 2024 15:30:47 -0700 Subject: [PATCH 17/18] Update opentelemetry-otlp/examples/basic-otlp-http/src/main.rs Co-authored-by: Zhongyang Wu --- opentelemetry-otlp/examples/basic-otlp-http/src/main.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs index 4ac66ae8fe..dd72682ff1 100644 --- a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs @@ -57,10 +57,7 @@ fn init_metrics() -> Result<(), MetricsError> { .with_export_config(export_config), ) .build(); - match provider { - Ok(provider) => Ok(()), - Err(err) => Err(err), - } + provider.map(|_| ()) } const LEMONS_KEY: Key = Key::from_static_str("ex.com/lemons"); From aefbaafbd86cfb1c0c426cce690906c4dd85e1ab Mon Sep 17 00:00:00 2001 From: Lalit Date: Fri, 15 Mar 2024 15:39:15 -0700 Subject: [PATCH 18/18] review comments --- .../examples/basic-otlp-http/src/main.rs | 2 +- opentelemetry-sdk/src/metrics/meter_provider.rs | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs index dd72682ff1..f06ba1d9de 100644 --- a/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs +++ b/opentelemetry-otlp/examples/basic-otlp-http/src/main.rs @@ -57,7 +57,7 @@ fn init_metrics() -> Result<(), MetricsError> { .with_export_config(export_config), ) .build(); - provider.map(|_| ()) + provider.map(|_| ()) } const LEMONS_KEY: Key = Key::from_static_str("ex.com/lemons"); diff --git a/opentelemetry-sdk/src/metrics/meter_provider.rs b/opentelemetry-sdk/src/metrics/meter_provider.rs index 9c04735f6f..d02b3fc2bc 100644 --- a/opentelemetry-sdk/src/metrics/meter_provider.rs +++ b/opentelemetry-sdk/src/metrics/meter_provider.rs @@ -116,9 +116,6 @@ impl SdkMeterProvider { impl Drop for SdkMeterProvider { fn drop(&mut self) { - if self.is_shutdown.load(Ordering::Relaxed) { - return; - } if let Err(err) = self.shutdown() { global::handle_error(err); } @@ -336,9 +333,8 @@ mod tests { .is_shutdown .load(std::sync::atomic::Ordering::Relaxed)); assert!(reader.is_shutdown()); - // TODO - assert that the instrument is no longer usable - // As of now, even after shutdown, the instrument can still be used. - // Even though reader is shutdown, and no collect will happen. - assert!(counter.add(1, &[]) == ()); + // TODO Fix: the instrument is still available, and can be used. + // While the reader is shutdown, and no collect is happening + counter.add(1, &[]); } }