Skip to content

Commit

Permalink
Reduce visibility of trace_utils::send_data::RetryStrategy struct fields
Browse files Browse the repository at this point in the history 
Added a new() impl and max_retries() function.
  • Loading branch information
@ekump
ekump committed Jun 6, 2024
1 parent 1ba0525 commit 4b8b9ad
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 59 deletions.
53 changes: 14 additions & 39 deletions trace-utils/src/send_data/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,8 @@ pub(crate) enum RequestResult {
/// let mut send_data = SendData::new(size, tracer_payload, tracer_header_tags, &target);
///
/// // Set a custom retry strategy
/// let retry_strategy = RetryStrategy {
/// max_retries: 3,
/// delay_ms: Duration::from_millis(10),
/// backoff_type: RetryBackoffType::Exponential,
/// jitter: Some(Duration::from_millis(5)),
/// };
/// let retry_strategy = RetryStrategy::new(3, 10, RetryBackoffType::Exponential, Some(5));
///
/// send_data.set_retry_strategy(retry_strategy);
///
/// // Send the data
Expand Down Expand Up @@ -272,7 +268,7 @@ impl SendData {
);
match request_result {
RequestResult::Error(_)
if request_attempt < self.retry_strategy.max_retries =>
if request_attempt < self.retry_strategy.max_retries() =>
{
self.retry_strategy.delay(request_attempt).await;
continue;
Expand All @@ -281,7 +277,7 @@ impl SendData {
}
}
Err(e) => {
if request_attempt >= self.retry_strategy.max_retries {
if request_attempt >= self.retry_strategy.max_retries() {
return self.handle_request_error(e, request_attempt, payload_chunks);
} else {
self.retry_strategy.delay(request_attempt).await;
Expand Down Expand Up @@ -437,7 +433,6 @@ mod tests {
use httpmock::prelude::*;
use httpmock::MockServer;
use std::collections::HashMap;
use std::time::Duration;

const HEADER_TAGS: TracerHeaderTags = TracerHeaderTags {
lang: "test-lang",
Expand Down Expand Up @@ -818,12 +813,7 @@ mod tests {
let size = 512;

let mut send_data = create_send_data(size, &target_endpoint);
send_data.set_retry_strategy(RetryStrategy {
max_retries: 0,
delay_ms: Duration::from_millis(2),
backoff_type: RetryBackoffType::Constant,
jitter: None,
});
send_data.set_retry_strategy(RetryStrategy::new(0, 2, RetryBackoffType::Constant, None));

tokio::spawn(async move {
let result = send_data.send().await;
Expand Down Expand Up @@ -862,12 +852,7 @@ mod tests {
let size = 512;

let mut send_data = create_send_data(size, &target_endpoint);
send_data.set_retry_strategy(RetryStrategy {
max_retries: 2,
delay_ms: Duration::from_millis(250),
backoff_type: RetryBackoffType::Constant,
jitter: None,
});
send_data.set_retry_strategy(RetryStrategy::new(2, 250, RetryBackoffType::Constant, None));

tokio::spawn(async move {
let result = send_data.send().await;
Expand Down Expand Up @@ -911,12 +896,7 @@ mod tests {
let size = 512;

let mut send_data = create_send_data(size, &target_endpoint);
send_data.set_retry_strategy(RetryStrategy {
max_retries: 2,
delay_ms: Duration::from_millis(250),
backoff_type: RetryBackoffType::Constant,
jitter: None,
});
send_data.set_retry_strategy(RetryStrategy::new(2, 250, RetryBackoffType::Constant, None));

tokio::spawn(async move {
let result = send_data.send().await;
Expand Down Expand Up @@ -950,12 +930,12 @@ mod tests {
let size = 512;

let mut send_data = create_send_data(size, &target_endpoint);
send_data.set_retry_strategy(RetryStrategy {
max_retries: expected_retry_attempts,
delay_ms: Duration::from_millis(10),
backoff_type: RetryBackoffType::Constant,
jitter: None,
});
send_data.set_retry_strategy(RetryStrategy::new(
expected_retry_attempts,
10,
RetryBackoffType::Constant,
None,
));

tokio::spawn(async move {
send_data.send().await;
Expand Down Expand Up @@ -994,12 +974,7 @@ mod tests {
let size = 512;

let mut send_data = create_send_data(size, &target_endpoint);
send_data.set_retry_strategy(RetryStrategy {
max_retries: 2,
delay_ms: Duration::from_millis(10),
backoff_type: RetryBackoffType::Constant,
jitter: None,
});
send_data.set_retry_strategy(RetryStrategy::new(2, 10, RetryBackoffType::Constant, None));

tokio::spawn(async move {
send_data.send().await;
Expand Down
81 changes: 61 additions & 20 deletions trace-utils/src/send_data/retry_strategy.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,36 +16,21 @@ pub enum RetryBackoffType {
Exponential,
}

// TODO: APMSP-1076 - RetryStrategy should be moved to a separate file when send_data is refactored.
/// Struct representing the retry strategy for sending data.
///
/// This struct contains the parameters that define how retries should be handled when sending data.
/// It includes the maximum number of retries, the delay between retries, the type of backoff to
/// use, and an optional jitter to add randomness to the delay.
///
/// # Examples
///
/// ```rust
/// use datadog_trace_utils::send_data::{RetryBackoffType, RetryStrategy};
/// use std::time::Duration;
///
/// let retry_strategy = RetryStrategy {
/// max_retries: 5,
/// delay_ms: Duration::from_millis(100),
/// backoff_type: RetryBackoffType::Exponential,
/// jitter: Some(Duration::from_millis(50)),
/// };
/// ```
#[derive(Debug, Clone)]
pub struct RetryStrategy {
/// The maximum number of retries to attempt.
pub max_retries: u32,
/// The minimum delay between retries.
pub delay_ms: Duration,
max_retries: u32,
// The minimum delay between retries.
delay_ms: Duration,
/// The type of backoff to use for the delay between retries.
pub backoff_type: RetryBackoffType,
backoff_type: RetryBackoffType,
/// An optional jitter to add randomness to the delay.
pub jitter: Option<Duration>,
jitter: Option<Duration>,
}

impl Default for RetryStrategy {
Expand All @@ -60,6 +45,40 @@ impl Default for RetryStrategy {
}

impl RetryStrategy {
/// Creates a new `RetryStrategy` with the specified parameters.
///
/// # Arguments
///
/// * `max_retries`: The maximum number of retries to attempt.
/// * `delay_ms`: The minimum delay between retries, in milliseconds.
/// * `backoff_type`: The type of backoff to use for the delay between retries.
/// * `jitter`: An optional jitter to add randomness to the delay, in milliseconds.
///
/// # Returns
///
/// A `RetryStrategy` instance with the specified parameters.
///
/// # Examples
///
/// ```rust
/// use datadog_trace_utils::send_data::{RetryBackoffType, RetryStrategy};
/// use std::time::Duration;
///
/// let retry_strategy = RetryStrategy::new(5, 100, RetryBackoffType::Exponential, Some(50));
/// ```
pub fn new(
max_retries: u32,
delay_ms: u64,
backoff_type: RetryBackoffType,
jitter: Option<u64>,
) -> RetryStrategy {
RetryStrategy {
max_retries,
delay_ms: Duration::from_millis(delay_ms),
backoff_type,
jitter: jitter.map(Duration::from_millis),
}
}
/// Delays the next request attempt based on the retry strategy.
///
/// If a jitter duration is specified in the retry strategy, a random duration up to the jitter
Expand All @@ -82,6 +101,11 @@ impl RetryStrategy {
sleep(delay).await;
}
}

/// Returns the maximum number of retries.
pub(crate) fn max_retries(&self) -> u32 {
self.max_retries
}
}

#[cfg(test)]
Expand Down Expand Up @@ -225,4 +249,21 @@ mod tests {
"Elapsed time was not within expected range"
);
}

#[cfg_attr(miri, ignore)]
#[tokio::test]
async fn test_retry_strategy_max_retries() {
let retry_strategy = RetryStrategy {
max_retries: 17,
delay_ms: Duration::from_millis(100),
backoff_type: RetryBackoffType::Constant,
jitter: Some(Duration::from_millis(50)),
};

assert_eq!(
retry_strategy.max_retries(),
17,
"Max retries did not match expected value"
);
}
}

0 comments on commit 4b8b9ad

Please sign in to comment.