Skip to content

Commit 4d40f35

Browse files
authored
feat(tls): add tls_handshake_timeout config option (#2303)
## Human Summary Adds support for `tls_handlshake_timeout` using [newly documented method in tokio-rustls](rustls/tokio-rustls#187). This is roughly follows #178 but based on latest main. It doesn't handle configuring the connect timeout when using an HTTPS proxy. That requires changes to hyper-http-proxy so I'll follow up with that. Closes: #178 ## AI Summary TLS handshakes to the Datadog intake have no built-in timeout independent of the overall request timeout: `hyper_rustls`'s connector fuses the transport connect and the TLS handshake into a single opaque future, so a stalled handshake (e.g. a peer that accepts the TCP connection but never completes the TLS negotiation) is only bounded by `forwarder_timeout`, which is meant to bound the whole request, not just the handshake. This adds a `tls_handshake_timeout` config option by having the HTTP client connector own the TLS layer directly, so it can time out just the handshake step and still distinguish that failure mode from a slow request. This picks up the intent of #1819, an older PR for the same issue, rewritten against the current typed configuration system rather than resurrected via rebase. ```mermaid sequenceDiagram participant Before as Before (hyper_rustls::HttpsConnector) participant After as After (owned TLS layer) Note over Before: connect + handshake fused into one future Before->>Before: TCP connect Before->>Before: TLS handshake Note over Before: only forwarder_timeout bounds both steps combined Note over After: connect and handshake are separate steps After->>After: TCP connect (connect_timeout) After->>After: TLS handshake (tls_handshake_timeout, new) Note over After: a stalled handshake times out on its own,<br/>without racing the whole request ``` ## Test plan - [x] Added `tls_handshake_timeout` to the Datadog config schema overlay (`support: full`) and wired it through the typed config system (`DatadogTranslator`, `SalukiConfiguration`) and the legacy `ForwarderConfiguration` facet-based config, both consumed by the HTTP client builder. - [x] Added/updated unit tests in `saluki-io`'s `conn.rs` for the new connector split (ALPN protocol selection, including an explicit `http/1.1` ALPN advertisement for `HttpProtocol::Http1` to avoid an ALPN regression from the previous implicit behavior). - [x] Existing `config_smoke::smoke_test` in `saluki-components` (`ForwarderConfiguration`) exercises the new field's default/deserialization against the config registry. - [x] Updated classifier unit tests in `datadog-agent-config` that previously used `tls_handshake_timeout` as an example unsupported/incompatible key, substituting other still-unsupported keys since this key is now fully supported. ## Known limitation Connections made through `proxy_https` bypass this connector and aren't covered by `tls_handshake_timeout` (flagged on the original PR). Left out of scope here; can be addressed separately if needed. Co-authored-by: jesse.szwedko <jesse.szwedko@datadoghq.com>
1 parent d155eaf commit 4d40f35

16 files changed

Lines changed: 338 additions & 103 deletions

File tree

docs/agent-data-plane/configuration/configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ tracking.
2929
| -------------------------------------------- | ----------------------------------------------- | ------- |
3030
| `dogstatsd_experimental_http.enabled` | Enable experimental HTTP/H2C DSD listener | [#1682] |
3131
| `dogstatsd_experimental_http.listen_address` | Bind address for experimental HTTP DSD listener | [#1682] |
32-
| `tls_handshake_timeout` | HTTP TLS handshake timeout | [#178] |
3332

3433
<!-- section:unsupported-not-planned -->
3534
### Not Planned
@@ -801,6 +800,7 @@ Both commands scrub recognized secret values before writing JSON to standard out
801800
| `syslog_rfc` | Use RFC-style syslog header |
802801
| `syslog_uri` | Syslog destination URI |
803802
| `telemetry.dogstatsd_origin` | Per-origin processed-metrics telemetry |
803+
| `tls_handshake_timeout` | HTTP TLS handshake timeout |
804804
| `use_proxy_for_cloud_metadata` | Proxy cloud metadata endpoints |
805805
| `use_v2_api.series` | Send series via V2 protobuf endpoint |
806806
| `use_v3_api.series.enabled` | Global V3 series mode |

lib/agent-data-plane-config-system/src/translators/datadog_translator.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -841,6 +841,10 @@ impl DatadogConfigWitness for DatadogTranslator<'_> {
841841
self.config.shared.endpoints.tls.min_tls_version = value;
842842
}
843843

844+
fn consume_tls_handshake_timeout(&mut self, value: Duration) {
845+
self.config.shared.endpoints.tls.handshake_timeout = value;
846+
}
847+
844848
fn consume_multi_region_failover_api_key(&mut self, value: String) {
845849
self.config.domains.multi_region_failover.api_key = non_empty(value);
846850
}

lib/agent-data-plane-config/src/shared.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,12 @@ pub struct Tls {
120120

121121
/// Path to which TLS session keys are logged, for debugging.
122122
pub sslkeylogfile: String,
123+
124+
/// Timeout for completing the TLS handshake after a connection is established.
125+
///
126+
/// Defaults to 10 seconds. Bounds only the handshake step, distinct from the overall request timeout. A value
127+
/// of zero disables the handshake-specific deadline, leaving the overall request timeout as the only bound.
128+
pub handshake_timeout: Duration,
123129
}
124130

125131
/// Payload compression settings applied before transmission.

lib/datadog-agent/config-testing/build/registry_gen.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,7 @@ static GOLDEN_ORDER: &[(&str, &[&str])] = &[
172172
"forwarder_storage_max_size_in_bytes",
173173
"forwarder_storage_path",
174174
"forwarder_outdated_file_in_days",
175+
"tls_handshake_timeout",
175176
],
176177
),
177178
(

lib/datadog-agent/config-testing/src/config_registry/forwarder.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,17 @@ crate::declare_annotations! {
313313
test_json: None,
314314
pipeline_affinity: PipelineAffinity::CrossCutting,
315315
};
316+
/// `tls_handshake_timeout`-HTTP TLS handshake timeout
317+
TLS_HANDSHAKE_TIMEOUT = SalukiAnnotation {
318+
schema: &schema::TLS_HANDSHAKE_TIMEOUT,
319+
support_level: SupportLevel::Full,
320+
additional_yaml_paths: &[],
321+
env_var_override: None,
322+
used_by: &[structs::FORWARDER_CONFIGURATION],
323+
value_type_override: None,
324+
test_json: None,
325+
pipeline_affinity: PipelineAffinity::CrossCutting,
326+
};
316327
/// `forwarder_apikey_validation_interval`-API key check interval (minutes)
317328
FORWARDER_APIKEY_VALIDATION_INTERVAL = SalukiAnnotation {
318329
schema: &schema::FORWARDER_APIKEY_VALIDATION_INTERVAL,

lib/datadog-agent/config-testing/src/config_registry/unsupported.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,6 @@ crate::declare_annotations! {
6060
test_json: None,
6161
pipeline_affinity: PipelineAffinity::CrossCutting,
6262
};
63-
/// `tls_handshake_timeout`-HTTP TLS handshake timeout
64-
TLS_HANDSHAKE_TIMEOUT = SalukiAnnotation {
65-
schema: &schema::TLS_HANDSHAKE_TIMEOUT,
66-
support_level: SupportLevel::Incompatible(Severity::Medium),
67-
additional_yaml_paths: &[],
68-
env_var_override: None,
69-
used_by: &[],
70-
value_type_override: None,
71-
test_json: None,
72-
pipeline_affinity: PipelineAffinity::CrossCutting,
73-
};
7463
/// `aggregator_buffer_size`-Channel buffer depth for aggregator queues
7564
AGGREGATOR_BUFFER_SIZE = SalukiAnnotation {
7665
schema: &schema::AGGREGATOR_BUFFER_SIZE,

lib/datadog-agent/config/schema/schema_overlay.yaml

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,12 +2593,18 @@ inventory:
25932593
config_registry_filename: dogstatsd.rs
25942594

25952595
tls_handshake_timeout:
2596-
support: none
2597-
severity: medium
2598-
planned: true
2596+
support: full
25992597
pipelines: [cross_cutting]
26002598
description: "HTTP TLS handshake timeout"
2601-
documentation: "Existing request timeout covers the gap."
2599+
documentation: |
2600+
ADP applies `tls_handshake_timeout` to the TLS handshake step of outbound Datadog intake connections,
2601+
independent of `forwarder_timeout`, which bounds the full request. ADP owns the TLS layer directly rather than
2602+
delegating the handshake to an opaque connect-and-handshake future, so it can time out the handshake without
2603+
also aborting an in-progress TCP connect.
2604+
test_support:
2605+
used_by: [ForwarderConfiguration]
2606+
additional_attributes:
2607+
config_registry_filename: forwarder.rs
26022608
issue: "#178"
26032609

26042610
use_dogstatsd:

lib/datadog-agent/config/src/classifier/classifier.rs

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ mod tests {
115115
#[test]
116116
fn incompatible_non_default() {
117117
let c = classifier();
118-
let result = c.classify("tls_handshake_timeout", &Value::Number(999.into())).unwrap();
118+
let result = c
119+
.classify("dogstatsd_stats_buffer", &Value::Number(999.into()))
120+
.unwrap();
119121
assert!(matches!(result.support_level, SupportLevel::Incompatible(_)));
120122
assert!(!result.is_default);
121123
}
@@ -139,8 +141,10 @@ mod tests {
139141
#[test]
140142
fn duration_default_null_is_not_default() {
141143
let c = classifier();
142-
// tls_handshake_timeout has a duration default (10s); a null value can't be normalized.
143-
let result = c.classify("tls_handshake_timeout", &Value::Null).unwrap();
144+
// dogstatsd_packet_buffer_flush_timeout has a duration default (100ms); a null value can't be normalized.
145+
let result = c
146+
.classify("dogstatsd_packet_buffer_flush_timeout", &Value::Null)
147+
.unwrap();
144148
assert!(!result.is_default);
145149
}
146150

@@ -149,14 +153,17 @@ mod tests {
149153
let c = classifier();
150154
// Neither an empty string nor arbitrary text parses as a duration, so neither matches.
151155
assert!(
152-
!c.classify("tls_handshake_timeout", &Value::String("".into()))
156+
!c.classify("dogstatsd_packet_buffer_flush_timeout", &Value::String("".into()))
153157
.unwrap()
154158
.is_default
155159
);
156160
assert!(
157-
!c.classify("tls_handshake_timeout", &Value::String("something".into()))
158-
.unwrap()
159-
.is_default
161+
!c.classify(
162+
"dogstatsd_packet_buffer_flush_timeout",
163+
&Value::String("something".into())
164+
)
165+
.unwrap()
166+
.is_default
160167
);
161168
}
162169

@@ -165,15 +172,15 @@ mod tests {
165172
let c = classifier();
166173
// The default is also matched when supplied as a Go duration string rather than nanoseconds.
167174
let result = c
168-
.classify("tls_handshake_timeout", &Value::String("10s".into()))
175+
.classify("dogstatsd_packet_buffer_flush_timeout", &Value::String("100ms".into()))
169176
.unwrap();
170177
assert!(result.is_default);
171178
}
172179

173180
#[test]
174181
fn incompatible_severity_levels() {
175182
let c = classifier();
176-
let result = c.classify("tls_handshake_timeout", &Value::Number(30.into())).unwrap();
183+
let result = c.classify("dogstatsd_stats_buffer", &Value::Number(30.into())).unwrap();
177184
assert!(matches!(
178185
result.support_level,
179186
SupportLevel::Incompatible(Severity::Medium)
@@ -183,20 +190,26 @@ mod tests {
183190
#[test]
184191
fn duration_default_as_nanoseconds_is_default() {
185192
let c = classifier();
186-
// The Agent transmits tls_handshake_timeout (schema default "10s") as integer nanoseconds.
187-
// The classifier must recognize this as the default and not flag it as an override.
193+
// The Agent transmits dogstatsd_packet_buffer_flush_timeout (schema default "100ms") as integer
194+
// nanoseconds. The classifier must recognize this as the default and not flag it as an override.
188195
let result = c
189-
.classify("tls_handshake_timeout", &Value::Number(10_000_000_000i64.into()))
196+
.classify(
197+
"dogstatsd_packet_buffer_flush_timeout",
198+
&Value::Number(100_000_000i64.into()),
199+
)
190200
.unwrap();
191201
assert!(result.is_default);
192202
}
193203

194204
#[test]
195205
fn duration_non_default_nanoseconds_is_not_default() {
196206
let c = classifier();
197-
// 5s in nanoseconds is not the 10s default.
207+
// 5ms in nanoseconds is not the 100ms default.
198208
let result = c
199-
.classify("tls_handshake_timeout", &Value::Number(5_000_000_000i64.into()))
209+
.classify(
210+
"dogstatsd_packet_buffer_flush_timeout",
211+
&Value::Number(5_000_000i64.into()),
212+
)
200213
.unwrap();
201214
assert!(!result.is_default);
202215
}

lib/datadog-agent/config/src/generated/classifier_data.rs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -319,13 +319,6 @@ pub(crate) static CLASSIFIER_ENTRIES: &[ClassifierEntry] = &[
319319
pipeline_affinity: PipelineAffinity::Pipelines(&[Pipeline::DogStatsD]),
320320
default: DefaultValue::Json("[]"),
321321
},
322-
ClassifierEntry {
323-
yaml_path: "tls_handshake_timeout",
324-
aliases: &[],
325-
support_level: SupportLevel::Incompatible(Severity::Medium),
326-
pipeline_affinity: PipelineAffinity::CrossCutting,
327-
default: DefaultValue::DurationNanos(10000000000),
328-
},
329322
ClassifierEntry {
330323
yaml_path: "use_dogstatsd",
331324
aliases: &[],

lib/datadog-agent/config/src/generated/datadog_configuration.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,13 @@ pub struct DatadogConfiguration {
412412
#[serde(default)]
413413
pub telemetry: Telemetry,
414414

415+
#[serde(
416+
default = "duration_defaults::tls_handshake_timeout",
417+
418+
deserialize_with = "crate::duration_de::deserialize_go_duration"
419+
)]
420+
pub tls_handshake_timeout: std::time::Duration,
421+
415422
#[serde(default)]
416423
pub use_proxy_for_cloud_metadata: bool,
417424

@@ -561,6 +568,7 @@ impl Default for DatadogConfiguration {
561568
syslog_rfc: Default::default(),
562569
syslog_uri: Default::default(),
563570
telemetry: Default::default(),
571+
tls_handshake_timeout: duration_defaults::tls_handshake_timeout(),
564572
use_proxy_for_cloud_metadata: Default::default(),
565573
use_v2_api: Default::default(),
566574
use_v3_api: Default::default(),
@@ -1869,4 +1877,7 @@ mod duration_defaults {
18691877
pub(super) fn expected_tags_duration() -> std::time::Duration {
18701878
std::time::Duration::from_nanos(0)
18711879
}
1880+
pub(super) fn tls_handshake_timeout() -> std::time::Duration {
1881+
std::time::Duration::from_nanos(10000000000)
1882+
}
18721883
}

0 commit comments

Comments
 (0)