update error handling in OTEL with no_std

- use core::Error unstable feature. It's an upcoming stable feature (rust-lang/rust#103765) - remove thiserror dependency. There's no _good_ way to support thiserror in no_std. Definitely not without bringing in some experimental code or forked thiserror crate Change-Id: I245311bb82596fdb3a1912da25a831693370e164
project-oak · Jul 29, 2024 · 38adfac · 38adfac
1 parent fa6abaa
commit 38adfac
Show file tree

Hide file tree

Showing 8 changed files with 81 additions and 67 deletions.
diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/BUILD b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/BUILD
@@ -22,14 +22,14 @@ package(
 
 rust_library(
     name = "opentelemetry_rk",
-    srcs = glob(["src/metrics/**"])
-    + [
-     "src/global/metrics.rs",
-     "src/global/mod.rs",
-     "src/baggage.rs",
-     "src/common.rs",
-     "src/context.rs",
-     "src/lib.rs",
+    srcs = glob(["src/metrics/**"]) + [
+        "src/baggage.rs",
+        "src/common.rs",
+        "src/context.rs",
+        "src/global/error_handler.rs",
+        "src/global/metrics.rs",
+        "src/global/mod.rs",
+        "src/lib.rs",
     ],
     deps = [
         "//oak_core",

diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/Cargo.toml b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/Cargo.toml
@@ -19,6 +19,7 @@ include = [
   "src/metrics/*",
   "src/global/metrics.rs",
   "src/global/mod.rs",
+  "src/global/error_handler.rs",
   "benches/*",
 ]
 exclude = [
@@ -38,16 +39,14 @@ all-features = true
 rustdoc-args = ["--cfg", "docsrs"]
 
 [dependencies]
-pin-project-lite = { version = "0.2", optional = true }
 oak_core = { path = "../../../../oak_core" }
-once_cell = { version = "1.13" }
 
 [target.'cfg(all(target_arch = "wasm32", not(target_os = "wasi")))'.dependencies]
 js-sys = "0.3.63"
 
 [features]
 default = ["metrics"]
-trace = ["pin-project-lite"]
+trace = []
 metrics = []
 testing = ["metrics"]
 logs = []

diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/common.rs b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/common.rs
@@ -437,9 +437,8 @@ impl KeyValue {
     }
 }
 
-// (TODO) Port errors to no_std
 /// Marker trait for errors returned by exporters
-pub trait ExportError: std::error::Error + Send + Sync + 'static {
+pub trait ExportError: core::error::Error + Send + Sync + 'static {
     /// The name of exporter that returned this error
     fn exporter_name(&self) -> &'static str;
 }

diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/global/error_handler.rs b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/global/error_handler.rs
@@ -1,50 +1,56 @@
-use std::sync::PoisonError;
-use std::sync::RwLock;
+use core::fmt;
+use oak_core::sync::OnceCell;
 
 #[cfg(feature = "logs")]
 use crate::logs::LogError;
 #[cfg(feature = "metrics")]
 use crate::metrics::MetricsError;
-use crate::propagation::PropagationError;
+// use crate::propagation::PropagationError;
 #[cfg(feature = "trace")]
 use crate::trace::TraceError;
-use once_cell::sync::Lazy;
 
-static GLOBAL_ERROR_HANDLER: Lazy<RwLock<Option<ErrorHandler>>> = Lazy::new(|| RwLock::new(None));
+static GLOBAL_ERROR_HANDLER: OnceCell<Option<ErrorHandler>> = OnceCell::new();
 
 /// Wrapper for error from both tracing and metrics part of open telemetry.
-#[derive(thiserror::Error, Debug)]
+#[derive(Debug)]
 #[non_exhaustive]
 pub enum Error {
     #[cfg(feature = "trace")]
     #[cfg_attr(docsrs, doc(cfg(feature = "trace")))]
-    #[error(transparent)]
+    // #[error(transparent)]
     /// Failed to export traces.
     Trace(#[from] TraceError),
     #[cfg(feature = "metrics")]
     #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))]
-    #[error(transparent)]
+    // #[error(transparent)]
     /// An issue raised by the metrics module.
-    Metric(#[from] MetricsError),
+    Metric(MetricsError),
 
     #[cfg(feature = "logs")]
     #[cfg_attr(docsrs, doc(cfg(feature = "logs")))]
-    #[error(transparent)]
+    // #[error(transparent)]
     /// Failed to export logs.
-    Log(#[from] LogError),
+    Log(LogError),
 
-    #[error(transparent)]
-    /// Error happens when injecting and extracting information using propagators.
-    Propagation(#[from] PropagationError),
-
-    #[error("{0}")]
+    // #[error(transparent)]
+    // /// Error happens when injecting and extracting information using propagators.
+    // Propagation(#[from] PropagationError),
+    // #[error("{0}")]
     /// Other types of failures not covered by the variants above.
     Other(String),
 }
 
-impl<T> From<PoisonError<T>> for Error {
-    fn from(err: PoisonError<T>) -> Self {
-        Error::Other(err.to_string())
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            #[cfg(feature = "trace")]
+            Error::Trace(err) => write!(f, "OpenTelemetry trace error occurred: {}", err),
+            #[cfg(feature = "metrics")]
+            Error::Metric(err) => write!(f, "OpenTelemetry metrics error occurred: {}", err),
+            #[cfg(feature = "logs")]
+            Error::Log(err) => write!(f, "OpenTelemetry log error occurred: {}", err),
+            Error::Other(msg) => write!(f, "OpenTelemetry error occurred: {}", msg),
+        }
     }
 }
 
@@ -54,9 +60,10 @@ struct ErrorHandler(Box<dyn Fn(Error) + Send + Sync>);
 ///
 /// Writes to stderr if unset.
 pub fn handle_error<T: Into<Error>>(err: T) {
-    match GLOBAL_ERROR_HANDLER.read() {
-        Ok(handler) if handler.is_some() => (handler.as_ref().unwrap().0)(err.into()),
-        _ => match err.into() {
+    if let Some(handler) = GLOBAL_ERROR_HANDLER.get() {
+        (handler.as_ref().unwrap().0)(err.into());
+    } else {
+        match err.into() {
             #[cfg(feature = "metrics")]
             #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))]
             Error::Metric(err) => eprintln!("OpenTelemetry metrics error occurred. {}", err),
@@ -66,21 +73,20 @@ pub fn handle_error<T: Into<Error>>(err: T) {
             #[cfg(feature = "logs")]
             #[cfg_attr(docsrs, doc(cfg(feature = "logs")))]
             Error::Log(err) => eprintln!("OpenTelemetry log error occurred. {}", err),
-            Error::Propagation(err) => {
-                eprintln!("OpenTelemetry propagation error occurred. {}", err)
-            }
+            // Error::Propagation(err) => {
+            //     eprintln!("OpenTelemetry propagation error occurred. {}", err)
+            // }
             Error::Other(err_msg) => eprintln!("OpenTelemetry error occurred. {}", err_msg),
-        },
+        }
     }
 }
 
 /// Set global error handler.
-pub fn set_error_handler<F>(f: F) -> std::result::Result<(), Error>
+pub fn set_error_handler<F>(f: F) -> Result<(), Error>
 where
     F: Fn(Error) + Send + Sync + 'static,
 {
     GLOBAL_ERROR_HANDLER
-        .write()
-        .map(|mut handler| *handler = Some(ErrorHandler(Box::new(f))))
-        .map_err(Into::into)
+        .set(Some(ErrorHandler(Box::new(f))))
+        .map_err(|_| Error::Other(String::from("Couldn't set GLOBAL_ERROR_HANDLER")))
 }
diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/global/metrics.rs b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/global/metrics.rs
@@ -2,13 +2,9 @@ extern crate alloc;
 
 use crate::metrics::{self, Meter, MeterProvider};
 use crate::KeyValue;
+use alloc::{borrow::Cow, sync::Arc, vec::Vec};
 use core::fmt;
 use oak_core::sync::OnceCell;
-use alloc::{
-  borrow::Cow,
-  vec::Vec,
-  sync::Arc,
-};
 
 /// The global `MeterProvider` singleton.
 static GLOBAL_METER_PROVIDER: OnceCell<GlobalMeterProvider> = OnceCell::new();
@@ -90,23 +86,25 @@ impl GlobalMeterProvider {
 pub fn set_meter_provider<P>(new_provider: P)
 where
     P: metrics::MeterProvider + Send + Sync + 'static,
-
 {
-    let _ = GLOBAL_METER_PROVIDER.set(GlobalMeterProvider::new(new_provider));
+    GLOBAL_METER_PROVIDER
+        .set(GlobalMeterProvider::new(new_provider))
+        .expect("Couldn't set GLOBAL_METER_PROVIDER");
 }
 
 /// Returns an instance of the currently configured global [`MeterProvider`]
 /// through [`GlobalMeterProvider`].
 pub fn meter_provider() -> GlobalMeterProvider {
-  if GLOBAL_METER_PROVIDER.get().is_none() {
-    let _ = GLOBAL_METER_PROVIDER.set(
-      GlobalMeterProvider::new(metrics::noop::NoopMeterProvider::new()));
-}
+    if GLOBAL_METER_PROVIDER.get().is_none() {
+        let _ = GLOBAL_METER_PROVIDER.set(GlobalMeterProvider::new(
+            metrics::noop::NoopMeterProvider::new(),
+        ));
+    }
 
-  GLOBAL_METER_PROVIDER
-      .get()
-      .expect("GLOBAL_METER_PROVIDER not initialized")
-      .clone()
+    GLOBAL_METER_PROVIDER
+        .get()
+        .expect("GLOBAL_METER_PROVIDER not initialized")
+        .clone()
 }
 
 /// Creates a named [`Meter`] via the configured [`GlobalMeterProvider`].

diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/global/mod.rs b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/global/mod.rs
@@ -125,15 +125,15 @@
 //! [`MeterProvider`]: crate::metrics::MeterProvider
 //! [`set_meter_provider`]: crate::global::set_meter_provider
 
-// mod error_handler;
+mod error_handler;
 #[cfg(feature = "metrics")]
 mod metrics;
 #[cfg(feature = "trace")]
 mod propagation;
 #[cfg(feature = "trace")]
 mod trace;
 
-// pub use error_handler::{handle_error, set_error_handler, Error};
+pub use error_handler::{handle_error, set_error_handler, Error};
 #[cfg(feature = "metrics")]
 #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))]
 pub use metrics::*;

diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/lib.rs b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/lib.rs
@@ -186,6 +186,7 @@
     html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/main/assets/logo.svg"
 )]
 #![cfg_attr(test, deny(warnings))]
+#![feature(error_in_core)]
 
 pub mod global;
 

diff --git a/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/metrics/mod.rs b/third_party/opentelemetry/opentelemetry-rust/opentelemetry/src/metrics/mod.rs
@@ -6,10 +6,10 @@ use alloc::{borrow::Cow, boxed::Box, string::String, sync::Arc, vec::Vec};
 use core::any::Any;
 use core::{
     cmp::Ordering,
+    fmt,
     hash::{Hash, Hasher},
     result,
 };
-// use thiserror::Error;
 
 mod instruments;
 mod meter;
@@ -28,7 +28,6 @@ pub use meter::{CallbackRegistration, Meter, MeterProvider, Observer};
 /// A specialized `Result` type for metric operations.
 pub type Result<T> = result::Result<T, MetricsError>;
 
-// (TODO) Port errors to no_std
 /// Errors returned by the metrics API.
 #[derive(Debug)]
 #[non_exhaustive]
@@ -55,11 +54,23 @@ impl<T: ExportError> From<T> for MetricsError {
     }
 }
 
-// impl<T> From<PoisonError<T>> for MetricsError {
-//     fn from(err: PoisonError<T>) -> Self {
-//         MetricsError::Other(err.to_string())
-//     }
-// }
+impl fmt::Display for MetricsError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            MetricsError::Other(msg) => write!(f, "Metrics error: {}", msg),
+            MetricsError::Config(msg) => write!(f, "Config error: {}", msg),
+            MetricsError::ExportErr(err) => write!(
+                f,
+                "Metrics exporter {} failed with {}",
+                err.exporter_name(),
+                err
+            ),
+            MetricsError::InvalidInstrumentConfiguration(msg) => {
+                write!(f, "Invalid instrument configuration: {}", msg)
+            }
+        }
+    }
+}
 
 struct F64Hashable(f64);