Skip to content

Commit 5be0b19

Browse files
PlexSheepPlexSheep
committed
fix: time grouping grouped everything in 1min groups
1 parent 46e09fb commit 5be0b19

File tree

2 files changed

+76
-49
lines changed

2 files changed

+76
-49
lines changed

‎src/analyze.rs‎

Lines changed: 63 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -340,30 +340,44 @@ pub(crate) fn fail_groups<'check>(checks: &[&'check Check]) -> Vec<CheckGroup<'c
340340
let by_time = group_by_time(checks);
341341
let mut time_sorted_values: Vec<&Vec<&Check>> = by_time.values().collect();
342342
time_sorted_values.sort();
343-
let max_time_inbetween = chrono::TimeDelta::seconds(OUTAGE_TIME_SPAN);
344-
let mut continuous_outage_groups: Vec<Vec<Vec<&Check>>> = Vec::new();
345-
let mut group_first_time: DateTime<chrono::Local> = chrono::DateTime::UNIX_EPOCH.into();
346-
let mut group_current = Vec::new();
343+
let mut continuous_outage_groups: Vec<Vec<&Check>> = Vec::new();
344+
let mut group_first_time = time_sorted_values[0][0].timestamp();
345+
let mut group_current: Vec<&Check> = Vec::new();
347346
let mut first;
348347

349348
for time_group in time_sorted_values {
349+
#[cfg(debug_assertions)]
350+
{
351+
let t = time_group[0].timestamp();
352+
debug_assert!(
353+
time_group.iter().all(|c| c.timestamp() == t),
354+
"time group does not share time"
355+
)
356+
}
357+
if time_group.iter().all(|c| c.is_success()) {
358+
if !group_current.is_empty() {
359+
continuous_outage_groups.push(group_current.clone());
360+
group_current.clear();
361+
}
362+
continue;
363+
}
350364
first = time_group[0];
351365
if group_current.is_empty() {
352-
group_first_time = first.timestamp_parsed();
366+
group_first_time = first.timestamp();
353367
}
354-
if first.timestamp_parsed() - group_first_time > max_time_inbetween {
368+
if first.timestamp() - group_first_time > OUTAGE_TIME_SPAN {
355369
continuous_outage_groups.push(group_current.clone());
356370
group_current.clear();
371+
group_first_time = first.timestamp(); // i don't understand why this needs to be here
357372
}
358-
group_current.push(time_group.clone());
373+
group_current.extend(time_group);
374+
}
375+
if !group_current.is_empty() {
376+
continuous_outage_groups.push(group_current);
359377
}
360-
continuous_outage_groups.push(group_current.clone());
361378

362379
continuous_outage_groups.sort();
363380
continuous_outage_groups
364-
.into_iter()
365-
.map(|v| v.into_iter().flatten().collect())
366-
.collect()
367381
}
368382

369383
/// Analyze metrics for a specific check type.
@@ -537,42 +551,43 @@ mod tests {
537551
let ip4 = TARGETS[0].parse().unwrap();
538552
let ip6 = TARGETS[1].parse().unwrap();
539553
let time = Utc::now().with_minute(0).unwrap();
540-
let time2 = Utc::now().with_minute(time.minute()+1).unwrap();
541-
let time3 = Utc::now().with_minute(time.minute()+2).unwrap();
542-
let time4 = Utc::now().with_minute(time.minute()+3).unwrap();
543-
let time5 = Utc::now().with_minute(time.minute()+4).unwrap();
544-
let time50 = Utc::now().with_minute(time.minute()+50).unwrap();
554+
let time = |min: u32| Utc::now().with_minute(time.minute()+min).unwrap();
545555

546556
let mut a = vec![
547-
Check::new(time, CheckFlag::Success | CheckFlag::TypeHTTP, None, ip4),
548-
Check::new(time, CheckFlag::Success | CheckFlag::TypeIcmp, None, ip4),
549-
Check::new(time, CheckFlag::Success | CheckFlag::TypeHTTP, None, ip6),
550-
Check::new(time, CheckFlag::Success | CheckFlag::TypeIcmp, None, ip6),
551-
552-
Check::new(time2, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
553-
Check::new(time2, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
554-
Check::new(time2, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
555-
Check::new(time2, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
556-
557-
Check::new(time3, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
558-
Check::new(time3, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
559-
Check::new(time3, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
560-
Check::new(time3, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
561-
562-
Check::new(time4, CheckFlag::Success | CheckFlag::TypeHTTP, None, ip4),
563-
Check::new(time4, CheckFlag::Success | CheckFlag::TypeIcmp, None, ip4),
564-
Check::new(time4, CheckFlag::Success | CheckFlag::TypeHTTP, None, ip6),
565-
Check::new(time4, CheckFlag::Success | CheckFlag::TypeIcmp, None, ip6),
566-
567-
Check::new(time5, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
568-
Check::new(time5, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
569-
Check::new(time5, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
570-
Check::new(time5, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
571-
572-
Check::new(time50, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
573-
Check::new(time50, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
574-
Check::new(time50, CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
575-
Check::new(time50, CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
557+
Check::new(time(0), CheckFlag::Success | CheckFlag::TypeHTTP, None, ip4),
558+
Check::new(time(0), CheckFlag::Success | CheckFlag::TypeIcmp, None, ip4),
559+
Check::new(time(0), CheckFlag::Success | CheckFlag::TypeHTTP, None, ip6),
560+
Check::new(time(0), CheckFlag::Success | CheckFlag::TypeIcmp, None, ip6),
561+
562+
Check::new(time(2), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
563+
Check::new(time(2), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
564+
Check::new(time(2), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
565+
Check::new(time(2), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
566+
567+
Check::new(time(3), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
568+
Check::new(time(3), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
569+
Check::new(time(3), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
570+
Check::new(time(3), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
571+
572+
Check::new(time(4), CheckFlag::Success | CheckFlag::TypeHTTP, None, ip4),
573+
Check::new(time(4), CheckFlag::Success | CheckFlag::TypeIcmp, None, ip4),
574+
Check::new(time(4), CheckFlag::Success | CheckFlag::TypeHTTP, None, ip6),
575+
Check::new(time(4), CheckFlag::Success | CheckFlag::TypeIcmp, None, ip6),
576+
577+
Check::new(time(5), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
578+
Check::new(time(5), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
579+
Check::new(time(5), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
580+
Check::new(time(5), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
581+
582+
Check::new(time(50), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
583+
Check::new(time(50), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
584+
Check::new(time(50), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
585+
Check::new(time(50), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
586+
587+
Check::new(time(51), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip4),
588+
Check::new(time(51), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip4),
589+
Check::new(time(51), CheckFlag::Unreachable | CheckFlag::TypeHTTP, None, ip6),
590+
Check::new(time(51), CheckFlag::Unreachable | CheckFlag::TypeIcmp, None, ip6),
576591
] ;
577592
a.sort();
578593
a
@@ -587,9 +602,10 @@ mod tests {
587602
// fail_groups has been non deterministic in the past, because of not-sorting
588603
for _ in 0..40 {
589604
let fg = fail_groups(&checks);
590-
assert_eq!(fg.len(), 2);
605+
assert_eq!(fg.len(), 3);
591606
assert_eq!(fg[0].len(), 8);
592607
assert_eq!(fg[1].len(), 4);
608+
assert_eq!(fg[2].len(), 8);
593609

594610
let _outages = [
595611
Outage::try_from(fg[0].clone()).unwrap(),
@@ -605,7 +621,7 @@ mod tests {
605621
let checks: Vec<&Check> = base_checks.iter().collect();
606622

607623
let tg = group_by_time(&checks);
608-
assert_eq!(tg.len(), 6);
624+
assert_eq!(tg.len(), 7);
609625
for (k, v) in tg {
610626
assert_eq!(v.len(), 4);
611627
for c in v {

‎src/records.rs‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
//! # }
3939
//! ```
4040
41-
use std::fmt::{Display, Write};
41+
use std::fmt::{Debug, Display, Write};
4242
use std::hash::Hash;
4343
use std::net::IpAddr;
4444

@@ -227,7 +227,7 @@ impl Display for CheckType {
227227
/// - Whether it succeeded
228228
/// - Measured latency (if successful)
229229
/// - Target address
230-
#[derive(Debug, PartialEq, Eq, Hash, Deserialize, Serialize, Clone, Copy)]
230+
#[derive(PartialEq, Eq, Hash, Deserialize, Serialize, Clone, Copy)]
231231
pub struct Check {
232232
/// Unix timestamp when check was performed (seconds since UNIX_EPOCH)
233233
timestamp: i64,
@@ -450,6 +450,17 @@ impl From<IpAddr> for IpType {
450450
}
451451
}
452452

453+
impl Debug for Check {
454+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
455+
f.debug_struct("Check")
456+
.field("timestamp", &self.timestamp_parsed())
457+
.field("flags", &self.flags())
458+
.field("latency", &self.latency())
459+
.field("target", &self.target())
460+
.finish()
461+
}
462+
}
463+
453464
/// Display a formatted list of checks.
454465
///
455466
/// Each check is formatted with:

0 commit comments

Comments
 (0)