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

Enable reqwest and reqwest-blocking client creation with custom timeout #2584

Merged
merged 11 commits into from
Feb 3, 2025
9 changes: 9 additions & 0 deletions opentelemetry-otlp/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@
- Remove unnecessarily public trait `opentelemetry_otlp::metrics::MetricsClient`
and `MetricExporter::new(..)` method. Use
`MetricExporter::builder()...build()` to obtain `MetricExporter`.
- The HTTP clients (reqwest, reqwest-blocking, hyper) now support the
timeout internal configured in below order
- Signal specific env variable `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT`,
`OTEL_EXPORTER_OTLP_LOGS_TIMEOUT` or `OTEL_EXPORTER_OTLP_TIMEOUT`.
- `OTEL_EXPORTER_OTLP_TIMEOUT` env variable.
- `with_http().with_timeout()` API method of
`LogExporterBuilder` and `SpanExporterBuilder` and `MetricsExporterBuilder`.
- The default interval of 30sec is used if none is configured.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. It's indeed 10sec. Have update the changelog accordingly.

pub const OTEL_EXPORTER_OTLP_TIMEOUT_DEFAULT: u64 = 10;



## 0.27.0

Expand Down
103 changes: 51 additions & 52 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,7 @@
use opentelemetry_http::hyper::HyperClient;

/// Configuration of the http transport
#[derive(Debug)]
#[cfg_attr(
all(
not(feature = "reqwest-client"),
not(feature = "reqwest-blocking-client"),
not(feature = "hyper-client")
),
derive(Default)
)]
#[derive(Debug, Default)]
pub struct HttpConfig {
/// Select the HTTP client
client: Option<Arc<dyn HttpClient>>,
Expand All @@ -61,44 +53,6 @@
headers: Option<HashMap<String, String>>,
}

#[cfg(any(
feature = "reqwest-blocking-client",
feature = "reqwest-client",
feature = "hyper-client"
))]
impl Default for HttpConfig {
fn default() -> Self {
#[cfg(feature = "reqwest-blocking-client")]
let default_client = std::thread::spawn(|| {
Some(Arc::new(reqwest::blocking::Client::new()) as Arc<dyn HttpClient>)
})
.join()
.expect("creating reqwest::blocking::Client on a new thread not to fail");
#[cfg(all(not(feature = "reqwest-blocking-client"), feature = "reqwest-client"))]
let default_client = Some(Arc::new(reqwest::Client::new()) as Arc<dyn HttpClient>);
#[cfg(all(
not(feature = "reqwest-client"),
not(feature = "reqwest-blocking-client"),
feature = "hyper-client"
))]
// TODO - support configuring custom connector and executor
let default_client = Some(Arc::new(HyperClient::with_default_connector(
Duration::from_secs(10),
None,
)) as Arc<dyn HttpClient>);
#[cfg(all(
not(feature = "reqwest-client"),
not(feature = "reqwest-blocking-client"),
not(feature = "hyper-client")
))]
let default_client = None;
HttpConfig {
client: default_client,
headers: None,
}
}
}

/// Configuration for the OTLP HTTP exporter.
///
/// ## Examples
Expand Down Expand Up @@ -171,11 +125,56 @@
},
None => self.exporter_config.timeout,
};
let http_client = self
.http_config
.client
.take()
.ok_or(crate::Error::NoHttpClient)?;

#[allow(unused_mut)] // TODO - clippy thinks mut is not needed, but it is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is another issue about parsing env variables for timeout, I opened #2591 to track it separate from this PR

let mut http_client = self.http_config.client.take();

if http_client.is_none() {
#[cfg(all(
not(feature = "reqwest-client"),
not(feature = "reqwest-blocking-client"),
feature = "hyper-client"
))]
{
// TODO - support configuring custom connector and executor
http_client = Some(Arc::new(HyperClient::with_default_connector(timeout, None))
as Arc<dyn HttpClient>);
}
#[cfg(all(
not(feature = "hyper-client"),
not(feature = "reqwest-blocking-client"),
feature = "reqwest-client"
))]
{
http_client = Some(Arc::new(
reqwest::Client::builder()
.timeout(timeout)
.build()
.unwrap_or_default(),
) as Arc<dyn HttpClient>);
}
#[cfg(all(
not(feature = "hyper-client"),
not(feature = "reqwest-client"),
feature = "reqwest-blocking-client"
))]
{
let timeout_clone = timeout;
http_client = Some(Arc::new(
std::thread::spawn(move || {
reqwest::blocking::Client::builder()
.timeout(timeout_clone)
.build()
.unwrap_or_else(|_| reqwest::blocking::Client::new())
})
.join()
.unwrap(), // Unwrap thread result
) as Arc<dyn HttpClient>);
}
}

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

View check run for this annotation

Codecov / codecov/patch

opentelemetry-otlp/src/exporter/http/mod.rs#L130-L174

Added lines #L130 - L174 were not covered by tests

let http_client = http_client.ok_or(crate::Error::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
.http_config
Expand Down