Skip to content

Commit e108b1e

Browse files
hdostTommyCpp
hdost
and
TommyCpp
authored
opentelemetry-jaeger: Add Timeout Environment Var (open-telemetry#729)
OTEL_EXPORTER_JAEGER_TIMEOUT is now supported for all cases where a non-custom HTTP Client is utilized. Closes open-telemetry#528 Signed-off-by: Harold Dost <[email protected]> Co-authored-by: Zhongyang Wu <[email protected]>
1 parent ed71d80 commit e108b1e

File tree

3 files changed

+84
-21
lines changed

3 files changed

+84
-21
lines changed

‎opentelemetry-jaeger/src/exporter/env.rs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use crate::PipelineBuilder;
22
use std::env;
3+
#[cfg(feature = "collector_client")]
4+
use std::time::Duration;
35

46
/// The hostname for the Jaeger agent.
57
/// e.g. "localhost"
@@ -14,6 +16,14 @@ const ENV_AGENT_PORT: &str = "OTEL_EXPORTER_JAEGER_AGENT_PORT";
1416
#[cfg(feature = "collector_client")]
1517
const ENV_ENDPOINT: &str = "OTEL_EXPORTER_JAEGER_ENDPOINT";
1618

19+
/// Timeout for Jaeger collector.
20+
#[cfg(feature = "collector_client")]
21+
pub(crate) const ENV_TIMEOUT: &str = "OTEL_EXPORTER_JAEGER_TIMEOUT";
22+
23+
/// Default of 10s
24+
#[cfg(feature = "collector_client")]
25+
pub(crate) const DEFAULT_COLLECTOR_TIMEOUT: Duration = Duration::from_secs(10);
26+
1727
/// Username to send as part of "Basic" authentication to the collector endpoint.
1828
#[cfg(feature = "collector_client")]
1929
const ENV_USER: &str = "OTEL_EXPORTER_JAEGER_USER";
@@ -30,20 +40,23 @@ pub(crate) fn assign_attrs(mut builder: PipelineBuilder) -> PipelineBuilder {
3040

3141
#[cfg(feature = "collector_client")]
3242
{
43+
if let Some(timeout) = env::var(ENV_TIMEOUT).ok().filter(|var| !var.is_empty()) {
44+
let timeout = match timeout.parse() {
45+
Ok(timeout) => Duration::from_millis(timeout),
46+
Err(e) => {
47+
eprintln!("{} malformed defaulting to 10000: {}", ENV_TIMEOUT, e);
48+
DEFAULT_COLLECTOR_TIMEOUT
49+
}
50+
};
51+
builder = builder.with_collector_timeout(timeout);
52+
}
3353
if let Some(endpoint) = env::var(ENV_ENDPOINT).ok().filter(|var| !var.is_empty()) {
3454
builder = builder.with_collector_endpoint(endpoint);
3555
}
36-
}
3756

38-
#[cfg(feature = "collector_client")]
39-
{
4057
if let Some(user) = env::var(ENV_USER).ok().filter(|var| !var.is_empty()) {
4158
builder = builder.with_collector_username(user);
4259
}
43-
}
44-
45-
#[cfg(feature = "collector_client")]
46-
{
4760
if let Some(password) = env::var(ENV_PASSWORD).ok().filter(|var| !var.is_empty()) {
4861
builder = builder.with_collector_password(password);
4962
}

‎opentelemetry-jaeger/src/exporter/mod.rs

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ mod env;
1111
pub(crate) mod transport;
1212
mod uploader;
1313

14+
// Linting isn't detecting that it's used seems like linting bug.
15+
#[allow(unused_imports)]
16+
#[cfg(feature = "surf_collector_client")]
17+
use std::convert::TryFrom;
18+
1419
use self::runtime::JaegerTraceRuntime;
1520
use self::thrift::jaeger;
1621
use agent::AgentAsyncClientUdp;
@@ -110,6 +115,10 @@ impl trace::SpanExporter for Exporter {
110115
#[derive(Debug)]
111116
pub struct PipelineBuilder {
112117
agent_endpoint: Vec<net::SocketAddr>,
118+
// There are many variations in which it's read unclear which is causing not to be.
119+
#[allow(dead_code)]
120+
#[cfg(feature = "collector_client")]
121+
collector_timeout: Duration,
113122
#[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))]
114123
collector_endpoint: Option<Result<http::Uri, http::uri::InvalidUri>>,
115124
#[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))]
@@ -131,6 +140,8 @@ impl Default for PipelineBuilder {
131140
fn default() -> Self {
132141
let builder_defaults = PipelineBuilder {
133142
agent_endpoint: vec![DEFAULT_AGENT_ENDPOINT.parse().unwrap()],
143+
#[cfg(feature = "collector_client")]
144+
collector_timeout: env::DEFAULT_COLLECTOR_TIMEOUT,
134145
#[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))]
135146
collector_endpoint: None,
136147
#[cfg(any(feature = "collector_client", feature = "wasm_collector_client"))]
@@ -173,6 +184,18 @@ impl PipelineBuilder {
173184
}
174185
}
175186

187+
/// Assign the collector timeout
188+
///
189+
/// E.g. "10s"
190+
#[cfg(feature = "collector_client")]
191+
#[cfg_attr(docsrs, doc(cfg(feature = "collector_client")))]
192+
pub fn with_collector_timeout(self, collector_timeout: Duration) -> Self {
193+
PipelineBuilder {
194+
collector_timeout,
195+
..self
196+
}
197+
}
198+
176199
/// Assign the collector endpoint.
177200
///
178201
/// E.g. "http://localhost:14268/api/traces"
@@ -519,7 +542,8 @@ impl PipelineBuilder {
519542

520543
#[cfg(feature = "isahc_collector_client")]
521544
let client = self.client.unwrap_or({
522-
let mut builder = isahc::HttpClient::builder();
545+
let mut builder = isahc::HttpClient::builder().timeout(self.collector_timeout);
546+
523547
if let (Some(username), Some(password)) =
524548
(self.collector_username, self.collector_password)
525549
{
@@ -546,12 +570,13 @@ impl PipelineBuilder {
546570
))]
547571
let client = self.client.unwrap_or({
548572
#[cfg(feature = "reqwest_collector_client")]
549-
let mut builder = reqwest::ClientBuilder::new();
573+
let mut builder = reqwest::ClientBuilder::new().timeout(self.collector_timeout);
550574
#[cfg(all(
551575
not(feature = "reqwest_collector_client"),
552576
feature = "reqwest_blocking_collector_client"
553577
))]
554-
let mut builder = reqwest::blocking::ClientBuilder::new();
578+
let mut builder =
579+
reqwest::blocking::ClientBuilder::new().timeout(self.collector_timeout);
555580
if let (Some(username), Some(password)) =
556581
(self.collector_username, self.collector_password)
557582
{
@@ -573,13 +598,18 @@ impl PipelineBuilder {
573598
not(feature = "reqwest_blocking_collector_client")
574599
))]
575600
let client = self.client.unwrap_or({
601+
let client = surf::Client::try_from(
602+
surf::Config::new().set_timeout(Some(self.collector_timeout)),
603+
)
604+
.unwrap_or_else(|_| surf::Client::new());
605+
576606
let client = if let (Some(username), Some(password)) =
577607
(self.collector_username, self.collector_password)
578608
{
579609
let auth = surf::http::auth::BasicAuth::new(username, password);
580-
surf::Client::new().with(BasicAuthMiddleware(auth))
610+
client.with(BasicAuthMiddleware(auth))
581611
} else {
582-
surf::Client::new()
612+
client
583613
};
584614

585615
Box::new(client)
@@ -830,6 +860,32 @@ impl ExportError for Error {
830860
}
831861
}
832862

863+
#[cfg(test)]
864+
#[cfg(all(feature = "collector_client"))]
865+
mod timeout_env_tests {
866+
use crate::exporter::env;
867+
use crate::exporter::PipelineBuilder;
868+
use std::time::Duration;
869+
870+
#[test]
871+
fn test_collector_defaults() {
872+
// No Env Variable
873+
std::env::remove_var(env::ENV_TIMEOUT);
874+
let builder = PipelineBuilder::default();
875+
assert_eq!(env::DEFAULT_COLLECTOR_TIMEOUT, builder.collector_timeout);
876+
877+
// Bad Env Variable
878+
std::env::set_var(env::ENV_TIMEOUT, "a");
879+
let builder = PipelineBuilder::default();
880+
assert_eq!(env::DEFAULT_COLLECTOR_TIMEOUT, builder.collector_timeout);
881+
882+
// Good Env Variable
883+
std::env::set_var(env::ENV_TIMEOUT, "777");
884+
let builder = PipelineBuilder::default();
885+
assert_eq!(Duration::from_millis(777), builder.collector_timeout);
886+
}
887+
}
888+
833889
#[cfg(test)]
834890
#[cfg(all(feature = "collector_client", feature = "rt-tokio"))]
835891
mod collector_client_tests {

‎opentelemetry-zipkin/src/exporter/env.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,16 @@ fn test_collector_defaults() {
4141
env::remove_var(ENV_ENDPOINT);
4242
assert_eq!(DEFAULT_COLLECTOR_TIMEOUT, get_timeout());
4343
assert_eq!(DEFAULT_COLLECTOR_ENDPOINT, get_endpoint());
44-
}
4544

46-
#[test]
47-
fn test_collector_bad_timeout() {
45+
// Bad Timeout Value
4846
env::set_var(ENV_TIMEOUT, "a");
4947
assert_eq!(DEFAULT_COLLECTOR_TIMEOUT, get_timeout());
50-
}
5148

52-
#[test]
53-
fn test_collector_good_timeout() {
49+
// Good Timeout Value
5450
env::set_var(ENV_TIMEOUT, "777");
5551
assert_eq!(Duration::from_millis(777), get_timeout());
56-
}
5752

58-
#[test]
59-
fn test_collector_custom_endpoint() {
53+
// Custom Endpoint
6054
let custom_endpoint = "https://example.com/api/v2/spans";
6155
env::set_var(ENV_ENDPOINT, custom_endpoint);
6256
assert_eq!(custom_endpoint, get_endpoint());

0 commit comments

Comments
 (0)