Marshall Bowers created
This PR updates the `EventCoalescer` to use the `SystemClock` trait to
abstract over the clock.
This allows us to test the advancement of time without relying on the
caller passing in the current time.
Release Notes:
- N/Acrates/client/src/telemetry.rs | 2
crates/client/src/telemetry/event_coalescer.rs | 107 +++++++++++--------
2 files changed, 62 insertions(+), 47 deletions(-)
@@ -96,7 +96,7 @@ impl Telemetry {
log_file: None,
is_staff: None,
first_event_date_time: None,
- event_coalescer: EventCoalescer::new(),
+ event_coalescer: EventCoalescer::new(clock.clone()),
max_queue_size: MAX_QUEUE_LEN,
}));
@@ -1,6 +1,9 @@
-use chrono::{DateTime, Duration, Utc};
+use std::sync::Arc;
use std::time;
+use chrono::{DateTime, Duration, Utc};
+use clock::SystemClock;
+
const COALESCE_TIMEOUT: time::Duration = time::Duration::from_secs(20);
const SIMULATED_DURATION_FOR_SINGLE_EVENT: time::Duration = time::Duration::from_millis(1);
@@ -12,30 +15,20 @@ struct PeriodData {
}
pub struct EventCoalescer {
+ clock: Arc<dyn SystemClock>,
state: Option<PeriodData>,
}
impl EventCoalescer {
- pub fn new() -> Self {
- Self { state: None }
+ pub fn new(clock: Arc<dyn SystemClock>) -> Self {
+ Self { clock, state: None }
}
pub fn log_event(
&mut self,
environment: &'static str,
) -> Option<(DateTime<Utc>, DateTime<Utc>, &'static str)> {
- self.log_event_with_time(Utc::now(), environment)
- }
-
- // pub fn close_current_period(&mut self) -> Option<(DateTime<Utc>, DateTime<Utc>)> {
- // self.environment.map(|env| self.log_event(env)).flatten()
- // }
-
- fn log_event_with_time(
- &mut self,
- log_time: DateTime<Utc>,
- environment: &'static str,
- ) -> Option<(DateTime<Utc>, DateTime<Utc>, &'static str)> {
+ let log_time = self.clock.utc_now();
let coalesce_timeout = Duration::from_std(COALESCE_TIMEOUT).unwrap();
let Some(state) = &mut self.state else {
@@ -78,18 +71,22 @@ impl EventCoalescer {
#[cfg(test)]
mod tests {
use chrono::TimeZone;
+ use clock::FakeSystemClock;
use super::*;
#[test]
fn test_same_context_exceeding_timeout() {
+ let clock = Arc::new(FakeSystemClock::new(
+ Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap(),
+ ));
let environment_1 = "environment_1";
- let mut event_coalescer = EventCoalescer::new();
+ let mut event_coalescer = EventCoalescer::new(clock.clone());
assert_eq!(event_coalescer.state, None);
- let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
- let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
+ let period_start = clock.utc_now();
+ let period_data = event_coalescer.log_event(environment_1);
assert_eq!(period_data, None);
assert_eq!(
@@ -102,12 +99,12 @@ mod tests {
);
let within_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT / 2).unwrap();
- let mut period_end = period_start;
// Ensure that many calls within the timeout don't start a new period
for _ in 0..100 {
- period_end += within_timeout_adjustment;
- let period_data = event_coalescer.log_event_with_time(period_end, environment_1);
+ clock.advance(within_timeout_adjustment);
+ let period_data = event_coalescer.log_event(environment_1);
+ let period_end = clock.utc_now();
assert_eq!(period_data, None);
assert_eq!(
@@ -120,10 +117,12 @@ mod tests {
);
}
+ let period_end = clock.utc_now();
let exceed_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT * 2).unwrap();
// Logging an event exceeding the timeout should start a new period
- let new_period_start = period_end + exceed_timeout_adjustment;
- let period_data = event_coalescer.log_event_with_time(new_period_start, environment_1);
+ clock.advance(exceed_timeout_adjustment);
+ let new_period_start = clock.utc_now();
+ let period_data = event_coalescer.log_event(environment_1);
assert_eq!(period_data, Some((period_start, period_end, environment_1)));
assert_eq!(
@@ -138,13 +137,16 @@ mod tests {
#[test]
fn test_different_environment_under_timeout() {
+ let clock = Arc::new(FakeSystemClock::new(
+ Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap(),
+ ));
let environment_1 = "environment_1";
- let mut event_coalescer = EventCoalescer::new();
+ let mut event_coalescer = EventCoalescer::new(clock.clone());
assert_eq!(event_coalescer.state, None);
- let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
- let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
+ let period_start = clock.utc_now();
+ let period_data = event_coalescer.log_event(environment_1);
assert_eq!(period_data, None);
assert_eq!(
@@ -157,8 +159,9 @@ mod tests {
);
let within_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT / 2).unwrap();
- let period_end = period_start + within_timeout_adjustment;
- let period_data = event_coalescer.log_event_with_time(period_end, environment_1);
+ clock.advance(within_timeout_adjustment);
+ let period_end = clock.utc_now();
+ let period_data = event_coalescer.log_event(environment_1);
assert_eq!(period_data, None);
assert_eq!(
@@ -170,10 +173,12 @@ mod tests {
})
);
+ clock.advance(within_timeout_adjustment);
+
// Logging an event within the timeout but with a different environment should start a new period
- let period_end = period_end + within_timeout_adjustment;
+ let period_end = clock.utc_now();
let environment_2 = "environment_2";
- let period_data = event_coalescer.log_event_with_time(period_end, environment_2);
+ let period_data = event_coalescer.log_event(environment_2);
assert_eq!(period_data, Some((period_start, period_end, environment_1)));
assert_eq!(
@@ -188,13 +193,16 @@ mod tests {
#[test]
fn test_switching_environment_while_within_timeout() {
+ let clock = Arc::new(FakeSystemClock::new(
+ Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap(),
+ ));
let environment_1 = "environment_1";
- let mut event_coalescer = EventCoalescer::new();
+ let mut event_coalescer = EventCoalescer::new(clock.clone());
assert_eq!(event_coalescer.state, None);
- let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
- let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
+ let period_start = clock.utc_now();
+ let period_data = event_coalescer.log_event(environment_1);
assert_eq!(period_data, None);
assert_eq!(
@@ -207,9 +215,10 @@ mod tests {
);
let within_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT / 2).unwrap();
- let period_end = period_start + within_timeout_adjustment;
+ clock.advance(within_timeout_adjustment);
+ let period_end = clock.utc_now();
let environment_2 = "environment_2";
- let period_data = event_coalescer.log_event_with_time(period_end, environment_2);
+ let period_data = event_coalescer.log_event(environment_2);
assert_eq!(period_data, Some((period_start, period_end, environment_1)));
assert_eq!(
@@ -221,22 +230,26 @@ mod tests {
})
);
}
- // // 0 20 40 60
- // // |-------------------|-------------------|-------------------|-------------------
- // // |--------|----------env change
- // // |-------------------
- // // |period_start |period_end
- // // |new_period_start
+
+ // 0 20 40 60
+ // |-------------------|-------------------|-------------------|-------------------
+ // |--------|----------env change
+ // |-------------------
+ // |period_start |period_end
+ // |new_period_start
#[test]
fn test_switching_environment_while_exceeding_timeout() {
+ let clock = Arc::new(FakeSystemClock::new(
+ Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap(),
+ ));
let environment_1 = "environment_1";
- let mut event_coalescer = EventCoalescer::new();
+ let mut event_coalescer = EventCoalescer::new(clock.clone());
assert_eq!(event_coalescer.state, None);
- let period_start = Utc.with_ymd_and_hms(1990, 4, 12, 0, 0, 0).unwrap();
- let period_data = event_coalescer.log_event_with_time(period_start, environment_1);
+ let period_start = clock.utc_now();
+ let period_data = event_coalescer.log_event(environment_1);
assert_eq!(period_data, None);
assert_eq!(
@@ -249,9 +262,10 @@ mod tests {
);
let exceed_timeout_adjustment = Duration::from_std(COALESCE_TIMEOUT * 2).unwrap();
- let period_end = period_start + exceed_timeout_adjustment;
+ clock.advance(exceed_timeout_adjustment);
+ let period_end = clock.utc_now();
let environment_2 = "environment_2";
- let period_data = event_coalescer.log_event_with_time(period_end, environment_2);
+ let period_data = event_coalescer.log_event(environment_2);
assert_eq!(
period_data,
@@ -270,6 +284,7 @@ mod tests {
})
);
}
+
// 0 20 40 60
// |-------------------|-------------------|-------------------|-------------------
// |--------|----------------------------------------env change