more error changes

Author: scottgerring

opentelemetry-otlp/tests/integration_test/tests/logs.rs

@@ -94,7 +94,7 @@ mod logtests {
 
         tokio::time::sleep(Duration::from_secs(10)).await;
 
-        assert_logs_results(test_utils::LOGS_FILE, "expected/logs.json");
+        let _ = assert_logs_results(test_utils::LOGS_FILE, "expected/logs.json");
 
         Ok(())
     }
@@ -122,7 +122,7 @@ mod logtests {
         }
         let _ = logger_provider.shutdown();
         // tokio::time::sleep(Duration::from_secs(10)).await;
-        assert_logs_results(test_utils::LOGS_FILE, "expected/logs.json");
+        let _ = assert_logs_results(test_utils::LOGS_FILE, "expected/logs.json");
 
         Ok(())
     }

opentelemetry-sdk/src/export/logs/mod.rs

@@ -1,6 +1,6 @@
 //! Log exporters
 use crate::logs::LogRecord;
-use crate::logs::{LogError, LogResult, ShutdownError};
+use crate::logs::{LogError, ShutdownError};
 use crate::Resource;
 use async_trait::async_trait;
 #[cfg(feature = "spec_unstable_logs_enabled")]
@@ -81,8 +81,10 @@ pub trait LogExporter: Send + Sync + Debug {
     /// A `LogResult<()>`, which is a result type indicating either a successful export (with
     /// `Ok(())`) or an error (`Err(LogError)`) if the export operation failed.
     ///
-    async fn export(&self, batch: LogBatch<'_>) -> LogResult<()>;
-    /// Shuts down the exporter.
+    async fn export(&self, batch: LogBatch<'_>) -> ExportResult;
+
+    /// Shuts down the exporter. This function is idempotent; calling it
+    /// more than once has no additional effect.
     fn shutdown(&mut self) -> ShutdownResult {
         Ok(())
     }

opentelemetry-sdk/src/export/trace.rs

@@ -118,18 +118,19 @@ pub enum ShutdownError {
     #[error("Shutdown timed out after {0:?}")]
     Timeout(Duration),
 
+    /// The export client failed while holding the client lock. It is not
+    /// possible to complete the shutdown and a retry will not help.
+    /// This is something that should not happen and should likely emit some diagnostic.
+    #[error("export client failed while holding lock; cannot retry.")]
+    ClientFailed(String),
+
     /// An unexpected error occurred during shutdown.
     #[error(transparent)]
     Other(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
 }
 
-/// Custom error wrapper for string messages.
-#[derive(Error, Debug)]
-#[error("{0}")]
-struct CustomError(String);
-
 impl<T> From<PoisonError<T>> for ShutdownError {
     fn from(err: PoisonError<T>) -> Self {
-        ShutdownError::Other(Box::new(CustomError(err.to_string())))
+        ShutdownError::ClientFailed(format!("Mutex poisoned during shutdown: {}", err))
     }
 }

opentelemetry-sdk/src/logs/error.rs

@@ -18,14 +18,16 @@ pub enum LogError {
     #[error("Exporter timed out after {} seconds", .0.as_secs())]
     ExportTimedOut(Duration),
 
+    /// The export client failed while holding the client lock. It is not
+    /// possible to complete the shutdown and a retry will not help.
+    /// This is something that should not happen and should likely emit some diagnostic.
+    #[error("export client failed while holding lock; cannot retry.")]
+    ClientFailed(String),
+
     /// Processor is already shutdown
     #[error("{0} already shutdown")]
     AlreadyShutdown(String),
 
-    /// Mutex lock poisoning
-    #[error("mutex lock poisioning for {0}")]
-    MutexPoisoned(String),
-
     /// Other errors propagated from log SDK that weren't covered above.
     #[error(transparent)]
     Other(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
@@ -54,9 +56,10 @@ impl From<&'static str> for LogError {
 
 impl<T> From<PoisonError<T>> for LogError {
     fn from(err: PoisonError<T>) -> Self {
-        LogError::Other(err.to_string().into())
+        LogError::ClientFailed(err.to_string().into())
     }
 }
+
 /// Wrap type for string
 #[derive(Error, Debug)]
 #[error("{0}")]
@@ -66,9 +69,11 @@ struct Custom(String);
 #[derive(Error, Debug)]
 #[non_exhaustive]
 pub enum ShutdownError {
-    /// Mutex lock poisoning
-    #[error("mutex lock poisioning for {0}")]
-    MutexPoisoned(String),
+    /// The export client failed while holding the client lock. It is not
+    /// possible to complete the shutdown and a retry will not help.
+    /// This is something that should not happen and should likely emit some diagnostic.
+    #[error("export client failed while holding lock; cannot retry.")]
+    ClientFailed(String),
 
     /// Other errors propagated from log SDK that weren't covered above.
     #[error(transparent)]
@@ -77,6 +82,6 @@ pub enum ShutdownError {
 
 impl<T> From<PoisonError<T>> for ShutdownError {
     fn from(err: PoisonError<T>) -> Self {
-        ShutdownError::Other(err.to_string().into())
+        ShutdownError::ClientFailed(format!("Mutex poisoned during shutdown: {}", err))
     }
 }

opentelemetry-sdk/src/logs/log_emitter.rs

@@ -141,9 +141,9 @@ impl LoggerProviderInner {
                 //    which is non-actionable by the user
                 match err {
                     // specific handling for mutex poisioning
-                    LogError::MutexPoisoned(_) => {
+                    LogError::ClientFailed(_) => {
                         otel_debug!(
-                            name: "LoggerProvider.Drop.ShutdownMutexPoisoned",
+                            name: "LoggerProvider.Drop.ShutdownClientFailed",
                         );
                     }
                     _ => {

opentelemetry-sdk/src/logs/log_processor.rs

@@ -113,14 +113,14 @@ impl<T: LogExporter> LogProcessor for SimpleLogProcessor<T> {
         let result = self
             .exporter
             .lock()
-            .map_err(|_| LogError::MutexPoisoned("SimpleLogProcessor".into()))
+            .map_err(|_| LogError::ClientFailed("SimpleLogProcessor".into()))
             .and_then(|exporter| {
                 let log_tuple = &[(record as &LogRecord, instrumentation)];
                 futures_executor::block_on(exporter.export(LogBatch::new(log_tuple)))
             });
         // Handle errors with specific static names
         match result {
-            Err(LogError::MutexPoisoned(_)) => {
+            Err(LogError::ClientFailed(_)) => {
                 // logging as debug as this is not a user error
                 otel_debug!(
                     name: "SimpleLogProcessor.Emit.MutexPoisoning",
@@ -149,7 +149,8 @@ impl<T: LogExporter> LogProcessor for SimpleLogProcessor<T> {
                 .map_err(|e| LogError::Other(Box::new(e)))?;
             Ok(())
         } else {
-            Err(LogError::MutexPoisoned("SimpleLogProcessor".into()))
+            // Failing to get the mutex means the export client failed whilst holding it
+            Err(LogError::ClientFailed("SimpleLogProcessor".into()))
         }
     }
 

opentelemetry-sdk/src/testing/logs/in_memory_exporter.rs

@@ -1,4 +1,4 @@
-use crate::export::logs::{LogBatch, LogExporter};
+use crate::export::logs::{ExportResult, LogBatch, LogExporter};
 use crate::logs::{LogError, LogResult};
 use crate::logs::{LogRecord, ShutdownError};
 use crate::Resource;
@@ -172,19 +172,21 @@ impl InMemoryLogExporter {
     /// exporter.reset();
     /// ```
     ///
-    pub fn reset(&self) {
-        let _ = self
-            .logs
-            .lock()
-            .map(|mut logs_guard| logs_guard.clear())
-            .map_err(LogError::from);
+    pub fn reset(&self) -> Result<(), LogError> {
+        let _ = self.logs.lock().map(|mut logs_guard| logs_guard.clear())?;
+
+        Ok(())
     }
 }
 
 #[async_trait]
 impl LogExporter for InMemoryLogExporter {
-    async fn export(&self, batch: LogBatch<'_>) -> LogResult<()> {
-        let mut logs_guard = self.logs.lock().map_err(LogError::from)?;
+    async fn export(&self, batch: LogBatch<'_>) -> ExportResult {
+        let mut logs_guard = self
+            .logs
+            .lock()
+            .map_err(|e| LogError::ClientFailed(e.to_string()))?;
+
         for (log_record, instrumentation) in batch.iter() {
             let owned_log = OwnedLogData {
                 record: (*log_record).clone(),
@@ -197,7 +199,8 @@ impl LogExporter for InMemoryLogExporter {
 
     fn shutdown(&mut self) -> Result<(), ShutdownError> {
         if self.should_reset_on_shutdown {
-            self.reset();
+            self.reset()
+                .map_err(|e| ShutdownError::Other(Box::new(e)))?;
         }
         Ok(())
     }

opentelemetry-stdout/src/trace/exporter.rs

@@ -1,7 +1,7 @@
 use chrono::{DateTime, Utc};
 use core::fmt;
 use futures_util::future::BoxFuture;
-use opentelemetry::trace::{TraceError};
+use opentelemetry::trace::TraceError;
 use opentelemetry_sdk::export::trace::ShutdownResult;
 use opentelemetry_sdk::export::{self, trace::ExportResult};
 use opentelemetry_sdk::resource::Resource;