From 0a7be318a11f508654aae67765c7ee5d045cdbdf Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Thu, 12 Dec 2024 07:48:00 -0800 Subject: [PATCH 1/5] Small improvements to PeriodicReader --- .../src/metrics/periodic_reader.rs | 56 +++++++------------ 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/opentelemetry-sdk/src/metrics/periodic_reader.rs b/opentelemetry-sdk/src/metrics/periodic_reader.rs index 9a720497ca..304b6736dd 100644 --- a/opentelemetry-sdk/src/metrics/periodic_reader.rs +++ b/opentelemetry-sdk/src/metrics/periodic_reader.rs @@ -117,9 +117,6 @@ where /// return metric data to the user. It will not automatically send that data to /// the exporter outside of the predefined interval. /// -/// As this spuns up own background thread, this is recommended to be used with push exporters -/// that do not require any particular async runtime. As of now, this cannot be used with -/// OTLP exporters as they requires async runtime /// /// [collect]: MetricReader::collect /// @@ -160,7 +157,7 @@ impl PeriodicReader { mpsc::channel(); let reader = PeriodicReader { inner: Arc::new(PeriodicReaderInner { - message_sender: Arc::new(Mutex::new(message_sender)), + message_sender: Arc::new(message_sender), is_shutdown: AtomicBool::new(false), producer: Mutex::new(None), exporter: Arc::new(exporter), @@ -223,6 +220,11 @@ impl PeriodicReader { } else { response_sender.send(true).unwrap(); } + + otel_debug!( + name: "PeriodReaderThreadExiting", + reason = "ShutdownRequested" + ); break; } Err(mpsc::RecvTimeoutError::Timeout) => { @@ -255,8 +257,13 @@ impl PeriodicReader { interval_start = Instant::now(); } } - Err(_) => { - // Some other error. Break out and exit the thread. + Err(mpsc::RecvTimeoutError::Disconnected) => { + // Channel disconnected, only thing to do is break + // out (i.e exit the thread) + otel_debug!( + name: "PeriodReaderThreadExiting", + reason = "MessageReceiverDisconnected" + ); break; } } @@ -271,6 +278,7 @@ impl PeriodicReader { if let Err(e) = result_thread_creation { otel_error!( name: "PeriodReaderThreadStartError", + message = "Failed to start PeriodicReader thread. Metrics will not be exported.", error = format!("{:?}", e) ); } @@ -290,7 +298,7 @@ impl fmt::Debug for PeriodicReader { struct PeriodicReaderInner { exporter: Arc, - message_sender: Arc>>, + message_sender: Arc>, producer: Mutex>>, is_shutdown: AtomicBool, } @@ -374,20 +382,9 @@ impl PeriodicReaderInner { return Err(MetricError::Other("reader is shut down".into())); } let (response_tx, response_rx) = mpsc::channel(); - match self.message_sender.lock() { - Ok(sender) => { - sender - .send(Message::Flush(response_tx)) - .map_err(|e| MetricError::Other(e.to_string()))?; - } - Err(e) => { - otel_debug!( - name: "PeriodReaderForceFlushError", - error = format!("{:?}", e) - ); - return Err(MetricError::Other(e.to_string())); - } - } + self.message_sender + .send(Message::Flush(response_tx)) + .map_err(|e| MetricError::Other(e.to_string()))?; if let Ok(response) = response_rx.recv() { // TODO: call exporter's force_flush method. @@ -408,20 +405,9 @@ impl PeriodicReaderInner { // TODO: See if this is better to be created upfront. let (response_tx, response_rx) = mpsc::channel(); - match self.message_sender.lock() { - Ok(sender) => { - sender - .send(Message::Shutdown(response_tx)) - .map_err(|e| MetricError::Other(e.to_string()))?; - } - Err(e) => { - otel_debug!( - name: "PeriodReaderShutdownError", - error = format!("{:?}", e) - ); - return Err(MetricError::Other(e.to_string())); - } - } + self.message_sender + .send(Message::Shutdown(response_tx)) + .map_err(|e| MetricError::Other(e.to_string()))?; if let Ok(response) = response_rx.recv() { self.is_shutdown From e4e4cbf48961a6125a26178b3d8e6690a4304633 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Thu, 12 Dec 2024 11:09:31 -0800 Subject: [PATCH 2/5] try 1.75 for sdk --- scripts/msrv_config.json | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/scripts/msrv_config.json b/scripts/msrv_config.json index e86cda1f8b..418add0e27 100644 --- a/scripts/msrv_config.json +++ b/scripts/msrv_config.json @@ -1,7 +1,6 @@ { "1.70.0": [ "opentelemetry/Cargo.toml", - "opentelemetry-sdk/Cargo.toml", "opentelemetry-stdout/Cargo.toml", "opentelemetry-http/Cargo.toml", "opentelemetry-jaeger-propagator/Cargo.toml", @@ -13,4 +12,8 @@ "opentelemetry-otlp/Cargo.toml", "opentelemetry-proto/Cargo.toml" ] + , + "1.75.0": [ + "opentelemetry-sdk/Cargo.toml" + ] } From 4db1a2a97d6927c9e63cf338207ff81daee0e3b9 Mon Sep 17 00:00:00 2001 From: Cijo Thomas Date: Thu, 12 Dec 2024 11:20:32 -0800 Subject: [PATCH 3/5] try 1.75 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 376730ac7b..5fc64be771 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,7 +88,7 @@ jobs: strategy: matrix: os: [windows-latest, ubuntu-latest] - rust: [1.70.0, 1.71.1] + rust: [1.70.0, 1.71.1, 1.75.0] runs-on: ${{ matrix.os }} continue-on-error: true steps: From a82f665cb17913495e87340510ab2a9ad927a38a Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 12 Dec 2024 23:36:27 -0800 Subject: [PATCH 4/5] Update msrv_config.json --- scripts/msrv_config.json | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/scripts/msrv_config.json b/scripts/msrv_config.json index 3653f34f70..7f5b29b746 100644 --- a/scripts/msrv_config.json +++ b/scripts/msrv_config.json @@ -1,6 +1,7 @@ { "1.75.0": [ "opentelemetry/Cargo.toml", + "opentelemetry-sdk/Cargo.toml" "opentelemetry-stdout/Cargo.toml", "opentelemetry-http/Cargo.toml", "opentelemetry-jaeger-propagator/Cargo.toml", @@ -10,8 +11,4 @@ "opentelemetry-otlp/Cargo.toml", "opentelemetry-proto/Cargo.toml" ] - , - "1.75.0": [ - "opentelemetry-sdk/Cargo.toml" - ] } From 0154773bc3f4c5d4ccac64e33829a6da821fb1e8 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Thu, 12 Dec 2024 23:36:51 -0800 Subject: [PATCH 5/5] Update msrv_config.json --- scripts/msrv_config.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/msrv_config.json b/scripts/msrv_config.json index 7f5b29b746..05f9f5615c 100644 --- a/scripts/msrv_config.json +++ b/scripts/msrv_config.json @@ -1,7 +1,7 @@ { "1.75.0": [ "opentelemetry/Cargo.toml", - "opentelemetry-sdk/Cargo.toml" + "opentelemetry-sdk/Cargo.toml", "opentelemetry-stdout/Cargo.toml", "opentelemetry-http/Cargo.toml", "opentelemetry-jaeger-propagator/Cargo.toml",