Skip to content

Commit 53c8c16

Browse files
authored
feat(http-client): add knob to disable connection pooling (#1933)
# What does this PR do? This PR adds a configuration knob to disable connection pooling for the HTTP client common component. This doesn't assume that the backend has specific support for connection pooling, but allows to say "if you have connection pooling, do not use it". # Motivation This PR has been split off #1806, which builds an agent-level HTTP client on top of the basic HTTP client. To communicate with the agent, the current behavior of existing helpers in e.g. libdd-common is to disable connection pooling by default, because the agent has a low keep-alive timeout and the most common case is to flush data on a periodic schedule. There, connection pooling is detrimental as the connection will be invalidated as soon as it is re-used, see https://github.com/DataDog/libdatadog/blob/cff72917bfe6a4f916db03294ae1ae895f58b882/libdd-common/src/http_common.rs#L118-L128. # Additional Notes N/A # How to test the change? N/A Co-authored-by: yann.hamdaoui <yann.hamdaoui@datadoghq.com>
1 parent b1b58fc commit 53c8c16

5 files changed

Lines changed: 86 additions & 15 deletions

File tree

libdd-http-client/src/backend/hyper_backend.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,19 @@ fn map_hyper_error(e: hyper_util::client::legacy::Error) -> HttpClientError {
145145

146146
impl super::Backend for HyperBackend {
147147
fn new(
148-
_timeout: std::time::Duration,
148+
client_config: &HttpClientConfig,
149149
transport: TransportConfig,
150150
) -> Result<Self, HttpClientError> {
151-
let client = http_common::client_builder().build(Connector::default());
152-
Ok(Self { client, transport })
151+
let mut builder = http_common::client_builder();
152+
153+
if !client_config.allow_connection_pooling() {
154+
builder.pool_max_idle_per_host(0);
155+
}
156+
157+
Ok(Self {
158+
client: builder.build(Connector::default()),
159+
transport,
160+
})
153161
}
154162

155163
async fn send(

libdd-http-client/src/backend/mod.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
// Copyright 2026-Present Datadog, Inc. https://www.datadoghq.com/
22
// SPDX-License-Identifier: Apache-2.0
33

4-
use crate::{config, HttpClientError, HttpRequest, HttpResponse};
5-
use std::time::Duration;
4+
use crate::{config, HttpClientConfig, HttpClientError, HttpRequest, HttpResponse};
65

76
#[cfg(all(feature = "hyper-backend", not(feature = "reqwest-backend")))]
87
pub(crate) mod hyper_backend;
@@ -16,7 +15,10 @@ pub(crate) mod reqwest_backend;
1615
/// Backend`.
1716
pub(crate) trait Backend: Sized {
1817
/// Construct a new backend with the given timeout and transport.
19-
fn new(timeout: Duration, transport: config::TransportConfig) -> Result<Self, HttpClientError>;
18+
fn new(
19+
client_config: &HttpClientConfig,
20+
transport: config::TransportConfig,
21+
) -> Result<Self, HttpClientError>;
2022

2123
/// Send an HTTP request and return the response.
2224
async fn send(

libdd-http-client/src/backend/reqwest_backend.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@ pub(crate) struct ReqwestBackend {
1818
impl super::Backend for ReqwestBackend {
1919
// Creates a `reqwest::Client` with connection pooling enabled.
2020
fn new(
21-
timeout: std::time::Duration,
21+
client_config: &HttpClientConfig,
2222
transport: TransportConfig,
2323
) -> Result<Self, HttpClientError> {
24-
let mut builder = reqwest::Client::builder().timeout(timeout);
24+
let mut builder = reqwest::Client::builder().timeout(client_config.timeout());
2525

2626
match transport {
2727
TransportConfig::Tcp => {}
@@ -35,6 +35,10 @@ impl super::Backend for ReqwestBackend {
3535
}
3636
}
3737

38+
if !client_config.allow_connection_pooling() {
39+
builder = builder.pool_max_idle_per_host(0);
40+
}
41+
3842
let client = builder
3943
.build()
4044
.map_err(|e| HttpClientError::InvalidConfig(e.to_string()))?;

libdd-http-client/src/client.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,10 @@ impl HttpClient {
4646
config: HttpClientConfig,
4747
transport: TransportConfig,
4848
) -> Result<Self, HttpClientError> {
49-
let backend = BackendImpl::new(config.timeout(), transport)?;
50-
Ok(Self { backend, config })
49+
Ok(Self {
50+
backend: BackendImpl::new(&config, transport)?,
51+
config,
52+
})
5153
}
5254

5355
/// The client's configuration.

libdd-http-client/src/config.rs

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ pub struct HttpClientConfig {
3333
timeout: Duration,
3434
treat_http_errors_as_errors: bool,
3535
retry: Option<RetryConfig>,
36+
allow_connection_pooling: bool,
3637
}
3738

3839
impl HttpClientConfig {
@@ -44,6 +45,7 @@ impl HttpClientConfig {
4445
timeout,
4546
treat_http_errors_as_errors: true,
4647
retry: None,
48+
allow_connection_pooling: true,
4749
}
4850
}
4951

@@ -66,28 +68,45 @@ impl HttpClientConfig {
6668
pub fn retry(&self) -> Option<&RetryConfig> {
6769
self.retry.as_ref()
6870
}
71+
72+
/// Whether connection pooling can be used, when available. See
73+
/// [HttpClientBuilder::allow_connection_pooling].
74+
pub fn allow_connection_pooling(&self) -> bool {
75+
self.allow_connection_pooling
76+
}
6977
}
7078

7179
/// Builder for [`crate::HttpClient`].
7280
///
7381
/// Obtain via [`crate::HttpClient::builder`].
74-
#[derive(Debug, Default)]
82+
#[derive(Debug)]
7583
pub struct HttpClientBuilder {
7684
base_url: Option<String>,
7785
timeout: Option<Duration>,
7886
treat_http_errors_as_errors: bool,
7987
retry: Option<RetryConfig>,
8088
transport: TransportConfig,
89+
allow_connection_pooling: bool,
8190
}
8291

83-
impl HttpClientBuilder {
84-
/// Create a new builder with default settings.
85-
pub fn new() -> Self {
92+
impl Default for HttpClientBuilder {
93+
fn default() -> Self {
8694
Self {
95+
base_url: Default::default(),
96+
timeout: Default::default(),
8797
treat_http_errors_as_errors: true,
88-
..Default::default()
98+
retry: Default::default(),
99+
transport: Default::default(),
100+
allow_connection_pooling: true,
89101
}
90102
}
103+
}
104+
105+
impl HttpClientBuilder {
106+
/// Create a new builder with default settings.
107+
pub fn new() -> Self {
108+
Self::default()
109+
}
91110

92111
/// Set the base URL.
93112
pub fn base_url(mut self, url: String) -> Self {
@@ -136,6 +155,18 @@ impl HttpClientBuilder {
136155
self
137156
}
138157

158+
/// Allow connection pooling. Defaults to `true`.
159+
///
160+
/// Note that whether pooling is actually used depends on the HTTP backend of
161+
/// [crate], though both currently available backends (reqwest and hyper) support
162+
/// pooling. This setting should be understood as: if set to `true`, the default behavior of the
163+
/// underlying backend will be selected, which might or might not do connection pooling by
164+
/// default. If set to `false`, we guarantee no connection pooling will happen.
165+
pub fn allow_connection_pooling(mut self, allow: bool) -> Self {
166+
self.allow_connection_pooling = allow;
167+
self
168+
}
169+
139170
/// Build the [`crate::HttpClient`].
140171
///
141172
/// Returns [`crate::HttpClientError::InvalidConfig`] if required fields
@@ -152,6 +183,7 @@ impl HttpClientBuilder {
152183
timeout,
153184
treat_http_errors_as_errors: self.treat_http_errors_as_errors,
154185
retry: self.retry,
186+
allow_connection_pooling: self.allow_connection_pooling,
155187
};
156188
crate::HttpClient::from_config_and_transport(config, self.transport)
157189
}
@@ -230,4 +262,27 @@ mod tests {
230262
.unwrap();
231263
assert!(!client.config().treat_http_errors_as_errors());
232264
}
265+
266+
#[test]
267+
fn builder_allow_connection_pooling_defaults_true() {
268+
ensure_crypto_provider();
269+
let client = HttpClientBuilder::new()
270+
.base_url("http://localhost".to_owned())
271+
.timeout(Duration::from_secs(1))
272+
.build()
273+
.unwrap();
274+
assert!(client.config().allow_connection_pooling());
275+
}
276+
277+
#[test]
278+
fn builder_allow_connection_pooling_set_false() {
279+
ensure_crypto_provider();
280+
let client = HttpClientBuilder::new()
281+
.base_url("http://localhost".to_owned())
282+
.timeout(Duration::from_secs(1))
283+
.allow_connection_pooling(false)
284+
.build()
285+
.unwrap();
286+
assert!(!client.config().allow_connection_pooling());
287+
}
233288
}

0 commit comments

Comments
 (0)