Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog.d/aws_timeout_all_components.enhancement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
All AWS-backed components now bound their AWS API requests with client timeouts (`connect_timeout_seconds = 5`, `read_timeout_seconds = 30` by default), exposed as configurable `connect_timeout_seconds`, `operation_timeout_seconds`, and `read_timeout_seconds` options.

The options are now available on the `aws_cloudwatch_logs`, `aws_cloudwatch_metrics`, `aws_kinesis_firehose`, `aws_kinesis_streams`, `aws_s3`, `aws_sns`, and `aws_sqs` sinks, the `aws_s3` and `aws_sqs` sources, and the `aws_secrets_manager` secrets backend (as `client_timeout`, to avoid colliding with the unrelated `timeout` option on the `exec` secrets backend).

authors: petere-datadog
24 changes: 11 additions & 13 deletions src/aws/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ pub async fn create_client<T>(
endpoint: Option<String>,
proxy: &ProxyConfig,
tls_options: Option<&TlsConfig>,
timeout: Option<&AwsTimeout>,
timeout: &AwsTimeout,
) -> crate::Result<T::Client>
where
T: ClientBuilder,
Expand All @@ -190,7 +190,7 @@ pub async fn create_client_and_region<T>(
endpoint: Option<String>,
proxy: &ProxyConfig,
tls_options: Option<&TlsConfig>,
timeout: Option<&AwsTimeout>,
timeout: &AwsTimeout,
) -> crate::Result<(T::Client, Region)>
where
T: ClientBuilder,
Expand Down Expand Up @@ -238,20 +238,18 @@ where
config_builder = config_builder.use_fips(use_fips);
}

if let Some(timeout) = timeout {
let mut timeout_config_builder = TimeoutConfig::builder();
let mut timeout_config_builder = TimeoutConfig::builder();

let operation_timeout = timeout.operation_timeout();
let connect_timeout = timeout.connect_timeout();
let read_timeout = timeout.read_timeout();
let operation_timeout = timeout.operation_timeout();
let connect_timeout = timeout.connect_timeout();
let read_timeout = timeout.read_timeout();

timeout_config_builder
.set_operation_timeout(operation_timeout.map(Duration::from_secs))
.set_connect_timeout(connect_timeout.map(Duration::from_secs))
.set_read_timeout(read_timeout.map(Duration::from_secs));
timeout_config_builder
.set_operation_timeout(operation_timeout.map(Duration::from_secs))
.set_connect_timeout(Some(Duration::from_secs(connect_timeout)))
.set_read_timeout(Some(Duration::from_secs(read_timeout)));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid forcing first-byte timeout on slow uploads

With the new defaults, every AWS client now gets a 30s read_timeout even when the component config does not opt in. That timeout is documented in AwsTimeout as time to the first response byte from request start, so for sinks that upload request bodies—especially aws_s3 with large batches or slow/S3-compatible endpoints—the response headers may legitimately arrive only after more than 30s of uploading/service work, causing previously valid writes to time out and retry indefinitely unless users discover and raise this new setting. Consider not applying a default read timeout to upload-oriented clients, or making the default opt-in/large enough for existing sink workloads.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, need to take the time to rework this to not apply for sinks


config_builder = config_builder.timeout_config(timeout_config_builder.build());
}
config_builder = config_builder.timeout_config(timeout_config_builder.build());

let config = config_builder.build();

Expand Down
36 changes: 28 additions & 8 deletions src/aws/timeout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
//use std::time::Duration;
use serde_with::serde_as;
use vector_lib::configurable::configurable_component;
const fn default_aws_connect_timeout_seconds() -> u64 {
5
}
const fn default_aws_read_timeout_seconds() -> u64 {
30
}

/// Client timeout configuration for AWS operations.
#[serde_as]
Expand All @@ -16,9 +22,10 @@ pub struct AwsTimeout {
#[configurable(metadata(docs::examples = 20))]
#[configurable(metadata(docs::human_name = "Connect Timeout"))]
#[configurable(metadata(docs::type_unit = "seconds"))]
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(rename = "connect_timeout_seconds")]
connect_timeout: Option<u64>,
#[serde(default = "default_aws_connect_timeout_seconds")]
#[derivative(Default(value = "default_aws_connect_timeout_seconds()"))]
connect_timeout: u64,

/// The operation timeout for AWS requests
///
Expand All @@ -41,14 +48,15 @@ pub struct AwsTimeout {
#[configurable(metadata(docs::examples = 20))]
#[configurable(metadata(docs::human_name = "Read Timeout"))]
#[configurable(metadata(docs::type_unit = "seconds"))]
#[serde(skip_serializing_if = "Option::is_none")]
#[serde(rename = "read_timeout_seconds")]
read_timeout: Option<u64>,
#[serde(default = "default_aws_read_timeout_seconds")]
#[derivative(Default(value = "default_aws_read_timeout_seconds()"))]
read_timeout: u64,
}

impl AwsTimeout {
/// returns the connection timeout
pub const fn connect_timeout(&self) -> Option<u64> {
pub const fn connect_timeout(&self) -> u64 {
self.connect_timeout
}

Expand All @@ -58,7 +66,7 @@ impl AwsTimeout {
}

/// returns the read timeout
pub const fn read_timeout(&self) -> Option<u64> {
pub const fn read_timeout(&self) -> u64 {
self.read_timeout
}
}
Expand All @@ -76,8 +84,20 @@ mod tests {
"})
.unwrap();

assert_eq!(config.connect_timeout, Some(20));
assert_eq!(config.connect_timeout, 20);
assert_eq!(config.operation_timeout, Some(20));
assert_eq!(config.read_timeout, Some(60));
assert_eq!(config.read_timeout, 60);
}

#[test]
fn default_matches_serde_defaults() {
let default = AwsTimeout::default();

assert_eq!(
default.connect_timeout,
default_aws_connect_timeout_seconds()
);
assert_eq!(default.operation_timeout, None);
assert_eq!(default.read_timeout, default_aws_read_timeout_seconds());
}
}
14 changes: 12 additions & 2 deletions src/secrets/aws_secrets_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use aws_sdk_secretsmanager::{Client, config};
use vector_lib::configurable::{component::GenerateConfig, configurable_component};

use crate::{
aws::{AwsAuthentication, ClientBuilder, RegionOrEndpoint, create_client},
aws::{AwsAuthentication, AwsTimeout, ClientBuilder, RegionOrEndpoint, create_client},
config::{ProxyConfig, SecretBackend},
signal,
tls::TlsConfig,
Expand Down Expand Up @@ -38,6 +38,15 @@ pub struct AwsSecretsManagerBackend {

#[configurable(derived)]
pub tls: Option<TlsConfig>,

/// Client timeout configuration for AWS requests.
///
/// These settings bound how long the client waits when connecting to and reading from the
/// AWS API. Any dimension left unset falls back to a default (`connect_timeout_seconds = 5`,
/// `read_timeout_seconds = 30`).
#[configurable(derived)]
#[serde(default)]
pub client_timeout: AwsTimeout,
}

impl GenerateConfig for AwsSecretsManagerBackend {
Expand All @@ -47,6 +56,7 @@ impl GenerateConfig for AwsSecretsManagerBackend {
region: Default::default(),
auth: Default::default(),
tls: None,
client_timeout: Default::default(),
})
.unwrap()
}
Expand All @@ -65,7 +75,7 @@ impl SecretBackend for AwsSecretsManagerBackend {
self.region.endpoint(),
&ProxyConfig::default(),
self.tls.as_ref(),
None,
&self.client_timeout,
)
.await?;

Expand Down
14 changes: 12 additions & 2 deletions src/sinks/aws_cloudwatch_logs/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use vector_lib::{codecs::JsonSerializerConfig, configurable::configurable_compon
use vrl::value::Kind;

use crate::{
aws::{AwsAuthentication, ClientBuilder, RegionOrEndpoint, create_client},
aws::{AwsAuthentication, AwsTimeout, ClientBuilder, RegionOrEndpoint, create_client},
codecs::{Encoder, EncodingConfig},
config::{
AcknowledgementsConfig, DataType, GenerateConfig, Input, ProxyConfig, SinkConfig,
Expand Down Expand Up @@ -157,6 +157,15 @@ pub struct CloudwatchLogsSinkConfig {
#[serde(default)]
pub auth: AwsAuthentication,

/// Client timeout configuration for AWS requests.
///
/// These settings bound how long the client waits when connecting to and reading from the
/// AWS API. Any dimension left unset falls back to a default (`connect_timeout_seconds = 5`,
/// `read_timeout_seconds = 30`).
#[configurable(derived)]
#[serde(default)]
pub timeout: AwsTimeout,

#[configurable(derived)]
#[serde(
default,
Expand Down Expand Up @@ -197,7 +206,7 @@ impl CloudwatchLogsSinkConfig {
self.region.endpoint(),
proxy,
self.tls.as_ref(),
None,
&self.timeout,
)
.await
}
Expand Down Expand Up @@ -279,6 +288,7 @@ fn default_config(encoding: EncodingConfig) -> CloudwatchLogsSinkConfig {
tls: Default::default(),
assume_role: Default::default(),
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: Default::default(),
tags: Default::default(),
Expand Down
14 changes: 11 additions & 3 deletions src/sinks/aws_cloudwatch_logs/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use vrl::event_path;

use super::*;
use crate::{
aws::{AwsAuthentication, ClientBuilder, RegionOrEndpoint, create_client},
aws::{AwsAuthentication, AwsTimeout, ClientBuilder, RegionOrEndpoint, create_client},
config::{ProxyConfig, SinkConfig, SinkContext, log_schema},
event::{Event, LogEvent, Value},
sinks::{aws_cloudwatch_logs::config::CloudwatchLogsClientBuilder, util::BatchConfig},
Expand Down Expand Up @@ -63,6 +63,7 @@ async fn cloudwatch_insert_log_event() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand Down Expand Up @@ -117,6 +118,7 @@ async fn cloudwatch_insert_log_events_sorted() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand Down Expand Up @@ -196,6 +198,7 @@ async fn cloudwatch_insert_out_of_range_timestamp() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand Down Expand Up @@ -276,6 +279,7 @@ async fn cloudwatch_dynamic_group_and_stream_creation() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand Down Expand Up @@ -330,6 +334,7 @@ async fn cloudwatch_dynamic_group_and_stream_creation_with_kms_key_and_tags() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: Some(
create_kms_client_test()
Expand Down Expand Up @@ -420,6 +425,7 @@ async fn cloudwatch_insert_log_event_batched() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand Down Expand Up @@ -474,6 +480,7 @@ async fn cloudwatch_insert_log_event_partitioned() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand Down Expand Up @@ -570,6 +577,7 @@ async fn cloudwatch_healthcheck() {
tls: Default::default(),
assume_role: None,
auth: Default::default(),
timeout: Default::default(),
acknowledgements: Default::default(),
kms_key: None,
tags: None,
Expand All @@ -593,7 +601,7 @@ async fn create_client_test() -> CloudwatchLogsClient {
endpoint,
&proxy,
None,
None,
&AwsTimeout::default(),
)
.await
.unwrap()
Expand All @@ -612,7 +620,7 @@ async fn create_kms_client_test() -> KMSClient {
endpoint,
&proxy,
None,
None,
&AwsTimeout::default(),
)
.await
.unwrap()
Expand Down
14 changes: 12 additions & 2 deletions src/sinks/aws_cloudwatch_metrics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ use vector_lib::{
use super::util::service::TowerRequestConfigDefaults;
use crate::{
aws::{
ClientBuilder, RegionOrEndpoint, auth::AwsAuthentication, create_client, is_retriable_error,
AwsTimeout, ClientBuilder, RegionOrEndpoint, auth::AwsAuthentication, create_client,
is_retriable_error,
},
config::{AcknowledgementsConfig, Input, ProxyConfig, SinkConfig, SinkContext},
event::{
Expand Down Expand Up @@ -107,6 +108,15 @@ pub struct CloudWatchMetricsSinkConfig {
#[serde(default)]
pub auth: AwsAuthentication,

/// Client timeout configuration for AWS requests.
///
/// These settings bound how long the client waits when connecting to and reading from the
/// AWS API. Any dimension left unset falls back to a default (`connect_timeout_seconds = 5`,
/// `read_timeout_seconds = 30`).
#[configurable(derived)]
#[serde(default)]
pub timeout: AwsTimeout,

#[configurable(derived)]
#[serde(
default,
Expand Down Expand Up @@ -191,7 +201,7 @@ impl CloudWatchMetricsSinkConfig {
self.region.endpoint(),
proxy,
self.tls.as_ref(),
None,
&self.timeout,
)
.await
}
Expand Down
11 changes: 10 additions & 1 deletion src/sinks/aws_kinesis/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use super::{
sink::{BatchKinesisRequest, KinesisSink},
};
use crate::{
aws::{AwsAuthentication, RegionOrEndpoint},
aws::{AwsAuthentication, AwsTimeout, RegionOrEndpoint},
sinks::{
prelude::*,
util::{TowerRequestConfig, retries::RetryLogic},
Expand Down Expand Up @@ -51,6 +51,15 @@ pub struct KinesisSinkBaseConfig {
#[serde(default)]
pub auth: AwsAuthentication,

/// Client timeout configuration for AWS requests.
///
/// These settings bound how long the client waits when connecting to and reading from the
/// AWS API. Any dimension left unset falls back to a default (`connect_timeout_seconds = 5`,
/// `read_timeout_seconds = 30`).
#[configurable(derived)]
#[serde(default)]
pub timeout: AwsTimeout,

/// Whether or not to retry successful requests containing partial failures.
#[serde(default)]
#[configurable(metadata(docs::advanced))]
Expand Down
2 changes: 1 addition & 1 deletion src/sinks/aws_kinesis/firehose/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl KinesisFirehoseSinkConfig {
self.base.region.endpoint(),
proxy,
self.base.tls.as_ref(),
None,
&self.base.timeout,
)
.await
}
Expand Down
Loading
Loading