From 74e683d15d78e1ea6dbb0273a7bb243439062fcc Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sat, 28 Nov 2020 15:14:23 -0500 Subject: [PATCH] fix: address comments * Fix typo and naming * Use thiserror for stdout exporter and datadog exporter * Implement Error for opentelemetry::Error, renamed from OpenTelemetryError --- examples/actix-http/src/main.rs | 4 +- examples/actix-udp/src/main.rs | 4 +- examples/async/src/main.rs | 3 +- examples/basic-otlp/src/main.rs | 4 +- examples/basic/src/main.rs | 3 +- examples/grpc/src/client.rs | 4 +- examples/grpc/src/server.rs | 3 +- .../src/trace/exporter/datadog/model/mod.rs | 43 +++++-------------- opentelemetry-jaeger/src/lib.rs | 3 +- opentelemetry-otlp/src/lib.rs | 2 +- opentelemetry/src/api/mod.rs | 31 ++++--------- opentelemetry/src/exporter/trace/stdout.rs | 20 ++------- opentelemetry/src/global/error_handler.rs | 16 +++---- 13 files changed, 47 insertions(+), 93 deletions(-) diff --git a/examples/actix-http/src/main.rs b/examples/actix-http/src/main.rs index 355c2002cf..2a3ea6d1ec 100644 --- a/examples/actix-http/src/main.rs +++ b/examples/actix-http/src/main.rs @@ -6,11 +6,11 @@ use opentelemetry::{ trace::{FutureExt, TraceContextExt, Tracer}, Key, }; -use std::error::Error; +use opentelemetry::trace::TraceError; fn init_tracer() -> Result< (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - Box, + TraceError > { opentelemetry_jaeger::new_pipeline() .with_collector_endpoint("http://127.0.0.1:14268/api/traces") diff --git a/examples/actix-udp/src/main.rs b/examples/actix-udp/src/main.rs index 6204cad87c..b7ccc35b64 100644 --- a/examples/actix-udp/src/main.rs +++ b/examples/actix-udp/src/main.rs @@ -6,11 +6,11 @@ use opentelemetry::{ trace::{FutureExt, TraceContextExt, Tracer}, Key, }; -use std::error::Error; +use opentelemetry::trace::TraceError; fn init_tracer() -> Result< (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - Box, + TraceError > { opentelemetry_jaeger::new_pipeline() .with_agent_endpoint("localhost:6831") diff --git a/examples/async/src/main.rs b/examples/async/src/main.rs index b762c5e821..e0e84d8a2a 100644 --- a/examples/async/src/main.rs +++ b/examples/async/src/main.rs @@ -26,6 +26,7 @@ use opentelemetry::{ use std::{error::Error, io, net::SocketAddr}; use tokio::io::AsyncWriteExt; use tokio::net::TcpStream; +use opentelemetry::trace::TraceError; async fn connect(addr: &SocketAddr) -> io::Result { let tracer = global::tracer("connector"); @@ -54,7 +55,7 @@ async fn run(addr: &SocketAddr) -> io::Result { fn init_tracer() -> Result< (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - Box, + TraceError > { opentelemetry_jaeger::new_pipeline() .with_service_name("trace-demo") diff --git a/examples/basic-otlp/src/main.rs b/examples/basic-otlp/src/main.rs index cb61c2df9e..483f374e0b 100644 --- a/examples/basic-otlp/src/main.rs +++ b/examples/basic-otlp/src/main.rs @@ -10,13 +10,13 @@ use opentelemetry::{ use opentelemetry::{global, sdk::trace as sdktrace}; use std::error::Error; use std::time::Duration; +use opentelemetry::trace::TraceError; fn init_tracer( -) -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), Box> +) -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), TraceError> { opentelemetry_otlp::new_pipeline() .install() - .map_err::, _>(|err| Box::new(err)) } // Skip first immediate tick from tokio, not needed for async_std. diff --git a/examples/basic/src/main.rs b/examples/basic/src/main.rs index 24a04e53b7..fd5bb2f430 100644 --- a/examples/basic/src/main.rs +++ b/examples/basic/src/main.rs @@ -10,10 +10,11 @@ use opentelemetry::{ }; use std::error::Error; use std::time::Duration; +use opentelemetry::trace::TraceError; fn init_tracer() -> Result< (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - Box, + TraceError, > { opentelemetry_jaeger::new_pipeline() .with_service_name("trace-demo") diff --git a/examples/grpc/src/client.rs b/examples/grpc/src/client.rs index 721207eea7..c76ad459f0 100644 --- a/examples/grpc/src/client.rs +++ b/examples/grpc/src/client.rs @@ -6,14 +6,14 @@ use opentelemetry::{ trace::{TraceContextExt, Tracer}, Context, KeyValue, }; -use std::error::Error; +use opentelemetry::trace::TraceError; pub mod hello_world { tonic::include_proto!("helloworld"); } fn tracing_init( -) -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall), Box> +) -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall),TraceError> { global::set_text_map_propagator(TraceContextPropagator::new()); opentelemetry_jaeger::new_pipeline() diff --git a/examples/grpc/src/server.rs b/examples/grpc/src/server.rs index aec588b8f2..12c0ea09b1 100644 --- a/examples/grpc/src/server.rs +++ b/examples/grpc/src/server.rs @@ -9,6 +9,7 @@ use opentelemetry::{ KeyValue, }; use std::error::Error; +use opentelemetry::trace::TraceError; pub mod hello_world { tonic::include_proto!("helloworld"); // The string specified here must match the proto package name. @@ -37,7 +38,7 @@ impl Greeter for MyGreeter { } fn tracing_init( -) -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall), Box> +) -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall), TraceError> { global::set_text_map_propagator(TraceContextPropagator::new()); opentelemetry_jaeger::new_pipeline() diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs index 69bfcf0d89..b1def406da 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs @@ -1,58 +1,35 @@ -use http::uri::InvalidUri; use opentelemetry::exporter::trace; use opentelemetry::exporter::trace::ExportError; -use std::fmt; mod v03; mod v05; /// Wrap type for errors from opentelemetry datadog exporter -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum Error { /// Message pack error + #[error("message pack error")] MessagePackError, - /// No http client founded. User should provide one or enbale features + /// No http client founded. User should provide one or enable features + #[error("http client must be set, users can enable reqwest or surf feature to use http client implementation within create")] NoHttpClient, /// Http requests failed with following errors - RequestError(http::Error), - /// The Uri was invalid. - InvalidUri(http::uri::InvalidUri), + #[error(transparent)] + RequestError(#[from] http::Error), + /// The Uri was invalid + #[error(transparent)] + InvalidUri(#[from] http::uri::InvalidUri), /// Other errors + #[error("{0}")] Other(String), } -impl std::error::Error for Error {} - impl ExportError for Error { fn exporter_name(&self) -> &'static str { "datadog" } } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Error::MessagePackError => write!(f, "message pack error"), - Error::NoHttpClient => write!(f, "http client must be set, users can enable reqwest or surf feature to use http client implementation within create"), - Error::RequestError(err) => write!(f, "{}", err), - Error::InvalidUri(err) => write!(f, "{}", err), - Error::Other(msg) => write!(f, "{}", msg) - } - } -} - -impl From for Error { - fn from(err: InvalidUri) -> Self { - Error::InvalidUri(err) - } -} - -impl From for Error { - fn from(err: http::Error) -> Self { - Error::RequestError(err) - } -} - impl From for Error { fn from(_: rmp::encode::ValueWriteError) -> Self { Self::MessagePackError diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index c2ae20c5bf..19a7edee2f 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -206,6 +206,7 @@ use std::{ time::{Duration, SystemTime}, }; use uploader::BatchUploader; +use opentelemetry::trace::TraceError; /// Default service name if no service is configured. const DEFAULT_SERVICE_NAME: &str = "OpenTelemetry"; @@ -409,7 +410,7 @@ impl PipelineBuilder { /// Install a Jaeger pipeline with the recommended defaults. pub fn install( self, - ) -> Result<(sdk::trace::Tracer, Uninstall), Box> + ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { let tracer_provider = self.build()?; let tracer = diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 0a043f06db..dae73a5919 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -246,6 +246,6 @@ pub enum Error { impl ExportError for Error { fn exporter_name(&self) -> &'static str { - "otel" + "otlp" } } diff --git a/opentelemetry/src/api/mod.rs b/opentelemetry/src/api/mod.rs index 80807af80c..27cd1bea4c 100644 --- a/opentelemetry/src/api/mod.rs +++ b/opentelemetry/src/api/mod.rs @@ -19,35 +19,22 @@ pub mod propagation; pub mod trace; /// Wrapper for error from both tracing and metrics part of open telemetry. -#[derive(Debug)] -pub enum OpenTelemetryError { +#[derive(thiserror::Error, Debug)] +pub enum Error { #[cfg(feature = "trace")] #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] - TraceErr(TraceError), + #[error(transparent)] + Trace(#[from] TraceError), #[cfg(feature = "metrics")] #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] - MetricErr(MetricsError), + #[error(transparent)] + Metric(#[from] MetricsError), + #[error("{0}")] Other(String), } -#[cfg(feature = "trace")] -#[cfg_attr(docsrs, doc(cfg(feature = "trace")))] -impl From for OpenTelemetryError { - fn from(err: TraceError) -> Self { - OpenTelemetryError::TraceErr(err) - } -} - -#[cfg(feature = "metrics")] -#[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] -impl From for OpenTelemetryError { - fn from(err: MetricsError) -> Self { - OpenTelemetryError::MetricErr(err) - } -} - -impl From> for OpenTelemetryError { +impl From> for Error { fn from(err: PoisonError) -> Self { - OpenTelemetryError::Other(err.to_string()) + Error::Other(err.to_string()) } } diff --git a/opentelemetry/src/exporter/trace/stdout.rs b/opentelemetry/src/exporter/trace/stdout.rs index 44a14c50d2..cbf01635da 100644 --- a/opentelemetry/src/exporter/trace/stdout.rs +++ b/opentelemetry/src/exporter/trace/stdout.rs @@ -31,7 +31,6 @@ use crate::{ trace::TracerProvider, }; use async_trait::async_trait; -use std::fmt::{Debug, Display, Formatter}; use std::io::{stdout, Stdout, Write}; /// Pipeline builder @@ -150,22 +149,9 @@ where pub struct Uninstall(global::TracerProviderGuard); /// Stdout exporter's error -#[derive(Debug)] -struct Error(std::io::Error); - -impl Display for Error { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - f.write_str(self.0.to_string().as_str()) - } -} - -impl std::error::Error for Error {} - -impl From for Error { - fn from(err: std::io::Error) -> Self { - Error(err) - } -} +#[derive(thiserror::Error, Debug)] +#[error(transparent)] +struct Error(#[from] std::io::Error); impl ExportError for Error { fn exporter_name(&self) -> &'static str { diff --git a/opentelemetry/src/global/error_handler.rs b/opentelemetry/src/global/error_handler.rs index 7fe58b9d8b..fcf92cebba 100644 --- a/opentelemetry/src/global/error_handler.rs +++ b/opentelemetry/src/global/error_handler.rs @@ -1,4 +1,4 @@ -use crate::api::OpenTelemetryError; +use crate::api::Error; use std::sync::RwLock; lazy_static::lazy_static! { @@ -6,26 +6,26 @@ lazy_static::lazy_static! { static ref GLOBAL_ERROR_HANDLER: RwLock> = RwLock::new(None); } -struct ErrorHandler(Box); +struct ErrorHandler(Box); /// Handle error using the globally configured error handler. /// /// Writes to stderr if unset. -pub fn handle_error>(err: T) { +pub fn handle_error>(err: T) { match GLOBAL_ERROR_HANDLER.read() { Ok(handler) if handler.is_some() => (handler.as_ref().unwrap().0)(err.into()), _ => match err.into() { #[cfg(feature = "metrics")] #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] - OpenTelemetryError::MetricErr(err) => { + Error::Metric(err) => { eprintln!("OpenTelemetry metrics error occurred {:?}", err) } #[cfg(feature = "trace")] #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] - OpenTelemetryError::TraceErr(err) => { + Error::Trace(err) => { eprintln!("OpenTelemetry trace error occurred {:?}", err) } - OpenTelemetryError::Other(err_msg) => { + Error::Other(err_msg) => { println!("OpenTelemetry error occurred {}", err_msg) } }, @@ -33,9 +33,9 @@ pub fn handle_error>(err: T) { } /// Set global error handler. -pub fn set_error_handler(f: F) -> std::result::Result<(), OpenTelemetryError> +pub fn set_error_handler(f: F) -> std::result::Result<(), Error> where - F: Fn(OpenTelemetryError) + Send + Sync + 'static, + F: Fn(Error) + Send + Sync + 'static, { GLOBAL_ERROR_HANDLER .write()