-
Notifications
You must be signed in to change notification settings - Fork 525
feat: OTLP Exporter builders to return specific Error type #2790
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
Changes from all commits
f4b3606
9783477
786df2d
35e9b17
3171749
9dfe6c5
3f2a208
9b5c7f2
157fb81
d131ea7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 @@ | |
} | ||
} | ||
|
||
#[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, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will only happens if reqwest-blocking is used right? If so can we guard it in feature flag? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, thats why I put a big TODO on this. we definitely need to refine it and see which ones makes sense and remove others. |
||
|
||
/// 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}'")] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm looks like a little bit inconsistent if the first letter is upper case across the variant? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes will address them in follow ups. |
||
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 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 @@ | |
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() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unintended change for the
{
?