diff --git a/.workhorse/specs/monitoring/incidents.md b/.workhorse/specs/monitoring/incidents.md index a6c8ce07..0a2c3abe 100644 --- a/.workhorse/specs/monitoring/incidents.md +++ b/.workhorse/specs/monitoring/incidents.md @@ -18,7 +18,12 @@ An incident opens when a check's effective result becomes failed on a target wit While an incident is open, every issue on its target joins it — effective warnings included — so the incident carries the full context of what was wrong during its span. An issue leaves the incident when it stops being one: its effective result recovers (to passed or skipped, whether by report or by policy), it is resolved or snoozed, or its server stops being monitored. -The incident closes when its last effective failure leaves; warnings never hold an incident open. +Warnings never hold an incident open. + +When the last effective failure leaves because its result recovered, the incident does not close immediately: it **lingers** for its target's linger window, remaining the target's open incident. +A check whose effective result becomes failed during the window — a fresh failure or the same one returning — ends the lingering and the incident continues. +An incident whose linger window elapses without an effective failure closes, recording the close as of when its last effective failure left. +Lingering damps reporter flapping, not operator action: a last failure leaving through resolution, snooze, silence, or its server's monitoring being turned off closes the incident immediately. The membership history — which issues joined and left, and when — is kept and presented as the incident's timeline. An issue can leave and rejoin the same incident. @@ -29,7 +34,8 @@ Operator actions that change what counts (monitoring toggles, group membership c Operators are notified over the notification channel: group incidents to the group's configured channel, Canopy-wide incidents to the operator channel. -An incident notifies when it has stayed open past its target's grace period; an incident that closes within grace never notifies. +An incident notifies when it has stayed open past its target's grace period; the notification additionally waits out any lingering, so it is sent only while an effective failure is live. +An incident that closes before its notification was sent never notifies. Whether an incident notified is recorded as its **published** flag, so flaps can be excluded from reporting. An escalating check's effective failure (see [CHK](checks.md), "Policy") notifies immediately, bypassing any remaining grace; if the incident has already notified, the join escalates it with a further notification, at most once per incident. A notified incident notifies again when it closes. diff --git a/crates/database/src/bin/seed.rs b/crates/database/src/bin/seed.rs index cfacec5c..da5253e0 100644 --- a/crates/database/src/bin/seed.rs +++ b/crates/database/src/bin/seed.rs @@ -524,6 +524,7 @@ async fn seed_groups(conn: &mut AsyncPgConnection) -> Result { notes: "Production deployments across the Pacific facilities.".to_string(), tags: tags(&[("region", "pacific"), ("tier", "production")]), slack_open_delay: Some(PgDuration(SignedDuration::from_secs(300))), + slack_close_delay: None, }, ) .await?; @@ -535,6 +536,7 @@ async fn seed_groups(conn: &mut AsyncPgConnection) -> Result { notes: "Mixed production + clone for the highlands rollout.".to_string(), tags: tags(&[("region", "highlands")]), slack_open_delay: None, + slack_close_delay: None, }, ) .await?; @@ -546,6 +548,7 @@ async fn seed_groups(conn: &mut AsyncPgConnection) -> Result { notes: "Demo and training environments — noisy, low priority.".to_string(), tags: tags(&[("env", "demo")]), slack_open_delay: Some(PgDuration(SignedDuration::from_secs(1800))), + slack_close_delay: None, }, ) .await?; @@ -557,6 +560,7 @@ async fn seed_groups(conn: &mut AsyncPgConnection) -> Result { notes: "Deliberately empty so adding the first server is testable.".to_string(), tags: TagMap::default(), slack_open_delay: None, + slack_close_delay: None, }, ) .await?; diff --git a/crates/database/src/issues.rs b/crates/database/src/issues.rs index c0e4b571..9479b6b5 100644 --- a/crates/database/src/issues.rs +++ b/crates/database/src/issues.rs @@ -174,6 +174,13 @@ pub struct Incident { pub resolved_reason: Option, #[diesel(deserialize_as = jiff_diesel::NullableTimestamp, serialize_as = jiff_diesel::NullableTimestamp)] pub escalated_at: Option, + /// When the incident's last effective failure left and lingering began. + /// `None` while a failure is live. A failure returning clears it (the + /// same incident continues); otherwise the linger sweep closes the + /// incident once this is older than the target's linger window, + /// backdating `closed_at` to it. Not cleared by the close itself. + #[diesel(deserialize_as = jiff_diesel::NullableTimestamp, serialize_as = jiff_diesel::NullableTimestamp)] + pub closing_at: Option, } #[derive(Clone, Debug, Serialize, Deserialize, Queryable, Selectable, Associations)] @@ -1095,13 +1102,22 @@ pub async fn get_global_issue(conn: &mut AsyncPgConnection, r#ref: &str) -> Resu /// warnings included, joins it. The threshold only governs incident /// *creation*; once an incident is in progress everything else piles /// in for context. -/// - **Close**: the incident auto-closes when the last effective-failure -/// contributor leaves. Lesser contributors that joined because the -/// target had an open incident stay attached (so the audit trail and -/// Slack thread retain them) but **do not** hold the incident open by -/// themselves. Without this asymmetry, a check that's stuck firing at -/// warning could keep an incident open indefinitely after the failure -/// that opened it has long since gone away. +/// - **Close**: when the last effective-failure contributor leaves, the +/// incident starts **lingering** (`closing_at` stamped) rather than +/// closing: a failure returning within the target's linger window clears +/// the stamp and the same incident continues, so a red check that blips +/// green doesn't turn one span of trouble into a resolve/re-open pair. +/// The linger sweep ([`sweep_lingering_incidents`]) closes it once the +/// window elapses, backdating the close to when the failure left. Only a +/// leave caused by the check actually recovering lingers: resolution, +/// snooze, silence, and monitoring-off are explicit operator actions, +/// not flaps, and close immediately — as does a zero linger window. +/// Lesser contributors that joined because +/// the target had an open incident stay attached (so the audit trail and +/// Slack thread retain them) but **do not** hold the incident open (or +/// lingering) by themselves. Without this asymmetry, a check that's +/// stuck firing at warning could keep an incident open indefinitely +/// after the failure that opened it has long since gone away. /// /// `monitored` reflects the server's `is_monitored()` at call time. When /// `false`, the issue is treated as a "leave": this is what makes @@ -1160,6 +1176,18 @@ async fn re_evaluate_incident_membership( )) .execute(conn) .await?; + // An effective failure joining a lingering incident ends the + // lingering: the trouble is back, the same incident continues. + if !newly_opened && issue.opens_incident() { + diesel::update( + incidents::table + .filter(incidents::id.eq(incident_id)) + .filter(incidents::closing_at.is_not_null()), + ) + .set(incidents::closing_at.eq(None::)) + .execute(conn) + .await?; + } if newly_opened { enqueue_slack_open(conn, incident_id, target, issue).await?; } else if issue.escalates_now() { @@ -1249,27 +1277,80 @@ async fn re_evaluate_incident_membership( .get_result(conn) .await?; if remaining_open == 0 { - // Filter on `closed_at IS NULL` so that when a stranded - // lesser contributor eventually leaves an already-closed - // incident (because the failure-filter close above already - // retired it), we skip both the no-op update and the - // double Slack resolve. - let closed: Option = diesel::update( + // Linger damps *reporter* flapping: only a leave caused by + // the check actually recovering (inactive, with no operator + // suppression in play) waits out the window. Resolution, + // snooze, silence, and monitoring-off are explicit operator + // actions — not flaps — and close immediately, Slack resolve + // attributed where `by` is known. A zero window is the + // operator opting out of lingering. + let check_recovery = !issue.active + && issue.resolved_at.is_none() + && !snoozed && !silenced + && monitored; + let window = linger_window(conn, target).await?; + if by.is_some() || !check_recovery || window.is_zero() { + // + // Filter on `closed_at IS NULL` so that when a stranded + // lesser contributor eventually leaves an already-closed + // incident (because the failure-filter close above already + // retired it), we skip both the no-op update and the + // double Slack resolve. + let closed: Option = diesel::update( + incidents::table + .filter(incidents::id.eq(open_link.incident_id)) + .filter(incidents::closed_at.is_null()), + ) + .set(incidents::closed_at.eq(jiff_diesel::Timestamp::from(transition_time))) + .returning(Incident::as_select()) + .get_result(conn) + .await + .optional()?; + if let Some(closed) = closed { + enqueue_slack_resolve_inner(conn, &closed, by).await?; + } + } else { + // Start lingering: record when the last effective failure + // left. Stamped once — a lesser contributor leaving an + // already-lingering incident doesn't move the mark — and + // the linger sweep closes the incident when the stamp + // outlives the window (see `sweep_lingering_incidents`). + diesel::update( + incidents::table + .filter(incidents::id.eq(open_link.incident_id)) + .filter(incidents::closed_at.is_null()) + .filter(incidents::closing_at.is_null()), + ) + .set(incidents::closing_at.eq(jiff_diesel::Timestamp::from(transition_time))) + .execute(conn) + .await?; + } + } + } + _ => { + // A member issue re-filing as an effective failure while its + // incident lingers ends the lingering — the trouble is back. A + // leave-and-rejoin lands in the join arm above; this catches the + // member that never left, e.g. a warning contributor re-graded + // to failed. + if was_in && should_join && !should_leave && issue.opens_incident() { + let member_of: Vec = incident_issues::table + .select(incident_issues::incident_id) + .filter(incident_issues::issue_id.eq(issue.id)) + .filter(incident_issues::left_at.is_null()) + .load(conn) + .await?; + diesel::update( incidents::table - .filter(incidents::id.eq(open_link.incident_id)) - .filter(incidents::closed_at.is_null()), + .filter(incidents::id.eq_any(member_of)) + .filter(incidents::closed_at.is_null()) + .filter(incidents::closing_at.is_not_null()), ) - .set(incidents::closed_at.eq(jiff_diesel::Timestamp::from(transition_time))) - .returning(Incident::as_select()) - .get_result(conn) - .await - .optional()?; - if let Some(closed) = closed { - enqueue_slack_resolve_inner(conn, &closed, by).await?; - } + .set(incidents::closing_at.eq(None::)) + .execute(conn) + .await?; } } - _ => {} } Ok(()) } @@ -1545,6 +1626,87 @@ pub async fn reconcile_open_incidents(db: &mut AsyncPgConnection) -> Result<(usi }) .await } +/// Close incidents whose linger window has expired: `closing_at` (when the +/// last effective failure left) has outlived the target's window without a +/// failure returning. The close is backdated to `closing_at` — the linger +/// is damping machinery, not part of the incident's span — and the Slack +/// cancel-or-resolve runs exactly as an immediate close would have: +/// a never-shipped open is cancelled (the flap stays silent), a shipped +/// open gets its resolve. +/// +/// Runs on the monitor pod's minute cadence. Each close is its own +/// transaction, re-checked under a row lock: a failure rejoining (which +/// clears `closing_at`) or an operator resolve (which sets `closed_at`) +/// between the scan and the lock wins. Belt-and-braces, a lingering +/// incident that somehow still has a live effective-failure member is +/// un-lingered instead of closed. +/// +/// Returns the number of incidents closed. +pub async fn sweep_lingering_incidents(db: &mut AsyncPgConnection) -> Result { + use crate::schema::{incident_issues, incidents, issues}; + + let now = Timestamp::now(); + let candidates: Vec = incidents::table + .select(Incident::as_select()) + .filter(incidents::closed_at.is_null()) + .filter(incidents::closing_at.is_not_null()) + .load(db) + .await?; + + let mut closed = 0usize; + for candidate in candidates { + let did_close = db + .transaction::<_, AppError, _>(async |conn| { + let incident: Option = incidents::table + .select(Incident::as_select()) + .filter(incidents::id.eq(candidate.id)) + .filter(incidents::closed_at.is_null()) + .for_update() + .first(conn) + .await + .optional()?; + let Some(incident) = incident else { + return Ok(false); + }; + let Some(closing_at) = incident.closing_at else { + return Ok(false); + }; + let target = IncidentTarget::of_incident(&incident); + if closing_at + linger_window(conn, target).await? > now { + return Ok(false); + } + let live_failures: i64 = incident_issues::table + .inner_join(issues::table.on(issues::id.eq(incident_issues::issue_id))) + .filter(incident_issues::incident_id.eq(incident.id)) + .filter(incident_issues::left_at.is_null()) + .filter(issues::effective_result.eq("failed")) + .count() + .get_result(conn) + .await?; + if live_failures > 0 { + diesel::update(incidents::table.filter(incidents::id.eq(incident.id))) + .set(incidents::closing_at.eq(None::)) + .execute(conn) + .await?; + return Ok(false); + } + let closed_incident: Incident = + diesel::update(incidents::table.filter(incidents::id.eq(incident.id))) + .set(incidents::closed_at.eq(jiff_diesel::Timestamp::from(closing_at))) + .returning(Incident::as_select()) + .get_result(conn) + .await?; + enqueue_slack_resolve_inner(conn, &closed_incident, None).await?; + Ok(true) + }) + .await?; + if did_close { + closed += 1; + } + } + Ok(closed) +} + async fn is_issue_in_open_incident(db: &mut AsyncPgConnection, issue_id: Uuid) -> Result { use crate::schema::incident_issues; @@ -1670,6 +1832,22 @@ async fn find_or_open_incident( /// `slack_open_delay` default (the global target has no config row). const GLOBAL_OPEN_GRACE: SignedDuration = SignedDuration::from_secs(3 * 60); +/// Linger window for canopy-wide incidents, mirroring the per-group +/// `slack_close_delay` default (the global target has no config row). +const GLOBAL_CLOSE_GRACE: SignedDuration = SignedDuration::from_secs(5 * 60); + +/// The target's linger window: how long an incident outlives its last +/// effective failure before it closes. +async fn linger_window( + conn: &mut AsyncPgConnection, + target: IncidentTarget, +) -> Result { + Ok(match target { + IncidentTarget::Group(gid) => ServerGroup::get_by_id(conn, gid).await?.slack_close_delay.0, + IncidentTarget::Global => GLOBAL_CLOSE_GRACE, + }) +} + async fn enqueue_slack_open( conn: &mut AsyncPgConnection, incident_id: Uuid, diff --git a/crates/database/src/schema.rs b/crates/database/src/schema.rs index 1334d4ab..8c797ce5 100644 --- a/crates/database/src/schema.rs +++ b/crates/database/src/schema.rs @@ -270,6 +270,7 @@ diesel::table! { resolved_reason -> Nullable, server_group_id -> Nullable, escalated_at -> Nullable, + closing_at -> Nullable, } } @@ -458,6 +459,7 @@ diesel::table! { version_server_id -> Nullable, effective_version -> Nullable, deleted_at -> Nullable, + slack_close_delay -> Interval, } } diff --git a/crates/database/src/server_groups.rs b/crates/database/src/server_groups.rs index 8f9b0e83..ef8bad0c 100644 --- a/crates/database/src/server_groups.rs +++ b/crates/database/src/server_groups.rs @@ -95,6 +95,14 @@ pub struct ServerGroup { treat_none_as_default_value = false )] pub deleted_at: Option, + /// How long, in seconds, an incident lingers after its last failure + /// recovers before it closes and the resolved notification is sent. A + /// failure returning within this window continues the same incident + /// instead of opening (and notifying about) a new one — the close-side + /// mirror of `slack_open_delay`, for a red check that briefly blips + /// green. + #[schema(value_type = i64, format = "int64")] + pub slack_close_delay: PgDuration, } /// Fields required to create a new server group. @@ -117,6 +125,12 @@ pub struct NewServerGroup { #[serde(default)] #[schema(value_type = Option, format = "int64")] pub slack_open_delay: Option, + /// How long, in seconds, an incident lingers after its last failure + /// recovers before it closes and notifies as resolved. Defaults to the + /// system default if omitted. + #[serde(default)] + #[schema(value_type = Option, format = "int64")] + pub slack_close_delay: Option, } /// Fields to update on an existing server group. Only the fields present @@ -135,6 +149,9 @@ pub struct PartialServerGroup { /// New incident-opened notification delay, in seconds. #[schema(value_type = Option, format = "int64")] pub slack_open_delay: Option, + /// New incident linger window, in seconds. + #[schema(value_type = Option, format = "int64")] + pub slack_close_delay: Option, } impl ServerGroup { diff --git a/crates/database/src/slack_outbox/mod.rs b/crates/database/src/slack_outbox/mod.rs index fd759c1f..2ed35084 100644 --- a/crates/database/src/slack_outbox/mod.rs +++ b/crates/database/src/slack_outbox/mod.rs @@ -208,13 +208,26 @@ impl SlackOutbox { /// [`mark_failed`](Self::mark_failed), or /// [`mark_given_up`](Self::mark_given_up) before committing. pub async fn claim_pending(db: &mut AsyncPgConnection, limit: i64) -> Result> { - use crate::schema::slack_outbox::dsl; + use crate::schema::{incidents, slack_outbox::dsl}; + use diesel::dsl::{exists, not}; let now = Timestamp::now(); dsl::slack_outbox .select(Self::as_select()) .filter(dsl::delivered_at.is_null()) .filter(dsl::gave_up_at.is_null()) .filter(dsl::deliver_after.le(jiff_diesel::Timestamp::from(now))) + // An `incident_open` for a lingering incident (its last effective + // failure has left, the linger window hasn't elapsed) must not + // ship: a one-off blip would otherwise notify purely because the + // linger held the incident open past its `deliver_after`. The row + // stays pending — it ships when a failure returns, or is + // cancelled when the linger expires and the incident closes. + .filter(not(dsl::kind.eq(KIND_INCIDENT_OPEN).and(exists( + incidents::table + .filter(incidents::id.nullable().eq(dsl::incident_id)) + .filter(incidents::closed_at.is_null()) + .filter(incidents::closing_at.is_not_null()), + )))) .order(dsl::created_at.asc()) .limit(limit) .for_update() diff --git a/crates/database/tests/it/backup_detection.rs b/crates/database/tests/it/backup_detection.rs index c809e4da..ec1c80c8 100644 --- a/crates/database/tests/it/backup_detection.rs +++ b/crates/database/tests/it/backup_detection.rs @@ -944,12 +944,33 @@ async fn group_event_pages_even_when_all_members_unmonitored() { 0, "recovery removes the issue from its incident", ); + // The incident lingers (close-side grace) rather than closing on + // the spot; once the window elapses the sweep closes it. let still_open = database::issues::Incident::list_for_group(&mut conn, group_id, false, 10) .await .expect("list incidents 2"); + assert_eq!( + still_open.len(), + 1, + "incident lingers after its only contributor recovers", + ); + sql_query( + "UPDATE incidents SET closing_at = closing_at - INTERVAL '1 hour' \ + WHERE server_group_id = $1", + ) + .bind::(group_id) + .execute(&mut conn) + .await + .expect("expire linger"); + database::issues::sweep_lingering_incidents(&mut conn) + .await + .expect("linger sweep"); + let still_open = database::issues::Incident::list_for_group(&mut conn, group_id, false, 10) + .await + .expect("list incidents 3"); assert!( still_open.is_empty(), - "incident auto-closes when its only contributor recovers", + "linger sweep closes the incident once the window elapses", ); }) .await; diff --git a/crates/database/tests/it/incident_close_result.rs b/crates/database/tests/it/incident_close_result.rs index b10f6e38..c92a8fe6 100644 --- a/crates/database/tests/it/incident_close_result.rs +++ b/crates/database/tests/it/incident_close_result.rs @@ -86,6 +86,20 @@ async fn mark_open_delivered(conn: &mut diesel_async::AsyncPgConnection, inciden .expect("mark delivered"); } +/// Backdate a lingering incident's `closing_at` past any realistic linger +/// window, then run the sweep — the test-speed way to let the close-side +/// grace elapse. +async fn expire_linger(conn: &mut diesel_async::AsyncPgConnection, incident_id: Uuid) { + sql_query("UPDATE incidents SET closing_at = closing_at - INTERVAL '1 hour' WHERE id = $1") + .bind::(incident_id) + .execute(conn) + .await + .expect("expire linger"); + database::issues::sweep_lingering_incidents(conn) + .await + .expect("linger sweep"); +} + async fn count_resolve_rows(conn: &mut diesel_async::AsyncPgConnection, incident_id: Uuid) -> i64 { #[derive(QueryableByName)] struct Count { @@ -166,13 +180,24 @@ async fn warning_does_not_hold_incident_open_after_error_resolves() { ) .await; - // New logic: incident closes despite the warning still being active. + // The incident lingers rather than closing on the spot — the still- + // active warning must not hold it past the window, so once the + // linger elapses the sweep closes it despite the warning. + let lingering = Incident::list_for_server(&mut conn, server_id, false, 10) + .await + .expect("list"); + assert_eq!( + lingering.len(), + 1, + "incident lingers after the failure recovers" + ); + expire_linger(&mut conn, incident.id).await; let still_open = Incident::list_for_server(&mut conn, server_id, false, 10) .await .expect("list"); assert!( still_open.is_empty(), - "incident must close once no severity≥error contributor is alive, got: {still_open:?}", + "incident must close once no failure contributor is alive, got: {still_open:?}", ); // One Slack resolve enqueued (not two — the guard against re-close fires). @@ -269,7 +294,8 @@ async fn stranded_warning_resolve_does_not_re_enqueue_slack() { ) .await; - // Error resolves → incident closes (one slack resolve). + // Error resolves → incident lingers, then the sweep closes it + // (one slack resolve). save_event( &mut conn, server_id, @@ -279,6 +305,7 @@ async fn stranded_warning_resolve_does_not_re_enqueue_slack() { "recovered", ) .await; + expire_linger(&mut conn, incident.id).await; assert_eq!(count_resolve_rows(&mut conn, incident.id).await, 1); // Warning eventually resolves too. Incident is already closed; this diff --git a/crates/database/tests/it/incident_linger.rs b/crates/database/tests/it/incident_linger.rs new file mode 100644 index 00000000..b95b84b2 --- /dev/null +++ b/crates/database/tests/it/incident_linger.rs @@ -0,0 +1,395 @@ +//! Close-side grace ("linger"): when an incident's last effective failure +//! recovers, the incident stays open for the group's `slack_close_delay`. +//! A failure returning within the window continues the same incident (no +//! new row, no new Slack open); the linger sweep closes it once the window +//! elapses, backdating the close to when the failure left. Operator-driven +//! leaves and zero-window groups close immediately, and the outbox drainer +//! won't ship an `incident_open` while its incident lingers. + +use commons_types::status::CheckResult; +use database::issues::{Incident, NewEvent}; +use database::slack_outbox::{KIND_INCIDENT_OPEN, KIND_INCIDENT_RESOLVE, SlackOutbox}; +use diesel::prelude::*; +use diesel::{QueryableByName, sql_query, sql_types}; +use diesel_async::RunQueryDsl; +use uuid::Uuid; + +#[derive(QueryableByName)] +struct RowId { + #[diesel(sql_type = sql_types::Uuid)] + id: Uuid, +} + +/// Server in a fresh group carrying the given linger window (as an SQL +/// interval literal, e.g. `'5 minutes'`). +async fn insert_grouped_server( + conn: &mut diesel_async::AsyncPgConnection, + linger: &str, +) -> (Uuid, Uuid) { + let group: RowId = sql_query(format!( + "INSERT INTO server_groups (name, slack_close_delay) \ + VALUES ('linger-group', INTERVAL '{linger}') RETURNING id" + )) + .get_result(conn) + .await + .expect("group"); + let row: RowId = sql_query( + "INSERT INTO servers (host, group_id) VALUES ('http://linger.invalid/', $1) RETURNING id", + ) + .bind::(group.id) + .get_result(conn) + .await + .expect("server"); + (group.id, row.id) +} + +async fn save_event( + conn: &mut diesel_async::AsyncPgConnection, + server_id: Uuid, + r#ref: &str, + result: CheckResult, + message: &str, +) { + let active = matches!( + result, + CheckResult::Failed | CheckResult::Warning | CheckResult::Broken + ); + let stamp = database::issues::CheckStateStamp { + check: r#ref.into(), + observed: result, + effective: result, + escalates: false, + detail: None, + }; + NewEvent { + source: "test".into(), + r#ref: r#ref.into(), + description: None, + message: message.into(), + active: Some(active), + occurred_at: None, + } + .save_with_state(conn, server_id, None, Some(&stamp)) + .await + .expect("save event"); +} + +async fn incident_row(conn: &mut diesel_async::AsyncPgConnection, id: Uuid) -> Incident { + use database::schema::incidents::dsl; + dsl::incidents + .select(Incident::as_select()) + .filter(dsl::id.eq(id)) + .first(conn) + .await + .expect("incident row") +} + +async fn incident_count(conn: &mut diesel_async::AsyncPgConnection, group_id: Uuid) -> i64 { + use database::schema::incidents::dsl; + dsl::incidents + .filter(dsl::server_group_id.eq(group_id)) + .count() + .get_result(conn) + .await + .expect("count incidents") +} + +async fn the_open_incident(conn: &mut diesel_async::AsyncPgConnection, group_id: Uuid) -> Incident { + use database::schema::incidents::dsl; + dsl::incidents + .select(Incident::as_select()) + .filter(dsl::server_group_id.eq(group_id)) + .filter(dsl::closed_at.is_null()) + .first(conn) + .await + .expect("open incident") +} + +async fn outbox_rows(conn: &mut diesel_async::AsyncPgConnection) -> Vec { + use database::schema::slack_outbox::dsl; + dsl::slack_outbox + .select(SlackOutbox::as_select()) + .order(dsl::created_at.asc()) + .load(conn) + .await + .expect("load outbox") +} + +async fn mark_open_delivered(conn: &mut diesel_async::AsyncPgConnection, incident_id: Uuid) { + let row: RowId = + sql_query("SELECT id FROM slack_outbox WHERE incident_id = $1 AND kind = $2 LIMIT 1") + .bind::(incident_id) + .bind::(KIND_INCIDENT_OPEN) + .get_result(conn) + .await + .expect("pending open row exists"); + SlackOutbox::mark_delivered(conn, row.id, "ok") + .await + .expect("mark delivered"); +} + +/// Backdate `closing_at` past any realistic window, then sweep. +async fn expire_linger(conn: &mut diesel_async::AsyncPgConnection, incident_id: Uuid) -> usize { + sql_query("UPDATE incidents SET closing_at = closing_at - INTERVAL '1 hour' WHERE id = $1") + .bind::(incident_id) + .execute(conn) + .await + .expect("expire linger"); + database::issues::sweep_lingering_incidents(conn) + .await + .expect("linger sweep") +} + +/// The headline behaviour: a red check that blips green and comes back +/// within the window stays one incident — one open, one resolve, however +/// many blips. +#[tokio::test(flavor = "multi_thread")] +async fn rejoin_within_linger_continues_the_same_incident() { + commons_tests::db::TestDb::run(async |mut conn, _| { + let (group_id, server_id) = insert_grouped_server(&mut conn, "5 minutes").await; + + save_event(&mut conn, server_id, "flappy", CheckResult::Failed, "boom").await; + let incident = the_open_incident(&mut conn, group_id).await; + assert_eq!(incident.closing_at, None); + mark_open_delivered(&mut conn, incident.id).await; + + // Blip green: the incident lingers instead of closing. + save_event(&mut conn, server_id, "flappy", CheckResult::Passed, "ok?").await; + let lingering = incident_row(&mut conn, incident.id).await; + assert_eq!(lingering.closed_at, None, "incident stays open"); + assert!(lingering.closing_at.is_some(), "lingering is stamped"); + assert_eq!( + outbox_rows(&mut conn).await.len(), + 1, + "no resolve enqueued while lingering" + ); + + // Red again within the window: same incident, lingering cleared, + // still no extra Slack traffic. + save_event(&mut conn, server_id, "flappy", CheckResult::Failed, "boom").await; + let resumed = incident_row(&mut conn, incident.id).await; + assert_eq!(resumed.closing_at, None, "rejoin ends the lingering"); + assert_eq!(resumed.closed_at, None); + assert_eq!( + incident_count(&mut conn, group_id).await, + 1, + "the blip did not fragment the trouble into a second incident" + ); + assert_eq!( + outbox_rows(&mut conn).await.len(), + 1, + "no second open for the rejoin" + ); + + // Final recovery, window elapses: closed, backdated, one resolve. + save_event(&mut conn, server_id, "flappy", CheckResult::Passed, "ok").await; + let closed_count = expire_linger(&mut conn, incident.id).await; + assert_eq!(closed_count, 1, "sweep closed exactly this incident"); + let closed = incident_row(&mut conn, incident.id).await; + assert_eq!( + closed.closed_at, closed.closing_at, + "close is backdated to the (test-shifted) lingering stamp" + ); + assert!(closed.closed_at.is_some()); + let rows = outbox_rows(&mut conn).await; + assert_eq!(rows.len(), 2, "one open, one resolve: {rows:?}"); + assert_eq!(rows[1].kind, KIND_INCIDENT_RESOLVE); + }) + .await +} + +/// A member that never left — a warning contributor — re-grading to an +/// effective failure while the incident lingers ends the lingering too. +#[tokio::test(flavor = "multi_thread")] +async fn member_regrade_to_failure_ends_lingering() { + commons_tests::db::TestDb::run(async |mut conn, _| { + let (group_id, server_id) = insert_grouped_server(&mut conn, "5 minutes").await; + + save_event(&mut conn, server_id, "opener", CheckResult::Failed, "boom").await; + let incident = the_open_incident(&mut conn, group_id).await; + // A warning joins the open incident but can't hold (or re-open) it. + save_event( + &mut conn, + server_id, + "grumbler", + CheckResult::Warning, + "meh", + ) + .await; + + save_event(&mut conn, server_id, "opener", CheckResult::Passed, "ok").await; + assert!( + incident_row(&mut conn, incident.id) + .await + .closing_at + .is_some(), + "warning member alone leaves the incident lingering" + ); + + // The warning escalates to a failure without ever leaving: the + // lingering ends, the incident continues. + save_event(&mut conn, server_id, "grumbler", CheckResult::Failed, "ow").await; + let resumed = incident_row(&mut conn, incident.id).await; + assert_eq!(resumed.closing_at, None, "regrade ends the lingering"); + assert_eq!(resumed.closed_at, None); + assert_eq!(incident_count(&mut conn, group_id).await, 1); + }) + .await +} + +/// An operator resolving the last failure is not a flap: the incident +/// closes immediately and the resolve ships attributed to them. +#[tokio::test(flavor = "multi_thread")] +async fn operator_resolve_closes_immediately() { + commons_tests::db::TestDb::run(async |mut conn, _| { + use database::issues::Issue; + + let (group_id, server_id) = insert_grouped_server(&mut conn, "5 minutes").await; + save_event(&mut conn, server_id, "handled", CheckResult::Failed, "boom").await; + let incident = the_open_incident(&mut conn, group_id).await; + mark_open_delivered(&mut conn, incident.id).await; + + let issue: Issue = { + use database::schema::issues::dsl; + dsl::issues + .select(Issue::as_select()) + .filter(dsl::server_id.eq(server_id)) + .filter(dsl::ref_.eq("handled")) + .first(&mut conn) + .await + .expect("issue") + }; + Issue::resolve( + &mut conn, + issue.id, + "op@example.com", + commons_types::issue::ResolvedReason::Fixed, + ) + .await + .expect("operator resolve"); + + let closed = incident_row(&mut conn, incident.id).await; + assert!( + closed.closed_at.is_some(), + "operator-driven leave skips the linger" + ); + let rows = outbox_rows(&mut conn).await; + assert_eq!(rows.len(), 2, "resolve shipped at once: {rows:?}"); + assert_eq!(rows[1].kind, KIND_INCIDENT_RESOLVE); + }) + .await +} + +/// A zero `slack_close_delay` opts the group out of lingering entirely. +#[tokio::test(flavor = "multi_thread")] +async fn zero_window_closes_immediately() { + commons_tests::db::TestDb::run(async |mut conn, _| { + let (group_id, server_id) = insert_grouped_server(&mut conn, "0").await; + save_event(&mut conn, server_id, "quick", CheckResult::Failed, "boom").await; + let incident = the_open_incident(&mut conn, group_id).await; + + save_event(&mut conn, server_id, "quick", CheckResult::Passed, "ok").await; + let closed = incident_row(&mut conn, incident.id).await; + assert!(closed.closed_at.is_some(), "zero window: no linger"); + assert_eq!(closed.closing_at, None); + }) + .await +} + +/// The drainer must not ship an `incident_open` while its incident +/// lingers: a one-off blip would otherwise notify purely because the +/// linger held the incident open past its `deliver_after`. +#[tokio::test(flavor = "multi_thread")] +async fn drainer_holds_opens_while_lingering() { + commons_tests::db::TestDb::run(async |mut conn, _| { + let (group_id, server_id) = insert_grouped_server(&mut conn, "5 minutes").await; + save_event(&mut conn, server_id, "blippy", CheckResult::Failed, "boom").await; + let incident = the_open_incident(&mut conn, group_id).await; + + // Let the open-side grace elapse so the row is nominally claimable. + sql_query( + "UPDATE slack_outbox SET deliver_after = NOW() - INTERVAL '1 second' WHERE incident_id = $1", + ) + .bind::(incident.id) + .execute(&mut conn) + .await + .expect("age the open row"); + + // While red: claimable. + let claimed = SlackOutbox::claim_pending(&mut conn, 10) + .await + .expect("claim"); + assert_eq!( + claimed.len(), + 1, + "open is claimable while a failure is live" + ); + + // Blip green: lingering, the open must be held back. + save_event(&mut conn, server_id, "blippy", CheckResult::Passed, "ok?").await; + let claimed = SlackOutbox::claim_pending(&mut conn, 10) + .await + .expect("claim"); + assert!( + claimed.is_empty(), + "open is not claimable while the incident lingers: {claimed:?}" + ); + + // Red returns: claimable again — trouble has outlived the grace. + save_event(&mut conn, server_id, "blippy", CheckResult::Failed, "boom").await; + let claimed = SlackOutbox::claim_pending(&mut conn, 10) + .await + .expect("claim"); + assert_eq!(claimed.len(), 1, "open claimable again after the rejoin"); + + // And if instead the flap ends for good, expiry cancels the open: + // Slack never hears about the blip. + save_event(&mut conn, server_id, "blippy", CheckResult::Passed, "ok").await; + expire_linger(&mut conn, incident.id).await; + let rows = outbox_rows(&mut conn).await; + assert_eq!( + rows.len(), + 1, + "no resolve for a never-shipped open: {rows:?}" + ); + assert!( + rows[0].gave_up_at.is_some(), + "pending open cancelled at linger expiry" + ); + assert_eq!( + incident_row(&mut conn, incident.id).await.closed_at, + incident_row(&mut conn, incident.id).await.closing_at, + "close backdated to when the failure left" + ); + }) + .await +} + +/// Timestamps: `deliver_after` on the open row is untouched by lingering, +/// so an incident whose trouble keeps flapping still publishes once its +/// cumulative age passes the open grace — the fast flapper is no longer +/// permanently silent. +#[tokio::test(flavor = "multi_thread")] +async fn flapping_incident_still_publishes_after_grace() { + commons_tests::db::TestDb::run(async |mut conn, _| { + let (group_id, server_id) = insert_grouped_server(&mut conn, "5 minutes").await; + save_event(&mut conn, server_id, "flappy", CheckResult::Failed, "boom").await; + let incident = the_open_incident(&mut conn, group_id).await; + let before = outbox_rows(&mut conn).await; + assert_eq!(before[0].incident_id, Some(incident.id)); + let original_deliver_after = before[0].deliver_after; + + // A full flap cycle: green, red again. + save_event(&mut conn, server_id, "flappy", CheckResult::Passed, "ok?").await; + save_event(&mut conn, server_id, "flappy", CheckResult::Failed, "boom").await; + + let after = outbox_rows(&mut conn).await; + assert_eq!(after.len(), 1, "still exactly one open row"); + assert_eq!( + after[0].deliver_after, original_deliver_after, + "the open's grace counts from the incident's first open, not the latest red" + ); + assert!(after[0].gave_up_at.is_none()); + }) + .await +} diff --git a/crates/database/tests/it/main.rs b/crates/database/tests/it/main.rs index 12a38ca5..40d911ed 100644 --- a/crates/database/tests/it/main.rs +++ b/crates/database/tests/it/main.rs @@ -12,6 +12,7 @@ mod check_severity_map; mod event_validation; mod incident_close_result; mod incident_get_with_issues; +mod incident_linger; mod incident_result_semantics; mod incident_stats; mod mcp_tokens; diff --git a/crates/database/tests/it/self_alerts.rs b/crates/database/tests/it/self_alerts.rs index 094bdb55..109e5e76 100644 --- a/crates/database/tests/it/self_alerts.rs +++ b/crates/database/tests/it/self_alerts.rs @@ -1,9 +1,11 @@ //! Self-alert lifecycle: a raise files one coalescing canopy-wide issue //! which opens a canopy-wide incident, enqueuing exactly one Slack open on //! the not-alerting → alerting transition (with flap grace for -//! non-escalating checks, immediate for escalating ones); recovery inside -//! the grace cancels the open and sends nothing; recovery after delivery -//! enqueues the resolve; idle recovers write nothing. +//! non-escalating checks, immediate for escalating ones); recovery starts +//! the incident lingering, and a re-raise within the linger continues the +//! same incident without a new notification; a flap whose open never +//! shipped is cancelled silently at linger expiry; recovery after delivery +//! enqueues the resolve at linger expiry; idle recovers write nothing. use commons_types::status::CheckResult; use database::self_alerts; @@ -46,6 +48,22 @@ async fn outbox_rows(conn: &mut diesel_async::AsyncPgConnection) -> Vec JoinHandle<()> { Err(err) => error!("staleness sweep failed: {err}"), } + // Incidents whose linger window has expired: the last effective + // failure left, nothing came back, close them and ship the + // pending Slack cancel-or-resolve. + match database::issues::sweep_lingering_incidents(&mut db).await { + Ok(0) => {} + Ok(n) => debug!("closed {n} lingering incident(s)"), + Err(err) => error!("incident linger sweep failed: {err}"), + } + // Backup staleness + report-vs-inventory reconciliation: another // minute-cadence DB-only sweep, so it rides this loop rather than a // separate pod. diff --git a/crates/private-server/src/fns/incidents.rs b/crates/private-server/src/fns/incidents.rs index 567b7915..74cec6a2 100644 --- a/crates/private-server/src/fns/incidents.rs +++ b/crates/private-server/src/fns/incidents.rs @@ -63,6 +63,13 @@ pub struct IncidentData { /// Lets a client distinguish "open but quietly held" from "open and /// operators have been notified". pub notification_held_until: Option, + /// When set, the incident is lingering: its last effective failure + /// recovered at this time, and the incident stays open for the group's + /// linger window in case the failure comes back (which would continue + /// this incident rather than open a new one). Null while a failure is + /// live, and always null on a closed incident. Lets a client + /// distinguish "open and failing" from "open but currently recovered". + pub lingering_since: Option, /// When the incident record was created. pub created_at: Timestamp, /// When the incident record was last modified. @@ -78,6 +85,11 @@ impl IncidentData { notification_held_until: Option, ) -> Self { let (res_name, res_pic) = lookup_user(users, i.resolved_by.as_deref()); + let lingering_since = if i.closed_at.is_none() { + i.closing_at + } else { + None + }; Self { id: i.id, server_group_id: i.server_group_id, @@ -92,6 +104,7 @@ impl IncidentData { issue_count: stats.issue_count, note_count: stats.note_count, notification_held_until, + lingering_since, created_at: i.created_at, updated_at: i.updated_at, } diff --git a/crates/private-server/src/fns/server_groups.rs b/crates/private-server/src/fns/server_groups.rs index aad10eb3..d406a2ee 100644 --- a/crates/private-server/src/fns/server_groups.rs +++ b/crates/private-server/src/fns/server_groups.rs @@ -209,6 +209,13 @@ pub struct ServerGroupsCreateArgs { #[serde(default)] #[schema(value_type = Option, format = "int64")] pub slack_open_delay: Option, + /// Optional linger window, in whole seconds: how long an incident stays + /// open after its last failure recovers, so a failure returning within + /// the window continues the same incident instead of opening (and + /// notifying about) a new one. Omit to accept the default. + #[serde(default)] + #[schema(value_type = Option, format = "int64")] + pub slack_close_delay: Option, } /// Create a server group. @@ -240,6 +247,7 @@ pub async fn create( notes: args.notes, tags: args.tags, slack_open_delay: args.slack_open_delay, + slack_close_delay: args.slack_close_delay, }, ) .await?; diff --git a/crates/private-server/tests/it/issues.rs b/crates/private-server/tests/it/issues.rs index 8b761a3d..df208bb3 100644 --- a/crates/private-server/tests/it/issues.rs +++ b/crates/private-server/tests/it/issues.rs @@ -299,6 +299,19 @@ async fn issue_reopen_keeps_identity_and_joins_new_incident() { .to_string(); assert_eq!(issue_id_1, issue_id_2, "same identity through inactive"); + // The recovery leaves the incident lingering; let the window + // elapse so the reopen below is a genuinely new incident rather + // than a rejoin of the lingering one. + conn.batch_execute( + "UPDATE incidents SET closing_at = closing_at - INTERVAL '1 hour' \ + WHERE closing_at IS NOT NULL", + ) + .await + .expect("expire linger"); + database::issues::sweep_lingering_incidents(&mut conn) + .await + .expect("linger sweep"); + // 3. Reopen — same identity, severity ≥ error. let r3 = private .post("/api/issues/submit_manual_event") diff --git a/crates/public-server/tests/it/statuses.rs b/crates/public-server/tests/it/statuses.rs index 41ab64d1..2e407525 100644 --- a/crates/public-server/tests/it/statuses.rs +++ b/crates/public-server/tests/it/statuses.rs @@ -137,6 +137,22 @@ async fn fetch_open_incident( .ok() } +/// Recovery leaves the incident lingering (close-side grace) rather than +/// closing it on the spot; backdate the stamp past any window and sweep, +/// the test-speed way to let the linger elapse. +async fn expire_linger(conn: &mut diesel_async::AsyncPgConnection) { + sql_query( + "UPDATE incidents SET closing_at = closing_at - INTERVAL '1 hour' \ + WHERE closing_at IS NOT NULL", + ) + .execute(conn) + .await + .expect("expire linger"); + database::issues::sweep_lingering_incidents(conn) + .await + .expect("linger sweep"); +} + #[tokio::test(flavor = "multi_thread")] async fn submit_status() { commons_tests::server::run_with_device_auth( @@ -1340,6 +1356,7 @@ async fn submit_status_full_recovery_closes_incident() { ) .await; + expire_linger(&mut conn).await; assert!( fetch_open_incident(&mut conn, server_id).await.is_none(), "incident must auto-close when every contributing issue resolves" @@ -2145,6 +2162,7 @@ async fn submit_status_failed_then_broken_retains_the_failure() { .await .expect("issue exists"); assert!(!issue.active); + expire_linger(&mut conn).await; assert!(fetch_open_incident(&mut conn, server_id).await.is_none()); }, ) diff --git a/migrations/2026-07-18-043336-0000_incident_linger/down.sql b/migrations/2026-07-18-043336-0000_incident_linger/down.sql new file mode 100644 index 00000000..6037bb1b --- /dev/null +++ b/migrations/2026-07-18-043336-0000_incident_linger/down.sql @@ -0,0 +1,2 @@ +ALTER TABLE server_groups DROP COLUMN slack_close_delay; +ALTER TABLE incidents DROP COLUMN closing_at; diff --git a/migrations/2026-07-18-043336-0000_incident_linger/up.sql b/migrations/2026-07-18-043336-0000_incident_linger/up.sql new file mode 100644 index 00000000..1001560e --- /dev/null +++ b/migrations/2026-07-18-043336-0000_incident_linger/up.sql @@ -0,0 +1,14 @@ +-- Close-side grace ("linger"). When an incident's last effective failure +-- leaves, the incident lingers — `closing_at` records when — instead of +-- closing outright, so a failure returning within the window continues the +-- same incident rather than opening (and re-notifying) a new one. The +-- linger sweep closes incidents whose `closing_at` has outlived their +-- group's `slack_close_delay`, backdating `closed_at` to `closing_at`. +ALTER TABLE incidents + ADD COLUMN closing_at TIMESTAMPTZ; + +-- Per-group linger window, alongside `slack_open_delay`. Set to +-- `INTERVAL '0'` to close (and ship resolves) immediately, as before. +ALTER TABLE server_groups + ADD COLUMN slack_close_delay INTERVAL NOT NULL DEFAULT INTERVAL '5 minutes' + CHECK (slack_close_delay >= INTERVAL '0'); diff --git a/private-web/e2e/groups.spec.ts b/private-web/e2e/groups.spec.ts index 16a40907..058d6804 100644 --- a/private-web/e2e/groups.spec.ts +++ b/private-web/e2e/groups.spec.ts @@ -193,30 +193,38 @@ test.describe("group edit page", () => { expect(rows[0]!.tags).toEqual({ env: "prod", tier: "1" }); }); - test("editing the Slack cooldown minutes persists as the right interval", async ({ + test("editing the Slack cooldown and linger minutes persists as the right intervals", async ({ page, sql, }) => { - // Seeded at 3 minutes (180s); UI works in minutes; bumping to 5 - // should round-trip through the API as 300 seconds in the DB. + // Seeded at 3 minutes open / 4 minutes linger; UI works in minutes; + // bumping to 5 and 7 should round-trip through the API as seconds + // in the DB. const group = await seedServerGroup(sql, { name: "cooldown-group", slackOpenDelaySeconds: 180, + slackCloseDelaySeconds: 240, }); await page.goto(`/groups/${group.id}/edit`); - const minutesInput = page.getByLabel(/^minutes$/i); - await expect(minutesInput).toHaveValue("3"); - await minutesInput.fill("5"); + // Two "minutes" fields: the open cooldown first, the linger second. + const openInput = page.getByLabel(/^minutes$/i).first(); + const lingerInput = page.getByLabel(/^minutes$/i).nth(1); + await expect(openInput).toHaveValue("3"); + await expect(lingerInput).toHaveValue("4"); + await openInput.fill("5"); + await lingerInput.fill("7"); await page.getByRole("button", { name: /^save$/i }).click(); await page.waitForURL(`**/groups/${group.id}`); - const rows = await sql.query<{ secs: string }>( - "SELECT EXTRACT(EPOCH FROM slack_open_delay)::text AS secs \ + const rows = await sql.query<{ open: string; close: string }>( + "SELECT EXTRACT(EPOCH FROM slack_open_delay)::text AS open, \ + EXTRACT(EPOCH FROM slack_close_delay)::text AS close \ FROM server_groups WHERE id = $1", [group.id], ); - expect(Number(rows[0]!.secs)).toBe(300); + expect(Number(rows[0]!.open)).toBe(300); + expect(Number(rows[0]!.close)).toBe(420); }); }); diff --git a/private-web/e2e/lingering.spec.ts b/private-web/e2e/lingering.spec.ts new file mode 100644 index 00000000..0fd3698a --- /dev/null +++ b/private-web/e2e/lingering.spec.ts @@ -0,0 +1,84 @@ +import { expect, test } from "./test-fixtures"; +import { + resetSeededTables, + seedIncident, + seedIssue, + seedServer, + seedServerGroup, + seedVersion, +} from "./seed"; + +// A lingering incident — open, but its last effective failure has +// recovered and it is waiting out the group's linger window — reads +// info-toned ("recovering") rather than error/warning, on every surface +// that differentiates held incidents. +test.describe("lingering incidents", () => { + test.beforeEach(async ({ sql }) => { + await resetSeededTables(sql); + }); + + async function seedLingering(sql: Parameters[0]) { + const group = await seedServerGroup(sql, { name: "linger-group" }); + const server = await seedServer(sql, { + name: "linger-server", + kind: "central", + rank: "production", + groupId: group.id, + }); + // The failure that opened the incident, since recovered. + const issue = await seedIssue(sql, { + serverId: server.id, + ref: "health/flappy", + active: false, + message: "was failing, now recovered", + }); + const opened = new Date(Date.now() - 10 * 60_000).toISOString(); + const cleared = new Date(Date.now() - 2 * 60_000).toISOString(); + const incident = await seedIncident(sql, { + serverGroupId: group.id, + openedAt: opened, + closingAt: cleared, + issues: [{ issueId: issue.id, joinedAt: opened, leftAt: cleared }], + }); + return { group, server, incident }; + } + + test("incident page shows the recovering chip", async ({ page, sql }) => { + const { incident } = await seedLingering(sql); + + await page.goto(`/incidents/${incident.id}`); + await expect( + page.getByText(/recovering; last failure cleared/i), + ).toBeVisible(); + await expect( + page.getByText(/a failure returning within the linger window/i), + ).toBeVisible(); + }); + + test("server page incident button reads recovering", async ({ + page, + sql, + }) => { + const { server, incident } = await seedLingering(sql); + + await page.goto(`/servers/${server.id}`); + const button = page.getByRole("link", { + name: new RegExp(`incident ${incident.id.slice(0, 8)}.*recovering`, "i"), + }); + await expect(button).toBeVisible(); + }); + + test("status page group card shows the recovering chip", async ({ + page, + sql, + }) => { + // group_details computes version-distance against the latest + // published version; without one the card 404s. + await seedVersion(sql, { major: 1, minor: 0, patch: 0 }); + const { group } = await seedLingering(sql); + + await page.goto("/status"); + await expect(page.getByRole("heading", { name: group.name })).toBeVisible(); + await expect(page.getByText("incident (recovering)")).toBeVisible(); + }); +}); diff --git a/private-web/e2e/seed.ts b/private-web/e2e/seed.ts index ec54729a..00fc5b6d 100644 --- a/private-web/e2e/seed.ts +++ b/private-web/e2e/seed.ts @@ -69,29 +69,35 @@ export async function seedServerGroup( tags?: Record; /** Slack open cooldown in seconds. Omit to keep the migration default. */ slackOpenDelaySeconds?: number; + /** Incident linger window in seconds. Omit to keep the migration default. */ + slackCloseDelaySeconds?: number; } = {}, ): Promise { const id = randomUUID(); const name = opts.name ?? randomLabel("group"); + const columns = ["id", "name", "notes", "tags"]; + const values: unknown[] = [ + id, + name, + opts.notes ?? "", + JSON.stringify(opts.tags ?? {}), + ]; + const exprs = ["$1", "$2", "$3", "$4::jsonb"]; if (opts.slackOpenDelaySeconds !== undefined) { - await sql.query( - `INSERT INTO server_groups (id, name, notes, tags, slack_open_delay) - VALUES ($1, $2, $3, $4::jsonb, make_interval(secs => $5))`, - [ - id, - name, - opts.notes ?? "", - JSON.stringify(opts.tags ?? {}), - opts.slackOpenDelaySeconds, - ], - ); - } else { - await sql.query( - `INSERT INTO server_groups (id, name, notes, tags) - VALUES ($1, $2, $3, $4::jsonb)`, - [id, name, opts.notes ?? "", JSON.stringify(opts.tags ?? {})], - ); + columns.push("slack_open_delay"); + values.push(opts.slackOpenDelaySeconds); + exprs.push(`make_interval(secs => $${values.length})`); } + if (opts.slackCloseDelaySeconds !== undefined) { + columns.push("slack_close_delay"); + values.push(opts.slackCloseDelaySeconds); + exprs.push(`make_interval(secs => $${values.length})`); + } + await sql.query( + `INSERT INTO server_groups (${columns.join(", ")}) + VALUES (${exprs.join(", ")})`, + values, + ); return { id, name }; } @@ -458,6 +464,46 @@ export async function seedIssue( return { id }; } +export interface SeededIncident { + id: string; +} + +/** Seed an open incident directly, optionally lingering (`closingAt` set: + * its last effective failure recovered then and it closes if things stay + * quiet) and optionally linking issues into its timeline. */ +export async function seedIncident( + sql: Sql, + opts: { + /** Group the incident targets; null/absent seeds a canopy-wide one. */ + serverGroupId?: string | null; + /** ISO 8601; defaults to NOW(). */ + openedAt?: string; + /** ISO 8601; sets the incident lingering since this time. */ + closingAt?: string | null; + /** Issues to link, each with optional join/leave times. */ + issues?: Array<{ + issueId: string; + joinedAt?: string; + leftAt?: string | null; + }>; + } = {}, +): Promise { + const id = randomUUID(); + await sql.query( + `INSERT INTO incidents (id, server_group_id, opened_at, closing_at) + VALUES ($1, $2, COALESCE($3::timestamptz, NOW()), $4::timestamptz)`, + [id, opts.serverGroupId ?? null, opts.openedAt ?? null, opts.closingAt ?? null], + ); + for (const link of opts.issues ?? []) { + await sql.query( + `INSERT INTO incident_issues (incident_id, issue_id, joined_at, left_at) + VALUES ($1, $2, COALESCE($3::timestamptz, NOW()), $4::timestamptz)`, + [id, link.issueId, link.joinedAt ?? null, link.leftAt ?? null], + ); + } + return { id }; +} + export interface SeededVersion { id: string; major: number; diff --git a/private-web/openapi.json b/private-web/openapi.json index 12558ef1..6670dc03 100644 --- a/private-web/openapi.json +++ b/private-web/openapi.json @@ -8151,6 +8151,14 @@ "format": "int64", "description": "Number of distinct issues that have ever contributed to the incident." }, + "lingering_since": { + "type": [ + "string", + "null" + ], + "format": "date-time", + "description": "When set, the incident is lingering: its last effective failure\nrecovered at this time, and the incident stays open for the group's\nlinger window in case the failure comes back (which would continue\nthis incident rather than open a new one). Null while a failure is\nlive, and always null on a closed incident. Lets a client\ndistinguish \"open and failing\" from \"open but currently recovered\"." + }, "note_count": { "type": "integer", "format": "int64", @@ -9613,6 +9621,14 @@ ], "description": "New free-form operator notes for the group." }, + "slack_close_delay": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "description": "New incident linger window, in seconds." + }, "slack_open_delay": { "type": [ "integer", @@ -11210,7 +11226,8 @@ "created_at", "updated_at", "name", - "slack_open_delay" + "slack_open_delay", + "slack_close_delay" ], "properties": { "created_at": { @@ -11250,6 +11267,11 @@ "type": "string", "description": "Free-form operator notes about this group." }, + "slack_close_delay": { + "type": "integer", + "format": "int64", + "description": "How long, in seconds, an incident lingers after its last failure\nrecovers before it closes and the resolved notification is sent. A\nfailure returning within this window continues the same incident\ninstead of opening (and notifying about) a new one — the close-side\nmirror of `slack_open_delay`, for a red check that briefly blips\ngreen." + }, "slack_open_delay": { "type": "integer", "format": "int64", @@ -11378,6 +11400,14 @@ "type": "string", "description": "Free-form notes about the group. Defaults to empty." }, + "slack_close_delay": { + "type": [ + "integer", + "null" + ], + "format": "int64", + "description": "Optional linger window, in whole seconds: how long an incident stays\nopen after its last failure recovers, so a failure returning within\nthe window continues the same incident instead of opening (and\nnotifying about) a new one. Omit to accept the default." + }, "slack_open_delay": { "type": [ "integer", diff --git a/private-web/src/api-types.ts b/private-web/src/api-types.ts index a4197321..cc671674 100644 --- a/private-web/src/api-types.ts +++ b/private-web/src/api-types.ts @@ -4476,6 +4476,16 @@ export interface components { * @description Number of distinct issues that have ever contributed to the incident. */ issue_count: number; + /** + * Format: date-time + * @description When set, the incident is lingering: its last effective failure + * recovered at this time, and the incident stays open for the group's + * linger window in case the failure comes back (which would continue + * this incident rather than open a new one). Null while a failure is + * live, and always null on a closed incident. Lets a client + * distinguish "open and failing" from "open but currently recovered". + */ + lingering_since?: string | null; /** * Format: int64 * @description Combined count of notes on the incident itself plus notes on all its @@ -5378,6 +5388,11 @@ export interface components { name?: string | null; /** @description New free-form operator notes for the group. */ notes?: string | null; + /** + * Format: int64 + * @description New incident linger window, in seconds. + */ + slack_close_delay?: number | null; /** * Format: int64 * @description New incident-opened notification delay, in seconds. @@ -6437,6 +6452,16 @@ export interface components { name: string; /** @description Free-form operator notes about this group. */ notes?: string; + /** + * Format: int64 + * @description How long, in seconds, an incident lingers after its last failure + * recovers before it closes and the resolved notification is sent. A + * failure returning within this window continues the same incident + * instead of opening (and notifying about) a new one — the close-side + * mirror of `slack_open_delay`, for a red check that briefly blips + * green. + */ + slack_close_delay: number; /** * Format: int64 * @description How long, in seconds, an incident-opened notification waits before @@ -6519,6 +6544,14 @@ export interface components { name: string; /** @description Free-form notes about the group. Defaults to empty. */ notes?: string; + /** + * Format: int64 + * @description Optional linger window, in whole seconds: how long an incident stays + * open after its last failure recovers, so a failure returning within + * the window continues the same incident instead of opening (and + * notifying about) a new one. Omit to accept the default. + */ + slack_close_delay?: number | null; /** * Format: int64 * @description Optional initial delay, in whole seconds, before an "incident opened" diff --git a/private-web/src/components/IncidentCard.tsx b/private-web/src/components/IncidentCard.tsx index 1f703b44..2cadf902 100644 --- a/private-web/src/components/IncidentCard.tsx +++ b/private-web/src/components/IncidentCard.tsx @@ -4,16 +4,19 @@ import NotesIcon from "@mui/icons-material/StickyNote2"; import { Link as RouterLink } from "react-router-dom"; import TimeAgo from "./TimeAgo"; import { useIsNotificationHeld } from "../hooks/useIsNotificationHeld"; -import type { IncidentData } from "../types"; +import { type IncidentData, isIncidentLingering } from "../types"; /** Compact view of an open incident; click-through goes to the incident * detail page. The body has a stats row (bottom-right) with issue / event * / note counts. A warning-coloured border (rather than the default error) * signals that the Slack notice is still inside the per-group cooldown * window — operators scanning the /incidents grid can tell at a glance - * which cards have already paged out. */ + * which cards have already paged out — and an info-coloured border that + * every failure has recovered and the incident is lingering, closing if + * things stay quiet. */ export default function IncidentCard({ incident }: { incident: IncidentData }) { const held = useIsNotificationHeld(incident.notification_held_until); + const lingering = isIncidentLingering(incident); return ( opened - {held && incident.notification_held_until && ( + {lingering && incident.lingering_since && ( + + Recovering; last failure cleared{" "} + + + )} + {!lingering && held && incident.notification_held_until && ( Holding; posting{" "} diff --git a/private-web/src/components/IncidentsLink.tsx b/private-web/src/components/IncidentsLink.tsx index 9d571d34..d5bd1bc0 100644 --- a/private-web/src/components/IncidentsLink.tsx +++ b/private-web/src/components/IncidentsLink.tsx @@ -4,13 +4,15 @@ import WarningAmberIcon from "@mui/icons-material/WarningAmber"; import { Link as RouterLink } from "react-router-dom"; import { useApi } from "../api"; import { useIsNotificationHeld } from "../hooks/useIsNotificationHeld"; +import { isIncidentLingering } from "../types"; /** Server-detail header button into the incidents view. Any server id in * a group works — the backend resolves to the group root. Three states: - * - open incident exists → direct link to /incidents/:id (error-coloured, - * or warning-coloured when the Slack notice is still inside the - * per-group cooldown window so the operator can tell at a glance that - * nobody else has been paged yet) + * - open incident exists → direct link to /incidents/:id (error-coloured; + * warning-coloured when the Slack notice is still inside the per-group + * cooldown window so the operator can tell at a glance that nobody else + * has been paged yet; info-coloured when the incident is lingering — + * every failure has recovered and it closes if things stay quiet) * - no open incident, but active issues → /incidents filtered to this group * - nothing active → same, with `showAll=1` so closed/inactive surface */ export default function IncidentsLink({ @@ -39,25 +41,29 @@ export default function IncidentsLink({ ); const hasActive = issues.status === "ok" && issues.data.length > 0; - const held = useIsNotificationHeld( + const heldUntilActive = useIsNotificationHeld( openIncident?.notification_held_until ?? null, ); if (openIncident) { + const lingering = isIncidentLingering(openIncident); + const held = heldUntilActive && !lingering; return ( ); } diff --git a/private-web/src/routes/GroupDetail.tsx b/private-web/src/routes/GroupDetail.tsx index 2e8819ab..2a69c89e 100644 --- a/private-web/src/routes/GroupDetail.tsx +++ b/private-web/src/routes/GroupDetail.tsx @@ -28,6 +28,7 @@ import { type BackupConfigStatus, aggregateOperators, groupServersByRank, + isIncidentLingering, type AggregatedOperator, type IncidentData, } from "../types"; @@ -360,12 +361,14 @@ function OperatorsSection({ function ActiveIncidentCard({ incident }: { incident: IncidentData }) { const held = useIsNotificationHeld(incident.notification_held_until); + const lingering = isIncidentLingering(incident); + const tone = lingering ? "info" : held ? "warning" : "error"; return ( @@ -375,7 +378,7 @@ function ActiveIncidentCard({ incident }: { incident: IncidentData }) { sx={{ alignItems: "center", justifyContent: "space-between" }} > - + Active incident @@ -395,7 +398,13 @@ function ActiveIncidentCard({ incident }: { incident: IncidentData }) { opened - {held && incident.notification_held_until && ( + {lingering && incident.lingering_since && ( + + Recovering; last failure cleared{" "} + + + )} + {!lingering && held && incident.notification_held_until && ( Holding; posting{" "} @@ -407,7 +416,7 @@ function ActiveIncidentCard({ incident }: { incident: IncidentData }) { component={RouterLink} to={`/incidents/${incident.id}`} variant="outlined" - color={held ? "warning" : "error"} + color={tone} > Open diff --git a/private-web/src/routes/GroupEdit.tsx b/private-web/src/routes/GroupEdit.tsx index 5b2a3359..d36a312a 100644 --- a/private-web/src/routes/GroupEdit.tsx +++ b/private-web/src/routes/GroupEdit.tsx @@ -138,6 +138,12 @@ function EditForm({ const [slackOpenDelayMinutes, setSlackOpenDelayMinutes] = useState( Math.max(0, Math.round(group.slack_open_delay / 60)).toString(), ); + // Linger window: how long an incident stays open after its last failure + // recovers, so a failure coming straight back continues the same + // incident instead of opening (and re-notifying) a new one. + const [slackCloseDelayMinutes, setSlackCloseDelayMinutes] = useState( + Math.max(0, Math.round(group.slack_close_delay / 60)).toString(), + ); const onSubmit = async (e: React.FormEvent) => { e.preventDefault(); @@ -152,6 +158,10 @@ function EditForm({ 0, Math.round(Number(slackOpenDelayMinutes) * 60), ), + slack_close_delay: Math.max( + 0, + Math.round(Number(slackCloseDelayMinutes) * 60), + ), }, }); navigate(`/groups/${group.id}`); @@ -233,6 +243,30 @@ function EditForm({ sent for either edge — useful for flappy probes. Set to 0 to ship opens immediately. + + + Keep incidents open after recovery for + + setSlackCloseDelayMinutes(e.target.value)} + disabled={pending} + slotProps={{ htmlInput: { min: 0, step: 1 } }} + sx={{ width: 140 }} + /> + + + A failure coming back inside this window continues the same + incident instead of opening — and notifying about — a new one. + The resolved notice waits it out. Set to 0 to close incidents + the moment their last failure recovers. + diff --git a/private-web/src/routes/IncidentDetail.tsx b/private-web/src/routes/IncidentDetail.tsx index 7ab5b8e4..32a0b433 100644 --- a/private-web/src/routes/IncidentDetail.tsx +++ b/private-web/src/routes/IncidentDetail.tsx @@ -32,6 +32,7 @@ import { humanDuration } from "../lib/humanDuration"; import { RESOLVED_REASONS, RESOLVED_REASON_LABEL, + isIncidentLingering, type IncidentIssueData, type IncidentNoteData, type IncidentWithIssues, @@ -132,7 +133,10 @@ function Header({ const heldUntilActive = useIsNotificationHeld( incident.notification_held_until, ); - const held = open && heldUntilActive; + const lingering = isIncidentLingering(incident); + // A lingering incident's open notice is also gated by the drainer, so + // the held countdown would be wrong — the recovery state wins. + const held = open && heldUntilActive && !lingering; const isAdmin = useIsAdmin() === true; const resolve = useApiAction("incidents", "resolve"); const unresolve = useApiAction("incidents", "unresolve"); @@ -254,6 +258,32 @@ function Header({ )} + {lingering && incident.lingering_since && ( + + } + label={ + <> + Recovering; last failure cleared{" "} + + + } + color="info" + variant="outlined" + size="small" + /> + + A failure returning within the linger window continues + this incident; otherwise it closes as of when the last + failure cleared. + + + )} {incident.resolved_at && ( diff --git a/private-web/src/routes/Status.tsx b/private-web/src/routes/Status.tsx index e5e16bde..da10a1db 100644 --- a/private-web/src/routes/Status.tsx +++ b/private-web/src/routes/Status.tsx @@ -30,13 +30,17 @@ import { SERVER_RANK_ORDER, aggregateOperators, compareServersByRankThenKind, + isIncidentLingering, } from "../types"; -/// Held vs loud: a group has an open incident, but the Slack notice is -/// still inside the per-group cooldown window (held) or has already fired -/// or been cancelled (loud). Drives the warning-vs-error colouring on the -/// Status page and elsewhere. -export type IncidentLoudness = "held" | "loud"; +/// How an open incident should read at a glance: +/// - "loud": failing, and the Slack notice has fired (or been given up on); +/// - "held": failing, but the notice is still inside the per-group cooldown +/// window — nobody has been paged yet; +/// - "lingering": every failure has recovered, and the incident is waiting +/// out the group's linger window in case one comes back. +/// Drives the error/warning/info colouring on the Status page and elsewhere. +export type IncidentLoudness = "held" | "loud" | "lingering"; export default function Status() { usePageTitle("Status"); @@ -56,10 +60,14 @@ export default function Status() { .filter((i) => i.server_group_id != null) .map((i): [string, IncidentLoudness] => [ i.server_group_id as string, - i.notification_held_until && - Date.parse(i.notification_held_until) > now - ? "held" - : "loud", + // Lingering wins: the group is currently green, so + // painting it red/yellow would overstate the trouble. + isIncidentLingering(i) + ? "lingering" + : i.notification_held_until && + Date.parse(i.notification_held_until) > now + ? "held" + : "loud", ]) : [], ); @@ -193,13 +201,17 @@ function GroupCardLoader({ // Held incidents tone the border down to warning so an operator can see // at a glance "yes there's a thing, but Slack hasn't been told yet — it - // might still self-resolve". Loud incidents stay full red. + // might still self-resolve". Lingering incidents tone down further to + // info: everything has recovered and the incident is just waiting out + // its linger window. Loud incidents stay full red. const borderColor = openIncident === "loud" ? "error.main" : openIncident === "held" ? "warning.main" - : undefined; + : openIncident === "lingering" + ? "info.main" + : undefined; // Active operator presence anywhere in the group tints the card, so // "someone is already on it" reads at a glance — especially useful @@ -304,6 +316,15 @@ function GroupCard({ )} + {openIncident === "lingering" && ( + + + + )} diff --git a/private-web/src/types.ts b/private-web/src/types.ts index 63d3aecd..d8fab265 100644 --- a/private-web/src/types.ts +++ b/private-web/src/types.ts @@ -125,6 +125,16 @@ export type HealthcheckSampleResponse = Solidify; export type IssueIncidentLink = Solidify; export type IncidentData = Solidify; + +/// An open incident whose last effective failure has recovered: it stays +/// open for the group's linger window in case the failure comes back, and +/// closes (backdated) if things stay quiet. Distinct from "held", which is +/// about the Slack open notice still sitting inside the notification delay. +export function isIncidentLingering( + incident: Pick, +): boolean { + return incident.closed_at == null && incident.lingering_since != null; +} export type IncidentIssueData = Solidify; export type IncidentWithIssues = Solidify; export type IssueNoteData = Solidify;