Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/tracing-jaeger/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
6 changes: 4 additions & 2 deletions examples/tracing-jaeger/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down
9 changes: 9 additions & 0 deletions opentelemetry-otlp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion opentelemetry-otlp/src/exporter/http/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

let (body, content_type) = self
.build_logs_export_body(batch)
.map_err(|e| OTelSdkError::InternalFailure(e.to_string()))?;
.map_err(OTelSdkError::InternalFailure)?;

Check warning on line 18 in opentelemetry-otlp/src/exporter/http/logs.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L18 was not covered by tests

let mut request = http::Request::builder()
.method(Method::POST)
Expand Down
51 changes: 28 additions & 23 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -108,7 +108,7 @@
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,
Expand Down Expand Up @@ -168,12 +168,12 @@
.unwrap_or_else(|_| reqwest::blocking::Client::new())
})
.join()
.unwrap(), // Unwrap thread result
.unwrap(), // TODO: Return ExporterBuildError::ThreadSpawnFailed

Check warning on line 171 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L171 was not covered by tests
) as Arc<dyn HttpClient>);
}
}

let http_client = http_client.ok_or(crate::Error::NoHttpClient)?;
let http_client = http_client.ok_or(ExporterBuildError::NoHttpClient)?;

Check warning on line 176 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L176 was not covered by tests

#[allow(clippy::mutable_key_type)] // http headers are not mutated
let mut headers: HashMap<HeaderName, HeaderValue> = self
Expand Down Expand Up @@ -208,9 +208,7 @@

/// 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> {

Check warning on line 211 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L211 was not covered by tests
use crate::{
OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, OTEL_EXPORTER_OTLP_TRACES_HEADERS,
OTEL_EXPORTER_OTLP_TRACES_TIMEOUT,
Expand All @@ -228,7 +226,7 @@

/// 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,
Expand All @@ -249,7 +247,7 @@
pub fn build_metrics_exporter(
mut self,
temporality: opentelemetry_sdk::metrics::Temporality,
) -> opentelemetry_sdk::metrics::MetricResult<crate::MetricExporter> {
) -> Result<crate::MetricExporter, ExporterBuildError> {

Check warning on line 250 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L250 was not covered by tests
use crate::{
OTEL_EXPORTER_OTLP_METRICS_ENDPOINT, OTEL_EXPORTER_OTLP_METRICS_HEADERS,
OTEL_EXPORTER_OTLP_METRICS_TIMEOUT,
Expand Down Expand Up @@ -309,7 +307,7 @@
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")),

Check warning on line 310 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L310 was not covered by tests
Err(e) => Err(opentelemetry_sdk::trace::TraceError::from(e.to_string())),
},
_ => Ok((req.encode_to_vec(), "application/x-protobuf")),
Expand All @@ -320,7 +318,7 @@
fn build_logs_export_body(
&self,
logs: LogBatch<'_>,
) -> opentelemetry_sdk::logs::LogResult<(Vec<u8>, &'static str)> {
) -> Result<(Vec<u8>, &'static str), String> {

Check warning on line 321 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L321 was not covered by tests
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 };
Expand All @@ -329,7 +327,7 @@
#[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()),

Check warning on line 330 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L330 was not covered by tests
},
_ => Ok((req.encode_to_vec(), "application/x-protobuf")),
}
Expand Down Expand Up @@ -357,21 +355,24 @@
}
}

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())

Check warning on line 366 in opentelemetry-otlp/src/exporter/http/mod.rs

View check run for this annotation

Codecov / codecov/patch

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

Added line #L366 was not covered by tests
})
}

// 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()
Expand All @@ -388,14 +389,18 @@
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
Expand Down
90 changes: 87 additions & 3 deletions opentelemetry-otlp/src/exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -92,6 +93,44 @@
}
}

#[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)]
Expand All @@ -112,13 +151,15 @@
}

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(),
)),

Check warning on line 162 in opentelemetry-otlp/src/exporter/mod.rs

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/mod.rs#L160-L162

Added lines #L160 - L162 were not covered by tests
}
}
}
Expand Down Expand Up @@ -298,6 +339,49 @@
assert_eq!(exporter_builder.exporter_config.endpoint, None);
}

#[cfg(feature = "logs")]
#[cfg(any(feature = "http-proto", feature = "http-json"))]
#[test]
fn 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!(exporter_result.is_err());
}

#[cfg(feature = "grpc-tonic")]
#[test]
fn 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!(exporter_result.is_err());
}

#[cfg(feature = "grpc-tonic")]
#[test]
fn test_default_tonic_endpoint() {
Expand Down
Loading