diff --git a/opentelemetry-zipkin/CHANGELOG.md b/opentelemetry-zipkin/CHANGELOG.md index 01bb1ddd7a..560327f4b5 100644 --- a/opentelemetry-zipkin/CHANGELOG.md +++ b/opentelemetry-zipkin/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## unreleased + +## Added + +- Add support for OTEL_EXPORTER_ZIPKIN_* variables. #718 + +## Changed + +- Add defaults for timeouts to HTTP clients #718 + ## v0.15.0 ### Changed @@ -116,5 +126,5 @@ ### Added -- Exporter to Zipkin collector through HTTP API +- Exporter to Zipkin collector through HTTP API diff --git a/opentelemetry-zipkin/Cargo.toml b/opentelemetry-zipkin/Cargo.toml index b6f35241bc..9027218def 100644 --- a/opentelemetry-zipkin/Cargo.toml +++ b/opentelemetry-zipkin/Cargo.toml @@ -34,6 +34,7 @@ serde_json = "1.0" serde = { version = "1.0", features = ["derive"] } typed-builder = "0.9" lazy_static = "1.4" +log = "0.4" http = "0.2" reqwest = { version = "0.11", optional = true, default-features = false } surf = { version = "2.0", optional = true, default-features = false } diff --git a/opentelemetry-zipkin/src/exporter/env.rs b/opentelemetry-zipkin/src/exporter/env.rs new file mode 100644 index 0000000000..860545f2c4 --- /dev/null +++ b/opentelemetry-zipkin/src/exporter/env.rs @@ -0,0 +1,64 @@ +use log::warn; +use std::env; +use std::time::Duration; + +/// Default Zipkin collector endpoint +const DEFAULT_COLLECTOR_ENDPOINT: &str = "http://127.0.0.1:9411/api/v2/spans"; + +/// HTTP endpoint for Zipkin collector. +/// e.g. "http://localhost:9411/api/v2/spans" +const ENV_ENDPOINT: &str = "OTEL_EXPORTER_ZIPKIN_ENDPOINT"; + +/// Maximum time the Zipkin exporter will wait for each batch export +const ENV_TIMEOUT: &str = "OTEL_EXPORTER_ZIPKIN_TIMEOUT"; + +/// Default Zipkin timeout in milliseconds +const DEFAULT_COLLECTOR_TIMEOUT: Duration = Duration::from_millis(10_000); + +pub(crate) fn get_timeout() -> Duration { + match env::var(ENV_TIMEOUT).ok().filter(|var| !var.is_empty()) { + Some(timeout) => match timeout.parse() { + Ok(timeout) => Duration::from_millis(timeout), + Err(e) => { + warn!("{} malformed defaulting to 10000: {}", ENV_TIMEOUT, e); + DEFAULT_COLLECTOR_TIMEOUT + } + }, + None => DEFAULT_COLLECTOR_TIMEOUT, + } +} + +pub(crate) fn get_endpoint() -> String { + match env::var(ENV_ENDPOINT).ok().filter(|var| !var.is_empty()) { + Some(endpoint) => endpoint, + None => DEFAULT_COLLECTOR_ENDPOINT.to_string(), + } +} + +#[test] +fn test_collector_defaults() { + // Ensure the variables are undefined. + env::remove_var(ENV_TIMEOUT); + env::remove_var(ENV_ENDPOINT); + assert_eq!(DEFAULT_COLLECTOR_TIMEOUT, get_timeout()); + assert_eq!(DEFAULT_COLLECTOR_ENDPOINT, get_endpoint()); +} + +#[test] +fn test_collector_bad_timeout() { + env::set_var(ENV_TIMEOUT, "a"); + assert_eq!(DEFAULT_COLLECTOR_TIMEOUT, get_timeout()); +} + +#[test] +fn test_collector_good_timeout() { + env::set_var(ENV_TIMEOUT, "777"); + assert_eq!(Duration::from_millis(777), get_timeout()); +} + +#[test] +fn test_collector_custom_endpoint() { + let custom_endpoint = "https://example.com/api/v2/spans"; + env::set_var(ENV_ENDPOINT, custom_endpoint); + assert_eq!(custom_endpoint, get_endpoint()); +} diff --git a/opentelemetry-zipkin/src/exporter/mod.rs b/opentelemetry-zipkin/src/exporter/mod.rs index b57e0c79c8..4dc79811aa 100644 --- a/opentelemetry-zipkin/src/exporter/mod.rs +++ b/opentelemetry-zipkin/src/exporter/mod.rs @@ -1,3 +1,4 @@ +mod env; mod model; mod uploader; @@ -21,9 +22,6 @@ use std::net::SocketAddr; use std::sync::Arc; use std::time::Duration; -/// Default Zipkin collector endpoint -const DEFAULT_COLLECTOR_ENDPOINT: &str = "http://127.0.0.1:9411/api/v2/spans"; - /// Zipkin span exporter #[derive(Debug)] pub struct Exporter { @@ -57,21 +55,31 @@ pub struct ZipkinPipelineBuilder { impl Default for ZipkinPipelineBuilder { fn default() -> Self { + let timeout = env::get_timeout(); ZipkinPipelineBuilder { #[cfg(feature = "reqwest-blocking-client")] - client: Some(Box::new(reqwest::blocking::Client::new())), + client: Some(Box::new( + reqwest::blocking::Client::builder() + .timeout(timeout) + .build() + .unwrap(), + )), #[cfg(all( not(feature = "reqwest-blocking-client"), not(feature = "surf-client"), feature = "reqwest-client" ))] - client: Some(Box::new(reqwest::Client::new())), + client: Some(Box::new( + reqwest::Client::builder().timeout(timeout).build().unwrap(), + )), #[cfg(all( not(feature = "reqwest-client"), not(feature = "reqwest-blocking-client"), feature = "surf-client" ))] - client: Some(Box::new(surf::Client::new())), + client: Some(Box::new( + surf::Config::new().set_timeout(Some(timeout)).into(), + )), #[cfg(all( not(feature = "reqwest-client"), not(feature = "surf-client"), @@ -81,7 +89,7 @@ impl Default for ZipkinPipelineBuilder { service_name: None, service_addr: None, - collector_endpoint: DEFAULT_COLLECTOR_ENDPOINT.to_string(), + collector_endpoint: env::get_endpoint(), trace_config: None, } }