From d192589790a328abb5aef01ac4eaae7ee60b977b Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Mon, 24 Aug 2026 14:12:38 +0000 Subject: [PATCH] fix: maintenance repaint no longer zeroes no_data uptime; strengthen redaction test --- server/internal/services/statussnapshot.go | 36 ++++++++++--- .../internal/services/statussnapshot_test.go | 53 +++++++++++++++++-- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/server/internal/services/statussnapshot.go b/server/internal/services/statussnapshot.go index ee5cde1..15269e4 100644 --- a/server/internal/services/statussnapshot.go +++ b/server/internal/services/statussnapshot.go @@ -132,11 +132,15 @@ func assembleSnapshot(in snapshotInput) StatusSnapshot { name := publicName(entry, mon.MonitorID) names[entry.MonitorID] = name + // Uptime is computed from the days as reported by rollups, before + // any maintenance repaint — a no_data day must never be counted as + // zero uptime just because it is later redrawn as "maintenance". + days := buildDays(in.Rollups[entry.MonitorID], in.Now) comp := PublicComponent{ Name: name, - Days: buildDays(in.Rollups[entry.MonitorID], underMaintenance[entry.MonitorID], in.Now), } - comp.Uptime90d = uptimeFromDays(comp.Days) + comp.Uptime90d = uptimeFromDays(days) + comp.Days = applyMaintenanceRepaint(days, underMaintenance[entry.MonitorID]) comp.Status = componentStatus(mon, known, underMaintenance[entry.MonitorID], in.Now) out.Components = append(out.Components, comp) } @@ -272,7 +276,14 @@ func publicFromAuthored(inc models.StatusIncident, names map[string]string) Publ } // buildDays produces exactly HistoryDays cells, oldest first, ending today. -func buildDays(rollups []models.Rollup, inMaintenance bool, now time.Time) []PublicDay { +// It carries no maintenance state: a maintenance repaint is a display concern +// applied afterwards by applyMaintenanceRepaint, once uptimeFromDays has +// already read the true no_data/up/down state of each day. Folding the +// repaint in here would let a today cell with no rollups yet flip from +// no_data to maintenance before its uptime contribution was decided, and +// uptimeFromDays skips no_data days by their State — so that day would stop +// being skipped and start counting as a zero. +func buildDays(rollups []models.Rollup, now time.Time) []PublicDay { type bucket struct{ checks, up int } byDay := map[string]*bucket{} for _, r := range rollups { @@ -300,15 +311,26 @@ func buildDays(rollups []models.Rollup, inMaintenance bool, now time.Time) []Pub day.State = PublicDown } } - // Maintenance repaints today's cell only, and never touches Uptime. - if inMaintenance && i == 0 { - day.State = PublicMaintenance - } days = append(days, day) } return days } +// applyMaintenanceRepaint redraws today's cell as "maintenance" for display, +// after uptimeFromDays has already computed the component's Uptime90d from +// the unpainted days. It never touches Uptime, and it must run after that +// computation, not before: repainting first would turn a today cell with no +// rollups yet from no_data (skipped) into maintenance (a 0% day counted in +// the average), and repainting a day that DOES have rollups must still leave +// that day's real up/down contribution in the average — maintenance changes +// how a day is drawn, never what the numbers say. +func applyMaintenanceRepaint(days []PublicDay, inMaintenance bool) []PublicDay { + if inMaintenance && len(days) > 0 { + days[len(days)-1].State = PublicMaintenance + } + return days +} + // uptimeFromDays ignores no_data days rather than counting them as zero. A // component created last week is not 92% available. func uptimeFromDays(days []PublicDay) float64 { diff --git a/server/internal/services/statussnapshot_test.go b/server/internal/services/statussnapshot_test.go index 033c075..40c1b5e 100644 --- a/server/internal/services/statussnapshot_test.go +++ b/server/internal/services/statussnapshot_test.go @@ -77,6 +77,7 @@ func TestAssembleSnapshotRedactsMonitorInternals(t *testing.T) { "chan-123", "SECRETKEYWORD", "prod-api-internal", + "db-primary", "5432", } for _, leak := range leaks { @@ -88,9 +89,10 @@ func TestAssembleSnapshotRedactsMonitorInternals(t *testing.T) { func TestAssembleSnapshotUsesDisplayNameThenMonitorName(t *testing.T) { in := testInput(time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC)) - // mon-2 has no override, and its monitor name is in the leak list above, - // so an un-overridden entry must fall back to something safe. It falls - // back to the monitor id, never the internal name. + // mon-2 has no DisplayName override, and its monitor name ("db-primary") + // is in TestAssembleSnapshotRedactsMonitorInternals's leak list, so an + // un-overridden entry must fall back to something safe. It falls back to + // the monitor id, never the internal name. snap := assembleSnapshot(in) comps := snap.Sections[0].Components if comps[0].Name != "Public API" { @@ -161,6 +163,51 @@ func TestAssembleSnapshotMaintenanceDoesNotChangeUptime(t *testing.T) { } } +// TestAssembleSnapshotMaintenanceRepaintDoesNotCountNoDataAsZero guards +// against the maintenance repaint corrupting Uptime90d for a component whose +// today rollup has not landed yet — an in-progress maintenance window on a +// young component, or one that simply started before today's hourly rollup +// was written. Repainting today's no_data cell to "maintenance" must never +// make uptimeFromDays stop skipping it: doing so would turn a component with +// one good day of history from 100% into 50%. +func TestAssembleSnapshotMaintenanceRepaintDoesNotCountNoDataAsZero(t *testing.T) { + now := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC) + in := testInput(now) + in.Rollups = map[string][]models.Rollup{ + // Only yesterday has a rollup, fully up. Today has none. + "mon-2": {{MonitorID: "mon-2", PeriodStart: now.Add(-24 * time.Hour), Checks: 60, UpCount: 60}}, + } + start := now.Add(-time.Hour) + end := now.Add(time.Hour) + in.Authored = []models.StatusIncident{{ + IncidentID: "mnt-2", + PageIDs: []string{"api"}, + Kind: models.StatusKindMaintenance, + Title: "Database upgrade", + Status: models.MaintenanceInProgress, + AffectedMonitors: []string{"mon-2"}, + ScheduledStart: &start, + ScheduledEnd: &end, + StartedAt: start, + }} + + snap := assembleSnapshot(in) + comp := snap.Sections[0].Components[1] + if comp.Status != "maintenance" { + t.Errorf("status = %q, want maintenance", comp.Status) + } + // Today's cell is redrawn as maintenance for display... + if comp.Days[89].State != "maintenance" { + t.Errorf("today state = %q, want maintenance", comp.Days[89].State) + } + // ...but it carries no rollup, so it must not have been averaged in as a + // zero. The only day with data was 100% up, so the 90-day figure is 100, + // not (100+0)/2 = 50. + if comp.Uptime90d != 100 { + t.Errorf("uptime = %v, want 100 (today's no_data must not count as zero)", comp.Uptime90d) + } +} + func TestAssembleSnapshotOnlyIncludesAuthoredIncidentsForThisPage(t *testing.T) { now := time.Date(2026, 8, 24, 12, 0, 0, 0, time.UTC) in := testInput(now)