From 1b3b8e531245ea8b48cfd9b6ded167dae3492404 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Sat, 19 Sep 2026 14:43:16 +0100 Subject: [PATCH] Clear the status window when a degraded spell starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The window was carried across a spell, so the results either side of it ended up adjacent: a monitor at [t, t, t, f, f] when egress dropped resumed at [t, t, f, f, f] on its first real failure afterwards, crossing the threshold on results taken before the outage. Cleared in the producer beside the maintenance gate, which already clears the window on entering a window, and guarded on its length so it is one write per spell rather than one per degraded check. The monitor still goes down once the rebuilt window holds enough real failures. The two integration tests that asserted the old behaviour are rewritten: one expected the window preserved through a spell, the other expected failures either side of a spell to combine. 🔧 --- server/src/worker/worker.check-producer.ts | 11 ++++- .../heartbeatJobDegradedEgress.test.ts | 19 ++++++--- .../test/unit/services/checkProducer.test.ts | 40 +++++++++++++++++++ 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/server/src/worker/worker.check-producer.ts b/server/src/worker/worker.check-producer.ts index d7b93ae3d..1d25bdd92 100644 --- a/server/src/worker/worker.check-producer.ts +++ b/server/src/worker/worker.check-producer.ts @@ -133,10 +133,19 @@ export class CheckProducer implements ICheckProducer { if (egressStatus !== null) { check.egressStatus = egressStatus; } + // Step 2c: Add to buffer this.bufferService.addToBuffer(check); - // Step 2d: Handle docker logs + // Step 2d: As with the maintenance gate above, the window is cleared at the point the monitor stops + // being evaluated, so the results either side of a spell never end up adjacent and cross the threshold + // on stale data. The length guard keeps this to one write per spell rather than one per degraded check, + // and it runs after the check is buffered so that a failed write cannot cost us the check. + if (egressStatus === "degraded" && monitor.statusWindow?.length) { + await this.monitorsRepository.updateById(monitor.id, monitor.teamId, { statusWindow: [] }); + } + + // Step 2e: Handle docker logs if (status.type === "docker") { const dockerLogs = await this.dockerLogsService.buildDockerLogs(status as MonitorStatusResponse); for (const dockerLog of dockerLogs) { diff --git a/server/test/integration/heartbeatJobDegradedEgress.test.ts b/server/test/integration/heartbeatJobDegradedEgress.test.ts index df8378790..f4eb11cdd 100644 --- a/server/test/integration/heartbeatJobDegradedEgress.test.ts +++ b/server/test/integration/heartbeatJobDegradedEgress.test.ts @@ -33,7 +33,8 @@ describe("Heartbeat job: degraded egress", () => { // Nothing downstream moved const storedMonitor = await h.monitorsRepo.findById("mon-1", "team-1"); expect(storedMonitor.status).toBe("up"); - expect(storedMonitor.statusWindow).toEqual([true, true, true, true, true]); + // The window is cleared once as the spell starts, as entering a maintenance window does. + expect(storedMonitor.statusWindow).toEqual([]); expect(h.incidentsRepo.getAll()).toHaveLength(0); expect(h.notificationsService.handleNotifications).not.toHaveBeenCalled(); }); @@ -158,7 +159,7 @@ describe("Heartbeat job: degraded egress", () => { expect(h.notificationsService.handleNotifications).toHaveBeenCalledTimes(1); }); - it("keeps counting real failures taken either side of a degraded spell", async () => { + it("starts a fresh status window after a degraded spell", async () => { const monitor = makeMonitor(); h.monitorsRepo.seed(monitor); @@ -167,16 +168,22 @@ describe("Heartbeat job: degraded egress", () => { await h.heartbeatJob(monitor); await h.heartbeatJob(monitor); - // A degraded spell must not push the window over the threshold h.setEgressStatus("degraded"); await h.heartbeatJob(monitor); await h.heartbeatJob(monitor); + expect((await h.monitorsRepo.findById("mon-1", "team-1")).statusWindow).toEqual([]); + + // A third real failure would have taken the old window to 60%. Against a window that starts again + // at the end of the spell it is one result, and the results either side are never adjacent. + h.setEgressStatus(null); + await h.heartbeatJob(monitor); expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("up"); expect(h.incidentsRepo.getAll()).toHaveLength(0); - // 3rd real failure: window [t, t, f, f, f] = 60% >= 60% - h.setEgressStatus(null); - await h.heartbeatJob(monitor); + // The monitor still goes down, once the rebuilt window holds enough real failures to cross. + for (let i = 0; i < 4; i++) { + await h.heartbeatJob(monitor); + } expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); expect(h.incidentsRepo.getAll()).toHaveLength(1); diff --git a/server/test/unit/services/checkProducer.test.ts b/server/test/unit/services/checkProducer.test.ts index c83a890cc..85763bfa9 100644 --- a/server/test/unit/services/checkProducer.test.ts +++ b/server/test/unit/services/checkProducer.test.ts @@ -373,6 +373,46 @@ describe("CheckProducer", () => { expect(result?.check.egressStatus).toBe("degraded"); }); + // ── status window across a spell ───────────────────────────────────────── + // Matching the maintenance gate: cleared once as the monitor stops being evaluated, so the results either + // side of a spell are never adjacent. + + it("clears the status window on the first degraded check of a spell", async () => { + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue({ id: "check-1" }) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ statusWindow: [true, true, true, false, false] })); + + expect(defaults.monitorsRepository.updateById).toHaveBeenCalledWith("m1", "team", { statusWindow: [] }); + }); + + it("does not re-write the status window for later degraded checks in the same spell", async () => { + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue({ id: "check-1" }) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ statusWindow: [] })); + + expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); + }); + + it("leaves the status window alone when the probe found egress fine", async () => { + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue({ id: "check-1" }) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("ok") }, + }); + + await producer.produce(makeMonitor({ statusWindow: [true, true, true, false, false] })); + + expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); + }); + it("flags a failing check as ok when the probe found egress fine", async () => { const check: Record = { id: "check-1" }; const { producer } = createProducer({