feat: OTLP Exporter builders to return specific Error type (#2790)

Author: cijothomas

examples/tracing-jaeger/Cargo.toml

@@ -8,5 +8,5 @@ publish = false
 [dependencies]
 opentelemetry = { path = "../../opentelemetry" }
 opentelemetry_sdk = { path = "../../opentelemetry-sdk", features = ["rt-tokio"] }
-opentelemetry-otlp = { workspace = true, features = ["tonic"] }
+opentelemetry-otlp = { workspace = true, features = ["grpc-tonic"] }
 tokio = { workspace = true, features = ["full"] }

examples/tracing-jaeger/src/main.rs

@@ -3,12 +3,14 @@ use opentelemetry::{
     trace::{TraceContextExt, Tracer},
     KeyValue,
 };
-use opentelemetry_sdk::trace::{SdkTracerProvider, TraceError};
+use opentelemetry_otlp::ExporterBuildError;
+use opentelemetry_sdk::trace::SdkTracerProvider;
 use opentelemetry_sdk::Resource;
 
 use std::error::Error;
 
-fn init_tracer_provider() -> Result<opentelemetry_sdk::trace::SdkTracerProvider, TraceError> {
+fn init_tracer_provider() -> Result<opentelemetry_sdk::trace::SdkTracerProvider, ExporterBuildError>
+{
     let exporter = opentelemetry_otlp::SpanExporter::builder()
         .with_tonic()
         .build()?;

opentelemetry-otlp/CHANGELOG.md

@@ -8,6 +8,15 @@
   [#2770](https://github.com/open-telemetry/opentelemetry-rust/issues/2770)
   partially to properly handle `shutdown()` when using `http`. (`tonic` still
   does not do proper shutdown)
+- *Breaking*
+ ExporterBuilder's build() method now Result with `ExporterBuildError` being the
+ Error variant. Previously it returned signal specific errors like `LogError`
+ from the `opentelemetry_sdk`, which are no longer part of the sdk. No changes
+ required if you were using unwrap/expect. If you were matching on the returning
+ Error enum, replace with the enum `ExporterBuildError`. Unlike the previous
+ `Error` which contained many variants unrelated to building an exporter, the
+ new one returns specific variants applicable to building an exporter. Some
+ variants might be applicable only on select features.
 
 ## 0.28.0
 

opentelemetry-otlp/src/exporter/http/logs.rs

@@ -15,7 +15,7 @@ impl LogExporter for OtlpHttpClient {
 
         let (body, content_type) = self
             .build_logs_export_body(batch)
-            .map_err(|e| OTelSdkError::InternalFailure(e.to_string()))?;
+            .map_err(OTelSdkError::InternalFailure)?;
 
         let mut request = http::Request::builder()
             .method(Method::POST)

opentelemetry-otlp/src/exporter/http/mod.rs

@@ -1,5 +1,5 @@
 use super::{
-    default_headers, default_protocol, parse_header_string,
+    default_headers, default_protocol, parse_header_string, ExporterBuildError,
     OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT,
 };
 use crate::{
@@ -108,7 +108,7 @@ impl HttpExporterBuilder {
         signal_endpoint_path: &str,
         signal_timeout_var: &str,
         signal_http_headers_var: &str,
-    ) -> Result<OtlpHttpClient, crate::Error> {
+    ) -> Result<OtlpHttpClient, ExporterBuildError> {
         let endpoint = resolve_http_endpoint(
             signal_endpoint_var,
             signal_endpoint_path,
@@ -168,12 +168,12 @@ impl HttpExporterBuilder {
                             .unwrap_or_else(|_| reqwest::blocking::Client::new())
                     })
                     .join()
-                    .unwrap(), // Unwrap thread result
+                    .unwrap(), // TODO: Return ExporterBuildError::ThreadSpawnFailed
                 ) as Arc<dyn HttpClient>);
             }
         }
 
-        let http_client = http_client.ok_or(crate::Error::NoHttpClient)?;
+        let http_client = http_client.ok_or(ExporterBuildError::NoHttpClient)?;
 
         #[allow(clippy::mutable_key_type)] // http headers are not mutated
         let mut headers: HashMap<HeaderName, HeaderValue> = self
@@ -208,9 +208,7 @@ impl HttpExporterBuilder {
 
     /// Create a log exporter with the current configuration
     #[cfg(feature = "trace")]
-    pub fn build_span_exporter(
-        mut self,
-    ) -> Result<crate::SpanExporter, opentelemetry_sdk::trace::TraceError> {
+    pub fn build_span_exporter(mut self) -> Result<crate::SpanExporter, ExporterBuildError> {
         use crate::{
             OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, OTEL_EXPORTER_OTLP_TRACES_HEADERS,
             OTEL_EXPORTER_OTLP_TRACES_TIMEOUT,
@@ -228,7 +226,7 @@ impl HttpExporterBuilder {
 
     /// Create a log exporter with the current configuration
     #[cfg(feature = "logs")]
-    pub fn build_log_exporter(mut self) -> opentelemetry_sdk::logs::LogResult<crate::LogExporter> {
+    pub fn build_log_exporter(mut self) -> Result<crate::LogExporter, ExporterBuildError> {
         use crate::{
             OTEL_EXPORTER_OTLP_LOGS_ENDPOINT, OTEL_EXPORTER_OTLP_LOGS_HEADERS,
             OTEL_EXPORTER_OTLP_LOGS_TIMEOUT,
@@ -249,7 +247,7 @@ impl HttpExporterBuilder {
     pub fn build_metrics_exporter(
         mut self,
         temporality: opentelemetry_sdk::metrics::Temporality,
-    ) -> opentelemetry_sdk::metrics::MetricResult<crate::MetricExporter> {
+    ) -> Result<crate::MetricExporter, ExporterBuildError> {
         use crate::{
             OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, OTEL_EXPORTER_OTLP_METRICS_HEADERS,
             OTEL_EXPORTER_OTLP_METRICS_TIMEOUT,
@@ -309,7 +307,7 @@ impl OtlpHttpClient {
         match self.protocol {
             #[cfg(feature = "http-json")]
             Protocol::HttpJson => match serde_json::to_string_pretty(&req) {
-                Ok(json) => Ok((json.into(), "application/json")),
+                Ok(json) => Ok((json.into_bytes(), "application/json")),
                 Err(e) => Err(opentelemetry_sdk::trace::TraceError::from(e.to_string())),
             },
             _ => Ok((req.encode_to_vec(), "application/x-protobuf")),
@@ -320,7 +318,7 @@ impl OtlpHttpClient {
     fn build_logs_export_body(
         &self,
         logs: LogBatch<'_>,
-    ) -> opentelemetry_sdk::logs::LogResult<(Vec<u8>, &'static str)> {
+    ) -> Result<(Vec<u8>, &'static str), String> {
         use opentelemetry_proto::tonic::collector::logs::v1::ExportLogsServiceRequest;
         let resource_logs = group_logs_by_resource_and_scope(logs, &self.resource);
         let req = ExportLogsServiceRequest { resource_logs };
@@ -329,7 +327,7 @@ impl OtlpHttpClient {
             #[cfg(feature = "http-json")]
             Protocol::HttpJson => match serde_json::to_string_pretty(&req) {
                 Ok(json) => Ok((json.into(), "application/json")),
-                Err(e) => Err(opentelemetry_sdk::logs::LogError::from(e.to_string())),
+                Err(e) => Err(e.to_string()),
             },
             _ => Ok((req.encode_to_vec(), "application/x-protobuf")),
         }
@@ -357,21 +355,24 @@ impl OtlpHttpClient {
     }
 }
 
-fn build_endpoint_uri(endpoint: &str, path: &str) -> Result<Uri, crate::Error> {
+fn build_endpoint_uri(endpoint: &str, path: &str) -> Result<Uri, ExporterBuildError> {
     let path = if endpoint.ends_with('/') && path.starts_with('/') {
         path.strip_prefix('/').unwrap()
     } else {
         path
     };
-    format!("{endpoint}{path}").parse().map_err(From::from)
+    let endpoint = format!("{endpoint}{path}");
+    endpoint.parse().map_err(|er: http::uri::InvalidUri| {
+        ExporterBuildError::InvalidUri(endpoint, er.to_string())
+    })
 }
 
 // see https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#endpoint-urls-for-otlphttp
 fn resolve_http_endpoint(
     signal_endpoint_var: &str,
     signal_endpoint_path: &str,
     provided_endpoint: Option<String>,
-) -> Result<Uri, crate::Error> {
+) -> Result<Uri, ExporterBuildError> {
     // per signal env var is not modified
     if let Some(endpoint) = env::var(signal_endpoint_var)
         .ok()
@@ -388,14 +389,18 @@ fn resolve_http_endpoint(
         return Ok(endpoint);
     }
 
-    provided_endpoint
-        .map(|e| e.parse().map_err(From::from))
-        .unwrap_or_else(|| {
-            build_endpoint_uri(
-                OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT,
-                signal_endpoint_path,
-            )
-        })
+    if let Some(provider_endpoint) = provided_endpoint {
+        provider_endpoint
+            .parse()
+            .map_err(|er: http::uri::InvalidUri| {
+                ExporterBuildError::InvalidUri(provider_endpoint, er.to_string())
+            })
+    } else {
+        build_endpoint_uri(
+            OTEL_EXPORTER_OTLP_HTTP_ENDPOINT_DEFAULT,
+            signal_endpoint_path,
+        )
+    }
 }
 
 #[allow(clippy::mutable_key_type)] // http headers are not mutated

opentelemetry-otlp/src/exporter/mod.rs

@@ -6,12 +6,13 @@
 use crate::exporter::http::HttpExporterBuilder;
 #[cfg(feature = "grpc-tonic")]
 use crate::exporter::tonic::TonicExporterBuilder;
-use crate::{Error, Protocol};
+use crate::Protocol;
 #[cfg(feature = "serialize")]
 use serde::{Deserialize, Serialize};
 use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 use std::time::Duration;
+use thiserror::Error;
 
 /// Target to which the exporter is going to send signals, defaults to https://localhost:4317.
 /// Learn about the relationship between this constant and metrics/spans/logs at
@@ -92,6 +93,44 @@ impl Default for ExportConfig {
     }
 }
 
+#[derive(Error, Debug)]
+/// Errors that can occur while building an exporter.
+// TODO: Refine and polish this.
+// Non-exhaustive to allow for future expansion without breaking changes.
+// This could be refined after polishing and finalizing the errors.
+#[non_exhaustive]
+pub enum ExporterBuildError {
+    /// Spawning a new thread failed.
+    #[error("Spawning a new thread failed. Unable to create Reqwest-Blocking client.")]
+    ThreadSpawnFailed,
+
+    /// Feature required to use the specified compression algorithm.
+    #[cfg(any(not(feature = "gzip-tonic"), not(feature = "zstd-tonic")))]
+    #[error("feature '{0}' is required to use the compression algorithm '{1}'")]
+    FeatureRequiredForCompressionAlgorithm(&'static str, Compression),
+
+    /// No Http client specified.
+    #[error("no http client specified")]
+    NoHttpClient,
+
+    /// Unsupported compression algorithm.
+    #[error("unsupported compression algorithm '{0}'")]
+    UnsupportedCompressionAlgorithm(String),
+
+    /// Invalid URI.
+    #[cfg(any(feature = "grpc-tonic", feature = "http-proto", feature = "http-json"))]
+    #[error("invalid URI {0}. Reason {1}")]
+    InvalidUri(String, String),
+
+    /// Failed due to an internal error.
+    /// The error message is intended for logging purposes only and should not
+    /// be used to make programmatic decisions. It is implementation-specific
+    /// and subject to change without notice. Consumers of this error should not
+    /// rely on its content beyond logging.
+    #[error("Reason: {0}")]
+    InternalFailure(String),
+}
+
 /// The compression algorithm to use when sending data.
 #[cfg_attr(feature = "serialize", derive(Deserialize, Serialize))]
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
@@ -112,13 +151,15 @@ impl Display for Compression {
 }
 
 impl FromStr for Compression {
-    type Err = Error;
+    type Err = ExporterBuildError;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "gzip" => Ok(Compression::Gzip),
             "zstd" => Ok(Compression::Zstd),
-            _ => Err(Error::UnsupportedCompressionAlgorithm(s.to_string())),
+            _ => Err(ExporterBuildError::UnsupportedCompressionAlgorithm(
+                s.to_string(),
+            )),
         }
     }
 }
@@ -298,6 +339,55 @@ mod tests {
         assert_eq!(exporter_builder.exporter_config.endpoint, None);
     }
 
+    #[cfg(feature = "logs")]
+    #[cfg(any(feature = "http-proto", feature = "http-json"))]
+    #[test]
+    fn export_builder_error_invalid_http_endpoint() {
+        use std::time::Duration;
+
+        use crate::{ExportConfig, LogExporter, Protocol, WithExportConfig};
+
+        let ex_config = ExportConfig {
+            endpoint: Some("invalid_uri/something".to_string()),
+            protocol: Protocol::HttpBinary,
+            timeout: Duration::from_secs(10),
+        };
+
+        let exporter_result = LogExporter::builder()
+            .with_http()
+            .with_export_config(ex_config)
+            .build();
+
+        assert!(matches!(
+            exporter_result,
+            Err(crate::exporter::ExporterBuildError::InvalidUri(_, _))
+        ));
+    }
+
+    #[cfg(feature = "grpc-tonic")]
+    #[test]
+    fn export_builder_error_invalid_grpc_endpoint() {
+        use std::time::Duration;
+
+        use crate::{ExportConfig, LogExporter, Protocol, WithExportConfig};
+
+        let ex_config = ExportConfig {
+            endpoint: Some("invalid_uri/something".to_string()),
+            protocol: Protocol::Grpc,
+            timeout: Duration::from_secs(10),
+        };
+
+        let exporter_result = LogExporter::builder()
+            .with_tonic()
+            .with_export_config(ex_config)
+            .build();
+
+        assert!(matches!(
+            exporter_result,
+            Err(crate::exporter::ExporterBuildError::InvalidUri(_, _))
+        ));
+    }
+
     #[cfg(feature = "grpc-tonic")]
     #[test]
     fn test_default_tonic_endpoint() {