From 128178dabb83f93a5399254160bbefd4290d9b1b Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sun, 22 Nov 2020 20:20:27 -0500 Subject: [PATCH 01/18] Add Boxed variant in TraceError enum. It could help propagate error from upstream components. --- opentelemetry/src/api/trace/mod.rs | 31 ++++++++- opentelemetry/src/sdk/trace/span_processor.rs | 64 ++++++++++--------- 2 files changed, 63 insertions(+), 32 deletions(-) diff --git a/opentelemetry/src/api/trace/mod.rs b/opentelemetry/src/api/trace/mod.rs index 075cd88697..cc636e4849 100644 --- a/opentelemetry/src/api/trace/mod.rs +++ b/opentelemetry/src/api/trace/mod.rs @@ -110,6 +110,7 @@ //! Please review the W3C specification for details on the [Tracestate //! field](https://www.w3.org/TR/trace-context/#tracestate-field). //! +use ::futures::channel::{mpsc::TrySendError, oneshot::Canceled}; use thiserror::Error; mod context; @@ -139,11 +140,39 @@ pub use self::{ tracer::{SpanBuilder, Tracer}, }; +/// Describe the result of operations in tracing API. +pub type TraceResult = Result; + /// Errors returned by the trace API. -#[derive(Error, Debug, PartialEq)] +#[derive(Error, Debug)] #[non_exhaustive] pub enum TraceError { /// Other errors not covered by specific cases. #[error("Trace error: {0}")] Other(String), + + /// Error propagated from trace SDK + #[error(transparent)] + Boxed(#[from] Box), +} + +impl From> for TraceError +where + T: std::error::Error + Send + Sync + 'static, +{ + fn from(error: Box) -> Self { + TraceError::Boxed(error) + } +} + +impl From> for TraceError { + fn from(err: TrySendError) -> Self { + TraceError::Boxed(Box::new(err.into_send_error())) + } +} + +impl From for TraceError { + fn from(err: Canceled) -> Self { + TraceError::Boxed(Box::new(err)) + } } diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 7ce7c2b5f3..002832a3be 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -33,18 +33,20 @@ //! //! [`is_recording`]: ../span/trait.Span.html#method.is_recording //! [`TracerProvider`]: ../provider/trait.TracerProvider.html -use crate::api::trace::TraceError; +use std::{fmt, str::FromStr, sync::Mutex, time}; + +use futures::{ + channel::mpsc, channel::oneshot, executor, future::BoxFuture, future::Either, pin_mut, Future, + FutureExt, Stream, StreamExt, +}; + +use crate::api::trace::{TraceError, TraceResult}; use crate::exporter::trace::ExportTimedOutError; use crate::sdk::trace::Span; use crate::{ exporter::trace::{ExportResult, SpanData, SpanExporter}, Context, }; -use futures::{ - channel::mpsc, channel::oneshot, executor, future::BoxFuture, future::Either, pin_mut, Future, - FutureExt, Stream, StreamExt, -}; -use std::{fmt, str::FromStr, sync::Mutex, time}; /// Delay interval between two consecutive exports, default to be 5000. const OTEL_BSP_SCHEDULE_DELAY_MILLIS: &str = "OTEL_BSP_SCHEDULE_DELAY_MILLIS"; @@ -76,10 +78,10 @@ pub trait SpanProcessor: Send + Sync + std::fmt::Debug { /// API, therefore it should not block or throw an exception. fn on_end(&self, span: SpanData); /// Force the spans lying in the cache to be exported. - fn force_flush(&self) -> Result<(), Box>; + fn force_flush(&self) -> TraceResult<()>; /// Shuts down the processor. Called when SDK is shut down. This is an /// opportunity for processors to do any cleanup required. - fn shutdown(&mut self) -> Result<(), Box>; + fn shutdown(&mut self) -> TraceResult<()>; } /// A [`SpanProcessor`] that exports synchronously when spans are finished. @@ -130,12 +132,12 @@ impl SpanProcessor for SimpleSpanProcessor { } } - fn force_flush(&self) -> Result<(), Box> { + fn force_flush(&self) -> TraceResult<()> { // Ignored since all spans in Simple Processor will be exported as they ended. Ok(()) } - fn shutdown(&mut self) -> Result<(), Box> { + fn shutdown(&mut self) -> TraceResult<()> { if let Ok(mut exporter) = self.exporter.lock() { exporter.shutdown(); Ok(()) @@ -143,8 +145,7 @@ impl SpanProcessor for SimpleSpanProcessor { Err(TraceError::Other( "When shutting down the SimpleSpanProcessor, the exporter's lock has been poisoned" .into(), - ) - .into()) + )) } } } @@ -214,26 +215,24 @@ impl SpanProcessor for BatchSpanProcessor { } } - fn force_flush(&self) -> Result<(), Box> { + fn force_flush(&self) -> TraceResult<()> { let mut sender = self.message_sender.lock().map_err(|_| TraceError::Other("When force flushing the BatchSpanProcessor, the message sender's lock has been poisoned".into()))?; let (res_sender, res_receiver) = oneshot::channel::>(); - sender.try_send(BatchMessage::Flush(Some(res_sender)))?; + sender + .try_send(BatchMessage::Flush(Some(res_sender))) + .map_err(|err| Box::new(err))?; for result in futures::executor::block_on(res_receiver)? { - if result.is_err() { - return result; - } + result?; } Ok(()) } - fn shutdown(&mut self) -> Result<(), Box> { + fn shutdown(&mut self) -> TraceResult<()> { let mut sender = self.message_sender.lock().map_err(|_| TraceError::Other("When shutting down the BatchSpanProcessor, the message sender's lock has been poisoned".into()))?; let (res_sender, res_receiver) = oneshot::channel::>(); sender.try_send(BatchMessage::Shutdown(res_sender))?; for result in futures::executor::block_on(res_receiver)? { - if result.is_err() { - return result; - } + result?; } Ok(()) } @@ -565,22 +564,25 @@ where #[cfg(test)] mod tests { - use super::{ - BatchSpanProcessor, SimpleSpanProcessor, SpanProcessor, OTEL_BSP_MAX_EXPORT_BATCH_SIZE, - OTEL_BSP_MAX_QUEUE_SIZE, OTEL_BSP_MAX_QUEUE_SIZE_DEFAULT, OTEL_BSP_SCHEDULE_DELAY_MILLIS, - OTEL_BSP_SCHEDULE_DELAY_MILLIS_DEFAULT, - }; + use std::fmt::Debug; + use std::time; + use std::time::Duration; + + use async_std::prelude::*; + use async_trait::async_trait; + use crate::exporter::trace::{stdout, ExportResult, SpanData, SpanExporter}; use crate::sdk::trace::span_processor::OTEL_BSP_EXPORT_TIMEOUT_MILLIS; use crate::sdk::trace::BatchConfig; use crate::testing::trace::{ new_test_export_span_data, new_test_exporter, new_tokio_test_exporter, }; - use async_std::prelude::*; - use async_trait::async_trait; - use std::fmt::Debug; - use std::time; - use std::time::Duration; + + use super::{ + BatchSpanProcessor, SimpleSpanProcessor, SpanProcessor, OTEL_BSP_MAX_EXPORT_BATCH_SIZE, + OTEL_BSP_MAX_QUEUE_SIZE, OTEL_BSP_MAX_QUEUE_SIZE_DEFAULT, OTEL_BSP_SCHEDULE_DELAY_MILLIS, + OTEL_BSP_SCHEDULE_DELAY_MILLIS_DEFAULT, + }; #[test] fn simple_span_processor_on_end_calls_export() { From ac2ffc8041426fe4b4b2913f19aeb5780e4dd677 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Tue, 24 Nov 2020 21:46:11 -0500 Subject: [PATCH 02/18] feat(error_handle): add export errors. Export error will consist two parts. The first one is two variant in TraceError. One for timed out, the other for all other errors. The other part is a trait ExportError, which extended the Error trait by asking user to provide the name of the exporter. Users should implement their own Error for their exporters and implement ExportError on those errors. (todo): Add errors for all exporters --- opentelemetry/src/api/mod.rs | 36 ++++++ opentelemetry/src/api/trace/mod.rs | 22 ++-- opentelemetry/src/exporter/trace/mod.rs | 22 ++-- opentelemetry/src/exporter/trace/stdout.rs | 43 +++++-- opentelemetry/src/global/error_handler.rs | 20 ++-- opentelemetry/src/sdk/trace/span_processor.rs | 108 +++++++++--------- 6 files changed, 165 insertions(+), 86 deletions(-) diff --git a/opentelemetry/src/api/mod.rs b/opentelemetry/src/api/mod.rs index bc02866dae..2007b8067d 100644 --- a/opentelemetry/src/api/mod.rs +++ b/opentelemetry/src/api/mod.rs @@ -1,3 +1,9 @@ +use crate::api::trace::TraceError; +#[cfg(feature = "metrics")] +#[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] +use crate::api::metrics::MetricsError; +use std::sync::PoisonError; + pub mod baggage; pub(crate) mod context; pub(crate) mod core; @@ -11,3 +17,33 @@ pub mod propagation; #[cfg(feature = "trace")] #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] pub mod trace; + +/// Wrapper for error from both tracing and metrics part of open telemetry. +#[derive(Debug)] +pub enum OpenTelemetryError { + #[cfg(feature = "trace")] + #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] + TraceErr(TraceError), + #[cfg(feature = "metrics")] + #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] + MetricErr(MetricsError), + Other(String), +} + +impl From for OpenTelemetryError { + fn from(err: TraceError) -> Self { + OpenTelemetryError::TraceErr(err) + } +} + +impl From for OpenTelemetryError { + fn from(err: MetricsError) -> Self { + OpenTelemetryError::MetricErr(err) + } +} + +impl From> for OpenTelemetryError { + fn from(err: PoisonError) -> Self { + OpenTelemetryError::Other(err.to_string()) + } +} \ No newline at end of file diff --git a/opentelemetry/src/api/trace/mod.rs b/opentelemetry/src/api/trace/mod.rs index cc636e4849..80b887f8e8 100644 --- a/opentelemetry/src/api/trace/mod.rs +++ b/opentelemetry/src/api/trace/mod.rs @@ -139,6 +139,8 @@ pub use self::{ }, tracer::{SpanBuilder, Tracer}, }; +use std::time; +use crate::exporter::trace::ExportError; /// Describe the result of operations in tracing API. pub type TraceResult = Result; @@ -151,17 +153,23 @@ pub enum TraceError { #[error("Trace error: {0}")] Other(String), - /// Error propagated from trace SDK + /// Export failed with the error returned by the exporter + #[error("Exporting failed with {0}")] + ExportFailed(Box), + + /// Export failed to finish after certain period and processor stopped the export. + #[error("Exporting timed out after {} seconds", .0.as_secs())] + ExportTimedOut(time::Duration), + + /// Other errors propagated from trace SDK that weren't covered above #[error(transparent)] Boxed(#[from] Box), } -impl From> for TraceError -where - T: std::error::Error + Send + Sync + 'static, -{ - fn from(error: Box) -> Self { - TraceError::Boxed(error) +impl From for TraceError + where T: ExportError { + fn from(err: T) -> Self { + TraceError::ExportFailed(Box::new(err)) } } diff --git a/opentelemetry/src/exporter/trace/mod.rs b/opentelemetry/src/exporter/trace/mod.rs index 549a448c42..7e8516b10f 100644 --- a/opentelemetry/src/exporter/trace/mod.rs +++ b/opentelemetry/src/exporter/trace/mod.rs @@ -10,27 +10,27 @@ use http::Request; use serde::{Deserialize, Serialize}; #[cfg(all(feature = "http", feature = "reqwest"))] use std::convert::TryInto; -use std::error::Error; -use std::fmt::{Debug, Display}; +use std::fmt::Debug; use std::sync::Arc; use std::time::SystemTime; +use crate::api::trace::TraceError; pub mod stdout; /// Describes the result of an export. -pub type ExportResult = Result<(), Box>; +pub type ExportResult = Result<(), TraceError>; -/// Timed out when exporting spans to remote -#[derive(Debug, Default)] -pub struct ExportTimedOutError {} - -impl Display for ExportTimedOutError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str("export timed out") +/// Marker trait for errors returned by exporters +pub trait ExportError: std::error::Error + Send + Sync + 'static { + /// The name of exporter that returned this error + fn exporter_name(&self) -> &'static str { + "N/A" } } -impl Error for ExportTimedOutError {} +#[cfg(all(feature = "reqwest", feature = "http"))] +#[async_trait] +impl ExportError for reqwest::Error {} /// `SpanExporter` defines the interface that protocol-specific exporters must /// implement so that they can be plugged into OpenTelemetry SDK and support diff --git a/opentelemetry/src/exporter/trace/stdout.rs b/opentelemetry/src/exporter/trace/stdout.rs index 1b257e8382..00f5c15d17 100644 --- a/opentelemetry/src/exporter/trace/stdout.rs +++ b/opentelemetry/src/exporter/trace/stdout.rs @@ -30,8 +30,10 @@ use crate::{ trace::TracerProvider, }; use async_trait::async_trait; -use std::fmt::Debug; +use std::fmt::{Debug, Display}; use std::io::{stdout, Stdout, Write}; +use crate::exporter::trace::ExportError; +use serde::export::Formatter; /// Pipeline builder #[derive(Debug)] @@ -81,8 +83,8 @@ impl PipelineBuilder { } impl PipelineBuilder -where - W: Write + Debug + Send + 'static, + where + W: Write + Debug + Send + 'static, { /// Install the stdout exporter pipeline with the recommended defaults. pub fn install(mut self) -> (sdk::trace::Tracer, Uninstall) { @@ -123,16 +125,16 @@ impl Exporter { #[async_trait] impl SpanExporter for Exporter -where - W: Write + Debug + Send + 'static, + where + W: Write + Debug + Send + 'static, { /// Export spans to stdout async fn export(&mut self, batch: Vec) -> ExportResult { for span in batch { if self.pretty_print { - self.writer.write_all(format!("{:#?}\n", span).as_bytes())?; + self.writer.write_all(format!("{:#?}\n", span).as_bytes()).map_err(|err| Error::from(err))?; } else { - self.writer.write_all(format!("{:?}\n", span).as_bytes())?; + self.writer.write_all(format!("{:?}\n", span).as_bytes()).map_err(|err| Error::from(err))?; } } @@ -140,6 +142,31 @@ where } } -/// Uninstalls the stdout pipeline on drop. +/// Uninstalls the stdout pipeline on dropping. #[derive(Debug)] 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) + } +} + +impl ExportError for Error { + fn exporter_name(&self) -> &'static str { + "stdout" + } +} + diff --git a/opentelemetry/src/global/error_handler.rs b/opentelemetry/src/global/error_handler.rs index bc4b1c65e9..a79c3ead75 100644 --- a/opentelemetry/src/global/error_handler.rs +++ b/opentelemetry/src/global/error_handler.rs @@ -1,27 +1,31 @@ -use crate::metrics::{MetricsError, Result}; use std::sync::RwLock; +use crate::api::OpenTelemetryError; lazy_static::lazy_static! { /// The global error handler. 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: MetricsError) { +pub fn handle_error>(err: T) { match GLOBAL_ERROR_HANDLER.read() { - Ok(handler) if handler.is_some() => (handler.as_ref().unwrap().0)(err), - _ => eprintln!("OpenTelemetry metrics error occurred {:?}", err), + Ok(handler) if handler.is_some() => (handler.as_ref().unwrap().0)(err.into()), + _ => match err.into() { + OpenTelemetryError::MetricErr(err) => eprintln!("OpenTelemetry metrics error occurred {:?}", err), + OpenTelemetryError::TraceErr(err) => eprintln!("OpenTelemetry trace error occurred {:?}", err), + OpenTelemetryError::Other(err_msg) => println!("OpenTelemetry error occurred {}", err_msg) + } } } /// Set global error handler. -pub fn set_error_handler(f: F) -> Result<()> -where - F: Fn(MetricsError) + Send + Sync + 'static, +pub fn set_error_handler(f: F) -> std::result::Result<(), OpenTelemetryError> + where + F: Fn(OpenTelemetryError) + Send + Sync + 'static, { GLOBAL_ERROR_HANDLER .write() diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 002832a3be..745a1caafc 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -41,7 +41,7 @@ use futures::{ }; use crate::api::trace::{TraceError, TraceResult}; -use crate::exporter::trace::ExportTimedOutError; +use crate::global; use crate::sdk::trace::Span; use crate::{ exporter::trace::{ExportResult, SpanData, SpanExporter}, @@ -127,8 +127,9 @@ impl SpanProcessor for SimpleSpanProcessor { fn on_end(&self, span: SpanData) { if let Ok(mut exporter) = self.exporter.lock() { - // TODO: Surface error through global error handler let _result = executor::block_on(exporter.export(vec![span])); + } else { + global::handle_error(TraceError::Other("When export span with the SimpleSpanProcessor, the exporter's lock has been poisoned".to_string())); } } @@ -219,8 +220,7 @@ impl SpanProcessor for BatchSpanProcessor { let mut sender = self.message_sender.lock().map_err(|_| TraceError::Other("When force flushing the BatchSpanProcessor, the message sender's lock has been poisoned".into()))?; let (res_sender, res_receiver) = oneshot::channel::>(); sender - .try_send(BatchMessage::Flush(Some(res_sender))) - .map_err(|err| Box::new(err))?; + .try_send(BatchMessage::Flush(Some(res_sender)))?; for result in futures::executor::block_on(res_receiver)? { result?; } @@ -253,13 +253,13 @@ impl BatchSpanProcessor { delay: D, config: BatchConfig, ) -> Self - where - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IS, - IS: Stream + Send + 'static, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IS, + IS: Stream + Send + 'static, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { let (message_sender, message_receiver) = mpsc::channel(config.max_queue_size); let ticker = interval(config.scheduled_delay).map(|_| BatchMessage::Flush(None)); @@ -293,11 +293,13 @@ impl BatchSpanProcessor { &delay, batch, ) - .await, + .await, ); } - // TODO: Surface error through global error handler - let _send_result = ch.send(results); + let send_result = ch.send(results); + if let Err(_) = send_result { + global::handle_error(TraceError::Other("fail to send the export response from worker handle in BatchProcessor".to_string())) + } } BatchMessage::Flush(None) => { while !spans.is_empty() { @@ -311,7 +313,7 @@ impl BatchSpanProcessor { &delay, batch, ) - .await; + .await; } } // Stream has terminated or processor is shutdown, return to finish execution. @@ -330,18 +332,20 @@ impl BatchSpanProcessor { &delay, batch, ) - .await, + .await, ); } exporter.shutdown(); - // TODO: Surface error through global error handler - let _send_result = ch.send(results); + let send_result = ch.send(results); + if let Err(_) = send_result { + global::handle_error(TraceError::Other("fail to send the export response from worker handle in BatchProcessor".to_string())) + } break; } } } })) - .map(|_| ()); + .map(|_| ()); // Return batch processor with link to worker BatchSpanProcessor { @@ -356,13 +360,13 @@ impl BatchSpanProcessor { delay: D, interval: I, ) -> BatchSpanProcessorBuilder - where - E: SpanExporter, - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IO, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + E: SpanExporter, + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IO, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { BatchSpanProcessorBuilder { exporter, @@ -386,13 +390,13 @@ impl BatchSpanProcessor { interval: I, delay: D, ) -> BatchSpanProcessorBuilder - where - E: SpanExporter, - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IO, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + E: SpanExporter, + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IO, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { let mut config = BatchConfig::default(); let schedule_delay = std::env::var(OTEL_BSP_SCHEDULE_DELAY_MILLIS) @@ -443,10 +447,10 @@ async fn export_with_timeout( delay: &D, batch: Vec, ) -> ExportResult -where - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, - E: SpanExporter + ?Sized, + where + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, + E: SpanExporter + ?Sized, { let export = exporter.export(batch); let timeout = delay(time_out); @@ -454,7 +458,7 @@ where pin_mut!(timeout); match futures::future::select(export, timeout).await { Either::Left((export_res, _)) => export_res, - Either::Right((_, _)) => ExportResult::Err(Box::new(ExportTimedOutError::default())), + Either::Right((_, _)) => ExportResult::Err(TraceError::ExportTimedOut(time_out)), } } @@ -503,14 +507,14 @@ pub struct BatchSpanProcessorBuilder { } impl BatchSpanProcessorBuilder -where - E: SpanExporter + 'static, - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IS, - IS: Stream + Send + 'static, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + E: SpanExporter + 'static, + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IS, + IS: Stream + Send + 'static, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { /// Set max queue size for batches pub fn with_max_queue_size(self, size: usize) -> Self { @@ -680,9 +684,9 @@ mod tests { } impl Debug for BlockingExporter - where - D: Fn(time::Duration) -> DS + 'static + Send + Sync, - DS: Future + Send + Sync + 'static, + where + D: Fn(time::Duration) -> DS + 'static + Send + Sync, + DS: Future + Send + Sync + 'static, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str("blocking exporter for testing") @@ -691,9 +695,9 @@ mod tests { #[async_trait] impl SpanExporter for BlockingExporter - where - D: Fn(time::Duration) -> DS + 'static + Send + Sync, - DS: Future + Send + Sync + 'static, + where + D: Fn(time::Duration) -> DS + 'static + Send + Sync, + DS: Future + Send + Sync + 'static, { async fn export(&mut self, batch: Vec) -> ExportResult { println!("Accepting {} spans", batch.len()); From 81bc7f25493439599fb92cfdf7a3a124a1cc9e0a Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 14:46:34 -0500 Subject: [PATCH 03/18] feat: add the error type in opentelemetry-zipkin --- opentelemetry-zipkin/Cargo.toml | 1 + opentelemetry-zipkin/src/lib.rs | 76 ++++++++++++++++++---------- opentelemetry-zipkin/src/uploader.rs | 3 +- 3 files changed, 52 insertions(+), 28 deletions(-) diff --git a/opentelemetry-zipkin/Cargo.toml b/opentelemetry-zipkin/Cargo.toml index e62a87b538..08ed750ca3 100644 --- a/opentelemetry-zipkin/Cargo.toml +++ b/opentelemetry-zipkin/Cargo.toml @@ -35,6 +35,7 @@ lazy_static = "1.4" http = "0.2" reqwest = { version = "0.10", optional = true } surf = { version = "2.0", optional = true } +thiserror = { version = "1.0"} [dev-dependencies] isahc = "=0.9.6" diff --git a/opentelemetry-zipkin/src/lib.rs b/opentelemetry-zipkin/src/lib.rs index 0538e8e57a..c9be54aa14 100644 --- a/opentelemetry-zipkin/src/lib.rs +++ b/opentelemetry-zipkin/src/lib.rs @@ -148,17 +148,17 @@ //! supported compiler version is not considered a semver breaking change as //! long as doing so complies with this policy. #![warn( - future_incompatible, - missing_debug_implementations, - missing_docs, - nonstandard_style, - rust_2018_idioms, - unreachable_pub, - unused +future_incompatible, +missing_debug_implementations, +missing_docs, +nonstandard_style, +rust_2018_idioms, +unreachable_pub, +unused )] #![cfg_attr(docsrs, feature(doc_cfg), deny(broken_intra_doc_links))] #![doc( - html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" +html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" )] #![cfg_attr(test, deny(warnings))] @@ -171,11 +171,10 @@ mod uploader; use async_trait::async_trait; use http::Uri; use model::endpoint::Endpoint; -use opentelemetry::exporter::trace::HttpClient; +use opentelemetry::exporter::trace::{HttpClient, ExportError}; use opentelemetry::{exporter::trace, global, sdk, trace::TracerProvider}; -use std::error::Error; -use std::io; use std::net::SocketAddr; +use opentelemetry::trace::TraceError; /// Default Zipkin collector endpoint const DEFAULT_COLLECTOR_ENDPOINT: &str = "http://127.0.0.1:9411/api/v2/spans"; @@ -220,21 +219,21 @@ impl Default for ZipkinPipelineBuilder { #[cfg(feature = "reqwest-blocking-client")] client: Some(Box::new(reqwest::blocking::Client::new())), #[cfg(all( - not(feature = "reqwest-blocking-client"), - not(feature = "surf-client"), - feature = "reqwest-client" + not(feature = "reqwest-blocking-client"), + not(feature = "surf-client"), + feature = "reqwest-client" ))] client: Some(Box::new(reqwest::Client::new())), #[cfg(all( - not(feature = "reqwest-client"), - not(feature = "reqwest-blocking-client"), - feature = "surf-client" + not(feature = "reqwest-client"), + not(feature = "reqwest-blocking-client"), + feature = "surf-client" ))] client: Some(Box::new(surf::Client::new())), #[cfg(all( - not(feature = "reqwest-client"), - not(feature = "surf-client"), - not(feature = "reqwest-blocking-client") + not(feature = "reqwest-client"), + not(feature = "surf-client"), + not(feature = "reqwest-blocking-client") ))] client: None, @@ -250,10 +249,10 @@ impl ZipkinPipelineBuilder { /// Create `ExporterConfig` struct from current `ExporterConfigBuilder` pub fn install( mut self, - ) -> Result<(sdk::trace::Tracer, Uninstall), Box> { + ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { if let Some(client) = self.client { let endpoint = Endpoint::new(self.service_name, self.service_addr); - let exporter = Exporter::new(endpoint, client, self.collector_endpoint.parse()?); + let exporter = Exporter::new(endpoint, client, self.collector_endpoint.parse().map_err::(Into::into)?); let mut provider_builder = sdk::trace::TracerProvider::builder().with_exporter(exporter); @@ -267,11 +266,7 @@ impl ZipkinPipelineBuilder { Ok((tracer, Uninstall(provider_guard))) } else { - Err(Box::new(io::Error::new( - io::ErrorKind::Other, - "http client must be set, users can enable reqwest or surf feature to use http \ - client implementation within create", - ))) + Err(Error::NoHttpClient.into()) } } @@ -322,3 +317,30 @@ impl trace::SpanExporter for Exporter { /// Uninstalls the Zipkin pipeline on drop. #[derive(Debug)] pub struct Uninstall(global::TracerProviderGuard); + +/// Wrap type for errors from opentelemetry zipkin +#[derive(thiserror::Error, Debug)] +#[non_exhaustive] +pub enum Error { + /// No http client implementation found. 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 + #[error("http request failed with {0}")] + RequestFailed(#[from] http::Error), + + /// The uri provided is invalid + #[error("invalid uri")] + InvalidUri(#[from] http::uri::InvalidUri), + + /// Other errors + #[error("export error: {0}")] + Other(String), +} + +impl ExportError for Error { + fn exporter_name(&self) -> &'static str { + "zipkin" + } +} \ No newline at end of file diff --git a/opentelemetry-zipkin/src/uploader.rs b/opentelemetry-zipkin/src/uploader.rs index 3c67d9a35a..31db25b53b 100644 --- a/opentelemetry-zipkin/src/uploader.rs +++ b/opentelemetry-zipkin/src/uploader.rs @@ -3,6 +3,7 @@ use crate::model::span::Span; use http::{header::CONTENT_TYPE, Method, Request, Uri}; use opentelemetry::exporter::trace::{ExportResult, HttpClient}; use std::fmt::Debug; +use crate::Error; #[derive(Debug)] pub(crate) enum Uploader { @@ -38,7 +39,7 @@ impl JsonV2Client { .method(Method::POST) .uri(self.collector_endpoint.clone()) .header(CONTENT_TYPE, "application/json") - .body(serde_json::to_vec(&spans).unwrap_or_default())?; + .body(serde_json::to_vec(&spans).unwrap_or_default()).map_err::(Into::into)?; self.client.send(req).await } } From 7f782f2ea05c0f863c8a62051ee6fec87132bf14 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 15:03:49 -0500 Subject: [PATCH 04/18] feat: add the error type in opentelemetry-otlp --- opentelemetry-otlp/Cargo.toml | 1 + opentelemetry-otlp/src/lib.rs | 20 ++++++++++++++++++-- opentelemetry-otlp/src/span.rs | 5 +++-- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/opentelemetry-otlp/Cargo.toml b/opentelemetry-otlp/Cargo.toml index beee91f5c6..353de4fff1 100644 --- a/opentelemetry-otlp/Cargo.toml +++ b/opentelemetry-otlp/Cargo.toml @@ -26,6 +26,7 @@ futures = "0.3" grpcio = "0.6" opentelemetry = { version = "0.10", default-features = false, features = ["trace"], path = "../opentelemetry" } protobuf = "2.18" +thiserror = "1.0" [features] openssl = ["grpcio/openssl"] diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 307ed2a8e2..103e24d29a 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -122,7 +122,6 @@ use opentelemetry::{global, sdk, trace::TracerProvider}; use std::collections::HashMap; -use std::error::Error; use std::time::Duration; #[allow(clippy::all, unreachable_pub, dead_code)] @@ -132,6 +131,8 @@ mod span; mod transform; pub use crate::span::{Compression, Credentials, Exporter, ExporterConfig, Protocol}; +use opentelemetry::trace::TraceError; +use opentelemetry::exporter::trace::ExportError; /// Create a new pipeline builder with the recommended configuration. /// @@ -217,7 +218,7 @@ impl OtlpPipelineBuilder { /// Install the OTLP exporter pipeline with the recommended defaults. pub fn install( mut self, - ) -> Result<(sdk::trace::Tracer, Uninstall), Box> { + ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { let exporter = Exporter::new(self.exporter_config); let mut provider_builder = sdk::trace::TracerProvider::builder().with_exporter(exporter); @@ -235,3 +236,18 @@ impl OtlpPipelineBuilder { /// Uninstalls the OTLP pipeline on drop #[derive(Debug)] pub struct Uninstall(global::TracerProviderGuard); + +/// Wrap type for errors from opentelemetry otel +#[derive(thiserror::Error, Debug)] +pub enum Error{ + // FIXME: wait until https://github.com/open-telemetry/opentelemetry-rust/pull/352 merged + /// Error from grpcio module + #[error("grpcio error {0}")] + Grpcio(#[from] grpcio::Error) +} + +impl ExportError for Error { + fn exporter_name(&self) -> &'static str{ + "otel" + } +} diff --git a/opentelemetry-otlp/src/span.rs b/opentelemetry-otlp/src/span.rs index ec28902d82..a2c5bc2fc2 100644 --- a/opentelemetry-otlp/src/span.rs +++ b/opentelemetry-otlp/src/span.rs @@ -167,8 +167,9 @@ impl SpanExporter for Exporter { let receiver = self .trace_exporter - .export_async_opt(&request, call_options)?; - receiver.await?; + .export_async_opt(&request, call_options).map_err::(Into::into)?; + receiver.await.map_err::(Into::into)?; Ok(()) } } + From b79b9e0171e85223cc6d60df97c750cb96b23448 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 15:29:29 -0500 Subject: [PATCH 05/18] feat: add the error type in opentelemetry-contrib datadog --- examples/basic-otlp/src/main.rs | 7 ++--- opentelemetry-contrib/Cargo.toml | 3 +- .../src/trace/exporter/datadog/mod.rs | 22 ++++++-------- .../src/trace/exporter/datadog/model/mod.rs | 29 ++++++++++++++++++- 4 files changed, 42 insertions(+), 19 deletions(-) diff --git a/examples/basic-otlp/src/main.rs b/examples/basic-otlp/src/main.rs index 911f8bf40b..a84995dd35 100644 --- a/examples/basic-otlp/src/main.rs +++ b/examples/basic-otlp/src/main.rs @@ -11,14 +11,13 @@ use opentelemetry::{global, sdk::trace as sdktrace}; use std::error::Error; use std::time::Duration; -fn init_tracer( -) -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), Box> +fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), Box> { - opentelemetry_otlp::new_pipeline().install() + opentelemetry_otlp::new_pipeline().install().map_err::, _>(|err| Box::new(err)) } // Skip first immediate tick from tokio, not needed for async_std. -fn delayed_interval(duration: Duration) -> impl Stream { +fn delayed_interval(duration: Duration) -> impl Stream { tokio::time::interval(duration).skip(1) } diff --git a/opentelemetry-contrib/Cargo.toml b/opentelemetry-contrib/Cargo.toml index 8bdeedd22d..d6c5b81cbe 100644 --- a/opentelemetry-contrib/Cargo.toml +++ b/opentelemetry-contrib/Cargo.toml @@ -22,7 +22,7 @@ rustdoc-args = ["--cfg", "docsrs"] default = [] base64_format = ["base64", "binary_propagator"] binary_propagator = [] -datadog = ["indexmap", "rmp", "async-trait"] +datadog = ["indexmap", "rmp", "async-trait", "thiserror"] reqwest-blocking-client = ["reqwest/blocking", "opentelemetry/reqwest"] reqwest-client = ["reqwest", "opentelemetry/reqwest"] surf-client = ["surf", "opentelemetry/surf"] @@ -37,6 +37,7 @@ reqwest = { version = "0.10", optional = true } surf = { version = "2.0", optional = true } http = "0.2" base64 = { version = "0.13", optional = true } +thiserror = { version = "1.0", optional = true } [dev-dependencies] base64 = "0.13" diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs index 5107649005..25b06e328b 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs @@ -97,8 +97,8 @@ //! } //! } //! -//! fn main() -> Result<(), Box> { -//! let (tracer, _uninstall) = new_pipeline() +//! fn main() -> Result<(), opentelemetry::trace::TraceError> { +//! let (tracer, _uninstall) = new_pipeline() //! .with_service_name("my_app") //! .with_version(ApiVersion::Version05) //! .with_agent_endpoint("http://localhost:8126") @@ -129,8 +129,8 @@ use http::{Method, Request, Uri}; use opentelemetry::exporter::trace; use opentelemetry::exporter::trace::{HttpClient, SpanData}; use opentelemetry::{global, sdk, trace::TracerProvider}; -use std::error::Error; -use std::io; +use opentelemetry::trace::TraceError; +use crate::trace::exporter::datadog::model::Error; /// Default Datadog collector endpoint const DEFAULT_AGENT_ENDPOINT: &str = "http://127.0.0.1:8126"; @@ -213,12 +213,12 @@ impl DatadogPipelineBuilder { /// Create `ExporterConfig` struct from current `ExporterConfigBuilder` pub fn install( mut self, - ) -> Result<(sdk::trace::Tracer, Uninstall), Box> { + ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { if let Some(client) = self.client { let endpoint = self.agent_endpoint + self.version.path(); let exporter = DatadogExporter::new( self.service_name.clone(), - endpoint.parse()?, + endpoint.parse().map_err::(Into::into)?, self.version, client, ); @@ -233,11 +233,7 @@ impl DatadogPipelineBuilder { let provider_guard = global::set_tracer_provider(provider); Ok((tracer, Uninstall(provider_guard))) } else { - Err(Box::new(io::Error::new( - io::ErrorKind::Other, - "http client must be set, users can enable reqwest or surf feature to use http\ - client implementation within create", - ))) + Err(Error::NoHttpClient.into()) } } @@ -284,11 +280,11 @@ impl trace::SpanExporter for DatadogExporter { .method(Method::POST) .uri(self.request_url.clone()) .header(http::header::CONTENT_TYPE, self.version.content_type()) - .body(data)?; + .body(data).map_err::(Into::into)?; self.client.send(req).await } } /// Uninstalls the Datadog pipeline on drop #[derive(Debug)] -pub struct Uninstall(global::TracerProviderGuard); +pub struct Uninstall(global::TracerProviderGuard); \ No newline at end of file diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs index b1a665a9e3..73216de1d3 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs @@ -1,24 +1,51 @@ use opentelemetry::exporter::trace; use std::fmt; +use opentelemetry::exporter::trace::ExportError; +use http::uri::InvalidUri; mod v03; mod v05; -#[derive(Debug, Clone, Copy)] +#[derive(Debug)] pub(crate) enum Error { MessagePackError, + NoHttpClient, + RequestError(http::Error), + InvalidUri(http::uri::InvalidUri) } 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), } } } +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 From 41ce652b5de44a2cd545f5c261042e9cd1546ae6 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 15:40:24 -0500 Subject: [PATCH 06/18] feat: add the error type in opentelemetry-jaeger --- opentelemetry-jaeger/Cargo.toml | 1 + opentelemetry-jaeger/src/lib.rs | 50 ++++++++++++++++++---------- opentelemetry-jaeger/src/uploader.rs | 6 ++-- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/opentelemetry-jaeger/Cargo.toml b/opentelemetry-jaeger/Cargo.toml index dc023b2c4c..3914853441 100644 --- a/opentelemetry-jaeger/Cargo.toml +++ b/opentelemetry-jaeger/Cargo.toml @@ -27,6 +27,7 @@ isahc = { version = "0.9", default-features = false, optional = true } opentelemetry = { version = "0.10", default-features = false, features = ["trace"], path = "../opentelemetry" } thrift = "0.13" tokio = { version = "0.2", features = ["udp", "sync"], optional = true } +thiserror = "1.0" [features] default = [] diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index d9f00acb2c..eafb99692b 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -165,17 +165,17 @@ //! supported compiler version is not considered a semver breaking change as //! long as doing so complies with this policy. #![warn( - future_incompatible, - missing_debug_implementations, - missing_docs, - nonstandard_style, - rust_2018_idioms, - unreachable_pub, - unused +future_incompatible, +missing_debug_implementations, +missing_docs, +nonstandard_style, +rust_2018_idioms, +unreachable_pub, +unused )] #![cfg_attr(docsrs, feature(doc_cfg), deny(broken_intra_doc_links))] #![doc( - html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" +html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" )] #![cfg_attr(test, deny(warnings))] @@ -200,12 +200,12 @@ use opentelemetry::{ trace::{Event, Link, SpanKind, StatusCode, TracerProvider}, Key, KeyValue, Value, }; -use std::error::Error; use std::{ net, time::{Duration, SystemTime}, }; use uploader::BatchUploader; +use opentelemetry::exporter::trace::ExportError; /// Default service name if no service is configured. const DEFAULT_SERVICE_NAME: &str = "OpenTelemetry"; @@ -357,8 +357,8 @@ impl PipelineBuilder { #[cfg(feature = "collector_client")] #[cfg_attr(docsrs, doc(cfg(feature = "collector_client")))] pub fn with_collector_endpoint(self, collector_endpoint: T) -> Self - where - http::Uri: core::convert::TryFrom, + where + http::Uri: core::convert::TryFrom, { PipelineBuilder { collector_endpoint: core::convert::TryFrom::try_from(collector_endpoint).ok(), @@ -393,7 +393,7 @@ impl PipelineBuilder { } /// Assign the process service tags. - pub fn with_tags>(mut self, tags: T) -> Self { + pub fn with_tags>(mut self, tags: T) -> Self { self.process.tags = tags.into_iter().collect(); self } @@ -409,7 +409,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), Box> { let tracer_provider = self.build()?; let tracer = tracer_provider.get_tracer("opentelemetry-jaeger", Some(env!("CARGO_PKG_VERSION"))); @@ -422,7 +422,7 @@ impl PipelineBuilder { /// Build a configured `sdk::trace::TracerProvider` with the recommended defaults. pub fn build( mut self, - ) -> Result> { + ) -> Result> { let config = self.config.take(); let exporter = self.init_exporter()?; @@ -438,7 +438,7 @@ impl PipelineBuilder { /// Initialize a new exporter. /// /// This is useful if you are manually constructing a pipeline. - pub fn init_exporter(self) -> Result> { + pub fn init_exporter(self) -> Result> { let export_instrumentation_lib = self.export_instrument_library; let (process, uploader) = self.init_uploader()?; @@ -452,7 +452,7 @@ impl PipelineBuilder { #[cfg(not(feature = "collector_client"))] fn init_uploader( self, - ) -> Result<(Process, BatchUploader), Box> { + ) -> Result<(Process, BatchUploader), Box> { let agent = AgentAsyncClientUDP::new(self.agent_endpoint.as_slice())?; Ok((self.process, BatchUploader::Agent(agent))) } @@ -460,7 +460,7 @@ impl PipelineBuilder { #[cfg(feature = "collector_client")] fn init_uploader( self, - ) -> Result<(Process, uploader::BatchUploader), Box> { + ) -> Result<(Process, uploader::BatchUploader), Box> { if let Some(collector_endpoint) = self.collector_endpoint { let collector = CollectorAsyncClientHttp::new( collector_endpoint, @@ -587,7 +587,7 @@ fn convert_otel_span_into_jaeger_span( fn build_process_tags( span_data: &trace::SpanData, -) -> Option + '_> { +) -> Option + '_> { if span_data.resource.is_empty() { None } else { @@ -677,3 +677,17 @@ fn events_to_logs(events: sdk::trace::EvictedQueue) -> Option &'static str { + "jaeger" + } +} \ No newline at end of file diff --git a/opentelemetry-jaeger/src/uploader.rs b/opentelemetry-jaeger/src/uploader.rs index dc4cef7317..620bdfb5d7 100644 --- a/opentelemetry-jaeger/src/uploader.rs +++ b/opentelemetry-jaeger/src/uploader.rs @@ -20,14 +20,14 @@ impl BatchUploader { match self { BatchUploader::Agent(client) => { // TODO Implement retry behaviour - client.emit_batch(batch).await?; + client.emit_batch(batch).await.map_err::(Into::into)?; } #[cfg(feature = "collector_client")] BatchUploader::Collector(collector) => { // TODO Implement retry behaviour - collector.submit_batch(batch).await?; + collector.submit_batch(batch).await.map_err::(Into::into)?; } - }; + } Ok(()) } } From 1b7803e8b0119a4c41e5191e1b765f5ac80d277e Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 15:52:02 -0500 Subject: [PATCH 07/18] feat: add the error type in opentelemetry testing --- opentelemetry/src/testing/trace.rs | 40 ++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/opentelemetry/src/testing/trace.rs b/opentelemetry/src/testing/trace.rs index da86e7f065..37f3fe25f6 100644 --- a/opentelemetry/src/testing/trace.rs +++ b/opentelemetry/src/testing/trace.rs @@ -1,4 +1,4 @@ -use crate::exporter::trace::SpanData; +use crate::exporter::trace::{SpanData, ExportError}; use crate::{ exporter::trace::{self as exporter, ExportResult, SpanExporter}, sdk::{ @@ -11,6 +11,8 @@ use crate::{ use async_trait::async_trait; use std::sync::mpsc::{channel, Receiver, Sender}; use std::time::SystemTime; +use std::fmt::Display; +use serde::export::Formatter; #[derive(Debug)] pub struct TestSpan(pub SpanContext); @@ -21,8 +23,7 @@ impl Span for TestSpan { _name: String, _timestamp: std::time::SystemTime, _attributes: Vec, - ) { - } + ) {} fn span_context(&self) -> &SpanContext { &self.0 } @@ -64,7 +65,7 @@ pub struct TestSpanExporter { impl SpanExporter for TestSpanExporter { async fn export(&mut self, batch: Vec) -> ExportResult { for span_data in batch { - self.tx_export.send(span_data)?; + self.tx_export.send(span_data).map_err::(Into::into)?; } Ok(()) } @@ -94,7 +95,7 @@ pub struct TokioSpanExporter { impl SpanExporter for TokioSpanExporter { async fn export(&mut self, batch: Vec) -> ExportResult { for span_data in batch { - self.tx_export.send(span_data)?; + self.tx_export.send(span_data).map_err::(Into::into)?; } Ok(()) } @@ -117,3 +118,32 @@ pub fn new_tokio_test_exporter() -> ( }; (exporter, rx_export, rx_shutdown) } + +#[derive(Debug)] +pub struct TestExportError(String); + +impl std::error::Error for TestExportError {} + +impl ExportError for TestExportError { + fn exporter_name(&self) -> &'static str { + "test" + } +} + +impl Display for TestExportError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl From> for TestExportError { + fn from(err: tokio::sync::mpsc::error::SendError) -> Self { + TestExportError(err.to_string()) + } +} + +impl From> for TestExportError { + fn from(err: std::sync::mpsc::SendError) -> Self { + TestExportError(err.to_string()) + } +} From ae0fe86c4ebbae58e824d29c626f1679a342af02 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 16:07:16 -0500 Subject: [PATCH 08/18] fix: errors in test and doc. --- .../src/trace/exporter/datadog/mod.rs | 8 ++++---- .../src/trace/exporter/datadog/model/mod.rs | 16 ++++++++++++---- opentelemetry-zipkin/src/lib.rs | 4 ++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs index 25b06e328b..a17f8369a4 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs @@ -76,7 +76,7 @@ //! use opentelemetry::exporter::trace::HttpClient; //! use opentelemetry_contrib::trace::exporter::datadog::{new_pipeline, ApiVersion}; //! use async_trait::async_trait; -//! use std::error::Error; +//! use opentelemetry_contrib::trace::exporter::datadog::Error; //! //! // `reqwest` and `surf` are supported through features, if you prefer an //! // alternate http client you can add support by implementing `HttpClient` as @@ -87,12 +87,12 @@ //! #[async_trait] //! impl HttpClient for IsahcClient { //! async fn send(&self, request: http::Request>) -> ExportResult { -//! let result = self.0.send_async(request).await?; +//! let result = self.0.send_async(request).await.map_err(|err| Error::Other(err.to_string()))?; //! //! if result.status().is_success() { //! Ok(()) //! } else { -//! Err(result.status().as_str().into()) +//! Err(Error::Other(result.status().to_string()).into()) //! } //! } //! } @@ -123,6 +123,7 @@ mod intern; mod model; pub use model::ApiVersion; +pub use model::Error; use async_trait::async_trait; use http::{Method, Request, Uri}; @@ -130,7 +131,6 @@ use opentelemetry::exporter::trace; use opentelemetry::exporter::trace::{HttpClient, SpanData}; use opentelemetry::{global, sdk, trace::TracerProvider}; use opentelemetry::trace::TraceError; -use crate::trace::exporter::datadog::model::Error; /// Default Datadog collector endpoint const DEFAULT_AGENT_ENDPOINT: &str = "http://127.0.0.1:8126"; diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs index 73216de1d3..dbcb6d171e 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs @@ -6,12 +6,19 @@ use http::uri::InvalidUri; mod v03; mod v05; +/// Wrap type for errors from opentelemetry datadog exporter #[derive(Debug)] -pub(crate) enum Error { +pub enum Error { + /// Message pack error MessagePackError, + /// No http client founded. User should provide one or enbale features NoHttpClient, + /// Http requests failed with following errors RequestError(http::Error), - InvalidUri(http::uri::InvalidUri) + /// The Uri was invalid. + InvalidUri(http::uri::InvalidUri), + /// Other errors + Other(String), } impl std::error::Error for Error {} @@ -29,17 +36,18 @@ impl fmt::Display for 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{ +impl From for Error { fn from(err: InvalidUri) -> Self { Error::InvalidUri(err) } } -impl From for Error{ +impl From for Error { fn from(err: http::Error) -> Self { Error::RequestError(err) } diff --git a/opentelemetry-zipkin/src/lib.rs b/opentelemetry-zipkin/src/lib.rs index c9be54aa14..3dc907f413 100644 --- a/opentelemetry-zipkin/src/lib.rs +++ b/opentelemetry-zipkin/src/lib.rs @@ -90,12 +90,12 @@ //! #[async_trait] //! impl HttpClient for IsahcClient { //! async fn send(&self, request: http::Request>) -> ExportResult { -//! let result = self.0.send_async(request).await?; +//! let result = self.0.send_async(request).await.map_err(|err| opentelemetry_zipkin::Error::Other(err.to_string()))?; //! //! if result.status().is_success() { //! Ok(()) //! } else { -//! Err(result.status().as_str().into()) +//! Err(opentelemetry_zipkin::Error::Other(result.status().to_string()).into()) //! } //! } //! } From 23823d033f7d7e1848de555935747176a4403717 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 16:08:11 -0500 Subject: [PATCH 09/18] fix: format --- examples/basic-otlp/src/main.rs | 9 ++- .../src/trace/exporter/datadog/mod.rs | 11 ++- .../src/trace/exporter/datadog/model/mod.rs | 5 +- opentelemetry-jaeger/src/lib.rs | 45 ++++++----- opentelemetry-jaeger/src/uploader.rs | 10 ++- opentelemetry-otlp/src/lib.rs | 12 ++- opentelemetry-otlp/src/span.rs | 4 +- opentelemetry-zipkin/src/lib.rs | 52 ++++++------ opentelemetry-zipkin/src/uploader.rs | 5 +- opentelemetry/src/api/mod.rs | 4 +- opentelemetry/src/api/trace/mod.rs | 6 +- opentelemetry/src/exporter/trace/mod.rs | 2 +- opentelemetry/src/exporter/trace/stdout.rs | 21 ++--- opentelemetry/src/global/error_handler.rs | 20 +++-- opentelemetry/src/sdk/trace/span_processor.rs | 81 +++++++++---------- opentelemetry/src/testing/trace.rs | 17 ++-- 16 files changed, 168 insertions(+), 136 deletions(-) diff --git a/examples/basic-otlp/src/main.rs b/examples/basic-otlp/src/main.rs index a84995dd35..cb61c2df9e 100644 --- a/examples/basic-otlp/src/main.rs +++ b/examples/basic-otlp/src/main.rs @@ -11,13 +11,16 @@ use opentelemetry::{global, sdk::trace as sdktrace}; use std::error::Error; use std::time::Duration; -fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), Box> +fn init_tracer( +) -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), Box> { - opentelemetry_otlp::new_pipeline().install().map_err::, _>(|err| Box::new(err)) + opentelemetry_otlp::new_pipeline() + .install() + .map_err::, _>(|err| Box::new(err)) } // Skip first immediate tick from tokio, not needed for async_std. -fn delayed_interval(duration: Duration) -> impl Stream { +fn delayed_interval(duration: Duration) -> impl Stream { tokio::time::interval(duration).skip(1) } diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs index a17f8369a4..71fc76fda6 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs @@ -129,8 +129,8 @@ use async_trait::async_trait; use http::{Method, Request, Uri}; use opentelemetry::exporter::trace; use opentelemetry::exporter::trace::{HttpClient, SpanData}; -use opentelemetry::{global, sdk, trace::TracerProvider}; use opentelemetry::trace::TraceError; +use opentelemetry::{global, sdk, trace::TracerProvider}; /// Default Datadog collector endpoint const DEFAULT_AGENT_ENDPOINT: &str = "http://127.0.0.1:8126"; @@ -211,9 +211,7 @@ impl Default for DatadogPipelineBuilder { impl DatadogPipelineBuilder { /// Create `ExporterConfig` struct from current `ExporterConfigBuilder` - pub fn install( - mut self, - ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { + pub fn install(mut self) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { if let Some(client) = self.client { let endpoint = self.agent_endpoint + self.version.path(); let exporter = DatadogExporter::new( @@ -280,11 +278,12 @@ impl trace::SpanExporter for DatadogExporter { .method(Method::POST) .uri(self.request_url.clone()) .header(http::header::CONTENT_TYPE, self.version.content_type()) - .body(data).map_err::(Into::into)?; + .body(data) + .map_err::(Into::into)?; self.client.send(req).await } } /// Uninstalls the Datadog pipeline on drop #[derive(Debug)] -pub struct Uninstall(global::TracerProviderGuard); \ No newline at end of file +pub struct Uninstall(global::TracerProviderGuard); diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs index dbcb6d171e..69bfcf0d89 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs @@ -1,7 +1,7 @@ +use http::uri::InvalidUri; use opentelemetry::exporter::trace; -use std::fmt; use opentelemetry::exporter::trace::ExportError; -use http::uri::InvalidUri; +use std::fmt; mod v03; mod v05; @@ -53,7 +53,6 @@ impl From for Error { } } - 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 eafb99692b..c2ae20c5bf 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -165,17 +165,17 @@ //! supported compiler version is not considered a semver breaking change as //! long as doing so complies with this policy. #![warn( -future_incompatible, -missing_debug_implementations, -missing_docs, -nonstandard_style, -rust_2018_idioms, -unreachable_pub, -unused + future_incompatible, + missing_debug_implementations, + missing_docs, + nonstandard_style, + rust_2018_idioms, + unreachable_pub, + unused )] #![cfg_attr(docsrs, feature(doc_cfg), deny(broken_intra_doc_links))] #![doc( -html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" + html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" )] #![cfg_attr(test, deny(warnings))] @@ -194,6 +194,7 @@ use agent::AgentAsyncClientUDP; use async_trait::async_trait; #[cfg(feature = "collector_client")] use collector::CollectorAsyncClientHttp; +use opentelemetry::exporter::trace::ExportError; use opentelemetry::{ exporter::trace, global, sdk, @@ -205,7 +206,6 @@ use std::{ time::{Duration, SystemTime}, }; use uploader::BatchUploader; -use opentelemetry::exporter::trace::ExportError; /// Default service name if no service is configured. const DEFAULT_SERVICE_NAME: &str = "OpenTelemetry"; @@ -357,8 +357,8 @@ impl PipelineBuilder { #[cfg(feature = "collector_client")] #[cfg_attr(docsrs, doc(cfg(feature = "collector_client")))] pub fn with_collector_endpoint(self, collector_endpoint: T) -> Self - where - http::Uri: core::convert::TryFrom, + where + http::Uri: core::convert::TryFrom, { PipelineBuilder { collector_endpoint: core::convert::TryFrom::try_from(collector_endpoint).ok(), @@ -393,7 +393,7 @@ impl PipelineBuilder { } /// Assign the process service tags. - pub fn with_tags>(mut self, tags: T) -> Self { + pub fn with_tags>(mut self, tags: T) -> Self { self.process.tags = tags.into_iter().collect(); self } @@ -409,7 +409,8 @@ 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), Box> + { let tracer_provider = self.build()?; let tracer = tracer_provider.get_tracer("opentelemetry-jaeger", Some(env!("CARGO_PKG_VERSION"))); @@ -422,7 +423,8 @@ impl PipelineBuilder { /// Build a configured `sdk::trace::TracerProvider` with the recommended defaults. pub fn build( mut self, - ) -> Result> { + ) -> Result> + { let config = self.config.take(); let exporter = self.init_exporter()?; @@ -438,7 +440,9 @@ impl PipelineBuilder { /// Initialize a new exporter. /// /// This is useful if you are manually constructing a pipeline. - pub fn init_exporter(self) -> Result> { + pub fn init_exporter( + self, + ) -> Result> { let export_instrumentation_lib = self.export_instrument_library; let (process, uploader) = self.init_uploader()?; @@ -460,7 +464,10 @@ impl PipelineBuilder { #[cfg(feature = "collector_client")] fn init_uploader( self, - ) -> Result<(Process, uploader::BatchUploader), Box> { + ) -> Result< + (Process, uploader::BatchUploader), + Box, + > { if let Some(collector_endpoint) = self.collector_endpoint { let collector = CollectorAsyncClientHttp::new( collector_endpoint, @@ -587,7 +594,7 @@ fn convert_otel_span_into_jaeger_span( fn build_process_tags( span_data: &trace::SpanData, -) -> Option + '_> { +) -> Option + '_> { if span_data.resource.is_empty() { None } else { @@ -683,11 +690,11 @@ fn events_to_logs(events: sdk::trace::EvictedQueue) -> Option &'static str { "jaeger" } -} \ No newline at end of file +} diff --git a/opentelemetry-jaeger/src/uploader.rs b/opentelemetry-jaeger/src/uploader.rs index 620bdfb5d7..c16446b721 100644 --- a/opentelemetry-jaeger/src/uploader.rs +++ b/opentelemetry-jaeger/src/uploader.rs @@ -20,12 +20,18 @@ impl BatchUploader { match self { BatchUploader::Agent(client) => { // TODO Implement retry behaviour - client.emit_batch(batch).await.map_err::(Into::into)?; + client + .emit_batch(batch) + .await + .map_err::(Into::into)?; } #[cfg(feature = "collector_client")] BatchUploader::Collector(collector) => { // TODO Implement retry behaviour - collector.submit_batch(batch).await.map_err::(Into::into)?; + collector + .submit_batch(batch) + .await + .map_err::(Into::into)?; } } Ok(()) diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index 103e24d29a..0a043f06db 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -131,8 +131,8 @@ mod span; mod transform; pub use crate::span::{Compression, Credentials, Exporter, ExporterConfig, Protocol}; -use opentelemetry::trace::TraceError; use opentelemetry::exporter::trace::ExportError; +use opentelemetry::trace::TraceError; /// Create a new pipeline builder with the recommended configuration. /// @@ -216,9 +216,7 @@ impl OtlpPipelineBuilder { } /// Install the OTLP exporter pipeline with the recommended defaults. - pub fn install( - mut self, - ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { + pub fn install(mut self) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { let exporter = Exporter::new(self.exporter_config); let mut provider_builder = sdk::trace::TracerProvider::builder().with_exporter(exporter); @@ -239,15 +237,15 @@ pub struct Uninstall(global::TracerProviderGuard); /// Wrap type for errors from opentelemetry otel #[derive(thiserror::Error, Debug)] -pub enum Error{ +pub enum Error { // FIXME: wait until https://github.com/open-telemetry/opentelemetry-rust/pull/352 merged /// Error from grpcio module #[error("grpcio error {0}")] - Grpcio(#[from] grpcio::Error) + Grpcio(#[from] grpcio::Error), } impl ExportError for Error { - fn exporter_name(&self) -> &'static str{ + fn exporter_name(&self) -> &'static str { "otel" } } diff --git a/opentelemetry-otlp/src/span.rs b/opentelemetry-otlp/src/span.rs index a2c5bc2fc2..1f6029ef09 100644 --- a/opentelemetry-otlp/src/span.rs +++ b/opentelemetry-otlp/src/span.rs @@ -167,9 +167,9 @@ impl SpanExporter for Exporter { let receiver = self .trace_exporter - .export_async_opt(&request, call_options).map_err::(Into::into)?; + .export_async_opt(&request, call_options) + .map_err::(Into::into)?; receiver.await.map_err::(Into::into)?; Ok(()) } } - diff --git a/opentelemetry-zipkin/src/lib.rs b/opentelemetry-zipkin/src/lib.rs index 3dc907f413..ac4acfd2b7 100644 --- a/opentelemetry-zipkin/src/lib.rs +++ b/opentelemetry-zipkin/src/lib.rs @@ -148,17 +148,17 @@ //! supported compiler version is not considered a semver breaking change as //! long as doing so complies with this policy. #![warn( -future_incompatible, -missing_debug_implementations, -missing_docs, -nonstandard_style, -rust_2018_idioms, -unreachable_pub, -unused + future_incompatible, + missing_debug_implementations, + missing_docs, + nonstandard_style, + rust_2018_idioms, + unreachable_pub, + unused )] #![cfg_attr(docsrs, feature(doc_cfg), deny(broken_intra_doc_links))] #![doc( -html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" + html_logo_url = "https://raw.githubusercontent.com/open-telemetry/opentelemetry-rust/master/assets/logo.svg" )] #![cfg_attr(test, deny(warnings))] @@ -171,10 +171,10 @@ mod uploader; use async_trait::async_trait; use http::Uri; use model::endpoint::Endpoint; -use opentelemetry::exporter::trace::{HttpClient, ExportError}; +use opentelemetry::exporter::trace::{ExportError, HttpClient}; +use opentelemetry::trace::TraceError; use opentelemetry::{exporter::trace, global, sdk, trace::TracerProvider}; use std::net::SocketAddr; -use opentelemetry::trace::TraceError; /// Default Zipkin collector endpoint const DEFAULT_COLLECTOR_ENDPOINT: &str = "http://127.0.0.1:9411/api/v2/spans"; @@ -219,21 +219,21 @@ impl Default for ZipkinPipelineBuilder { #[cfg(feature = "reqwest-blocking-client")] client: Some(Box::new(reqwest::blocking::Client::new())), #[cfg(all( - not(feature = "reqwest-blocking-client"), - not(feature = "surf-client"), - feature = "reqwest-client" + not(feature = "reqwest-blocking-client"), + not(feature = "surf-client"), + feature = "reqwest-client" ))] client: Some(Box::new(reqwest::Client::new())), #[cfg(all( - not(feature = "reqwest-client"), - not(feature = "reqwest-blocking-client"), - feature = "surf-client" + not(feature = "reqwest-client"), + not(feature = "reqwest-blocking-client"), + feature = "surf-client" ))] client: Some(Box::new(surf::Client::new())), #[cfg(all( - not(feature = "reqwest-client"), - not(feature = "surf-client"), - not(feature = "reqwest-blocking-client") + not(feature = "reqwest-client"), + not(feature = "surf-client"), + not(feature = "reqwest-blocking-client") ))] client: None, @@ -247,12 +247,16 @@ impl Default for ZipkinPipelineBuilder { impl ZipkinPipelineBuilder { /// Create `ExporterConfig` struct from current `ExporterConfigBuilder` - pub fn install( - mut self, - ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { + pub fn install(mut self) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { if let Some(client) = self.client { let endpoint = Endpoint::new(self.service_name, self.service_addr); - let exporter = Exporter::new(endpoint, client, self.collector_endpoint.parse().map_err::(Into::into)?); + let exporter = Exporter::new( + endpoint, + client, + self.collector_endpoint + .parse() + .map_err::(Into::into)?, + ); let mut provider_builder = sdk::trace::TracerProvider::builder().with_exporter(exporter); @@ -343,4 +347,4 @@ impl ExportError for Error { fn exporter_name(&self) -> &'static str { "zipkin" } -} \ No newline at end of file +} diff --git a/opentelemetry-zipkin/src/uploader.rs b/opentelemetry-zipkin/src/uploader.rs index 31db25b53b..d952f595cc 100644 --- a/opentelemetry-zipkin/src/uploader.rs +++ b/opentelemetry-zipkin/src/uploader.rs @@ -1,9 +1,9 @@ //! # Zipkin Span Exporter use crate::model::span::Span; +use crate::Error; use http::{header::CONTENT_TYPE, Method, Request, Uri}; use opentelemetry::exporter::trace::{ExportResult, HttpClient}; use std::fmt::Debug; -use crate::Error; #[derive(Debug)] pub(crate) enum Uploader { @@ -39,7 +39,8 @@ impl JsonV2Client { .method(Method::POST) .uri(self.collector_endpoint.clone()) .header(CONTENT_TYPE, "application/json") - .body(serde_json::to_vec(&spans).unwrap_or_default()).map_err::(Into::into)?; + .body(serde_json::to_vec(&spans).unwrap_or_default()) + .map_err::(Into::into)?; self.client.send(req).await } } diff --git a/opentelemetry/src/api/mod.rs b/opentelemetry/src/api/mod.rs index 2007b8067d..e5756152b2 100644 --- a/opentelemetry/src/api/mod.rs +++ b/opentelemetry/src/api/mod.rs @@ -1,7 +1,7 @@ -use crate::api::trace::TraceError; #[cfg(feature = "metrics")] #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] use crate::api::metrics::MetricsError; +use crate::api::trace::TraceError; use std::sync::PoisonError; pub mod baggage; @@ -46,4 +46,4 @@ impl From> for OpenTelemetryError { fn from(err: PoisonError) -> Self { OpenTelemetryError::Other(err.to_string()) } -} \ No newline at end of file +} diff --git a/opentelemetry/src/api/trace/mod.rs b/opentelemetry/src/api/trace/mod.rs index 80b887f8e8..19bb155160 100644 --- a/opentelemetry/src/api/trace/mod.rs +++ b/opentelemetry/src/api/trace/mod.rs @@ -139,8 +139,8 @@ pub use self::{ }, tracer::{SpanBuilder, Tracer}, }; -use std::time; use crate::exporter::trace::ExportError; +use std::time; /// Describe the result of operations in tracing API. pub type TraceResult = Result; @@ -167,7 +167,9 @@ pub enum TraceError { } impl From for TraceError - where T: ExportError { +where + T: ExportError, +{ fn from(err: T) -> Self { TraceError::ExportFailed(Box::new(err)) } diff --git a/opentelemetry/src/exporter/trace/mod.rs b/opentelemetry/src/exporter/trace/mod.rs index 7e8516b10f..06f4d02967 100644 --- a/opentelemetry/src/exporter/trace/mod.rs +++ b/opentelemetry/src/exporter/trace/mod.rs @@ -1,4 +1,5 @@ //! Trace exporters +use crate::api::trace::TraceError; use crate::{ sdk, trace::{Event, Link, SpanContext, SpanId, SpanKind, StatusCode}, @@ -13,7 +14,6 @@ use std::convert::TryInto; use std::fmt::Debug; use std::sync::Arc; use std::time::SystemTime; -use crate::api::trace::TraceError; pub mod stdout; diff --git a/opentelemetry/src/exporter/trace/stdout.rs b/opentelemetry/src/exporter/trace/stdout.rs index 00f5c15d17..27fa0e6e07 100644 --- a/opentelemetry/src/exporter/trace/stdout.rs +++ b/opentelemetry/src/exporter/trace/stdout.rs @@ -24,16 +24,16 @@ //! }); //! } //! ``` +use crate::exporter::trace::ExportError; use crate::{ exporter::trace::{ExportResult, SpanData, SpanExporter}, global, sdk, trace::TracerProvider, }; use async_trait::async_trait; +use serde::export::Formatter; use std::fmt::{Debug, Display}; use std::io::{stdout, Stdout, Write}; -use crate::exporter::trace::ExportError; -use serde::export::Formatter; /// Pipeline builder #[derive(Debug)] @@ -83,8 +83,8 @@ impl PipelineBuilder { } impl PipelineBuilder - where - W: Write + Debug + Send + 'static, +where + W: Write + Debug + Send + 'static, { /// Install the stdout exporter pipeline with the recommended defaults. pub fn install(mut self) -> (sdk::trace::Tracer, Uninstall) { @@ -125,16 +125,20 @@ impl Exporter { #[async_trait] impl SpanExporter for Exporter - where - W: Write + Debug + Send + 'static, +where + W: Write + Debug + Send + 'static, { /// Export spans to stdout async fn export(&mut self, batch: Vec) -> ExportResult { for span in batch { if self.pretty_print { - self.writer.write_all(format!("{:#?}\n", span).as_bytes()).map_err(|err| Error::from(err))?; + self.writer + .write_all(format!("{:#?}\n", span).as_bytes()) + .map_err(|err| Error::from(err))?; } else { - self.writer.write_all(format!("{:?}\n", span).as_bytes()).map_err(|err| Error::from(err))?; + self.writer + .write_all(format!("{:?}\n", span).as_bytes()) + .map_err(|err| Error::from(err))?; } } @@ -169,4 +173,3 @@ impl ExportError for Error { "stdout" } } - diff --git a/opentelemetry/src/global/error_handler.rs b/opentelemetry/src/global/error_handler.rs index a79c3ead75..ebf8969909 100644 --- a/opentelemetry/src/global/error_handler.rs +++ b/opentelemetry/src/global/error_handler.rs @@ -1,5 +1,5 @@ -use std::sync::RwLock; use crate::api::OpenTelemetryError; +use std::sync::RwLock; lazy_static::lazy_static! { /// The global error handler. @@ -15,17 +15,23 @@ 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() { - OpenTelemetryError::MetricErr(err) => eprintln!("OpenTelemetry metrics error occurred {:?}", err), - OpenTelemetryError::TraceErr(err) => eprintln!("OpenTelemetry trace error occurred {:?}", err), - OpenTelemetryError::Other(err_msg) => println!("OpenTelemetry error occurred {}", err_msg) - } + OpenTelemetryError::MetricErr(err) => { + eprintln!("OpenTelemetry metrics error occurred {:?}", err) + } + OpenTelemetryError::TraceErr(err) => { + eprintln!("OpenTelemetry trace error occurred {:?}", err) + } + OpenTelemetryError::Other(err_msg) => { + println!("OpenTelemetry error occurred {}", err_msg) + } + }, } } /// Set global error handler. pub fn set_error_handler(f: F) -> std::result::Result<(), OpenTelemetryError> - where - F: Fn(OpenTelemetryError) + Send + Sync + 'static, +where + F: Fn(OpenTelemetryError) + Send + Sync + 'static, { GLOBAL_ERROR_HANDLER .write() diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 745a1caafc..0655266aba 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -219,8 +219,7 @@ impl SpanProcessor for BatchSpanProcessor { fn force_flush(&self) -> TraceResult<()> { let mut sender = self.message_sender.lock().map_err(|_| TraceError::Other("When force flushing the BatchSpanProcessor, the message sender's lock has been poisoned".into()))?; let (res_sender, res_receiver) = oneshot::channel::>(); - sender - .try_send(BatchMessage::Flush(Some(res_sender)))?; + sender.try_send(BatchMessage::Flush(Some(res_sender)))?; for result in futures::executor::block_on(res_receiver)? { result?; } @@ -253,13 +252,13 @@ impl BatchSpanProcessor { delay: D, config: BatchConfig, ) -> Self - where - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IS, - IS: Stream + Send + 'static, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IS, + IS: Stream + Send + 'static, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { let (message_sender, message_receiver) = mpsc::channel(config.max_queue_size); let ticker = interval(config.scheduled_delay).map(|_| BatchMessage::Flush(None)); @@ -360,13 +359,13 @@ impl BatchSpanProcessor { delay: D, interval: I, ) -> BatchSpanProcessorBuilder - where - E: SpanExporter, - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IO, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + E: SpanExporter, + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IO, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { BatchSpanProcessorBuilder { exporter, @@ -390,13 +389,13 @@ impl BatchSpanProcessor { interval: I, delay: D, ) -> BatchSpanProcessorBuilder - where - E: SpanExporter, - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IO, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, + where + E: SpanExporter, + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IO, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { let mut config = BatchConfig::default(); let schedule_delay = std::env::var(OTEL_BSP_SCHEDULE_DELAY_MILLIS) @@ -447,10 +446,10 @@ async fn export_with_timeout( delay: &D, batch: Vec, ) -> ExportResult - where - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, - E: SpanExporter + ?Sized, +where + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, + E: SpanExporter + ?Sized, { let export = exporter.export(batch); let timeout = delay(time_out); @@ -507,14 +506,14 @@ pub struct BatchSpanProcessorBuilder { } impl BatchSpanProcessorBuilder - where - E: SpanExporter + 'static, - S: Fn(BoxFuture<'static, ()>) -> SH, - SH: Future + Send + Sync + 'static, - I: Fn(time::Duration) -> IS, - IS: Stream + Send + 'static, - D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, - DS: Future + 'static + Send + Sync, +where + E: SpanExporter + 'static, + S: Fn(BoxFuture<'static, ()>) -> SH, + SH: Future + Send + Sync + 'static, + I: Fn(time::Duration) -> IS, + IS: Stream + Send + 'static, + D: (Fn(time::Duration) -> DS) + Send + Sync + 'static, + DS: Future + 'static + Send + Sync, { /// Set max queue size for batches pub fn with_max_queue_size(self, size: usize) -> Self { @@ -684,9 +683,9 @@ mod tests { } impl Debug for BlockingExporter - where - D: Fn(time::Duration) -> DS + 'static + Send + Sync, - DS: Future + Send + Sync + 'static, + where + D: Fn(time::Duration) -> DS + 'static + Send + Sync, + DS: Future + Send + Sync + 'static, { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.write_str("blocking exporter for testing") @@ -695,9 +694,9 @@ mod tests { #[async_trait] impl SpanExporter for BlockingExporter - where - D: Fn(time::Duration) -> DS + 'static + Send + Sync, - DS: Future + Send + Sync + 'static, + where + D: Fn(time::Duration) -> DS + 'static + Send + Sync, + DS: Future + Send + Sync + 'static, { async fn export(&mut self, batch: Vec) -> ExportResult { println!("Accepting {} spans", batch.len()); diff --git a/opentelemetry/src/testing/trace.rs b/opentelemetry/src/testing/trace.rs index 37f3fe25f6..20a6976146 100644 --- a/opentelemetry/src/testing/trace.rs +++ b/opentelemetry/src/testing/trace.rs @@ -1,4 +1,4 @@ -use crate::exporter::trace::{SpanData, ExportError}; +use crate::exporter::trace::{ExportError, SpanData}; use crate::{ exporter::trace::{self as exporter, ExportResult, SpanExporter}, sdk::{ @@ -9,10 +9,10 @@ use crate::{ KeyValue, }; use async_trait::async_trait; +use serde::export::Formatter; +use std::fmt::Display; use std::sync::mpsc::{channel, Receiver, Sender}; use std::time::SystemTime; -use std::fmt::Display; -use serde::export::Formatter; #[derive(Debug)] pub struct TestSpan(pub SpanContext); @@ -23,7 +23,8 @@ impl Span for TestSpan { _name: String, _timestamp: std::time::SystemTime, _attributes: Vec, - ) {} + ) { + } fn span_context(&self) -> &SpanContext { &self.0 } @@ -65,7 +66,9 @@ pub struct TestSpanExporter { impl SpanExporter for TestSpanExporter { async fn export(&mut self, batch: Vec) -> ExportResult { for span_data in batch { - self.tx_export.send(span_data).map_err::(Into::into)?; + self.tx_export + .send(span_data) + .map_err::(Into::into)?; } Ok(()) } @@ -95,7 +98,9 @@ pub struct TokioSpanExporter { impl SpanExporter for TokioSpanExporter { async fn export(&mut self, batch: Vec) -> ExportResult { for span_data in batch { - self.tx_export.send(span_data).map_err::(Into::into)?; + self.tx_export + .send(span_data) + .map_err::(Into::into)?; } Ok(()) } From eea72373b1cef0c94fd6f2c9b758fcab3e0c5726 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Thu, 26 Nov 2020 17:30:23 -0500 Subject: [PATCH 10/18] feat: add HttpClientError --- opentelemetry/src/api/mod.rs | 4 + opentelemetry/src/exporter/trace/mod.rs | 104 ++++++++++++++++-- opentelemetry/src/exporter/trace/stdout.rs | 7 +- opentelemetry/src/global/error_handler.rs | 4 + opentelemetry/src/global/mod.rs | 3 - opentelemetry/src/sdk/trace/span_processor.rs | 4 +- opentelemetry/src/testing/trace.rs | 3 +- 7 files changed, 107 insertions(+), 22 deletions(-) diff --git a/opentelemetry/src/api/mod.rs b/opentelemetry/src/api/mod.rs index e5756152b2..80807af80c 100644 --- a/opentelemetry/src/api/mod.rs +++ b/opentelemetry/src/api/mod.rs @@ -30,12 +30,16 @@ pub enum OpenTelemetryError { 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) diff --git a/opentelemetry/src/exporter/trace/mod.rs b/opentelemetry/src/exporter/trace/mod.rs index 06f4d02967..d84ae7c875 100644 --- a/opentelemetry/src/exporter/trace/mod.rs +++ b/opentelemetry/src/exporter/trace/mod.rs @@ -12,6 +12,8 @@ use serde::{Deserialize, Serialize}; #[cfg(all(feature = "http", feature = "reqwest"))] use std::convert::TryInto; use std::fmt::Debug; +#[cfg(feature = "http")] +use std::fmt::{Display, Formatter}; use std::sync::Arc; use std::time::SystemTime; @@ -28,10 +30,6 @@ pub trait ExportError: std::error::Error + Send + Sync + 'static { } } -#[cfg(all(feature = "reqwest", feature = "http"))] -#[async_trait] -impl ExportError for reqwest::Error {} - /// `SpanExporter` defines the interface that protocol-specific exporters must /// implement so that they can be plugged into OpenTelemetry SDK and support /// sending of telemetry data. @@ -81,6 +79,65 @@ pub trait HttpClient: Debug + Send + Sync { async fn send(&self, request: Request>) -> ExportResult; } +/// Error when sending http requests visa HttpClient implementation +#[cfg(feature = "http")] +#[cfg_attr(docsrs, doc(cfg(feature = "http")))] +#[derive(Debug)] +pub enum HttpClientError { + /// Errors from reqwest + #[cfg(all(feature = "reqwest", feature = "http"))] + ReqwestError(reqwest::Error), + + /// Errors from surf + #[cfg(all(feature = "surf", feature = "http"))] + SurfError(surf::Error), + + /// Other errors + Other(String), +} + +#[cfg(feature = "http")] +#[cfg_attr(docsrs, doc(cfg(feature = "http")))] +impl std::error::Error for HttpClientError {} + +#[cfg(feature = "http")] +#[cfg_attr(docsrs, doc(cfg(feature = "http")))] +impl ExportError for HttpClientError {} + +#[cfg(feature = "http")] +#[cfg_attr(docsrs, doc(cfg(feature = "http")))] +impl Display for HttpClientError { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + #[cfg(all(feature = "reqwest", feature = "http"))] + HttpClientError::ReqwestError(err) => { + write!(f, "error when sending requests using reqwest, {}", err) + } + #[cfg(all(feature = "surf", feature = "http"))] + HttpClientError::SurfError(err) => write!( + f, + "error when sending requests using surf, {}", + err.to_string() + ), + HttpClientError::Other(msg) => write!(f, "{}", msg), + } + } +} + +#[cfg(all(feature = "reqwest", feature = "http"))] +impl From for HttpClientError { + fn from(err: reqwest::Error) -> Self { + HttpClientError::ReqwestError(err) + } +} + +#[cfg(all(feature = "surf", feature = "http"))] +impl From for HttpClientError { + fn from(err: surf::Error) -> Self { + HttpClientError::SurfError(err) + } +} + /// `SpanData` contains all the information collected by a `Span` and can be used /// by exporters as a standard input. #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))] @@ -120,9 +177,15 @@ pub struct SpanData { impl HttpClient for reqwest::Client { async fn send(&self, request: Request>) -> ExportResult { let _result = self - .execute(request.try_into()?) - .await? - .error_for_status()?; + .execute( + request + .try_into() + .map_err::(Into::into)?, + ) + .await + .map_err::(Into::into)? + .error_for_status() + .map_err::(Into::into)?; Ok(()) } } @@ -131,7 +194,15 @@ impl HttpClient for reqwest::Client { #[async_trait] impl HttpClient for reqwest::blocking::Client { async fn send(&self, request: Request>) -> ExportResult { - let _result = self.execute(request.try_into()?)?.error_for_status()?; + let _result = self + .execute( + request + .try_into() + .map_err::(Into::into)?, + ) + .map_err::(Into::into)? + .error_for_status() + .map_err::(Into::into)?; Ok(()) } } @@ -141,17 +212,28 @@ impl HttpClient for reqwest::blocking::Client { impl HttpClient for surf::Client { async fn send(&self, request: Request>) -> ExportResult { let (parts, body) = request.into_parts(); - let uri = parts.uri.to_string().parse()?; + let uri = parts + .uri + .to_string() + .parse() + .map_err(|err: surf::http::url::ParseError| HttpClientError::Other(err.to_string()))?; let req = surf::Request::builder(surf::http::Method::Post, uri) .content_type("application/json") .body(body); - let result = self.send(req).await?; + let result = self + .send(req) + .await + .map_err::(Into::into)?; if result.status().is_success() { Ok(()) } else { - Err(result.status().canonical_reason().into()) + Err(HttpClientError::SurfError(surf::Error::from_str( + result.status(), + result.status().canonical_reason(), + )) + .into()) } } } diff --git a/opentelemetry/src/exporter/trace/stdout.rs b/opentelemetry/src/exporter/trace/stdout.rs index 27fa0e6e07..44a14c50d2 100644 --- a/opentelemetry/src/exporter/trace/stdout.rs +++ b/opentelemetry/src/exporter/trace/stdout.rs @@ -31,8 +31,7 @@ use crate::{ trace::TracerProvider, }; use async_trait::async_trait; -use serde::export::Formatter; -use std::fmt::{Debug, Display}; +use std::fmt::{Debug, Display, Formatter}; use std::io::{stdout, Stdout, Write}; /// Pipeline builder @@ -134,11 +133,11 @@ where if self.pretty_print { self.writer .write_all(format!("{:#?}\n", span).as_bytes()) - .map_err(|err| Error::from(err))?; + .map_err::(Into::into)?; } else { self.writer .write_all(format!("{:?}\n", span).as_bytes()) - .map_err(|err| Error::from(err))?; + .map_err::(Into::into)?; } } diff --git a/opentelemetry/src/global/error_handler.rs b/opentelemetry/src/global/error_handler.rs index ebf8969909..7fe58b9d8b 100644 --- a/opentelemetry/src/global/error_handler.rs +++ b/opentelemetry/src/global/error_handler.rs @@ -15,9 +15,13 @@ 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) => { eprintln!("OpenTelemetry metrics error occurred {:?}", err) } + #[cfg(feature = "trace")] + #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] OpenTelemetryError::TraceErr(err) => { eprintln!("OpenTelemetry trace error occurred {:?}", err) } diff --git a/opentelemetry/src/global/mod.rs b/opentelemetry/src/global/mod.rs index 2148a22124..7318b656f2 100644 --- a/opentelemetry/src/global/mod.rs +++ b/opentelemetry/src/global/mod.rs @@ -129,7 +129,6 @@ //! [installing a metrics pipeline]: crate::exporter::metrics::stdout::StdoutExporterBuilder::try_init //! [`MeterProvider`]: crate::metrics::MeterProvider -#[cfg(feature = "metrics")] mod error_handler; #[cfg(feature = "metrics")] mod metrics; @@ -138,8 +137,6 @@ mod propagation; #[cfg(feature = "trace")] mod trace; -#[cfg(feature = "metrics")] -#[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] pub use error_handler::{handle_error, set_error_handler}; #[cfg(feature = "metrics")] #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 0655266aba..89f9208192 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -296,7 +296,7 @@ impl BatchSpanProcessor { ); } let send_result = ch.send(results); - if let Err(_) = send_result { + if send_result.is_err() { global::handle_error(TraceError::Other("fail to send the export response from worker handle in BatchProcessor".to_string())) } } @@ -336,7 +336,7 @@ impl BatchSpanProcessor { } exporter.shutdown(); let send_result = ch.send(results); - if let Err(_) = send_result { + if send_result.is_err() { global::handle_error(TraceError::Other("fail to send the export response from worker handle in BatchProcessor".to_string())) } break; diff --git a/opentelemetry/src/testing/trace.rs b/opentelemetry/src/testing/trace.rs index 20a6976146..2f148cc1ec 100644 --- a/opentelemetry/src/testing/trace.rs +++ b/opentelemetry/src/testing/trace.rs @@ -9,8 +9,7 @@ use crate::{ KeyValue, }; use async_trait::async_trait; -use serde::export::Formatter; -use std::fmt::Display; +use std::fmt::{Display, Formatter}; use std::sync::mpsc::{channel, Receiver, Sender}; use std::time::SystemTime; From 487c33d718df78fcd28d45c13eb4b8fab14817c7 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sat, 28 Nov 2020 15:14:23 -0500 Subject: [PATCH 11/18] 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 | 32 +++++--------- opentelemetry/src/exporter/trace/stdout.rs | 21 ++------- opentelemetry/src/global/error_handler.rs | 16 +++---- 13 files changed, 49 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..6193552f64 100644 --- a/opentelemetry/src/api/mod.rs +++ b/opentelemetry/src/api/mod.rs @@ -19,35 +19,23 @@ 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)] +#[non_exhaustive] +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..e90bb3c842 100644 --- a/opentelemetry/src/exporter/trace/stdout.rs +++ b/opentelemetry/src/exporter/trace/stdout.rs @@ -31,8 +31,8 @@ use crate::{ trace::TracerProvider, }; use async_trait::async_trait; -use std::fmt::{Debug, Display, Formatter}; use std::io::{stdout, Stdout, Write}; +use std::fmt::Debug; /// Pipeline builder #[derive(Debug)] @@ -150,22 +150,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() From 9ffbace2edec4657a25b00bfade6373396db7811 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sat, 28 Nov 2020 21:24:05 -0500 Subject: [PATCH 12/18] fix: address comments * Add ExportError in MetricsError. It can help unify the export error among trace, metric and logging in the future. * Merge TraceError::Other into TraceError::Boxed. Those two seems to have a lot in common. Created a custom wrap type for string and metric those two variants. * Remove default implement for exporter_name. It should have used in case like http client failure. But it could also encourage ppl not implement it at all. --- examples/actix-http/src/main.rs | 7 ++--- examples/actix-udp/src/main.rs | 7 ++--- examples/async/src/main.rs | 7 ++--- examples/basic-otlp/src/main.rs | 9 ++---- examples/basic/src/main.rs | 7 ++--- examples/grpc/src/client.rs | 6 ++-- examples/grpc/src/server.rs | 6 ++-- .../src/trace/exporter/datadog/model/mod.rs | 3 +- opentelemetry-jaeger/src/lib.rs | 9 ++---- opentelemetry-otlp/src/lib.rs | 2 +- opentelemetry-zipkin/src/lib.rs | 11 +++++-- opentelemetry/src/api/metrics/mod.rs | 12 +++++++- opentelemetry/src/api/trace/mod.rs | 29 ++++++++++++++----- opentelemetry/src/exporter/mod.rs | 6 ++++ opentelemetry/src/exporter/trace/mod.rs | 15 ++++------ opentelemetry/src/exporter/trace/stdout.rs | 8 +++-- opentelemetry/src/global/error_handler.rs | 12 ++------ .../src/sdk/metrics/aggregators/ddsketch.rs | 2 +- .../src/sdk/metrics/processors/basic.rs | 2 +- opentelemetry/src/sdk/trace/span_processor.rs | 10 +++---- opentelemetry/src/testing/trace.rs | 6 ++-- 21 files changed, 91 insertions(+), 85 deletions(-) diff --git a/examples/actix-http/src/main.rs b/examples/actix-http/src/main.rs index 2a3ea6d1ec..dbdd5d219d 100644 --- a/examples/actix-http/src/main.rs +++ b/examples/actix-http/src/main.rs @@ -1,17 +1,14 @@ use actix_service::Service; use actix_web::middleware::Logger; use actix_web::{web, App, HttpServer}; +use opentelemetry::trace::TraceError; use opentelemetry::{global, sdk::trace as sdktrace}; use opentelemetry::{ trace::{FutureExt, TraceContextExt, Tracer}, Key, }; -use opentelemetry::trace::TraceError; -fn init_tracer() -> Result< - (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - TraceError -> { +fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_jaeger::Uninstall), TraceError> { opentelemetry_jaeger::new_pipeline() .with_collector_endpoint("http://127.0.0.1:14268/api/traces") .with_service_name("trace-http-demo") diff --git a/examples/actix-udp/src/main.rs b/examples/actix-udp/src/main.rs index b7ccc35b64..11de6bdc3a 100644 --- a/examples/actix-udp/src/main.rs +++ b/examples/actix-udp/src/main.rs @@ -1,17 +1,14 @@ use actix_service::Service; use actix_web::middleware::Logger; use actix_web::{web, App, HttpServer}; +use opentelemetry::trace::TraceError; use opentelemetry::{global, sdk::trace as sdktrace}; use opentelemetry::{ trace::{FutureExt, TraceContextExt, Tracer}, Key, }; -use opentelemetry::trace::TraceError; -fn init_tracer() -> Result< - (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - TraceError -> { +fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_jaeger::Uninstall), TraceError> { opentelemetry_jaeger::new_pipeline() .with_agent_endpoint("localhost:6831") .with_service_name("trace-udp-demo") diff --git a/examples/async/src/main.rs b/examples/async/src/main.rs index e0e84d8a2a..aff021ba81 100644 --- a/examples/async/src/main.rs +++ b/examples/async/src/main.rs @@ -17,6 +17,7 @@ //! cargo run --example async_fn //! //! [`hello_world`]: https://github.com/tokio-rs/tokio/blob/132e9f1da5965530b63554d7a1c59824c3de4e30/tokio/examples/hello_world.rs +use opentelemetry::trace::TraceError; use opentelemetry::{ global, sdk::trace as sdktrace, @@ -26,7 +27,6 @@ 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"); @@ -53,10 +53,7 @@ async fn run(addr: &SocketAddr) -> io::Result { write(&mut stream).with_context(cx).await } -fn init_tracer() -> Result< - (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - TraceError -> { +fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_jaeger::Uninstall), TraceError> { opentelemetry_jaeger::new_pipeline() .with_service_name("trace-demo") .install() diff --git a/examples/basic-otlp/src/main.rs b/examples/basic-otlp/src/main.rs index 483f374e0b..74c140ade9 100644 --- a/examples/basic-otlp/src/main.rs +++ b/examples/basic-otlp/src/main.rs @@ -1,6 +1,7 @@ use futures::stream::{Stream, StreamExt}; use opentelemetry::exporter; use opentelemetry::sdk::metrics::PushController; +use opentelemetry::trace::TraceError; use opentelemetry::{ baggage::BaggageExt, metrics::{self, MetricsError, ObserverResult}, @@ -10,13 +11,9 @@ 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), TraceError> -{ - opentelemetry_otlp::new_pipeline() - .install() +fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_otlp::Uninstall), TraceError> { + opentelemetry_otlp::new_pipeline().install() } // 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 fd5bb2f430..fee5280057 100644 --- a/examples/basic/src/main.rs +++ b/examples/basic/src/main.rs @@ -2,6 +2,7 @@ use futures::stream::{Stream, StreamExt}; use opentelemetry::exporter; use opentelemetry::global; use opentelemetry::sdk::{metrics::PushController, trace as sdktrace}; +use opentelemetry::trace::TraceError; use opentelemetry::{ baggage::BaggageExt, metrics::{self, MetricsError, ObserverResult}, @@ -10,12 +11,8 @@ use opentelemetry::{ }; use std::error::Error; use std::time::Duration; -use opentelemetry::trace::TraceError; -fn init_tracer() -> Result< - (sdktrace::Tracer, opentelemetry_jaeger::Uninstall), - TraceError, -> { +fn init_tracer() -> Result<(sdktrace::Tracer, opentelemetry_jaeger::Uninstall), TraceError> { opentelemetry_jaeger::new_pipeline() .with_service_name("trace-demo") .with_tags(vec![ diff --git a/examples/grpc/src/client.rs b/examples/grpc/src/client.rs index c76ad459f0..ab2e95f9ec 100644 --- a/examples/grpc/src/client.rs +++ b/examples/grpc/src/client.rs @@ -2,19 +2,17 @@ use hello_world::greeter_client::GreeterClient; use hello_world::HelloRequest; use opentelemetry::global; use opentelemetry::sdk::propagation::TraceContextPropagator; +use opentelemetry::trace::TraceError; use opentelemetry::{ trace::{TraceContextExt, Tracer}, Context, KeyValue, }; -use opentelemetry::trace::TraceError; pub mod hello_world { tonic::include_proto!("helloworld"); } -fn tracing_init( -) -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall),TraceError> -{ +fn tracing_init() -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall), TraceError> { global::set_text_map_propagator(TraceContextPropagator::new()); opentelemetry_jaeger::new_pipeline() .with_service_name("grpc-client") diff --git a/examples/grpc/src/server.rs b/examples/grpc/src/server.rs index 12c0ea09b1..d066bdd99d 100644 --- a/examples/grpc/src/server.rs +++ b/examples/grpc/src/server.rs @@ -4,12 +4,12 @@ use hello_world::greeter_server::{Greeter, GreeterServer}; use hello_world::{HelloReply, HelloRequest}; use opentelemetry::global; use opentelemetry::sdk::propagation::TraceContextPropagator; +use opentelemetry::trace::TraceError; use opentelemetry::{ trace::{Span, Tracer}, 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,9 +37,7 @@ impl Greeter for MyGreeter { } } -fn tracing_init( -) -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall), TraceError> -{ +fn tracing_init() -> Result<(impl Tracer, opentelemetry_jaeger::Uninstall), TraceError> { global::set_text_map_propagator(TraceContextPropagator::new()); opentelemetry_jaeger::new_pipeline() .with_service_name("grpc-server") diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs index b1def406da..a24a98c740 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/model/mod.rs @@ -1,5 +1,4 @@ -use opentelemetry::exporter::trace; -use opentelemetry::exporter::trace::ExportError; +use opentelemetry::exporter::{trace, ExportError}; mod v03; mod v05; diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index 4b3e619d22..a317590d2a 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -194,7 +194,8 @@ use agent::AgentAsyncClientUDP; use async_trait::async_trait; #[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))] use collector::CollectorAsyncClientHttp; -use opentelemetry::exporter::trace::ExportError; +use opentelemetry::exporter::ExportError; +use opentelemetry::trace::TraceError; use opentelemetry::{ exporter::trace, global, sdk, @@ -206,7 +207,6 @@ 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"; @@ -417,10 +417,7 @@ impl PipelineBuilder { } /// Install a Jaeger pipeline with the recommended defaults. - pub fn install( - self, - ) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> - { + pub fn install(self) -> Result<(sdk::trace::Tracer, Uninstall), TraceError> { let tracer_provider = self.build()?; let tracer = tracer_provider.get_tracer("opentelemetry-jaeger", Some(env!("CARGO_PKG_VERSION"))); diff --git a/opentelemetry-otlp/src/lib.rs b/opentelemetry-otlp/src/lib.rs index dae73a5919..ab669f3073 100644 --- a/opentelemetry-otlp/src/lib.rs +++ b/opentelemetry-otlp/src/lib.rs @@ -131,7 +131,7 @@ mod span; mod transform; pub use crate::span::{Compression, Credentials, Exporter, ExporterConfig, Protocol}; -use opentelemetry::exporter::trace::ExportError; +use opentelemetry::exporter::ExportError; use opentelemetry::trace::TraceError; /// Create a new pipeline builder with the recommended configuration. diff --git a/opentelemetry-zipkin/src/lib.rs b/opentelemetry-zipkin/src/lib.rs index ac4acfd2b7..0459ee450d 100644 --- a/opentelemetry-zipkin/src/lib.rs +++ b/opentelemetry-zipkin/src/lib.rs @@ -171,9 +171,14 @@ mod uploader; use async_trait::async_trait; use http::Uri; use model::endpoint::Endpoint; -use opentelemetry::exporter::trace::{ExportError, HttpClient}; -use opentelemetry::trace::TraceError; -use opentelemetry::{exporter::trace, global, sdk, trace::TracerProvider}; +use opentelemetry::{ + exporter::{ + trace::{self, HttpClient}, + ExportError, + }, + global, sdk, + trace::{TraceError, TracerProvider}, +}; use std::net::SocketAddr; /// Default Zipkin collector endpoint diff --git a/opentelemetry/src/api/metrics/mod.rs b/opentelemetry/src/api/metrics/mod.rs index f3172a0682..7e3dc5927a 100644 --- a/opentelemetry/src/api/metrics/mod.rs +++ b/opentelemetry/src/api/metrics/mod.rs @@ -19,6 +19,7 @@ mod sync_instrument; mod up_down_counter; mod value_recorder; +use crate::exporter::ExportError; pub use async_instrument::{AsyncRunner, BatchObserverCallback, Observation, ObserverResult}; pub use config::InstrumentConfig; pub use counter::{BoundCounter, Counter, CounterBuilder}; @@ -38,7 +39,7 @@ pub use value_recorder::{BoundValueRecorder, ValueRecorder, ValueRecorderBuilder pub type Result = result::Result; /// Errors returned by the metrics API. -#[derive(Error, Debug, PartialEq)] +#[derive(Error, Debug)] #[non_exhaustive] pub enum MetricsError { /// Other errors not covered by specific cases. @@ -68,6 +69,15 @@ pub enum MetricsError { /// Errors when aggregator cannot subtract #[error("Aggregator does not subtract")] NoSubtraction, + /// Fail to export metrics + #[error("Export metrics failed with {0}")] + ExportErr(Box), +} + +impl From for MetricsError { + fn from(err: T) -> Self { + MetricsError::ExportErr(Box::new(err)) + } } impl From> for MetricsError { diff --git a/opentelemetry/src/api/trace/mod.rs b/opentelemetry/src/api/trace/mod.rs index 19bb155160..161c85fea1 100644 --- a/opentelemetry/src/api/trace/mod.rs +++ b/opentelemetry/src/api/trace/mod.rs @@ -139,7 +139,7 @@ pub use self::{ }, tracer::{SpanBuilder, Tracer}, }; -use crate::exporter::trace::ExportError; +use crate::exporter::ExportError; use std::time; /// Describe the result of operations in tracing API. @@ -149,10 +149,6 @@ pub type TraceResult = Result; #[derive(Error, Debug)] #[non_exhaustive] pub enum TraceError { - /// Other errors not covered by specific cases. - #[error("Trace error: {0}")] - Other(String), - /// Export failed with the error returned by the exporter #[error("Exporting failed with {0}")] ExportFailed(Box), @@ -163,7 +159,7 @@ pub enum TraceError { /// Other errors propagated from trace SDK that weren't covered above #[error(transparent)] - Boxed(#[from] Box), + Other(#[from] Box), } impl From for TraceError @@ -177,12 +173,29 @@ where impl From> for TraceError { fn from(err: TrySendError) -> Self { - TraceError::Boxed(Box::new(err.into_send_error())) + TraceError::Other(Box::new(err.into_send_error())) } } impl From for TraceError { fn from(err: Canceled) -> Self { - TraceError::Boxed(Box::new(err)) + TraceError::Other(Box::new(err)) } } + +impl From for TraceError { + fn from(err_msg: String) -> Self { + TraceError::Other(Box::new(Custom(err_msg))) + } +} + +impl From<&'static str> for TraceError { + fn from(err_msg: &'static str) -> Self { + TraceError::Other(Box::new(Custom(err_msg.into()))) + } +} + +/// Wrap type for string +#[derive(Error, Debug)] +#[error("{0}")] +struct Custom(String); diff --git a/opentelemetry/src/exporter/mod.rs b/opentelemetry/src/exporter/mod.rs index 85a5e842b5..7ded6fa9be 100644 --- a/opentelemetry/src/exporter/mod.rs +++ b/opentelemetry/src/exporter/mod.rs @@ -14,3 +14,9 @@ pub mod metrics; #[cfg(feature = "trace")] #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] pub mod trace; + +/// Marker trait for errors returned by exporters +pub trait ExportError: std::error::Error + Send + Sync + 'static { + /// The name of exporter that returned this error + fn exporter_name(&self) -> &'static str; +} diff --git a/opentelemetry/src/exporter/trace/mod.rs b/opentelemetry/src/exporter/trace/mod.rs index 623aa8c661..0b5bccf15c 100644 --- a/opentelemetry/src/exporter/trace/mod.rs +++ b/opentelemetry/src/exporter/trace/mod.rs @@ -1,5 +1,6 @@ //! Trace exporters use crate::api::trace::TraceError; +use crate::exporter::ExportError; use crate::{ sdk, trace::{Event, Link, SpanContext, SpanId, SpanKind, StatusCode}, @@ -22,14 +23,6 @@ pub mod stdout; /// Describes the result of an export. pub type ExportResult = Result<(), TraceError>; -/// Marker trait for errors returned by exporters -pub trait ExportError: std::error::Error + Send + Sync + 'static { - /// The name of exporter that returned this error - fn exporter_name(&self) -> &'static str { - "N/A" - } -} - /// `SpanExporter` defines the interface that protocol-specific exporters must /// implement so that they can be plugged into OpenTelemetry SDK and support /// sending of telemetry data. @@ -102,7 +95,11 @@ impl std::error::Error for HttpClientError {} #[cfg(feature = "http")] #[cfg_attr(docsrs, doc(cfg(feature = "http")))] -impl ExportError for HttpClientError {} +impl ExportError for HttpClientError { + fn exporter_name(&self) -> &'static str { + "http client" + } +} #[cfg(feature = "http")] #[cfg_attr(docsrs, doc(cfg(feature = "http")))] diff --git a/opentelemetry/src/exporter/trace/stdout.rs b/opentelemetry/src/exporter/trace/stdout.rs index e90bb3c842..437b39a520 100644 --- a/opentelemetry/src/exporter/trace/stdout.rs +++ b/opentelemetry/src/exporter/trace/stdout.rs @@ -24,15 +24,17 @@ //! }); //! } //! ``` -use crate::exporter::trace::ExportError; use crate::{ - exporter::trace::{ExportResult, SpanData, SpanExporter}, + exporter::{ + trace::{ExportResult, SpanData, SpanExporter}, + ExportError, + }, global, sdk, trace::TracerProvider, }; use async_trait::async_trait; -use std::io::{stdout, Stdout, Write}; use std::fmt::Debug; +use std::io::{stdout, Stdout, Write}; /// Pipeline builder #[derive(Debug)] diff --git a/opentelemetry/src/global/error_handler.rs b/opentelemetry/src/global/error_handler.rs index fcf92cebba..d982878175 100644 --- a/opentelemetry/src/global/error_handler.rs +++ b/opentelemetry/src/global/error_handler.rs @@ -17,17 +17,11 @@ pub fn handle_error>(err: T) { _ => match err.into() { #[cfg(feature = "metrics")] #[cfg_attr(docsrs, doc(cfg(feature = "metrics")))] - Error::Metric(err) => { - eprintln!("OpenTelemetry metrics error occurred {:?}", err) - } + Error::Metric(err) => eprintln!("OpenTelemetry metrics error occurred {:?}", err), #[cfg(feature = "trace")] #[cfg_attr(docsrs, doc(cfg(feature = "trace")))] - Error::Trace(err) => { - eprintln!("OpenTelemetry trace error occurred {:?}", err) - } - Error::Other(err_msg) => { - println!("OpenTelemetry error occurred {}", err_msg) - } + Error::Trace(err) => eprintln!("OpenTelemetry trace error occurred {:?}", err), + Error::Other(err_msg) => println!("OpenTelemetry error occurred {}", err_msg), }, } } diff --git a/opentelemetry/src/sdk/metrics/aggregators/ddsketch.rs b/opentelemetry/src/sdk/metrics/aggregators/ddsketch.rs index b989da3461..50d957353f 100644 --- a/opentelemetry/src/sdk/metrics/aggregators/ddsketch.rs +++ b/opentelemetry/src/sdk/metrics/aggregators/ddsketch.rs @@ -1019,7 +1019,7 @@ mod tests { (moved_ddsketch.sum().unwrap().to_f64(&NumberKind::F64) - expected_sum).abs() < std::f64::EPSILON ); - assert_eq!(moved_ddsketch.count(), Ok(expected_count)); + assert_eq!(moved_ddsketch.count().unwrap(), expected_count); // assert can generate same result for q in test_quantiles() { diff --git a/opentelemetry/src/sdk/metrics/processors/basic.rs b/opentelemetry/src/sdk/metrics/processors/basic.rs index 6a9df37fba..24ed972879 100644 --- a/opentelemetry/src/sdk/metrics/processors/basic.rs +++ b/opentelemetry/src/sdk/metrics/processors/basic.rs @@ -367,7 +367,7 @@ impl CheckpointSet for BasicProcessorState { start, self.interval_end, )); - if res == Err(MetricsError::NoDataCollected) { + if let Err(MetricsError::NoDataCollected) = res { Ok(()) } else { res diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 89f9208192..90619de0ba 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -129,7 +129,7 @@ impl SpanProcessor for SimpleSpanProcessor { if let Ok(mut exporter) = self.exporter.lock() { let _result = executor::block_on(exporter.export(vec![span])); } else { - global::handle_error(TraceError::Other("When export span with the SimpleSpanProcessor, the exporter's lock has been poisoned".to_string())); + global::handle_error(TraceError::from("When export span with the SimpleSpanProcessor, the exporter's lock has been poisoned")); } } @@ -217,7 +217,7 @@ impl SpanProcessor for BatchSpanProcessor { } fn force_flush(&self) -> TraceResult<()> { - let mut sender = self.message_sender.lock().map_err(|_| TraceError::Other("When force flushing the BatchSpanProcessor, the message sender's lock has been poisoned".into()))?; + let mut sender = self.message_sender.lock().map_err(|_| TraceError::from("When force flushing the BatchSpanProcessor, the message sender's lock has been poisoned"))?; let (res_sender, res_receiver) = oneshot::channel::>(); sender.try_send(BatchMessage::Flush(Some(res_sender)))?; for result in futures::executor::block_on(res_receiver)? { @@ -227,7 +227,7 @@ impl SpanProcessor for BatchSpanProcessor { } fn shutdown(&mut self) -> TraceResult<()> { - let mut sender = self.message_sender.lock().map_err(|_| TraceError::Other("When shutting down the BatchSpanProcessor, the message sender's lock has been poisoned".into()))?; + let mut sender = self.message_sender.lock().map_err(|_| TraceError::from("When shutting down the BatchSpanProcessor, the message sender's lock has been poisoned"))?; let (res_sender, res_receiver) = oneshot::channel::>(); sender.try_send(BatchMessage::Shutdown(res_sender))?; for result in futures::executor::block_on(res_receiver)? { @@ -297,7 +297,7 @@ impl BatchSpanProcessor { } let send_result = ch.send(results); if send_result.is_err() { - global::handle_error(TraceError::Other("fail to send the export response from worker handle in BatchProcessor".to_string())) + global::handle_error(TraceError::from("fail to send the export response from worker handle in BatchProcessor")) } } BatchMessage::Flush(None) => { @@ -337,7 +337,7 @@ impl BatchSpanProcessor { exporter.shutdown(); let send_result = ch.send(results); if send_result.is_err() { - global::handle_error(TraceError::Other("fail to send the export response from worker handle in BatchProcessor".to_string())) + global::handle_error(TraceError::from("fail to send the export response from worker handle in BatchProcessor")) } break; } diff --git a/opentelemetry/src/testing/trace.rs b/opentelemetry/src/testing/trace.rs index 0fd473df07..ea74b2e019 100644 --- a/opentelemetry/src/testing/trace.rs +++ b/opentelemetry/src/testing/trace.rs @@ -1,6 +1,8 @@ -use crate::exporter::trace::{ExportError, SpanData}; use crate::{ - exporter::trace::{self as exporter, ExportResult, SpanExporter}, + exporter::{ + trace::{self as exporter, ExportResult, SpanData, SpanExporter}, + ExportError, + }, sdk::{ trace::{Config, EvictedHashMap, EvictedQueue}, InstrumentationLibrary, From fb54263f7cdc20f46d9dfb3ccae8c4fc2fb09ebc Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sat, 28 Nov 2020 21:39:50 -0500 Subject: [PATCH 13/18] fix: address comments * Remove HttpClientError --- opentelemetry/src/exporter/trace/mod.rs | 100 +++++++----------------- 1 file changed, 28 insertions(+), 72 deletions(-) diff --git a/opentelemetry/src/exporter/trace/mod.rs b/opentelemetry/src/exporter/trace/mod.rs index 0b5bccf15c..5e16062e17 100644 --- a/opentelemetry/src/exporter/trace/mod.rs +++ b/opentelemetry/src/exporter/trace/mod.rs @@ -1,5 +1,6 @@ //! Trace exporters use crate::api::trace::TraceError; +#[cfg(feature = "http")] use crate::exporter::ExportError; use crate::{ sdk, @@ -13,7 +14,7 @@ use serde::{Deserialize, Serialize}; #[cfg(all(feature = "http", feature = "reqwest"))] use std::convert::TryInto; use std::fmt::Debug; -#[cfg(feature = "http")] +#[cfg(all(feature = "surf", feature = "http"))] use std::fmt::{Display, Formatter}; use std::sync::Arc; use std::time::SystemTime; @@ -72,66 +73,38 @@ pub trait HttpClient: Debug + Send + Sync { async fn send(&self, request: Request>) -> ExportResult; } -/// Error when sending http requests visa HttpClient implementation -#[cfg(feature = "http")] -#[cfg_attr(docsrs, doc(cfg(feature = "http")))] -#[derive(Debug)] -pub enum HttpClientError { - /// Errors from reqwest - #[cfg(all(feature = "reqwest", feature = "http"))] - ReqwestError(reqwest::Error), - - /// Errors from surf - #[cfg(all(feature = "surf", feature = "http"))] - SurfError(surf::Error), - - /// Other errors - Other(String), +#[cfg(all(feature = "reqwest", feature = "http"))] +impl ExportError for reqwest::Error { + fn exporter_name(&self) -> &'static str { + "reqwest" + } } -#[cfg(feature = "http")] -#[cfg_attr(docsrs, doc(cfg(feature = "http")))] -impl std::error::Error for HttpClientError {} - -#[cfg(feature = "http")] -#[cfg_attr(docsrs, doc(cfg(feature = "http")))] -impl ExportError for HttpClientError { +#[cfg(all(feature = "surf", feature = "http"))] +impl ExportError for SurfError { fn exporter_name(&self) -> &'static str { - "http client" + "surf" } } -#[cfg(feature = "http")] -#[cfg_attr(docsrs, doc(cfg(feature = "http")))] -impl Display for HttpClientError { +#[cfg(all(feature = "surf", feature = "http"))] +#[derive(Debug)] +struct SurfError(surf::Error); + +#[cfg(all(feature = "surf", feature = "http"))] +impl Display for SurfError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - #[cfg(all(feature = "reqwest", feature = "http"))] - HttpClientError::ReqwestError(err) => { - write!(f, "error when sending requests using reqwest, {}", err) - } - #[cfg(all(feature = "surf", feature = "http"))] - HttpClientError::SurfError(err) => write!( - f, - "error when sending requests using surf, {}", - err.to_string() - ), - HttpClientError::Other(msg) => write!(f, "{}", msg), - } + write!(f, "{}", self.0.to_string()) } } -#[cfg(all(feature = "reqwest", feature = "http"))] -impl From for HttpClientError { - fn from(err: reqwest::Error) -> Self { - HttpClientError::ReqwestError(err) - } -} +#[cfg(all(feature = "surf", feature = "http"))] +impl std::error::Error for SurfError {} #[cfg(all(feature = "surf", feature = "http"))] -impl From for HttpClientError { +impl From for SurfError { fn from(err: surf::Error) -> Self { - HttpClientError::SurfError(err) + SurfError(err) } } @@ -174,15 +147,9 @@ pub struct SpanData { impl HttpClient for reqwest::Client { async fn send(&self, request: Request>) -> ExportResult { let _result = self - .execute( - request - .try_into() - .map_err::(Into::into)?, - ) - .await - .map_err::(Into::into)? - .error_for_status() - .map_err::(Into::into)?; + .execute(request.try_into()?) + .await? + .error_for_status()?; Ok(()) } } @@ -191,15 +158,7 @@ impl HttpClient for reqwest::Client { #[async_trait] impl HttpClient for reqwest::blocking::Client { async fn send(&self, request: Request>) -> ExportResult { - let _result = self - .execute( - request - .try_into() - .map_err::(Into::into)?, - ) - .map_err::(Into::into)? - .error_for_status() - .map_err::(Into::into)?; + let _result = self.execute(request.try_into()?)?.error_for_status()?; Ok(()) } } @@ -213,20 +172,17 @@ impl HttpClient for surf::Client { .uri .to_string() .parse() - .map_err(|err: surf::http::url::ParseError| HttpClientError::Other(err.to_string()))?; + .map_err(|_err: surf::http::url::ParseError| TraceError::from("error parse url"))?; let req = surf::Request::builder(surf::http::Method::Post, uri) .content_type("application/json") .body(body); - let result = self - .send(req) - .await - .map_err::(Into::into)?; + let result = self.send(req).await.map_err::(Into::into)?; if result.status().is_success() { Ok(()) } else { - Err(HttpClientError::SurfError(surf::Error::from_str( + Err(SurfError(surf::Error::from_str( result.status(), result.status().canonical_reason(), )) From 118044481268bd7124a4213b6fdef7f727d06ab7 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sun, 29 Nov 2020 17:03:15 -0500 Subject: [PATCH 14/18] fix: fix msrv, address comments and add comments. --- .github/workflows/ci.yml | 2 +- CONTRIBUTING.md | 7 +++++++ opentelemetry-jaeger/src/lib.rs | 5 +---- opentelemetry/src/sdk/trace/span_processor.rs | 3 +-- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1321129e54..d9c697a688 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,7 +52,7 @@ jobs: override: true - name: Run tests run: cargo --version && - cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,async-std,serde,http,tonic,reqwest && + cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,serde,http,tonic,reqwest && cargo test --manifest-path=opentelemetry-jaeger/Cargo.toml && cargo test --manifest-path=opentelemetry-zipkin/Cargo.toml meta: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index fbe00c0f18..6fc6e4144a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -93,6 +93,13 @@ patterns in the spec. For a deeper discussion, see: https://github.com/open-telemetry/opentelemetry-specification/issues/165 +### Error Handling +Currently, the Opentelemetry Rust SDK has two ways to handle errors. In the situation where errors are not allowed to return. One should call global error handler to process the errors. Otherwise, one should return the errors. + +The Opentelemetry Rust SDK comes with an error type `openetelemetry::Error`. For different function, one error has been defined. All error returned by trace module MUST be wrapped in `opentelemetry::api::trace::TraceError`. All errors returned by metrics module MUST be wrapped in `opentelemetry::api::trace::MetricsError`. + +For users that want to implement their own exporters. It's RECOMMENDED to wrap all errors from the exporter into a crate-level error type, and implement `ExporterError` trait. + ## Style Guide * Run `cargo clippy --all` - this will catch common mistakes and improve diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index a317590d2a..25209fa6cf 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -428,10 +428,7 @@ impl PipelineBuilder { } /// Build a configured `sdk::trace::TracerProvider` with the recommended defaults. - pub fn build( - mut self, - ) -> Result> - { + pub fn build(mut self) -> Result { let config = self.config.take(); let exporter = self.init_exporter()?; diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 90619de0ba..7b805ae9c8 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -291,8 +291,7 @@ impl BatchSpanProcessor { exporter.as_mut(), &delay, batch, - ) - .await, + ).await, ); } let send_result = ch.send(results); From 1d1f65615d9e7d8e2a7751a12207f28a3e03f284 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sun, 29 Nov 2020 17:26:50 -0500 Subject: [PATCH 15/18] fix: fix msrv --- .github/workflows/ci.yml | 2 +- opentelemetry/Cargo.toml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9c697a688..1321129e54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,7 +52,7 @@ jobs: override: true - name: Run tests run: cargo --version && - cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,serde,http,tonic,reqwest && + cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,async-std,serde,http,tonic,reqwest && cargo test --manifest-path=opentelemetry-jaeger/Cargo.toml && cargo test --manifest-path=opentelemetry-zipkin/Cargo.toml meta: diff --git a/opentelemetry/Cargo.toml b/opentelemetry/Cargo.toml index 9cafb15292..3ae84a908b 100644 --- a/opentelemetry/Cargo.toml +++ b/opentelemetry/Cargo.toml @@ -22,6 +22,7 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] async-std = { version = "1.6", features = ["unstable"], default-features = false, optional = true } +async-io = { version = "~1.2.0", optional = true } async-trait = { version = "0.1", optional = true } bincode = { version = "1.2", optional = true } dashmap = { version = "4.0.0-rc6", optional = true } From 5aeeb22728759dd1f444e3911023ffa338fc80d3 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sun, 29 Nov 2020 18:47:03 -0500 Subject: [PATCH 16/18] fix: clean the remaining std::error::Error --- CONTRIBUTING.md | 2 +- opentelemetry-jaeger/src/lib.rs | 37 +++++++++++++-------------------- opentelemetry-zipkin/src/lib.rs | 4 ++-- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6fc6e4144a..d63c98f1d3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -96,7 +96,7 @@ https://github.com/open-telemetry/opentelemetry-specification/issues/165 ### Error Handling Currently, the Opentelemetry Rust SDK has two ways to handle errors. In the situation where errors are not allowed to return. One should call global error handler to process the errors. Otherwise, one should return the errors. -The Opentelemetry Rust SDK comes with an error type `openetelemetry::Error`. For different function, one error has been defined. All error returned by trace module MUST be wrapped in `opentelemetry::api::trace::TraceError`. All errors returned by metrics module MUST be wrapped in `opentelemetry::api::trace::MetricsError`. +The Opentelemetry Rust SDK comes with an error type `openetelemetry::Error`. For different function, one error has been defined. All error returned by trace module MUST be wrapped in `opentelemetry::trace::TraceError`. All errors returned by metrics module MUST be wrapped in `opentelemetry::metrics::MetricsError`. For users that want to implement their own exporters. It's RECOMMENDED to wrap all errors from the exporter into a crate-level error type, and implement `ExporterError` trait. diff --git a/opentelemetry-jaeger/src/lib.rs b/opentelemetry-jaeger/src/lib.rs index 25209fa6cf..4f8b3198b3 100644 --- a/opentelemetry-jaeger/src/lib.rs +++ b/opentelemetry-jaeger/src/lib.rs @@ -22,7 +22,7 @@ //! ```no_run //! use opentelemetry::trace::Tracer; //! -//! fn main() -> Result<(), Box> { +//! fn main() -> Result<(), opentelemetry::trace::TraceError> { //! let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline().install()?; //! //! tracer.in_span("doing_work", |cx| { @@ -59,9 +59,9 @@ //! [jaeger variables spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/sdk-environment-variables.md#jaeger-exporter //! //! ```no_run -//! use opentelemetry::trace::Tracer; +//! use opentelemetry::trace::{Tracer, TraceError}; //! -//! fn main() -> Result<(), Box> { +//! fn main() -> Result<(), TraceError> { //! // export OTEL_SERVICE_NAME=my-service-name //! let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline().from_env().install()?; //! @@ -90,9 +90,9 @@ //! //! ```ignore //! // Note that this requires the `collector_client` feature. -//! use opentelemetry::trace::Tracer; +//! use opentelemetry::trace::{Tracer, TraceError}; //! -//! fn main() -> Result<(), Box> { +//! fn main() -> Result<(), TraceError> { //! let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline() //! .with_collector_endpoint("http://localhost:14268/api/traces") //! // optionally set username and password as well. @@ -116,10 +116,10 @@ //! [`PipelineBuilder`]: struct.PipelineBuilder.html //! //! ```no_run -//! use opentelemetry::{KeyValue, trace::Tracer}; +//! use opentelemetry::{KeyValue, trace::{Tracer, TraceError}}; //! use opentelemetry::sdk::{trace::{self, IdGenerator, Sampler}, Resource}; //! -//! fn main() -> Result<(), Box> { +//! fn main() -> Result<(), TraceError> { //! let (tracer, _uninstall) = opentelemetry_jaeger::new_pipeline() //! .from_env() //! .with_agent_endpoint("localhost:6831") @@ -444,9 +444,7 @@ impl PipelineBuilder { /// Initialize a new exporter. /// /// This is useful if you are manually constructing a pipeline. - pub fn init_exporter( - self, - ) -> Result> { + pub fn init_exporter(self) -> Result { let export_instrumentation_lib = self.export_instrument_library; let (process, uploader) = self.init_uploader()?; @@ -458,30 +456,25 @@ impl PipelineBuilder { } #[cfg(not(any(feature = "collector_client", feature = "wasm_collector_client")))] - fn init_uploader( - self, - ) -> Result<(Process, BatchUploader), Box> { - let agent = AgentAsyncClientUDP::new(self.agent_endpoint.as_slice())?; + fn init_uploader(self) -> Result<(Process, BatchUploader), TraceError> { + let agent = AgentAsyncClientUDP::new(self.agent_endpoint.as_slice()) + .map_err::(Into::into)?; Ok((self.process, BatchUploader::Agent(agent))) } #[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))] - fn init_uploader( - self, - ) -> Result< - (Process, uploader::BatchUploader), - Box, - > { + fn init_uploader(self) -> Result<(Process, uploader::BatchUploader), TraceError> { if let Some(collector_endpoint) = self.collector_endpoint { let collector = CollectorAsyncClientHttp::new( collector_endpoint, self.collector_username, self.collector_password, - )?; + ) + .map_err::(Into::into)?; Ok((self.process, uploader::BatchUploader::Collector(collector))) } else { let endpoint = self.agent_endpoint.as_slice(); - let agent = AgentAsyncClientUDP::new(endpoint)?; + let agent = AgentAsyncClientUDP::new(endpoint).map_err::(Into::into)?; Ok((self.process, BatchUploader::Agent(agent))) } } diff --git a/opentelemetry-zipkin/src/lib.rs b/opentelemetry-zipkin/src/lib.rs index 0459ee450d..dda11342a9 100644 --- a/opentelemetry-zipkin/src/lib.rs +++ b/opentelemetry-zipkin/src/lib.rs @@ -21,9 +21,9 @@ //! telemetry: //! //! ```no_run -//! use opentelemetry::trace::Tracer; +//! use opentelemetry::trace::{Tracer, TraceError}; //! -//! fn main() -> Result<(), Box> { +//! fn main() -> Result<(), TraceError> { //! let (tracer, _uninstall) = opentelemetry_zipkin::new_pipeline().install()?; //! //! tracer.in_span("doing_work", |cx| { From fa8dae34fb0fce8c1c5de4e2140a694c4dd39f39 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sun, 29 Nov 2020 20:39:41 -0500 Subject: [PATCH 17/18] fix: format --- opentelemetry-contrib/src/trace/exporter/datadog/mod.rs | 2 +- opentelemetry/src/sdk/trace/span_processor.rs | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs index 71fc76fda6..1ac8dfe402 100644 --- a/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs +++ b/opentelemetry-contrib/src/trace/exporter/datadog/mod.rs @@ -98,7 +98,7 @@ //! } //! //! fn main() -> Result<(), opentelemetry::trace::TraceError> { -//! let (tracer, _uninstall) = new_pipeline() +//! let (tracer, _uninstall) = new_pipeline() //! .with_service_name("my_app") //! .with_version(ApiVersion::Version05) //! .with_agent_endpoint("http://localhost:8126") diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index 7b805ae9c8..ac7b93911f 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -310,8 +310,7 @@ impl BatchSpanProcessor { exporter.as_mut(), &delay, batch, - ) - .await; + ).await; } } // Stream has terminated or processor is shutdown, return to finish execution. @@ -329,8 +328,7 @@ impl BatchSpanProcessor { exporter.as_mut(), &delay, batch, - ) - .await, + ).await, ); } exporter.shutdown(); @@ -342,8 +340,7 @@ impl BatchSpanProcessor { } } } - })) - .map(|_| ()); + })).map(|_| ()); // Return batch processor with link to worker BatchSpanProcessor { From 47836eaab9042e8e03e3507f96b7bb844805d240 Mon Sep 17 00:00:00 2001 From: "zhongyang.wu" Date: Sun, 29 Nov 2020 22:34:46 -0500 Subject: [PATCH 18/18] fix: msrv --- .github/workflows/ci.yml | 2 +- opentelemetry/Cargo.toml | 2 -- opentelemetry/src/sdk/trace/span_processor.rs | 6 +++++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1321129e54..d9c697a688 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,7 +52,7 @@ jobs: override: true - name: Run tests run: cargo --version && - cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,async-std,serde,http,tonic,reqwest && + cargo test --verbose --manifest-path=opentelemetry/Cargo.toml --features trace,metrics,serialize,tokio,serde,http,tonic,reqwest && cargo test --manifest-path=opentelemetry-jaeger/Cargo.toml && cargo test --manifest-path=opentelemetry-zipkin/Cargo.toml meta: diff --git a/opentelemetry/Cargo.toml b/opentelemetry/Cargo.toml index 3ae84a908b..82338977d2 100644 --- a/opentelemetry/Cargo.toml +++ b/opentelemetry/Cargo.toml @@ -22,7 +22,6 @@ rustdoc-args = ["--cfg", "docsrs"] [dependencies] async-std = { version = "1.6", features = ["unstable"], default-features = false, optional = true } -async-io = { version = "~1.2.0", optional = true } async-trait = { version = "0.1", optional = true } bincode = { version = "1.2", optional = true } dashmap = { version = "4.0.0-rc6", optional = true } @@ -48,7 +47,6 @@ js-sys = "0.3" criterion = "0.3.1" rand_distr = "0.3.0" tokio = { version = "0.2", features = ["full"] } -async-std = { version = "1.6", features = ["unstable"] } [features] default = ["trace"] diff --git a/opentelemetry/src/sdk/trace/span_processor.rs b/opentelemetry/src/sdk/trace/span_processor.rs index ac7b93911f..43dbe41606 100644 --- a/opentelemetry/src/sdk/trace/span_processor.rs +++ b/opentelemetry/src/sdk/trace/span_processor.rs @@ -567,7 +567,6 @@ mod tests { use std::time; use std::time::Duration; - use async_std::prelude::*; use async_trait::async_trait; use crate::exporter::trace::{stdout, ExportResult, SpanData, SpanExporter}; @@ -577,6 +576,8 @@ mod tests { new_test_export_span_data, new_test_exporter, new_tokio_test_exporter, }; + use futures::Future; + use super::{ BatchSpanProcessor, SimpleSpanProcessor, SpanProcessor, OTEL_BSP_MAX_EXPORT_BATCH_SIZE, OTEL_BSP_MAX_QUEUE_SIZE, OTEL_BSP_MAX_QUEUE_SIZE_DEFAULT, OTEL_BSP_SCHEDULE_DELAY_MILLIS, @@ -726,17 +727,20 @@ mod tests { } #[test] + #[cfg(feature = "async-std")] fn test_timeout_async_std_timeout() { async_std::task::block_on(timeout_test_std_async(true)); } #[test] + #[cfg(feature = "async-std")] fn test_timeout_async_std_not_timeout() { async_std::task::block_on(timeout_test_std_async(false)); } // If the time_out is true, then the result suppose to ended with timeout. // otherwise the exporter should be able to export within time out duration. + #[cfg(feature = "async-std")] async fn timeout_test_std_async(time_out: bool) { let mut config = BatchConfig::default(); config.max_export_timeout = time::Duration::from_millis(if time_out { 5 } else { 60 });