From f52acb3f68e8738be1ab8cf3e79ce2f198db4001 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Mon, 14 Sep 2026 10:35:42 +0100 Subject: [PATCH 1/9] Flag checks taken during degraded egress and exclude them from uptime (#3937) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CheckProducer asks EgressService to assess the instance's own connectivity after a failing check and stamps the result on the check as egressStatus. CheckEvaluator short-circuits degraded checks so no incident opens and no spurious recovery fires later. Uptime and down-count aggregations exclude degraded checks; the response-time series keeps them. 🔧 --- server/src/config/services.worker.ts | 3 +- .../domain/checks/check.docker.aggregation.ts | 3 +- .../checks/check.hardware.aggregations.ts | 3 +- server/src/domain/checks/check.model.ts | 12 +++- .../domain/checks/check.repository.mongo.ts | 7 ++- server/src/domain/checks/check.type.ts | 4 ++ server/src/worker/worker.check-evaluator.ts | 37 +++++++++++- server/src/worker/worker.check-producer.ts | 9 +++ server/test/helpers/heartbeatTestHarness.ts | 4 +- .../test/unit/services/checkEvaluator.test.ts | 38 +++++++++++- .../test/unit/services/checkProducer.test.ts | 60 +++++++++++++++++++ 11 files changed, 172 insertions(+), 8 deletions(-) diff --git a/server/src/config/services.worker.ts b/server/src/config/services.worker.ts index 802ad4b0c..88b5a18e2 100644 --- a/server/src/config/services.worker.ts +++ b/server/src/config/services.worker.ts @@ -129,9 +129,10 @@ export const buildWorker = async (shared: SharedServices, envSettings: EnvConfig proxyResolver, bufferService, dockerLogsService, + egressService, logger ); - const checkEvaluator = new CheckEvaluator(statusService, monitorStatusPolicy); + const checkEvaluator = new CheckEvaluator(statusService, monitorStatusPolicy, logger); const geoCheckPipeline = new GeoChecksPipeline(maintenanceWindowsRepository, geoChecksService, bufferService, logger); // *********************** diff --git a/server/src/domain/checks/check.docker.aggregation.ts b/server/src/domain/checks/check.docker.aggregation.ts index 37d6d8e87..d2db06462 100644 --- a/server/src/domain/checks/check.docker.aggregation.ts +++ b/server/src/domain/checks/check.docker.aggregation.ts @@ -1,4 +1,4 @@ -import CheckModel from "@/domain/checks/check.model.js"; +import CheckModel, { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.model.js"; import { DockerContainerStatsBucket, DockerStatsBucket } from "@/domain/checks/check.type.js"; import mongoose from "mongoose"; @@ -9,6 +9,7 @@ export const getDockerTotalChecks = async (monitorId: string, dates: DateRange): "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }); export const getDockerUpChecks = async (monitorId: string, dates: DateRange): Promise<{ totalChecks: number }> => { diff --git a/server/src/domain/checks/check.hardware.aggregations.ts b/server/src/domain/checks/check.hardware.aggregations.ts index 6c672b630..495ee5278 100644 --- a/server/src/domain/checks/check.hardware.aggregations.ts +++ b/server/src/domain/checks/check.hardware.aggregations.ts @@ -1,4 +1,4 @@ -import { CheckModel } from "@/domain/checks/check.model.js"; +import { CheckModel, EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.model.js"; import mongoose from "mongoose"; import type { HardwareDiskStats, HardwareNetStats } from "@/domain/checks/check.type.js"; @@ -70,6 +70,7 @@ export const getHardwareTotalChecks = async (monitorId: string, dates: DateRange "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "hardware", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }); }; diff --git a/server/src/domain/checks/check.model.ts b/server/src/domain/checks/check.model.ts index a4319056e..094ced6b0 100644 --- a/server/src/domain/checks/check.model.ts +++ b/server/src/domain/checks/check.model.ts @@ -1,5 +1,6 @@ import { Schema, model, Types } from "mongoose"; import { MonitorTypes, type MonitorType } from "@/domain/monitors/monitor.type.js"; +import { EgressStatuses, type EgressStatus } from "@/domain/egress/egress.type.js"; import type { Check, CheckAudits, @@ -337,6 +338,11 @@ const CheckSchema = new Schema( type: containerSummarySchema, default: undefined, }, + egressStatus: { + type: String, + enum: EgressStatuses, + default: undefined, + }, }, { timestamps: true, @@ -359,6 +365,10 @@ CheckSchema.index({ "metadata.teamId": 1, status: 1, createdAt: -1 }); const CheckModel = model("Check", CheckSchema); +// Checks recorded while the instance's own egress was down cannot be attributed to the target, so they +// are left out of every uptime percentage and down-count. Spread this into the $match / filter of any such query. +const EXCLUDE_DEGRADED_EGRESS_MATCH = { egressStatus: { $ne: "degraded" satisfies EgressStatus } } as const; + export type { CheckDocument, CheckMetadataDocument }; -export { CheckModel, containerSummarySchema }; +export { CheckModel, containerSummarySchema, EXCLUDE_DEGRADED_EGRESS_MATCH }; export default CheckModel; diff --git a/server/src/domain/checks/check.repository.mongo.ts b/server/src/domain/checks/check.repository.mongo.ts index 82e89d0d9..e9ad42ad2 100644 --- a/server/src/domain/checks/check.repository.mongo.ts +++ b/server/src/domain/checks/check.repository.mongo.ts @@ -15,7 +15,7 @@ import type { HardwareCheckStats, } from "@/domain/checks/check.type.js"; import type { MonitorType } from "@/domain/monitors/monitor.type.js"; -import { CheckModel, type CheckDocument } from "@/domain/checks/check.model.js"; +import { CheckModel, EXCLUDE_DEGRADED_EGRESS_MATCH, type CheckDocument } from "@/domain/checks/check.model.js"; import mongoose from "mongoose"; import { getDateFormat, getDateForRange } from "@/utils/dataUtils.js"; import { ILogger } from "@/utils/logger.js"; @@ -179,6 +179,7 @@ class MongoChecksRepository implements IChecksRepository { audits: mapAudits(doc.audits), containers: doc.containers, containerSummary: doc.containerSummary, + ...(doc.egressStatus !== undefined && { egressStatus: doc.egressStatus }), createdAt: toDateString(doc.createdAt), updatedAt: toDateString(doc.updatedAt), }; @@ -348,6 +349,7 @@ class MongoChecksRepository implements IChecksRepository { const baseMatch = { "metadata.teamId": new mongoose.Types.ObjectId(teamId), createdAt: { $gte: getDateForRange(dateRange) }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }; const [totalResult, downResult] = await Promise.all([ @@ -393,6 +395,7 @@ class MongoChecksRepository implements IChecksRepository { $match: { "metadata.monitorId": { $in: objectIds }, createdAt: { $gte: windowStart }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }, }, { @@ -480,6 +483,8 @@ class MongoChecksRepository implements IChecksRepository { { $facet: { uptimePercentage: [ + // Response-time series (groupedUpChecks / groupedDownChecks) deliberately keep degraded checks so they still show as failures on the graph. + { $match: EXCLUDE_DEGRADED_EGRESS_MATCH }, { $group: { _id: null, diff --git a/server/src/domain/checks/check.type.ts b/server/src/domain/checks/check.type.ts index afea301be..ab304e007 100644 --- a/server/src/domain/checks/check.type.ts +++ b/server/src/domain/checks/check.type.ts @@ -1,6 +1,7 @@ import type { MonitorType } from "@/domain/monitors/monitor.type.js"; import { DockerContainerInfo, DockerContainerSummary } from "@/domain/docker/docker.type.js"; import type { Response } from "got"; +import type { EgressStatus } from "@/domain/egress/egress.type.js"; export const CHECK_TTL_SENTINEL = 366; @@ -116,6 +117,9 @@ export interface Check { seo?: number; performance?: number; audits?: CheckAudits; + // Set only on checks produced while the reliability probe ran (a failing check). + // "degraded" means every reliability target was unreachable, so the failure is not attributable to the monitored target. + egressStatus?: EgressStatus; createdAt: string; updatedAt: string; } diff --git a/server/src/worker/worker.check-evaluator.ts b/server/src/worker/worker.check-evaluator.ts index da7e13dbe..4c4c4a7ac 100644 --- a/server/src/worker/worker.check-evaluator.ts +++ b/server/src/worker/worker.check-evaluator.ts @@ -4,17 +4,52 @@ import { Check } from "@/domain/checks/check.type.js"; import { Monitor } from "@/domain/monitors/monitor.type.js"; import { IMonitorStatusPolicy } from "@/worker/worker.monitor-status-policy.js"; import { IStatusService } from "@/service/statusService.js"; +import { MonitorActionDecision } from "@/worker/worker.helper.js"; +import { ILogger } from "@/utils/logger.js"; export interface ICheckEvaluator { evaluate(status: MonitorStatusResponse, check: Check, monitor: Monitor): Promise; } +const SERVICE_NAME = "CheckEvaluator"; + export class CheckEvaluator implements ICheckEvaluator { constructor( private statusService: IStatusService, - private monitorStatusPolicy: IMonitorStatusPolicy + private monitorStatusPolicy: IMonitorStatusPolicy, + private logger: ILogger ) {} + + // A check taken while the instance had no outbound connectivity says nothing about the target. + // It is left out of the status window, running stats and monitor status entirely, so that no incident + // opens for it and no spurious "resolved"/"up" fires once egress returns. + private skipDegradedEgressCheck = (status: MonitorStatusResponse, check: Check, monitor: Monitor): MonitorEvaluation => { + this.logger.debug({ + message: `Skipping evaluation of check ${check.id} for monitor ${monitor.id}: instance egress was degraded`, + service: SERVICE_NAME, + method: "evaluate", + }); + const decision: MonitorActionDecision = { + shouldCreateIncident: false, + shouldResolveIncident: false, + shouldSendNotification: false, + incidentReason: null, + notificationReason: null, + }; + return { + monitor, + status, + check, + statusChange: { monitor, statusChanged: false, prevStatus: monitor.status, code: status.code, timestamp: Date.now() }, + decision, + }; + }; + evaluate = async (status: MonitorStatusResponse, check: Check, monitor: Monitor) => { + if (check.egressStatus === "degraded") { + return this.skipDegradedEgressCheck(status, check, monitor); + } + // **************************** // Step 3: Evaluate and return result to reactors // **************************** diff --git a/server/src/worker/worker.check-producer.ts b/server/src/worker/worker.check-producer.ts index 603eec8b6..194718a20 100644 --- a/server/src/worker/worker.check-producer.ts +++ b/server/src/worker/worker.check-producer.ts @@ -11,6 +11,7 @@ import { IMaintenanceWindowsRepository } from "@/domain/maintenance-windows/main import { isWindowActive } from "@/utils/maintenanceWindow.js"; import { IProxyResolver } from "@/service/network/ProxyResolver.js"; import { IDockerLogsService } from "@/domain/docker/docker-log.service.js"; +import { IEgressService } from "@/domain/egress/egress.service.js"; export interface ICheckProducer { produce(monitor: Monitor): Promise<{ status: MonitorStatusResponse; check: Check } | null>; @@ -27,6 +28,7 @@ export class CheckProducer implements ICheckProducer { private proxyResolver: IProxyResolver, private bufferService: IBufferService, private dockerLogsService: IDockerLogsService, + private egressService: IEgressService, private logger: ILogger ) {} @@ -74,6 +76,10 @@ export class CheckProducer implements ICheckProducer { throw new Error("No network response"); } + // Step 1c: On failure, ask whether the instance itself can reach anything before blaming the target. + // Null means the egress check is disabled (or failed internally) and the check is treated as usual. + const egressStatus = status.status === false ? await this.egressService.assessAfterFailure() : null; + // **************************** // Step 2: Record // **************************** @@ -89,6 +95,9 @@ export class CheckProducer implements ICheckProducer { }); return null; } + if (egressStatus !== null) { + check.egressStatus = egressStatus; + } // Step 2b: Add to buffer this.bufferService.addToBuffer(check); diff --git a/server/test/helpers/heartbeatTestHarness.ts b/server/test/helpers/heartbeatTestHarness.ts index 0661d6445..9652b021e 100644 --- a/server/test/helpers/heartbeatTestHarness.ts +++ b/server/test/helpers/heartbeatTestHarness.ts @@ -129,6 +129,7 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness { const maintenanceWindowsRepo = { findByMonitorId: jest.fn().mockResolvedValue([]) }; const proxyResolver = { resolve: jest.fn().mockResolvedValue(undefined) }; const dockerLogsService = { buildDockerLogs: jest.fn().mockResolvedValue([]) }; + const egressService = { assessAfterFailure: jest.fn().mockResolvedValue(null) }; const checkProducer = new CheckProducer( monitorsRepo as any, @@ -138,9 +139,10 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness { proxyResolver as any, bufferStub as any, dockerLogsService as any, + egressService as any, logger ); - const checkEvaluator = new CheckEvaluator(statusService as any, new MonitorStatusPolicy()); + const checkEvaluator = new CheckEvaluator(statusService as any, new MonitorStatusPolicy(), logger); const notificationReactor = new NotificationReactor(notificationsService as any); const incidentReactor = new IncidentReactor(incidentService as any); diff --git a/server/test/unit/services/checkEvaluator.test.ts b/server/test/unit/services/checkEvaluator.test.ts index 22919495a..23d64a0b0 100644 --- a/server/test/unit/services/checkEvaluator.test.ts +++ b/server/test/unit/services/checkEvaluator.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, jest } from "@jest/globals"; import { CheckEvaluator } from "../../../src/worker/worker.check-evaluator.ts"; import { MonitorStatusPolicy } from "../../../src/worker/worker.monitor-status-policy.ts"; import type { Monitor } from "../../../src/domain/monitors/monitor.type.ts"; +import { createMockLogger } from "../../helpers/createMockLogger.ts"; const makeMonitor = (overrides?: Partial): Monitor => ({ @@ -19,9 +20,10 @@ const createEvaluator = (overrides?: Record) => { updateMonitorStatus: jest.fn().mockResolvedValue({ monitor: makeMonitor({ status: "up" }), statusChanged: false, prevStatus: "up", code: 200 }), }, monitorStatusPolicy: new MonitorStatusPolicy(), + logger: createMockLogger(), ...overrides, }; - const evaluator = new CheckEvaluator(defaults.statusService as any, defaults.monitorStatusPolicy as any); + const evaluator = new CheckEvaluator(defaults.statusService as any, defaults.monitorStatusPolicy as any, defaults.logger as any); return { evaluator, defaults }; }; @@ -84,4 +86,38 @@ describe("CheckEvaluator", () => { shouldSendNotification: false, }); }); + + // ── degraded egress ─────────────────────────────────────────────────────── + + it("short-circuits a check flagged with degraded egress without touching monitor status", async () => { + const status = { monitorId: "m1", status: false, code: 500, message: "Error" } as any; + const check = { id: "check-1", egressStatus: "degraded" } as any; + const monitor = makeMonitor({ status: "up" }); + const { evaluator, defaults } = createEvaluator(); + + const result = await evaluator.evaluate(status, check, monitor); + + expect(defaults.statusService.updateMonitorStatus).not.toHaveBeenCalled(); + expect(result.monitor).toBe(monitor); + expect(result.statusChange).toMatchObject({ monitor, statusChanged: false, prevStatus: "up", code: 500 }); + expect(result.statusChange.timestamp).toEqual(expect.any(Number)); + expect(result.decision).toEqual({ + shouldCreateIncident: false, + shouldResolveIncident: false, + shouldSendNotification: false, + incidentReason: null, + notificationReason: null, + }); + expect(defaults.logger.debug).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress was degraded") })); + }); + + it("evaluates a check flagged with egress ok as normal", async () => { + const status = { monitorId: "m1", status: false, code: 500, message: "Error" } as any; + const check = { id: "check-1", egressStatus: "ok" } as any; + const { evaluator, defaults } = createEvaluator(); + + await evaluator.evaluate(status, check, makeMonitor()); + + expect(defaults.statusService.updateMonitorStatus).toHaveBeenCalledWith(status, check, expect.anything()); + }); }); diff --git a/server/test/unit/services/checkProducer.test.ts b/server/test/unit/services/checkProducer.test.ts index c10cc2f29..478262c20 100644 --- a/server/test/unit/services/checkProducer.test.ts +++ b/server/test/unit/services/checkProducer.test.ts @@ -29,6 +29,7 @@ const createProducer = (overrides?: Record) => { proxyResolver: { resolve: jest.fn().mockResolvedValue(undefined) }, buffer: { addToBuffer: jest.fn() }, dockerLogsService: { buildDockerLogs: jest.fn().mockResolvedValue([]) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue(null) }, ...overrides, }; const producer = new CheckProducer( @@ -39,6 +40,7 @@ const createProducer = (overrides?: Record) => { defaults.proxyResolver as any, defaults.buffer as any, defaults.dockerLogsService as any, + defaults.egressService as any, defaults.logger as any ); return { producer, defaults }; @@ -210,4 +212,62 @@ describe("CheckProducer", () => { expect(defaults.dockerLogsService.buildDockerLogs).not.toHaveBeenCalled(); }); + + // ── egress self-check ───────────────────────────────────────────────────── + + const failingStatus = { monitorId: "m1", status: false, code: 500, message: "Error" }; + + it("does not consult the egress service for a successful check and leaves the field unset", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + }); + + await producer.produce(makeMonitor()); + + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + }); + + it("flags a failing check as degraded when the egress service reports degraded egress", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + const result = await producer.produce(makeMonitor()); + + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check.egressStatus).toBe("degraded"); + expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(expect.objectContaining({ egressStatus: "degraded" })); + expect(result?.check.egressStatus).toBe("degraded"); + }); + + it("flags a failing check as ok when the probe found egress fine", async () => { + const check: Record = { id: "check-1" }; + const { producer } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("ok") }, + }); + + await producer.produce(makeMonitor()); + + expect(check.egressStatus).toBe("ok"); + }); + + it("leaves the field unset on a failing check when the egress check is disabled", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + }); + + await producer.produce(makeMonitor()); + + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check).not.toHaveProperty("egressStatus"); + }); }); From 4f5dba560d383fdc1088043380aad82b65084cbd Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Tue, 15 Sep 2026 19:47:35 +0100 Subject: [PATCH 2/9] Add integration test for degraded egress uptime exclusions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Runs the summary, daily bucket, date-range, hardware and docker aggregations against a live mongod with a mix of degraded and ordinary checks, and covers the egressStatus round trip through createChecks and findUnevaluatedByMonitorId. 🔧 --- .../checksRepositoryEgressExclusion.test.ts | 166 ++++++++++++++++++ 1 file changed, 166 insertions(+) create mode 100644 server/test/integration/checksRepositoryEgressExclusion.test.ts diff --git a/server/test/integration/checksRepositoryEgressExclusion.test.ts b/server/test/integration/checksRepositoryEgressExclusion.test.ts new file mode 100644 index 000000000..b2f488b66 --- /dev/null +++ b/server/test/integration/checksRepositoryEgressExclusion.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it, beforeAll, afterAll, beforeEach } from "@jest/globals"; +import mongoose from "mongoose"; +import { MongoMemoryServer } from "mongodb-memory-server"; +import MongoChecksRepository from "../../src/domain/checks/check.repository.mongo.ts"; +import { CheckModel } from "../../src/domain/checks/check.model.ts"; +import type { Check, DockerChecksResult, HardwareChecksResult, UptimeChecksResult } from "../../src/domain/checks/check.type.ts"; +import type { ILogger } from "../../src/utils/logger.ts"; +import { createMockLogger } from "../helpers/createMockLogger.ts"; + +// ── Real-Mongo harness ───────────────────────────────────────────────────────── +// The guarantee under test — a check flagged egressStatus "degraded" is left out of +// every uptime percentage and down-count while still appearing in listings and in +// the response-time series — lives in $match stages spread into several aggregation +// pipelines, so it can only be exercised against a live mongod. + +let mongod: MongoMemoryServer; + +beforeAll(async () => { + mongod = await MongoMemoryServer.create(); + await mongoose.connect(mongod.getUri()); + await CheckModel.createCollection(); // timeseries collections must exist before insert +}, 120_000); + +afterAll(async () => { + await mongoose.disconnect(); + await mongod.stop(); +}); + +beforeEach(async () => { + await CheckModel.deleteMany({}); +}); + +// ── Helpers ────────────────────────────────────────────────────────────────── + +const MONITOR_ID = new mongoose.Types.ObjectId(); +const TEAM_ID = new mongoose.Types.ObjectId(); + +// One shared createdAt keeps every seeded check in the same bucket of the "day" range. +const BUCKET_TIME = new Date(Date.now() - 10 * 60 * 1000); + +const seedCheck = (overrides: Record = {}) => + CheckModel.create({ + metadata: { monitorId: MONITOR_ID, teamId: TEAM_ID, type: "http" }, + status: true, + responseTime: 100, + createdAt: BUCKET_TIME, + ...overrides, + }); + +const seedDegradedFailure = (overrides: Record = {}) => + seedCheck({ status: false, responseTime: 300, egressStatus: "degraded", ...overrides }); + +const utcDate = (date: Date) => date.toISOString().slice(0, 10); + +const makeCheck = (overrides: Partial = {}): Check => + ({ + metadata: { monitorId: MONITOR_ID.toString(), teamId: TEAM_ID.toString(), type: "http" }, + status: false, + responseTime: 300, + statusCode: 500, + message: "Error", + createdAt: BUCKET_TIME.toISOString(), + updatedAt: BUCKET_TIME.toISOString(), + ...overrides, + }) as Check; + +describe("MongoChecksRepository degraded-egress exclusion", () => { + let repo: MongoChecksRepository; + + beforeEach(() => { + repo = new MongoChecksRepository(createMockLogger() as unknown as ILogger); + }); + + it("round-trips egressStatus through createChecks and the unevaluated-checks read, leaving it absent when unset", async () => { + await repo.createChecks([makeCheck({ egressStatus: "degraded" }), makeCheck({ egressStatus: "ok" }), makeCheck()]); + + const checks = await repo.findUnevaluatedByMonitorId(MONITOR_ID.toString(), BUCKET_TIME.getTime() - 1); + + // The evaluator runs from this read, so the flag must survive persistence for the short-circuit to fire. + expect(checks.map((check) => check.egressStatus)).toEqual(["degraded", "ok", undefined]); + expect(checks[2]).not.toHaveProperty("egressStatus"); + }); + + it("rejects a value outside the EgressStatuses tuple", async () => { + await expect(seedCheck({ status: false, egressStatus: "unknown" })).rejects.toThrow(/egressStatus/); + }); + + it("excludes degraded checks from the uptime percentage but keeps them in the response-time series", async () => { + await seedCheck(); // up + await seedCheck({ status: false }); // real failure + await seedDegradedFailure(); + await seedDegradedFailure(); + + const result = (await repo.findByDateRangeAndMonitorId(MONITOR_ID.toString(), "day", { type: "http" })) as UptimeChecksResult; + + // 1 up of 2 attributable checks. Counting the degraded pair would give 25%. + expect(result.uptimePercentage).toBe(0.5); + // The graph still shows the degraded checks as failures. + expect(result.groupedChecks[0]).toMatchObject({ totalChecks: 4 }); + expect(result.groupedDownChecks[0]).toMatchObject({ totalChecks: 3 }); + expect(result.groupedUpChecks[0]).toMatchObject({ totalChecks: 1 }); + }); + + it("does not flag a failure that carries egressStatus ok", async () => { + await seedCheck(); + await seedCheck({ status: false, egressStatus: "ok" }); + + const result = (await repo.findByDateRangeAndMonitorId(MONITOR_ID.toString(), "day", { type: "http" })) as UptimeChecksResult; + + expect(result.uptimePercentage).toBe(0.5); + }); + + it("excludes degraded checks from the team summary totals", async () => { + await seedCheck(); + await seedCheck({ status: false }); + await seedDegradedFailure(); + + const summary = await repo.findSummaryByTeamId(TEAM_ID.toString(), "day"); + + expect(summary).toEqual({ totalChecks: 2, downChecks: 1 }); + }); + + it("excludes degraded checks from the daily status buckets and yields no row for a day of only degraded checks", async () => { + const otherMonitor = new mongoose.Types.ObjectId(); + await seedCheck(); + await seedCheck({ status: false }); + await seedDegradedFailure(); + await seedDegradedFailure({ metadata: { monitorId: otherMonitor, teamId: TEAM_ID, type: "http" } }); + + const buckets = await repo.getDailyStatusBuckets([MONITOR_ID.toString(), otherMonitor.toString()], 7, "UTC"); + + expect(buckets).toEqual([ + { monitorId: MONITOR_ID.toString(), date: utcDate(BUCKET_TIME), totalChecks: 2, upChecks: 1, downChecks: 1, avgResponseTime: 100 }, + ]); + }); + + it("excludes degraded checks from the hardware and docker totals", async () => { + const hardwareMonitor = new mongoose.Types.ObjectId(); + const dockerMonitor = new mongoose.Types.ObjectId(); + const hardwareMeta = { monitorId: hardwareMonitor, teamId: TEAM_ID, type: "hardware" }; + const dockerMeta = { monitorId: dockerMonitor, teamId: TEAM_ID, type: "docker" }; + + await seedCheck({ metadata: hardwareMeta }); + await seedDegradedFailure({ metadata: hardwareMeta }); + await seedCheck({ metadata: dockerMeta }); + await seedDegradedFailure({ metadata: dockerMeta }); + + const hardware = (await repo.findByDateRangeAndMonitorId(hardwareMonitor.toString(), "day", { type: "hardware" })) as HardwareChecksResult; + const docker = (await repo.findByDateRangeAndMonitorId(dockerMonitor.toString(), "day", { type: "docker" })) as DockerChecksResult; + + expect(hardware.aggregateData.totalChecks).toBe(1); + expect(hardware.upChecks.totalChecks).toBe(1); + expect(docker.aggregateData.totalChecks).toBe(1); + expect(docker.upChecks.totalChecks).toBe(1); + }); + + it("keeps degraded checks in the paginated listing so they can be shown as such", async () => { + await seedCheck({ status: false }); + await seedDegradedFailure(); + + const { checksCount, checks } = await repo.findByMonitorId(MONITOR_ID.toString(), "desc", "day", 0, 10, undefined, "down"); + + expect(checksCount).toBe(2); + expect(checks.map((check) => check.egressStatus).sort()).toEqual([undefined, "degraded"].sort()); + }); +}); From 4335e6b7b661573bc6c388e96791d447b84c3bbc Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Tue, 15 Sep 2026 20:01:33 +0100 Subject: [PATCH 3/9] Address review of degraded-egress check persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CheckProducer consults egress only for a transport failure: the check failed, no HTTP status came back (isHttpStatusCode from PR 1), and the monitor type is one whose failure can be a loss of egress (EgressAttributableTypes; hardware and docker are excluded). A 4xx/5xx or a content mismatch is evaluated as usual while the instance is degraded. A rejected assessment is logged and treated as unknown so the check is still buffered. EXCLUDE_DEGRADED_EGRESS_MATCH moves out of the model into check.query.ts, with an expression form for $group accumulators. Daily status buckets and docker stats buckets exclude degraded checks from their up/total counts while keeping them in the response-time average, matching the uptime facet. toEntity maps egressStatus like its siblings. Adds heartbeatJobDegradedEgress integration tests, extends the repository exclusion tests, and makes the harness egress stub settable. 🔧 --- .../domain/checks/check.docker.aggregation.ts | 8 +- .../checks/check.hardware.aggregations.ts | 3 +- server/src/domain/checks/check.model.ts | 8 +- server/src/domain/checks/check.query.ts | 8 + .../domain/checks/check.repository.mongo.ts | 13 +- server/src/domain/checks/check.type.ts | 7 +- server/src/domain/egress/egress.service.ts | 2 +- server/src/domain/monitors/monitor.type.ts | 15 ++ server/src/worker/worker.check-producer.ts | 28 ++- server/test/helpers/heartbeatTestHarness.ts | 12 +- .../checksRepositoryEgressExclusion.test.ts | 26 ++- .../heartbeatJobDegradedEgress.test.ts | 160 ++++++++++++++++++ .../test/unit/services/checkProducer.test.ts | 73 +++++++- 13 files changed, 334 insertions(+), 29 deletions(-) create mode 100644 server/src/domain/checks/check.query.ts create mode 100644 server/test/integration/heartbeatJobDegradedEgress.test.ts diff --git a/server/src/domain/checks/check.docker.aggregation.ts b/server/src/domain/checks/check.docker.aggregation.ts index d2db06462..de6870e04 100644 --- a/server/src/domain/checks/check.docker.aggregation.ts +++ b/server/src/domain/checks/check.docker.aggregation.ts @@ -1,5 +1,6 @@ -import CheckModel, { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.model.js"; +import CheckModel from "@/domain/checks/check.model.js"; import { DockerContainerStatsBucket, DockerStatsBucket } from "@/domain/checks/check.type.js"; +import { EXCLUDE_DEGRADED_EGRESS_MATCH, IS_NOT_DEGRADED_EGRESS_EXPR } from "@/domain/checks/check.query.js"; import mongoose from "mongoose"; type DateRange = { start: Date; end: Date }; @@ -35,8 +36,9 @@ export const getDockerStats = async (monitorId: string, dates: DateRange, dateSt $group: { _id: { $dateToString: { format: dateString, date: "$createdAt" } }, avgResponseTime: { $avg: "$responseTime" }, - upCount: { $sum: { $cond: [{ $eq: ["$status", true] }, 1, 0] } }, - totalCount: { $sum: 1 }, + // Counts exclude degraded-egress checks like aggregateData.totalChecks does; the response-time average keeps them. + upCount: { $sum: { $cond: [{ $and: [{ $eq: ["$status", true] }, IS_NOT_DEGRADED_EGRESS_EXPR] }, 1, 0] } }, + totalCount: { $sum: { $cond: [IS_NOT_DEGRADED_EGRESS_EXPR, 1, 0] } }, avgRunning: { $avg: "$containerSummary.running" }, avgTotal: { $avg: "$containerSummary.total" }, avgUnhealthy: { $avg: "$containerSummary.unhealthy" }, diff --git a/server/src/domain/checks/check.hardware.aggregations.ts b/server/src/domain/checks/check.hardware.aggregations.ts index 495ee5278..9a67a8d0a 100644 --- a/server/src/domain/checks/check.hardware.aggregations.ts +++ b/server/src/domain/checks/check.hardware.aggregations.ts @@ -1,6 +1,7 @@ -import { CheckModel, EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.model.js"; +import { CheckModel } from "@/domain/checks/check.model.js"; import mongoose from "mongoose"; import type { HardwareDiskStats, HardwareNetStats } from "@/domain/checks/check.type.js"; +import { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.query.js"; type DateRange = { start: Date; end: Date }; type HardwareUpChecks = { totalChecks: number }; diff --git a/server/src/domain/checks/check.model.ts b/server/src/domain/checks/check.model.ts index 094ced6b0..1f92fce3c 100644 --- a/server/src/domain/checks/check.model.ts +++ b/server/src/domain/checks/check.model.ts @@ -1,6 +1,6 @@ import { Schema, model, Types } from "mongoose"; import { MonitorTypes, type MonitorType } from "@/domain/monitors/monitor.type.js"; -import { EgressStatuses, type EgressStatus } from "@/domain/egress/egress.type.js"; +import { EgressStatuses } from "@/domain/egress/egress.type.js"; import type { Check, CheckAudits, @@ -365,10 +365,6 @@ CheckSchema.index({ "metadata.teamId": 1, status: 1, createdAt: -1 }); const CheckModel = model("Check", CheckSchema); -// Checks recorded while the instance's own egress was down cannot be attributed to the target, so they -// are left out of every uptime percentage and down-count. Spread this into the $match / filter of any such query. -const EXCLUDE_DEGRADED_EGRESS_MATCH = { egressStatus: { $ne: "degraded" satisfies EgressStatus } } as const; - export type { CheckDocument, CheckMetadataDocument }; -export { CheckModel, containerSummarySchema, EXCLUDE_DEGRADED_EGRESS_MATCH }; +export { CheckModel, containerSummarySchema }; export default CheckModel; diff --git a/server/src/domain/checks/check.query.ts b/server/src/domain/checks/check.query.ts new file mode 100644 index 000000000..2ebaf59d3 --- /dev/null +++ b/server/src/domain/checks/check.query.ts @@ -0,0 +1,8 @@ +import type { EgressStatus } from "@/domain/egress/egress.type.js"; + +// Checks recorded while the instance's own egress was down cannot be attributed to the target, so they +// are left out of every uptime percentage and up/down count. Response-time series keep them. +// Spread this into the $match / filter of a query, or use the expression form inside a $group accumulator. +export const EXCLUDE_DEGRADED_EGRESS_MATCH = { egressStatus: { $ne: "degraded" satisfies EgressStatus } } as const; + +export const IS_NOT_DEGRADED_EGRESS_EXPR = { $ne: ["$egressStatus", "degraded" satisfies EgressStatus] } as const; diff --git a/server/src/domain/checks/check.repository.mongo.ts b/server/src/domain/checks/check.repository.mongo.ts index e9ad42ad2..1a9707aa5 100644 --- a/server/src/domain/checks/check.repository.mongo.ts +++ b/server/src/domain/checks/check.repository.mongo.ts @@ -15,7 +15,8 @@ import type { HardwareCheckStats, } from "@/domain/checks/check.type.js"; import type { MonitorType } from "@/domain/monitors/monitor.type.js"; -import { CheckModel, EXCLUDE_DEGRADED_EGRESS_MATCH, type CheckDocument } from "@/domain/checks/check.model.js"; +import { CheckModel, type CheckDocument } from "@/domain/checks/check.model.js"; +import { EXCLUDE_DEGRADED_EGRESS_MATCH, IS_NOT_DEGRADED_EGRESS_EXPR } from "@/domain/checks/check.query.js"; import mongoose from "mongoose"; import { getDateFormat, getDateForRange } from "@/utils/dataUtils.js"; import { ILogger } from "@/utils/logger.js"; @@ -179,7 +180,7 @@ class MongoChecksRepository implements IChecksRepository { audits: mapAudits(doc.audits), containers: doc.containers, containerSummary: doc.containerSummary, - ...(doc.egressStatus !== undefined && { egressStatus: doc.egressStatus }), + egressStatus: doc.egressStatus, createdAt: toDateString(doc.createdAt), updatedAt: toDateString(doc.updatedAt), }; @@ -395,7 +396,6 @@ class MongoChecksRepository implements IChecksRepository { $match: { "metadata.monitorId": { $in: objectIds }, createdAt: { $gte: windowStart }, - ...EXCLUDE_DEGRADED_EGRESS_MATCH, }, }, { @@ -404,11 +404,14 @@ class MongoChecksRepository implements IChecksRepository { monitorId: "$metadata.monitorId", day: { $dateTrunc: { date: "$createdAt", unit: "day", timezone } }, }, - totalChecks: { $sum: 1 }, - upChecks: { $sum: { $cond: [{ $eq: ["$status", true] }, 1, 0] } }, + // Counts exclude degraded-egress checks; the response-time average keeps them. + totalChecks: { $sum: { $cond: [IS_NOT_DEGRADED_EGRESS_EXPR, 1, 0] } }, + upChecks: { $sum: { $cond: [{ $and: [{ $eq: ["$status", true] }, IS_NOT_DEGRADED_EGRESS_EXPR] }, 1, 0] } }, avgResponseTime: { $avg: "$responseTime" }, }, }, + // A day with nothing attributable to the target renders as an empty bar; the client divides by totalChecks for the tooltip. + { $match: { totalChecks: { $gt: 0 } } }, { $sort: { "_id.day": 1 } }, { $project: { diff --git a/server/src/domain/checks/check.type.ts b/server/src/domain/checks/check.type.ts index ab304e007..f63cf5c83 100644 --- a/server/src/domain/checks/check.type.ts +++ b/server/src/domain/checks/check.type.ts @@ -1,7 +1,7 @@ import type { MonitorType } from "@/domain/monitors/monitor.type.js"; +import type { EgressStatus } from "@/domain/egress/egress.type.js"; import { DockerContainerInfo, DockerContainerSummary } from "@/domain/docker/docker.type.js"; import type { Response } from "got"; -import type { EgressStatus } from "@/domain/egress/egress.type.js"; export const CHECK_TTL_SENTINEL = 366; @@ -117,8 +117,9 @@ export interface Check { seo?: number; performance?: number; audits?: CheckAudits; - // Set only on checks produced while the reliability probe ran (a failing check). - // "degraded" means every reliability target was unreachable, so the failure is not attributable to the monitored target. + // Set on failing checks while the egress check is enabled; absent otherwise. + // "degraded": no reliability target was reachable (or the instance was already degraded), so the failure is not attributable to the target. + // "ok": a reliability target was reachable. egressStatus?: EgressStatus; createdAt: string; updatedAt: string; diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts index f336bab12..1088356ff 100644 --- a/server/src/domain/egress/egress.service.ts +++ b/server/src/domain/egress/egress.service.ts @@ -26,7 +26,7 @@ const RECOVERY_JOB_TYPE = "egress" as const; const RECOVERY_JOB_ID = jobId(RECOVERY_JOB_TYPE, null); // HttpProvider reports transport failures with NETWORK_ERROR (outside the HTTP range) and everything else with the real status code. -const isHttpStatusCode = (code: unknown): boolean => typeof code === "number" && code >= 100 && code <= 599; +export const isHttpStatusCode = (code: unknown): boolean => typeof code === "number" && code >= 100 && code <= 599; // Synthetic identity stamped on the probe monitors so provider responses are recognisable in logs. const PROBE_MONITOR_ID = "egress-probe"; diff --git a/server/src/domain/monitors/monitor.type.ts b/server/src/domain/monitors/monitor.type.ts index df8c52d9e..f2029bbb1 100644 --- a/server/src/domain/monitors/monitor.type.ts +++ b/server/src/domain/monitors/monitor.type.ts @@ -50,6 +50,21 @@ export const UptimeDetailsSupportedTypes = ["http", "ping", "port", "game", "grp export type UptimeDetailsSupportedType = (typeof UptimeDetailsSupportedTypes)[number]; export const supportsUptimeDetails = (type: MonitorType): type is UptimeDetailsSupportedType => UptimeDetailsSupportedTypes.some((t) => t === type); +// Types whose transport failure can be the instance's own loss of egress. Hardware (Capture agent) and docker +// (local socket or host) failures are local, so they are never attributed to egress. +export const EgressAttributableTypes = [ + "http", + "ping", + "pagespeed", + "port", + "game", + "grpc", + "websocket", + "dns", +] as const satisfies readonly MonitorType[]; +export type EgressAttributableType = (typeof EgressAttributableTypes)[number]; +export const isEgressAttributable = (type: MonitorType): type is EgressAttributableType => EgressAttributableTypes.some((t) => t === type); + export const MonitorStatuses = ["up", "down", "paused", "initializing", "maintenance", "breached"] as const; export type MonitorStatus = (typeof MonitorStatuses)[number]; diff --git a/server/src/worker/worker.check-producer.ts b/server/src/worker/worker.check-producer.ts index 194718a20..3f09e998d 100644 --- a/server/src/worker/worker.check-producer.ts +++ b/server/src/worker/worker.check-producer.ts @@ -11,7 +11,9 @@ import { IMaintenanceWindowsRepository } from "@/domain/maintenance-windows/main import { isWindowActive } from "@/utils/maintenanceWindow.js"; import { IProxyResolver } from "@/service/network/ProxyResolver.js"; import { IDockerLogsService } from "@/domain/docker/docker-log.service.js"; -import { IEgressService } from "@/domain/egress/egress.service.js"; +import { IEgressService, isHttpStatusCode } from "@/domain/egress/egress.service.js"; +import type { EgressStatus } from "@/domain/egress/egress.type.js"; +import { isEgressAttributable } from "@/domain/monitors/monitor.type.js"; export interface ICheckProducer { produce(monitor: Monitor): Promise<{ status: MonitorStatusResponse; check: Check } | null>; @@ -44,6 +46,26 @@ export class CheckProducer implements ICheckProducer { return dockerTlsKey ?? undefined; } + // Only a failure to reach the target at all can be the instance's own fault. Any HTTP response (4xx, 5xx, + // or a 200 with a content mismatch) proves the target was reached, and hardware/docker failures are local. + private isTransportFailure(monitor: Monitor, status: MonitorStatusResponse): boolean { + return status.status === false && isEgressAttributable(monitor.type) && !isHttpStatusCode(status.code); + } + + // The egress check must never stop a check being recorded, so a rejection is logged and treated as unknown. + private async assessEgress(monitorId: string): Promise { + try { + return await this.egressService.assessAfterFailure(); + } catch (error: unknown) { + this.logger.warn({ + message: `Egress assessment failed for monitor ${monitorId}: ${error instanceof Error ? error.message : String(error)}`, + service: SERVICE_NAME, + method: "assessEgress", + }); + return null; + } + } + produce = async (monitor: Monitor) => { if (!monitor.id) { throw new AppError({ message: "No monitor id", service: SERVICE_NAME, method: "produce" }); @@ -76,9 +98,9 @@ export class CheckProducer implements ICheckProducer { throw new Error("No network response"); } - // Step 1c: On failure, ask whether the instance itself can reach anything before blaming the target. + // Step 1c: On a transport failure, ask whether the instance itself can reach anything before blaming the target. // Null means the egress check is disabled (or failed internally) and the check is treated as usual. - const egressStatus = status.status === false ? await this.egressService.assessAfterFailure() : null; + const egressStatus = this.isTransportFailure(monitor, status) ? await this.assessEgress(monitor.id) : null; // **************************** // Step 2: Record diff --git a/server/test/helpers/heartbeatTestHarness.ts b/server/test/helpers/heartbeatTestHarness.ts index 9652b021e..47370df0b 100644 --- a/server/test/helpers/heartbeatTestHarness.ts +++ b/server/test/helpers/heartbeatTestHarness.ts @@ -14,6 +14,7 @@ import type { Monitor } from "../../src/domain/monitors/monitor.type.ts"; import type { MonitorStatusResponse } from "../../src/types/network.ts"; import type { Check } from "../../src/domain/checks/check.type.ts"; import type { MaintenanceWindow } from "../../src/domain/maintenance-windows/maintenance-window.type.ts"; +import type { EgressStatus } from "../../src/domain/egress/egress.type.ts"; let checkCounter = 0; @@ -79,9 +80,11 @@ export interface HeartbeatTestHarness { bufferStub: { addToBuffer: jest.Mock; addGeoCheckToBuffer: jest.Mock; scheduleNextFlush: jest.Mock }; maintenanceWindowsRepo: { findByMonitorId: jest.Mock }; messageBuilder: { extractThresholdBreaches: jest.Mock }; + egressService: { assessAfterFailure: jest.Mock }; heartbeatJob: (monitor: Monitor) => Promise; setNextResponse: (status: boolean, code: number) => void; setNextResponseFull: (response: MonitorStatusResponse) => void; + setEgressStatus: (status: EgressStatus | null) => void; } export function createHeartbeatTestHarness(): HeartbeatTestHarness { @@ -129,7 +132,12 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness { const maintenanceWindowsRepo = { findByMonitorId: jest.fn().mockResolvedValue([]) }; const proxyResolver = { resolve: jest.fn().mockResolvedValue(undefined) }; const dockerLogsService = { buildDockerLogs: jest.fn().mockResolvedValue([]) }; - const egressService = { assessAfterFailure: jest.fn().mockResolvedValue(null) }; + // Null mirrors the egress check being disabled, so the existing heartbeat suites run unchanged. + let nextEgressStatus: EgressStatus | null = null; + const egressService = { assessAfterFailure: jest.fn().mockImplementation(() => Promise.resolve(nextEgressStatus)) }; + const setEgressStatus = (status: EgressStatus | null) => { + nextEgressStatus = status; + }; const checkProducer = new CheckProducer( monitorsRepo as any, @@ -169,8 +177,10 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness { bufferStub, maintenanceWindowsRepo, messageBuilder, + egressService, heartbeatJob, setNextResponse, setNextResponseFull, + setEgressStatus, }; } diff --git a/server/test/integration/checksRepositoryEgressExclusion.test.ts b/server/test/integration/checksRepositoryEgressExclusion.test.ts index b2f488b66..eea421019 100644 --- a/server/test/integration/checksRepositoryEgressExclusion.test.ts +++ b/server/test/integration/checksRepositoryEgressExclusion.test.ts @@ -78,7 +78,7 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { // The evaluator runs from this read, so the flag must survive persistence for the short-circuit to fire. expect(checks.map((check) => check.egressStatus)).toEqual(["degraded", "ok", undefined]); - expect(checks[2]).not.toHaveProperty("egressStatus"); + expect(checks[2].egressStatus).toBeUndefined(); }); it("rejects a value outside the EgressStatuses tuple", async () => { @@ -95,8 +95,9 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { // 1 up of 2 attributable checks. Counting the degraded pair would give 25%. expect(result.uptimePercentage).toBe(0.5); - // The graph still shows the degraded checks as failures. - expect(result.groupedChecks[0]).toMatchObject({ totalChecks: 4 }); + // The graph still shows the degraded checks as failures, and their response times stay in the averages. + expect(result.avgResponseTime).toBe(200); + expect(result.groupedChecks[0]).toMatchObject({ totalChecks: 4, avgResponseTime: 200 }); expect(result.groupedDownChecks[0]).toMatchObject({ totalChecks: 3 }); expect(result.groupedUpChecks[0]).toMatchObject({ totalChecks: 1 }); }); @@ -120,7 +121,7 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { expect(summary).toEqual({ totalChecks: 2, downChecks: 1 }); }); - it("excludes degraded checks from the daily status buckets and yields no row for a day of only degraded checks", async () => { + it("excludes degraded checks from the daily status bucket counts but keeps them in its response-time average", async () => { const otherMonitor = new mongoose.Types.ObjectId(); await seedCheck(); await seedCheck({ status: false }); @@ -129,8 +130,10 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { const buckets = await repo.getDailyStatusBuckets([MONITOR_ID.toString(), otherMonitor.toString()], 7, "UTC"); + // (100 + 100 + 300) / 3 = 167: the degraded check's response time counts, its failure does not. + // The other monitor saw only degraded checks that day, so it gets no row rather than a 0/0 bucket. expect(buckets).toEqual([ - { monitorId: MONITOR_ID.toString(), date: utcDate(BUCKET_TIME), totalChecks: 2, upChecks: 1, downChecks: 1, avgResponseTime: 100 }, + { monitorId: MONITOR_ID.toString(), date: utcDate(BUCKET_TIME), totalChecks: 2, upChecks: 1, downChecks: 1, avgResponseTime: 167 }, ]); }); @@ -154,6 +157,19 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { expect(docker.upChecks.totalChecks).toBe(1); }); + it("excludes degraded checks from the docker bucket counts but keeps them in its response-time average", async () => { + const dockerMonitor = new mongoose.Types.ObjectId(); + const dockerMeta = { monitorId: dockerMonitor, teamId: TEAM_ID, type: "docker" }; + + await seedCheck({ metadata: dockerMeta }); + await seedDegradedFailure({ metadata: dockerMeta }); + + const docker = (await repo.findByDateRangeAndMonitorId(dockerMonitor.toString(), "day", { type: "docker" })) as DockerChecksResult; + + expect(docker.aggregate).toHaveLength(1); + expect(docker.aggregate[0]).toMatchObject({ upCount: 1, totalCount: 1, avgResponseTime: 200 }); + }); + it("keeps degraded checks in the paginated listing so they can be shown as such", async () => { await seedCheck({ status: false }); await seedDegradedFailure(); diff --git a/server/test/integration/heartbeatJobDegradedEgress.test.ts b/server/test/integration/heartbeatJobDegradedEgress.test.ts new file mode 100644 index 000000000..327adaf0e --- /dev/null +++ b/server/test/integration/heartbeatJobDegradedEgress.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it, beforeEach } from "@jest/globals"; +import { createHeartbeatTestHarness, makeMonitor, type HeartbeatTestHarness } from "../helpers/heartbeatTestHarness.ts"; +import { NETWORK_ERROR } from "../../src/types/network.ts"; + +// Egress is only consulted for transport failures, so these heartbeats fail with the network-error +// sentinel (timeout / connection refused) rather than an HTTP status code. + +describe("Heartbeat job: degraded egress", () => { + let h: HeartbeatTestHarness; + + beforeEach(() => { + h = createHeartbeatTestHarness(); + }); + + it("records failing checks as degraded without changing monitor status, opening an incident or notifying", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + // Enough failures to cross the 60% threshold if they were counted + h.setEgressStatus("degraded"); + h.setNextResponse(false, NETWORK_ERROR); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + // Every check was buffered, flagged, so the repository can exclude it from uptime + expect(h.egressService.assessAfterFailure).toHaveBeenCalledTimes(3); + expect(h.bufferStub.addToBuffer).toHaveBeenCalledTimes(3); + for (const [check] of h.bufferStub.addToBuffer.mock.calls) { + expect(check).toMatchObject({ status: false, egressStatus: "degraded" }); + } + + // 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]); + expect(h.incidentsRepo.getAll()).toHaveLength(0); + expect(h.notificationsService.handleNotifications).not.toHaveBeenCalled(); + }); + + it("does not fire a spurious recovery once egress returns", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + h.setEgressStatus("degraded"); + h.setNextResponse(false, NETWORK_ERROR); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + // Egress is back and the target is fine + h.setEgressStatus(null); + h.setNextResponse(true, 200); + await h.heartbeatJob(monitor); + + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("up"); + expect(h.incidentsRepo.getAll()).toHaveLength(0); + expect(h.notificationsService.handleNotifications).not.toHaveBeenCalled(); + }); + + it("detects down at the normal threshold when the egress check is disabled", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + h.setEgressStatus(null); + h.setNextResponse(false, NETWORK_ERROR); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + expect(h.egressService.assessAfterFailure).toHaveBeenCalledTimes(3); + for (const [check] of h.bufferStub.addToBuffer.mock.calls) { + expect(check).not.toHaveProperty("egressStatus"); + } + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); + expect(h.incidentsRepo.getAll()).toHaveLength(1); + expect(h.notificationsService.handleNotifications).toHaveBeenCalled(); + }); + + it("detects down at the normal threshold when egress is confirmed ok", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + h.setEgressStatus("ok"); + h.setNextResponse(false, NETWORK_ERROR); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + for (const [check] of h.bufferStub.addToBuffer.mock.calls) { + expect(check).toMatchObject({ status: false, egressStatus: "ok" }); + } + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); + expect(h.incidentsRepo.getAll()).toHaveLength(1); + expect(h.notificationsService.handleNotifications).toHaveBeenCalled(); + }); + + it("evaluates HTTP error responses normally while egress is degraded", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + // The target answered, so the failure is its own even though the instance is degraded + h.setEgressStatus("degraded"); + h.setNextResponse(false, 503); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + expect(h.egressService.assessAfterFailure).not.toHaveBeenCalled(); + for (const [check] of h.bufferStub.addToBuffer.mock.calls) { + expect(check).not.toHaveProperty("egressStatus"); + } + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); + expect(h.incidentsRepo.getAll()).toHaveLength(1); + expect(h.incidentsRepo.getAll()[0].statusCode).toBe(503); + expect(h.notificationsService.handleNotifications).toHaveBeenCalled(); + }); + + it("evaluates a failing docker check normally while egress is degraded", async () => { + const monitor = makeMonitor({ type: "docker", url: "unix:///var/run/docker.sock" }); + h.monitorsRepo.seed(monitor); + + h.setEgressStatus("degraded"); + h.setNextResponse(false, NETWORK_ERROR); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + expect(h.egressService.assessAfterFailure).not.toHaveBeenCalled(); + for (const [check] of h.bufferStub.addToBuffer.mock.calls) { + expect(check).not.toHaveProperty("egressStatus"); + } + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); + expect(h.incidentsRepo.getAll()).toHaveLength(1); + }); + + it("keeps counting real failures taken either side of a degraded spell", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + // 2 real failures: window [t, t, t, f, f] = 40% < 60% + h.setNextResponse(false, NETWORK_ERROR); + 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")).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); + + 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 478262c20..bb2a0b536 100644 --- a/server/test/unit/services/checkProducer.test.ts +++ b/server/test/unit/services/checkProducer.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, jest } from "@jest/globals"; import { CheckProducer } from "../../../src/worker/worker.check-producer.ts"; import type { Monitor } from "../../../src/domain/monitors/monitor.type.ts"; import { createMockLogger } from "../../helpers/createMockLogger.ts"; +import { NETWORK_ERROR } from "../../../src/types/network.ts"; const makeMonitor = (overrides?: Partial): Monitor => ({ @@ -215,7 +216,8 @@ describe("CheckProducer", () => { // ── egress self-check ───────────────────────────────────────────────────── - const failingStatus = { monitorId: "m1", status: false, code: 500, message: "Error" }; + // A transport failure: nothing came back from the target, so the code is the network-error sentinel. + const failingStatus = { monitorId: "m1", status: false, code: NETWORK_ERROR, message: "Timeout" }; it("does not consult the egress service for a successful check and leaves the field unset", async () => { const check: Record = { id: "check-1" }; @@ -229,6 +231,57 @@ describe("CheckProducer", () => { expect(check).not.toHaveProperty("egressStatus"); }); + it("does not consult egress when an HTTP response was received", async () => { + for (const status of [ + { monitorId: "m1", status: false, code: 500, message: "Internal Server Error" }, + { monitorId: "m1", status: false, code: 200, message: "Content mismatch" }, + ]) { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(status) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor()); + + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + } + }); + + it("consults egress for a timeout or network error", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor()); + + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check.egressStatus).toBe("degraded"); + }); + + it("never consults egress for a failing docker or hardware check", async () => { + for (const type of ["docker", "hardware"] as const) { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type })); + + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + } + }); + it("flags a failing check as degraded when the egress service reports degraded egress", async () => { const check: Record = { id: "check-1" }; const { producer, defaults } = createProducer({ @@ -258,6 +311,24 @@ describe("CheckProducer", () => { expect(check.egressStatus).toBe("ok"); }); + it("buffers the check when the egress assessment rejects", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue(failingStatus) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockRejectedValue(new Error("settings unavailable")) }, + }); + + const result = await producer.produce(makeMonitor()); + + expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + expect(check).not.toHaveProperty("egressStatus"); + expect(result).toEqual({ status: failingStatus, check }); + expect(defaults.logger.warn).toHaveBeenCalledWith( + expect.objectContaining({ message: expect.stringContaining("settings unavailable"), service: "CheckProducer", method: "assessEgress" }) + ); + }); + it("leaves the field unset on a failing check when the egress check is disabled", async () => { const check: Record = { id: "check-1" }; const { producer, defaults } = createProducer({ From d865103fb58742a553a90c6c176d14d8e0b709df Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Wed, 16 Sep 2026 21:58:55 +0100 Subject: [PATCH 4/9] Exclude degraded-egress checks from every aggregate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A degraded episode now reads as a gap in monitoring rather than as downtime or as a dip in response time. The exclusion is applied once per query, in the top-level $match, so no pipeline can count a degraded check by omission. - check.query.ts: EXCLUDE_DEGRADED_EGRESS_MATCH is the only helper; IS_NOT_DEGRADED_EGRESS_EXPR and the $cond accumulators it fed are gone. - findUptimeDateRangeChecks: excludes at the match, so the response-time series, the up/down series and the averages drop degraded checks too. - findPageSpeedDateRangeChecks: excludes as well; pagespeed is egress attributable and was previously missed. - getDailyStatusBuckets: excludes at the match; the group shapes and the totalChecks > 0 guard revert to their original form. - check.docker.aggregation.ts, check.hardware.aggregations.ts: reverted. Neither type is egress attributable, so their checks never carry the flag and the filters were dead code. A test now pins that decision. - Tests: the gap behaviour, a window that was entirely degraded, pagespeed, the team listing and the resolve filter, and an incident that stays open when egress drops during it. 🔧 --- .../domain/checks/check.docker.aggregation.ts | 7 +- .../checks/check.hardware.aggregations.ts | 2 - server/src/domain/checks/check.query.ts | 9 +- .../domain/checks/check.repository.mongo.ts | 14 +-- server/src/domain/monitors/monitor.type.ts | 5 +- .../checksRepositoryEgressExclusion.test.ts | 104 ++++++++++-------- .../heartbeatJobDegradedEgress.test.ts | 24 ++++ 7 files changed, 99 insertions(+), 66 deletions(-) diff --git a/server/src/domain/checks/check.docker.aggregation.ts b/server/src/domain/checks/check.docker.aggregation.ts index de6870e04..37d6d8e87 100644 --- a/server/src/domain/checks/check.docker.aggregation.ts +++ b/server/src/domain/checks/check.docker.aggregation.ts @@ -1,6 +1,5 @@ import CheckModel from "@/domain/checks/check.model.js"; import { DockerContainerStatsBucket, DockerStatsBucket } from "@/domain/checks/check.type.js"; -import { EXCLUDE_DEGRADED_EGRESS_MATCH, IS_NOT_DEGRADED_EGRESS_EXPR } from "@/domain/checks/check.query.js"; import mongoose from "mongoose"; type DateRange = { start: Date; end: Date }; @@ -10,7 +9,6 @@ export const getDockerTotalChecks = async (monitorId: string, dates: DateRange): "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", createdAt: { $gte: dates.start, $lte: dates.end }, - ...EXCLUDE_DEGRADED_EGRESS_MATCH, }); export const getDockerUpChecks = async (monitorId: string, dates: DateRange): Promise<{ totalChecks: number }> => { @@ -36,9 +34,8 @@ export const getDockerStats = async (monitorId: string, dates: DateRange, dateSt $group: { _id: { $dateToString: { format: dateString, date: "$createdAt" } }, avgResponseTime: { $avg: "$responseTime" }, - // Counts exclude degraded-egress checks like aggregateData.totalChecks does; the response-time average keeps them. - upCount: { $sum: { $cond: [{ $and: [{ $eq: ["$status", true] }, IS_NOT_DEGRADED_EGRESS_EXPR] }, 1, 0] } }, - totalCount: { $sum: { $cond: [IS_NOT_DEGRADED_EGRESS_EXPR, 1, 0] } }, + upCount: { $sum: { $cond: [{ $eq: ["$status", true] }, 1, 0] } }, + totalCount: { $sum: 1 }, avgRunning: { $avg: "$containerSummary.running" }, avgTotal: { $avg: "$containerSummary.total" }, avgUnhealthy: { $avg: "$containerSummary.unhealthy" }, diff --git a/server/src/domain/checks/check.hardware.aggregations.ts b/server/src/domain/checks/check.hardware.aggregations.ts index 9a67a8d0a..6c672b630 100644 --- a/server/src/domain/checks/check.hardware.aggregations.ts +++ b/server/src/domain/checks/check.hardware.aggregations.ts @@ -1,7 +1,6 @@ import { CheckModel } from "@/domain/checks/check.model.js"; import mongoose from "mongoose"; import type { HardwareDiskStats, HardwareNetStats } from "@/domain/checks/check.type.js"; -import { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.query.js"; type DateRange = { start: Date; end: Date }; type HardwareUpChecks = { totalChecks: number }; @@ -71,7 +70,6 @@ export const getHardwareTotalChecks = async (monitorId: string, dates: DateRange "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "hardware", createdAt: { $gte: dates.start, $lte: dates.end }, - ...EXCLUDE_DEGRADED_EGRESS_MATCH, }); }; diff --git a/server/src/domain/checks/check.query.ts b/server/src/domain/checks/check.query.ts index 2ebaf59d3..8b07a30cf 100644 --- a/server/src/domain/checks/check.query.ts +++ b/server/src/domain/checks/check.query.ts @@ -1,8 +1,7 @@ import type { EgressStatus } from "@/domain/egress/egress.type.js"; -// Checks recorded while the instance's own egress was down cannot be attributed to the target, so they -// are left out of every uptime percentage and up/down count. Response-time series keep them. -// Spread this into the $match / filter of a query, or use the expression form inside a $group accumulator. +// A check recorded while the instance's own egress was down says nothing about the target, so it is left out of +// every aggregate: uptime percentages, up/down counts and the response-time series alike. A degraded episode +// therefore reads as a gap in monitoring rather than as downtime or as a dip in response time. +// Spread this into the $match or filter of any query that reports statistics. Check listings keep degraded checks. export const EXCLUDE_DEGRADED_EGRESS_MATCH = { egressStatus: { $ne: "degraded" satisfies EgressStatus } } as const; - -export const IS_NOT_DEGRADED_EGRESS_EXPR = { $ne: ["$egressStatus", "degraded" satisfies EgressStatus] } as const; diff --git a/server/src/domain/checks/check.repository.mongo.ts b/server/src/domain/checks/check.repository.mongo.ts index 1a9707aa5..6a1648ce1 100644 --- a/server/src/domain/checks/check.repository.mongo.ts +++ b/server/src/domain/checks/check.repository.mongo.ts @@ -16,7 +16,7 @@ import type { } from "@/domain/checks/check.type.js"; import type { MonitorType } from "@/domain/monitors/monitor.type.js"; import { CheckModel, type CheckDocument } from "@/domain/checks/check.model.js"; -import { EXCLUDE_DEGRADED_EGRESS_MATCH, IS_NOT_DEGRADED_EGRESS_EXPR } from "@/domain/checks/check.query.js"; +import { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.query.js"; import mongoose from "mongoose"; import { getDateFormat, getDateForRange } from "@/utils/dataUtils.js"; import { ILogger } from "@/utils/logger.js"; @@ -396,6 +396,7 @@ class MongoChecksRepository implements IChecksRepository { $match: { "metadata.monitorId": { $in: objectIds }, createdAt: { $gte: windowStart }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }, }, { @@ -404,14 +405,11 @@ class MongoChecksRepository implements IChecksRepository { monitorId: "$metadata.monitorId", day: { $dateTrunc: { date: "$createdAt", unit: "day", timezone } }, }, - // Counts exclude degraded-egress checks; the response-time average keeps them. - totalChecks: { $sum: { $cond: [IS_NOT_DEGRADED_EGRESS_EXPR, 1, 0] } }, - upChecks: { $sum: { $cond: [{ $and: [{ $eq: ["$status", true] }, IS_NOT_DEGRADED_EGRESS_EXPR] }, 1, 0] } }, + totalChecks: { $sum: 1 }, + upChecks: { $sum: { $cond: [{ $eq: ["$status", true] }, 1, 0] } }, avgResponseTime: { $avg: "$responseTime" }, }, }, - // A day with nothing attributable to the target renders as an empty bar; the client divides by totalChecks for the tooltip. - { $match: { totalChecks: { $gt: 0 } } }, { $sort: { "_id.day": 1 } }, { $project: { @@ -479,6 +477,7 @@ class MongoChecksRepository implements IChecksRepository { const matchStage = { "metadata.monitorId": monitorObjectId, createdAt: { $gte: startDate, $lte: endDate }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }; const [result] = await CheckModel.aggregate([ { $match: matchStage }, @@ -486,8 +485,6 @@ class MongoChecksRepository implements IChecksRepository { { $facet: { uptimePercentage: [ - // Response-time series (groupedUpChecks / groupedDownChecks) deliberately keep degraded checks so they still show as failures on the graph. - { $match: EXCLUDE_DEGRADED_EGRESS_MATCH }, { $group: { _id: null, @@ -646,6 +643,7 @@ class MongoChecksRepository implements IChecksRepository { const matchStage = { "metadata.monitorId": monitorObjectId, createdAt: { $gte: startDate, $lte: endDate }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }; const [result] = await CheckModel.aggregate([ diff --git a/server/src/domain/monitors/monitor.type.ts b/server/src/domain/monitors/monitor.type.ts index f2029bbb1..5f40d6d80 100644 --- a/server/src/domain/monitors/monitor.type.ts +++ b/server/src/domain/monitors/monitor.type.ts @@ -50,8 +50,9 @@ export const UptimeDetailsSupportedTypes = ["http", "ping", "port", "game", "grp export type UptimeDetailsSupportedType = (typeof UptimeDetailsSupportedTypes)[number]; export const supportsUptimeDetails = (type: MonitorType): type is UptimeDetailsSupportedType => UptimeDetailsSupportedTypes.some((t) => t === type); -// Types whose transport failure can be the instance's own loss of egress. Hardware (Capture agent) and docker -// (local socket or host) failures are local, so they are never attributed to egress. +// Types whose transport failure can be the instance's own loss of egress. Hardware and docker are left out: +// a Capture agent or a docker socket is normally reached locally, and a docker monitor pointed at a remote +// host over TCP is deliberately out of scope rather than assumed local. export const EgressAttributableTypes = [ "http", "ping", diff --git a/server/test/integration/checksRepositoryEgressExclusion.test.ts b/server/test/integration/checksRepositoryEgressExclusion.test.ts index eea421019..0f0950928 100644 --- a/server/test/integration/checksRepositoryEgressExclusion.test.ts +++ b/server/test/integration/checksRepositoryEgressExclusion.test.ts @@ -3,14 +3,15 @@ import mongoose from "mongoose"; import { MongoMemoryServer } from "mongodb-memory-server"; import MongoChecksRepository from "../../src/domain/checks/check.repository.mongo.ts"; import { CheckModel } from "../../src/domain/checks/check.model.ts"; -import type { Check, DockerChecksResult, HardwareChecksResult, UptimeChecksResult } from "../../src/domain/checks/check.type.ts"; +import type { Check, DockerChecksResult, PageSpeedChecksResult, UptimeChecksResult } from "../../src/domain/checks/check.type.ts"; +import { NETWORK_ERROR } from "../../src/types/network.ts"; import type { ILogger } from "../../src/utils/logger.ts"; import { createMockLogger } from "../helpers/createMockLogger.ts"; // ── Real-Mongo harness ───────────────────────────────────────────────────────── // The guarantee under test — a check flagged egressStatus "degraded" is left out of -// every uptime percentage and down-count while still appearing in listings and in -// the response-time series — lives in $match stages spread into several aggregation +// every aggregate, so a degraded episode reads as a gap in monitoring, while the check +// still appears in listings — lives in $match stages spread into several aggregation // pipelines, so it can only be exercised against a live mongod. let mongod: MongoMemoryServer; @@ -85,7 +86,7 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { await expect(seedCheck({ status: false, egressStatus: "unknown" })).rejects.toThrow(/egressStatus/); }); - it("excludes degraded checks from the uptime percentage but keeps them in the response-time series", async () => { + it("excludes degraded checks from the uptime percentage and from the response-time series alike", async () => { await seedCheck(); // up await seedCheck({ status: false }); // real failure await seedDegradedFailure(); @@ -95,13 +96,33 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { // 1 up of 2 attributable checks. Counting the degraded pair would give 25%. expect(result.uptimePercentage).toBe(0.5); - // The graph still shows the degraded checks as failures, and their response times stay in the averages. - expect(result.avgResponseTime).toBe(200); - expect(result.groupedChecks[0]).toMatchObject({ totalChecks: 4, avgResponseTime: 200 }); - expect(result.groupedDownChecks[0]).toMatchObject({ totalChecks: 3 }); + // Every series sees the same two checks, so the episode is a gap rather than a dip. + expect(result.avgResponseTime).toBe(100); + expect(result.groupedChecks[0]).toMatchObject({ totalChecks: 2, avgResponseTime: 100 }); + expect(result.groupedDownChecks[0]).toMatchObject({ totalChecks: 1 }); expect(result.groupedUpChecks[0]).toMatchObject({ totalChecks: 1 }); }); + it("leaves a bucket out of the response-time series entirely when every check in it was degraded", async () => { + await seedDegradedFailure(); + await seedDegradedFailure(); + + const result = (await repo.findByDateRangeAndMonitorId(MONITOR_ID.toString(), "day", { type: "http" })) as UptimeChecksResult; + + expect(result.groupedChecks).toEqual([]); + expect(result.groupedDownChecks).toEqual([]); + expect(result.uptimePercentage).toBe(0); + }); + + it("excludes degraded checks from the pagespeed series", async () => { + await seedCheck({ metadata: { monitorId: MONITOR_ID, teamId: TEAM_ID, type: "pagespeed" }, performance: 0.9 }); + await seedDegradedFailure({ metadata: { monitorId: MONITOR_ID, teamId: TEAM_ID, type: "pagespeed" }, performance: 0.1 }); + + const result = (await repo.findByDateRangeAndMonitorId(MONITOR_ID.toString(), "day", { type: "pagespeed" })) as PageSpeedChecksResult; + + expect(result.groupedChecks[0]).toMatchObject({ totalChecks: 1, performance: 0.9 }); + }); + it("does not flag a failure that carries egressStatus ok", async () => { await seedCheck(); await seedCheck({ status: false, egressStatus: "ok" }); @@ -121,7 +142,7 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { expect(summary).toEqual({ totalChecks: 2, downChecks: 1 }); }); - it("excludes degraded checks from the daily status bucket counts but keeps them in its response-time average", async () => { + it("excludes degraded checks from the daily status buckets, dropping a day that saw nothing else", async () => { const otherMonitor = new mongoose.Types.ObjectId(); await seedCheck(); await seedCheck({ status: false }); @@ -130,46 +151,12 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { const buckets = await repo.getDailyStatusBuckets([MONITOR_ID.toString(), otherMonitor.toString()], 7, "UTC"); - // (100 + 100 + 300) / 3 = 167: the degraded check's response time counts, its failure does not. // The other monitor saw only degraded checks that day, so it gets no row rather than a 0/0 bucket. expect(buckets).toEqual([ - { monitorId: MONITOR_ID.toString(), date: utcDate(BUCKET_TIME), totalChecks: 2, upChecks: 1, downChecks: 1, avgResponseTime: 167 }, + { monitorId: MONITOR_ID.toString(), date: utcDate(BUCKET_TIME), totalChecks: 2, upChecks: 1, downChecks: 1, avgResponseTime: 100 }, ]); }); - it("excludes degraded checks from the hardware and docker totals", async () => { - const hardwareMonitor = new mongoose.Types.ObjectId(); - const dockerMonitor = new mongoose.Types.ObjectId(); - const hardwareMeta = { monitorId: hardwareMonitor, teamId: TEAM_ID, type: "hardware" }; - const dockerMeta = { monitorId: dockerMonitor, teamId: TEAM_ID, type: "docker" }; - - await seedCheck({ metadata: hardwareMeta }); - await seedDegradedFailure({ metadata: hardwareMeta }); - await seedCheck({ metadata: dockerMeta }); - await seedDegradedFailure({ metadata: dockerMeta }); - - const hardware = (await repo.findByDateRangeAndMonitorId(hardwareMonitor.toString(), "day", { type: "hardware" })) as HardwareChecksResult; - const docker = (await repo.findByDateRangeAndMonitorId(dockerMonitor.toString(), "day", { type: "docker" })) as DockerChecksResult; - - expect(hardware.aggregateData.totalChecks).toBe(1); - expect(hardware.upChecks.totalChecks).toBe(1); - expect(docker.aggregateData.totalChecks).toBe(1); - expect(docker.upChecks.totalChecks).toBe(1); - }); - - it("excludes degraded checks from the docker bucket counts but keeps them in its response-time average", async () => { - const dockerMonitor = new mongoose.Types.ObjectId(); - const dockerMeta = { monitorId: dockerMonitor, teamId: TEAM_ID, type: "docker" }; - - await seedCheck({ metadata: dockerMeta }); - await seedDegradedFailure({ metadata: dockerMeta }); - - const docker = (await repo.findByDateRangeAndMonitorId(dockerMonitor.toString(), "day", { type: "docker" })) as DockerChecksResult; - - expect(docker.aggregate).toHaveLength(1); - expect(docker.aggregate[0]).toMatchObject({ upCount: 1, totalCount: 1, avgResponseTime: 200 }); - }); - it("keeps degraded checks in the paginated listing so they can be shown as such", async () => { await seedCheck({ status: false }); await seedDegradedFailure(); @@ -179,4 +166,33 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { expect(checksCount).toBe(2); expect(checks.map((check) => check.egressStatus).sort()).toEqual([undefined, "degraded"].sort()); }); + + it("keeps degraded checks in the team listing and under the resolve filter", async () => { + await seedCheck({ status: false, statusCode: NETWORK_ERROR }); + await seedDegradedFailure({ statusCode: NETWORK_ERROR }); + + // The Checks page reads this listing when no single monitor is selected, and a degraded check has the + // same shape the resolve filter looks for, so neither may quietly drop it. + const team = await repo.findByTeamId("desc", "day", 0, 10, TEAM_ID.toString(), "down"); + const resolve = await repo.findByTeamId("desc", "day", 0, 10, TEAM_ID.toString(), "resolve"); + + expect(team.checksCount).toBe(2); + expect(resolve.checksCount).toBe(2); + expect(resolve.checks.map((check) => check.egressStatus).sort()).toEqual([undefined, "degraded"].sort()); + }); + + it("leaves the docker aggregates alone, since a docker check is never attributed to egress", async () => { + const dockerMonitor = new mongoose.Types.ObjectId(); + const dockerMeta = { monitorId: dockerMonitor, teamId: TEAM_ID, type: "docker" }; + + // The schema accepts the field on any type, so this pins the scope decision rather than the schema: + // the producer never sets it for docker or hardware, so their aggregations must not filter on it. + await seedCheck({ metadata: dockerMeta }); + await seedDegradedFailure({ metadata: dockerMeta }); + + const docker = (await repo.findByDateRangeAndMonitorId(dockerMonitor.toString(), "day", { type: "docker" })) as DockerChecksResult; + + expect(docker.aggregateData.totalChecks).toBe(2); + expect(docker.aggregate[0]).toMatchObject({ upCount: 1, totalCount: 2 }); + }); }); diff --git a/server/test/integration/heartbeatJobDegradedEgress.test.ts b/server/test/integration/heartbeatJobDegradedEgress.test.ts index 327adaf0e..df8378790 100644 --- a/server/test/integration/heartbeatJobDegradedEgress.test.ts +++ b/server/test/integration/heartbeatJobDegradedEgress.test.ts @@ -134,6 +134,30 @@ describe("Heartbeat job: degraded egress", () => { expect(h.incidentsRepo.getAll()).toHaveLength(1); }); + it("leaves an open incident open when egress drops during it", async () => { + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + // The target goes down first and is detected normally + h.setEgressStatus(null); + h.setNextResponse(false, NETWORK_ERROR); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); + expect(h.incidentsRepo.getAll()).toHaveLength(1); + + // Egress then drops. The monitor must stay down rather than be resolved by the silence. + h.setEgressStatus("degraded"); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("down"); + expect(h.incidentsRepo.getAll()).toHaveLength(1); + expect(h.notificationsService.handleNotifications).toHaveBeenCalledTimes(1); + }); + it("keeps counting real failures taken either side of a degraded spell", async () => { const monitor = makeMonitor(); h.monitorsRepo.seed(monitor); From 3dc529733a8ef97680d3801b0a7a665965c062c9 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Thu, 17 Sep 2026 17:22:03 +0100 Subject: [PATCH 5/9] Address the inline review on the egress check pipeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Distinguish "the peer answered" from "nothing answered", widen attribution to every check that leaves the instance, make the maintenance exit explicit, and fix the recovery poll cadence. Providers can now say outright whether the peer answered, via peerResponded on MonitorStatusResponse. Previously the producer inferred it from an HTTP status code, which two providers do not report: - GrpcProvider: NOT_SERVING shares NETWORK_ERROR with a connection failure, and gRPC codes 0-16 are not HTTP statuses, so PERMISSION_DENIED, UNIMPLEMENTED and the rest were all read as transport failures. Only DEADLINE_EXCEEDED and UNAVAILABLE, or a failure carrying no gRPC code, now count as unreachable. - DNSProvider: every resolver error became NETWORK_ERROR, so an authoritative NXDOMAIN, SERVFAIL or REFUSED was read as unreachable. The c-ares codes where the server replied are now separated from timeout and connection refused. Left unset, the HTTP-status heuristic still applies, so other providers are unchanged. Attribution now follows whether the check leaves the instance, not the monitor type. A hardware check is an ordinary outbound HTTP request to the Capture agent, and a Docker daemon can be reached over TCP, so both are attributable; only a Docker daemon on a unix socket is local and exempt. Leaving a maintenance window is now explicit in the producer, alongside the entry it already owned. It was a side effect of evaluating a check, which the degraded-egress short-circuit skips, so a monitor whose window ended during an outage kept reporting "maintenance" for the length of it. The recovery poll interval is fixed at 5 seconds and no longer configurable. While degraded, checks are flagged from stored state without probing, so the interval is the window in which a recovered instance still discards its checks; the only effect of raising it was a longer blind window. egressPollIntervalSeconds is removed from the settings, the validator and the OpenAPI spec. 🔧 --- server/openapi.json | 5 -- .../src/api/validation/settingsValidation.ts | 3 +- .../domain/app-settings/app-settings.model.ts | 3 +- .../app-settings.repository.mongo.ts | 3 +- .../domain/app-settings/app-settings.type.ts | 1 - server/src/domain/egress/egress.service.ts | 15 ++-- server/src/domain/egress/egress.type.ts | 8 +- server/src/domain/monitors/monitor.type.ts | 17 +++- server/src/service/network/DNSProvider.ts | 13 +++ server/src/service/network/GrpcProvider.ts | 14 +++ server/src/types/network.ts | 6 ++ server/src/worker/worker.check-producer.ts | 33 ++++--- .../providers/network/dnsProvider.test.ts | 43 +++++++++ .../providers/network/grpcProvider.test.ts | 53 +++++++++++ .../test/unit/services/checkProducer.test.ts | 89 +++++++++++++++++-- .../test/unit/services/egressService.test.ts | 20 +---- .../validation/settingsValidation.test.ts | 19 +--- 17 files changed, 265 insertions(+), 80 deletions(-) diff --git a/server/openapi.json b/server/openapi.json index 32f5013a1..237bcea29 100644 --- a/server/openapi.json +++ b/server/openapi.json @@ -13307,11 +13307,6 @@ }, "maxItems": 10 }, - "egressPollIntervalSeconds": { - "type": "integer", - "minimum": 5, - "maximum": 600 - }, "egressNotifications": { "type": "array", "items": { diff --git a/server/src/api/validation/settingsValidation.ts b/server/src/api/validation/settingsValidation.ts index 5bf635054..51ea3df5a 100644 --- a/server/src/api/validation/settingsValidation.ts +++ b/server/src/api/validation/settingsValidation.ts @@ -1,5 +1,5 @@ import { CHECK_TTL_SENTINEL } from "@/domain/checks/check.type.js"; -import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "@/domain/egress/egress.type.js"; +import { MAX_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; import { parseEgressTarget } from "@/utils/egressTarget.js"; import { z } from "zod"; @@ -56,7 +56,6 @@ export const updateAppSettingsBodyValidation = z egressCheckEnabled: z.boolean().optional(), egressCheckTargets: z.array(egressTargetValidation).max(MAX_EGRESS_TARGETS).optional(), // empty list falls back to DEFAULT_EGRESS_TARGETS - egressPollIntervalSeconds: z.number().int().min(MIN_EGRESS_POLL_INTERVAL_SECONDS).max(MAX_EGRESS_POLL_INTERVAL_SECONDS).optional(), egressNotifications: z.array(z.string().regex(/^[0-9a-fA-F]{24}$/, "Invalid notification id")).optional(), }) .strip() diff --git a/server/src/domain/app-settings/app-settings.model.ts b/server/src/domain/app-settings/app-settings.model.ts index dfe138b8f..194ee44a2 100644 --- a/server/src/domain/app-settings/app-settings.model.ts +++ b/server/src/domain/app-settings/app-settings.model.ts @@ -1,6 +1,6 @@ import { Schema, model, type Types } from "mongoose"; import type { Settings, SettingsThresholds } from "@/domain/app-settings/app-settings.type.js"; -import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; +import { DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; interface AppSettingsDocument extends Omit { _id: Types.ObjectId; @@ -45,7 +45,6 @@ const AppSettingsSchema = new Schema( globalProxyId: { type: String, default: null }, egressCheckEnabled: { type: Boolean, default: false }, egressCheckTargets: { type: [String], default: () => [...DEFAULT_EGRESS_TARGETS] }, - egressPollIntervalSeconds: { type: Number, default: DEFAULT_EGRESS_POLL_INTERVAL_SECONDS }, egressNotifications: { type: [String], default: () => [] }, // Notification ids, stored as strings like globalProxyId }, { timestamps: true } diff --git a/server/src/domain/app-settings/app-settings.repository.mongo.ts b/server/src/domain/app-settings/app-settings.repository.mongo.ts index 795a2db35..4527c4245 100644 --- a/server/src/domain/app-settings/app-settings.repository.mongo.ts +++ b/server/src/domain/app-settings/app-settings.repository.mongo.ts @@ -3,7 +3,7 @@ import { ISettingsRepository } from "@/domain/app-settings/app-settings-reposito import type { Settings, SettingsUpdate } from "@/domain/app-settings/app-settings.type.js"; import { AppSettingsModel, type AppSettingsDocument } from "@/domain/app-settings/app-settings.model.js"; import { toStringId, toDateString } from "@/utils/mongoMappers.js"; -import { DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; +import { DEFAULT_EGRESS_TARGETS } from "@/domain/egress/egress.type.js"; class MongoSettingsRepository implements ISettingsRepository { private toEntity = (doc: AppSettingsDocument): Settings => { @@ -33,7 +33,6 @@ class MongoSettingsRepository implements ISettingsRepository { globalProxyId: doc.globalProxyId ?? undefined, egressCheckEnabled: doc.egressCheckEnabled ?? false, egressCheckTargets: doc.egressCheckTargets ?? [...DEFAULT_EGRESS_TARGETS], - egressPollIntervalSeconds: doc.egressPollIntervalSeconds ?? DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, egressNotifications: doc.egressNotifications ?? [], createdAt: toDateString(doc.createdAt), updatedAt: toDateString(doc.updatedAt), diff --git a/server/src/domain/app-settings/app-settings.type.ts b/server/src/domain/app-settings/app-settings.type.ts index 9dc4eae45..dbb2d0665 100644 --- a/server/src/domain/app-settings/app-settings.type.ts +++ b/server/src/domain/app-settings/app-settings.type.ts @@ -54,7 +54,6 @@ export interface Settings { // Egress self-check (see domain/egress) egressCheckEnabled: boolean; egressCheckTargets: string[]; - egressPollIntervalSeconds: number; // recovery poll cadence while degraded, not a general schedule egressNotifications: string[]; // Notification ids to receive the "egress recovered" alert createdAt: string; updatedAt: string; diff --git a/server/src/domain/egress/egress.service.ts b/server/src/domain/egress/egress.service.ts index 1088356ff..4e6b0e515 100644 --- a/server/src/domain/egress/egress.service.ts +++ b/server/src/domain/egress/egress.service.ts @@ -7,7 +7,7 @@ import type { INetworkService } from "@/service/networkService.js"; import type { IProxyResolver } from "@/service/network/ProxyResolver.js"; import type { ILogger } from "@/utils/logger.js"; import { - DEFAULT_EGRESS_POLL_INTERVAL_SECONDS, + EGRESS_RECOVERY_POLL_SECONDS, DEFAULT_EGRESS_TARGETS, type EgressAssessment, type EgressProbeResult, @@ -136,11 +136,6 @@ export class EgressService implements IEgressService { return targets.length > 0 ? targets : [...DEFAULT_EGRESS_TARGETS]; }; - private toIntervalMs = (seconds: number | undefined): number => { - const valid = typeof seconds === "number" && Number.isFinite(seconds) && seconds > 0; - return (valid ? seconds : DEFAULT_EGRESS_POLL_INTERVAL_SECONDS) * 1000; - }; - // **************************** // Event-triggered assessment // **************************** @@ -181,7 +176,7 @@ export class EgressService implements IEgressService { const state = await this.egressStateRepository.findSingleton(); if (state.status === "degraded") { // Recovery is detected by the scheduled job. Make sure the row exists without touching its schedule. - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, false); + await this.scheduleRecoveryCheck(false); return "degraded"; } @@ -195,7 +190,7 @@ export class EgressService implements IEgressService { // Schedule the recovery job before recording the transition: a crash between the two then leaves a stray row // while ok, which the job's first run removes, rather than a degraded state that nothing is polling. - await this.scheduleRecoveryCheck(settings.egressPollIntervalSeconds, true); + await this.scheduleRecoveryCheck(true); const degraded = await this.egressStateRepository.markDegraded(results, now); this.logger.warn({ message: degraded @@ -224,8 +219,8 @@ export class EgressService implements IEgressService { // reschedule=true sets nextScheduledAt (start of an episode); false only inserts the row if it is missing, // so re-arming while degraded never pushes a pending run back. - private scheduleRecoveryCheck = async (pollIntervalSeconds: number | undefined, reschedule: boolean) => { - const intervalMs = this.toIntervalMs(pollIntervalSeconds); + private scheduleRecoveryCheck = async (reschedule: boolean) => { + const intervalMs = EGRESS_RECOVERY_POLL_SECONDS * 1000; const seed: JobSeed = { id: RECOVERY_JOB_ID, type: RECOVERY_JOB_TYPE, diff --git a/server/src/domain/egress/egress.type.ts b/server/src/domain/egress/egress.type.ts index 49c8d462f..080202ce1 100644 --- a/server/src/domain/egress/egress.type.ts +++ b/server/src/domain/egress/egress.type.ts @@ -9,11 +9,13 @@ export type EgressAssessment = EgressStatus | "disabled"; // Sensible defaults for a fresh install. Bare hosts are ICMP-pinged, host:port is a TCP connect, // http(s):// URLs are fetched. Any one target being reachable means egress is fine. export const DEFAULT_EGRESS_TARGETS = ["1.1.1.1", "8.8.8.8"] as const; -export const DEFAULT_EGRESS_POLL_INTERVAL_SECONDS = 30; -export const MIN_EGRESS_POLL_INTERVAL_SECONDS = 5; -export const MAX_EGRESS_POLL_INTERVAL_SECONDS = 600; export const MAX_EGRESS_TARGETS = 10; +// While the instance is marked degraded, every failing check is flagged from stored state without probing, so +// this interval is the window in which a recovered instance still discards its checks. It is deliberately fixed +// and not configurable: the only effect of raising it is a longer blind window. +export const EGRESS_RECOVERY_POLL_SECONDS = 5; + // Instance-global singleton persisted in MongoDB so that every worker process and the API // process share one view of the state (the queue worker may run separately from the API). export interface EgressState { diff --git a/server/src/domain/monitors/monitor.type.ts b/server/src/domain/monitors/monitor.type.ts index 5f40d6d80..bacaedb47 100644 --- a/server/src/domain/monitors/monitor.type.ts +++ b/server/src/domain/monitors/monitor.type.ts @@ -50,13 +50,15 @@ export const UptimeDetailsSupportedTypes = ["http", "ping", "port", "game", "grp export type UptimeDetailsSupportedType = (typeof UptimeDetailsSupportedTypes)[number]; export const supportsUptimeDetails = (type: MonitorType): type is UptimeDetailsSupportedType => UptimeDetailsSupportedTypes.some((t) => t === type); -// Types whose transport failure can be the instance's own loss of egress. Hardware and docker are left out: -// a Capture agent or a docker socket is normally reached locally, and a docker monitor pointed at a remote -// host over TCP is deliberately out of scope rather than assumed local. +// Types whose check leaves the instance, and whose failure to reach the target can therefore be the instance's +// own loss of egress. A hardware check is an ordinary outbound HTTP request to the Capture agent, so it counts +// however far away the agent is. `unknown` is excluded because no request is made for it. export const EgressAttributableTypes = [ "http", "ping", "pagespeed", + "hardware", + "docker", "port", "game", "grpc", @@ -64,7 +66,14 @@ export const EgressAttributableTypes = [ "dns", ] as const satisfies readonly MonitorType[]; export type EgressAttributableType = (typeof EgressAttributableTypes)[number]; -export const isEgressAttributable = (type: MonitorType): type is EgressAttributableType => EgressAttributableTypes.some((t) => t === type); + +// A Docker daemon reached over a unix socket is local IPC and cannot fail through egress; one reached over +// TCP or TLS is as remote as any other target. The URL, not the type, settles it. +const isLocalDockerSocket = (monitor: Pick): boolean => + monitor.type === "docker" && (monitor.url?.trim().startsWith("unix://") ?? false); + +export const isEgressAttributable = (monitor: Pick): boolean => + EgressAttributableTypes.some((t) => t === monitor.type) && !isLocalDockerSocket(monitor); export const MonitorStatuses = ["up", "down", "paused", "initializing", "maintenance", "breached"] as const; export type MonitorStatus = (typeof MonitorStatuses)[number]; diff --git a/server/src/service/network/DNSProvider.ts b/server/src/service/network/DNSProvider.ts index 10eff8d74..475026c54 100644 --- a/server/src/service/network/DNSProvider.ts +++ b/server/src/service/network/DNSProvider.ts @@ -1,5 +1,6 @@ import { DNSStatusPayload, MonitorStatusResponse } from "@/types/network.js"; import { IStatusProvider } from "@/service/network/IStatusProvider.js"; +import dns from "dns"; import type { Resolver } from "dns/promises"; import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js"; import { AppError } from "@/utils/AppError.js"; @@ -8,6 +9,16 @@ import { NETWORK_ERROR } from "@/types/network.js"; const SERVICE_NAME = "DNSProvider"; +// c-ares codes where the DNS server replied: the name does not exist, holds no such record, or the server +// declined or answered badly. Anything else (timeout, connection refused, a local resolver fault) means we +// never got an answer, so the failure could be our own loss of egress rather than the target's. +const ANSWERED_DNS_CODES: ReadonlySet = new Set([dns.NODATA, dns.FORMERR, dns.SERVFAIL, dns.NOTFOUND, dns.NOTIMP, dns.REFUSED, dns.BADRESP]); + +const dnsServerAnswered = (error: unknown): boolean => { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return typeof code === "string" && ANSWERED_DNS_CODES.has(code); +}; + export class DNSProvider implements IStatusProvider { readonly type = "dns"; constructor(private createResolver: () => Resolver) {} @@ -63,6 +74,7 @@ export class DNSProvider implements IStatusProvider { type: monitor.type, status: false, code: NETWORK_ERROR, + peerResponded: dnsServerAnswered(error), message: error instanceof Error ? error.message : String(error), responseTime, payload: { @@ -81,6 +93,7 @@ export class DNSProvider implements IStatusProvider { type: monitor.type, status: true, code: 200, + peerResponded: true, message: "Success", responseTime, payload: { diff --git a/server/src/service/network/GrpcProvider.ts b/server/src/service/network/GrpcProvider.ts index 340fdc01a..b651344cb 100644 --- a/server/src/service/network/GrpcProvider.ts +++ b/server/src/service/network/GrpcProvider.ts @@ -25,6 +25,17 @@ export class GrpcProvider implements IStatusProvider { return type === "grpc"; } + // gRPC codes that mean the call never reached a server. Everything else in 0-16 is the server's own answer, + // including NOT_SERVING, which shares NETWORK_ERROR with a connection failure and so cannot be read from `code`. + // A call that fails with no gRPC code at all never got as far as the protocol. + private peerAnswered(code: number | undefined): boolean { + const UNREACHABLE_GRPC_CODES: ReadonlySet = new Set([ + 4, // DEADLINE_EXCEEDED + 14, // UNAVAILABLE + ]); + return typeof code === "number" && !UNREACHABLE_GRPC_CODES.has(code); + } + private getGrpcStatusName(code: number): string { const statusNames: Record = { 0: "OK", @@ -148,6 +159,7 @@ export class GrpcProvider implements IStatusProvider { type: type, status: false, code: grpcError.grpcCode ?? 5000, + peerResponded: this.peerAnswered(grpcError.grpcCode), message: grpcError.message ?? "gRPC health check failed", responseTime, payload: payload ?? null, @@ -163,6 +175,8 @@ export class GrpcProvider implements IStatusProvider { type: type, status: isServing, code: isServing ? 200 : 5000, + // The server replied, whatever it said, so this failure is never the instance's own egress. + peerResponded: true, message: isServing ? `gRPC service healthy (${grpcPayload.servingStatus})` : `gRPC service unhealthy (${grpcPayload.servingStatus})`, responseTime, payload: grpcPayload, diff --git a/server/src/types/network.ts b/server/src/types/network.ts index 1f8afe267..b7a3d885a 100644 --- a/server/src/types/network.ts +++ b/server/src/types/network.ts @@ -33,6 +33,12 @@ export interface MonitorStatusResponse< status: boolean; code: number; message: string; + // Whether the peer answered, for failures where `code` alone cannot say. `code` carries an HTTP status for + // providers that speak HTTP, but DNS and gRPC report their own codes, and gRPC reuses NETWORK_ERROR for a + // server that replied NOT_SERVING. Providers that can tell the two apart set this; the rest leave it unset + // and the HTTP-status heuristic stands. Consumed by the egress check, which must only blame the instance's + // own connectivity when nothing answered. + peerResponded?: boolean; responseTime?: number; payload?: T | string | null; timings?: GotTimings; diff --git a/server/src/worker/worker.check-producer.ts b/server/src/worker/worker.check-producer.ts index 3f09e998d..d7b93ae3d 100644 --- a/server/src/worker/worker.check-producer.ts +++ b/server/src/worker/worker.check-producer.ts @@ -46,10 +46,15 @@ export class CheckProducer implements ICheckProducer { return dockerTlsKey ?? undefined; } - // Only a failure to reach the target at all can be the instance's own fault. Any HTTP response (4xx, 5xx, - // or a 200 with a content mismatch) proves the target was reached, and hardware/docker failures are local. + // Only a failure to reach the target at all can be the instance's own fault. A provider that can tell the + // two apart says so outright; for the rest, any HTTP status code proves the target answered (a 4xx, a 5xx, + // or a 200 with a content mismatch). + private peerAnswered(status: MonitorStatusResponse): boolean { + return status.peerResponded ?? isHttpStatusCode(status.code); + } + private isTransportFailure(monitor: Monitor, status: MonitorStatusResponse): boolean { - return status.status === false && isEgressAttributable(monitor.type) && !isHttpStatusCode(status.code); + return status.status === false && isEgressAttributable(monitor) && !this.peerAnswered(status); } // The egress check must never stop a check being recorded, so a rejection is logged and treated as unknown. @@ -89,6 +94,13 @@ export class CheckProducer implements ICheckProducer { return null; } + // Leaving the window is explicit rather than a side effect of evaluating the resulting check, which the + // degraded-egress short-circuit skips. "initializing" is resolved to up or down by the first check that is + // evaluated, so the monitor cannot sit in "maintenance" for the length of an egress outage. + if (monitor.status === "maintenance") { + monitor = await this.monitorsRepository.updateById(monitor.id, monitor.teamId, { status: "initializing" }); + } + // Step 1b: Acquire status const proxyUrl = await this.proxyResolver.resolve(monitor); const dockerTlsKey = await this.resolveDockerTlsKey(monitor); @@ -98,15 +110,12 @@ export class CheckProducer implements ICheckProducer { throw new Error("No network response"); } - // Step 1c: On a transport failure, ask whether the instance itself can reach anything before blaming the target. - // Null means the egress check is disabled (or failed internally) and the check is treated as usual. - const egressStatus = this.isTransportFailure(monitor, status) ? await this.assessEgress(monitor.id) : null; - // **************************** // Step 2: Record // **************************** - // Step 2a: Create & record a check, return null if fail + // Step 2a: Create & record a check, return null if fail. + // Built before the egress assessment below so that the probe's duration stays out of `createdAt`. const check = this.checkService.toCheck(status); if (!check) { this.logger.warn({ @@ -117,13 +126,17 @@ export class CheckProducer implements ICheckProducer { }); return null; } + + // Step 2b: On a transport failure, ask whether the instance itself can reach anything before blaming the + // target. Null means the egress check is disabled (or failed internally) and the check is treated as usual. + const egressStatus = this.isTransportFailure(monitor, status) ? await this.assessEgress(monitor.id) : null; if (egressStatus !== null) { check.egressStatus = egressStatus; } - // Step 2b: Add to buffer + // Step 2c: Add to buffer this.bufferService.addToBuffer(check); - // Step 2c: Handle docker logs + // Step 2d: 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/unit/providers/network/dnsProvider.test.ts b/server/test/unit/providers/network/dnsProvider.test.ts index d485ca599..c6cfff23b 100644 --- a/server/test/unit/providers/network/dnsProvider.test.ts +++ b/server/test/unit/providers/network/dnsProvider.test.ts @@ -209,6 +209,49 @@ describe("DNSProvider", () => { expect(mockResolverInstance.resolve4).toHaveBeenCalledWith("example.com"); }); + // ── Peer reachability ──────────────────────────────────────────────────── + // The egress check must only blame the instance's own connectivity when nothing answered. Every DNS failure + // carries NETWORK_ERROR, so `peerResponded` is the only thing separating an authoritative answer from silence. + + it.each([ + ["ENOTFOUND", "NXDOMAIN: the server answered that the name does not exist"], + ["ENODATA", "the server answered with no record of that type"], + ["ESERVFAIL", "the server answered with a failure"], + ["EREFUSED", "the server declined to answer"], + ["EFORMERR", "the server rejected the query as malformed"], + ["ENOTIMP", "the server does not implement the query"], + ["EBADRESP", "the server replied, badly"], + ])("reports the peer as having answered for %s", async (code) => { + const error = Object.assign(new Error("dns failure"), { code }); + mockResolverInstance.resolve4.mockRejectedValueOnce(error); + const provider = new DNSProvider(createResolver); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(NETWORK_ERROR); + expect(result.peerResponded).toBe(true); + }); + + it.each([["ETIMEOUT"], ["ECONNREFUSED"], ["ECANCELLED"], [undefined]])("reports the peer as unreachable for %s", async (code) => { + const error = code ? Object.assign(new Error("dns failure"), { code }) : new Error("dns failure"); + mockResolverInstance.resolve4.mockRejectedValueOnce(error); + const provider = new DNSProvider(createResolver); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as having answered on success", async () => { + const provider = new DNSProvider(createResolver); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + it("stringifies non-Error throws from outside timeRequest", async () => { mockResolverInstance.setServers.mockImplementationOnce(() => { throw 42; diff --git a/server/test/unit/providers/network/grpcProvider.test.ts b/server/test/unit/providers/network/grpcProvider.test.ts index 63e8859f2..0d53943d2 100644 --- a/server/test/unit/providers/network/grpcProvider.test.ts +++ b/server/test/unit/providers/network/grpcProvider.test.ts @@ -362,6 +362,59 @@ describe("GrpcProvider", () => { [99, "UNKNOWN"], ]; + // ── Peer reachability ──────────────────────────────────────────────── + // NOT_SERVING and a connection failure both report NETWORK_ERROR, and gRPC codes are not HTTP statuses, + // so `peerResponded` is what stops the egress check reading a server's answer as a transport failure. + + it("reports the peer as having answered when the server replies NOT_SERVING", async () => { + const { provider } = createProvider({ behavior: "not-serving" }); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(5000); + expect(result.peerResponded).toBe(true); + }); + + it.each([[5], [7], [12], [16], [2]])("reports the peer as having answered for gRPC code %i", async (code) => { + const client = { + Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ code, details: "test" })), + close: jest.fn(), + }; + const mocks = createMockGrpc(client as any); + const provider = new GrpcProvider(mocks.grpc as any, mocks.protoLoader as any); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + + it.each([[4], [14]])("reports the peer as unreachable for gRPC code %i", async (code) => { + const client = { + Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ code, details: "test" })), + close: jest.fn(), + }; + const mocks = createMockGrpc(client as any); + const provider = new GrpcProvider(mocks.grpc as any, mocks.protoLoader as any); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as unreachable when the failure carries no gRPC code", async () => { + const client = { + Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ message: "socket hang up" })), + close: jest.fn(), + }; + const mocks = createMockGrpc(client as any); + const provider = new GrpcProvider(mocks.grpc as any, mocks.protoLoader as any); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + it.each(statusCodes)("maps gRPC code %i to %s", async (code, expectedName) => { const client = { Check: jest.fn((_r: any, _o: any, cb: Function) => cb({ code, details: "test" })), diff --git a/server/test/unit/services/checkProducer.test.ts b/server/test/unit/services/checkProducer.test.ts index bb2a0b536..c83a890cc 100644 --- a/server/test/unit/services/checkProducer.test.ts +++ b/server/test/unit/services/checkProducer.test.ts @@ -79,6 +79,31 @@ describe("CheckProducer", () => { expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); }); + it("moves a monitor out of maintenance explicitly once the window is over", async () => { + const monitor = makeMonitor({ status: "maintenance" }); + const { producer, defaults } = createProducer({ + monitorsRepository: { + updateById: jest.fn().mockResolvedValue({ ...monitor, status: "initializing" }), + findDockerTlsKeyById: jest.fn().mockResolvedValue(null), + }, + }); + + await producer.produce(monitor); + + // The exit used to be a side effect of evaluating the resulting check, which the degraded-egress + // short-circuit skips. Left implicit, a monitor could sit in "maintenance" for a whole egress outage. + expect(defaults.monitorsRepository.updateById).toHaveBeenCalledWith("m1", "team", { status: "initializing" }); + expect(defaults.networkService.requestStatus).toHaveBeenCalled(); + }); + + it("does not touch the status of a monitor that was not in maintenance", async () => { + const { producer, defaults } = createProducer(); + + await producer.produce(makeMonitor({ status: "up" })); + + expect(defaults.monitorsRepository.updateById).not.toHaveBeenCalled(); + }); + it("proceeds with the check when the maintenance window is inactive", async () => { const { producer, defaults } = createProducer({ maintenanceWindowsRepository: { findByMonitorId: jest.fn().mockResolvedValue([{ ...activeWindow(), active: false }]) }, @@ -265,23 +290,73 @@ describe("CheckProducer", () => { expect(check.egressStatus).toBe("degraded"); }); - it("never consults egress for a failing docker or hardware check", async () => { - for (const type of ["docker", "hardware"] as const) { + it("never consults egress for a docker daemon on a local socket", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type: "docker" }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type: "docker", url: "unix:///var/run/docker.sock" })); + + // Local IPC never leaves the instance, so it cannot fail through egress. + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + }); + + it("consults egress for a hardware check and for a docker daemon reached over TCP", async () => { + for (const monitor of [ + makeMonitor({ type: "hardware", url: "http://capture.example.com:59232/api/v1/metrics" }), + makeMonitor({ type: "docker", url: "tcp://docker.example.com:2376" }), + ]) { const check: Record = { id: "check-1" }; const { producer, defaults } = createProducer({ - networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type }) }, + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, type: monitor.type }) }, checkService: { toCheck: jest.fn().mockReturnValue(check) }, egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, }); - await producer.produce(makeMonitor({ type })); + await producer.produce(monitor); - expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); - expect(check).not.toHaveProperty("egressStatus"); - expect(defaults.buffer.addToBuffer).toHaveBeenCalledWith(check); + // Both requests leave the instance, so our own loss of egress can be the cause. + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check.egressStatus).toBe("degraded"); } }); + it("does not consult egress when the provider reports that the peer answered", async () => { + // gRPC NOT_SERVING and an authoritative DNS NXDOMAIN both carry NETWORK_ERROR, so only this flag separates + // them from a connection failure. + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, peerResponded: true }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type: "dns" })); + + expect(defaults.egressService.assessAfterFailure).not.toHaveBeenCalled(); + expect(check).not.toHaveProperty("egressStatus"); + }); + + it("consults egress when the provider reports that nothing answered, despite an HTTP-shaped code", async () => { + const check: Record = { id: "check-1" }; + const { producer, defaults } = createProducer({ + networkService: { requestStatus: jest.fn().mockResolvedValue({ ...failingStatus, code: 503, peerResponded: false }) }, + checkService: { toCheck: jest.fn().mockReturnValue(check) }, + egressService: { assessAfterFailure: jest.fn().mockResolvedValue("degraded") }, + }); + + await producer.produce(makeMonitor({ type: "grpc" })); + + // An explicit report from the provider outranks the HTTP-status heuristic. + expect(defaults.egressService.assessAfterFailure).toHaveBeenCalledTimes(1); + expect(check.egressStatus).toBe("degraded"); + }); + it("flags a failing check as degraded when the egress service reports degraded egress", async () => { const check: Record = { id: "check-1" }; const { producer, defaults } = createProducer({ diff --git a/server/test/unit/services/egressService.test.ts b/server/test/unit/services/egressService.test.ts index e339ef7ec..adc79feb4 100644 --- a/server/test/unit/services/egressService.test.ts +++ b/server/test/unit/services/egressService.test.ts @@ -21,7 +21,6 @@ const makeState = (overrides?: Partial): EgressState => ({ const makeSettings = (overrides?: Record) => ({ egressCheckEnabled: true, egressCheckTargets: ["1.1.1.1", "8.8.8.8"], - egressPollIntervalSeconds: 30, egressNotifications: ["notif-1"], ...overrides, }); @@ -33,7 +32,7 @@ const makeJob = (overrides?: Partial): Job => ({ refId: null, isActive: true, nextScheduledAt: 1_000, - intervalMs: 30_000, + intervalMs: 5_000, lockedBy: "worker-1", lockedUntil: 61_000, runCount: 0, @@ -245,7 +244,7 @@ describe("EgressService", () => { refId: null, isActive: true, nextScheduledAt: expect.any(Number), - intervalMs: 30_000, + intervalMs: 5_000, }); expect(defaults.jobsRepository.upsertJob).not.toHaveBeenCalled(); // Job first, transition second: a crash in between leaves a stray row, never an unpolled degraded state @@ -266,7 +265,7 @@ describe("EgressService", () => { const result = await service.assessAfterFailure(); expect(result).toBe("degraded"); - expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 5_000 })); expect(defaults.logger.warn).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("already recorded") })); }); @@ -293,21 +292,10 @@ describe("EgressService", () => { expect(defaults.networkService.requestStatus).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.markDegraded).not.toHaveBeenCalled(); expect(defaults.egressStateRepository.recordProbe).not.toHaveBeenCalled(); - expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 30_000 })); + expect(defaults.jobsRepository.upsertJob).toHaveBeenCalledWith(expect.objectContaining({ id: "egress", intervalMs: 5_000 })); expect(defaults.jobsRepository.upsertCleanupJob).not.toHaveBeenCalled(); }); - it("uses the default poll interval for the recovery job when the setting is missing or invalid", async () => { - const { service, defaults } = createService({ - settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressPollIntervalSeconds: undefined })) }, - networkService: { requestStatus: statusFor([]) }, - }); - - await service.assessAfterFailure(); - - expect(defaults.jobsRepository.upsertCleanupJob).toHaveBeenCalledWith(expect.objectContaining({ intervalMs: 30_000 })); - }); - it("falls back to the default targets when the configured list is empty", async () => { const { service, defaults } = createService({ settingsService: { getCachedDBSettings: jest.fn().mockResolvedValue(makeSettings({ egressCheckTargets: [] })) }, diff --git a/server/test/unit/validation/settingsValidation.test.ts b/server/test/unit/validation/settingsValidation.test.ts index 745cbd81e..506090783 100644 --- a/server/test/unit/validation/settingsValidation.test.ts +++ b/server/test/unit/validation/settingsValidation.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "@jest/globals"; import { updateAppSettingsBodyValidation } from "../../../src/api/validation/settingsValidation.ts"; -import { MAX_EGRESS_POLL_INTERVAL_SECONDS, MAX_EGRESS_TARGETS, MIN_EGRESS_POLL_INTERVAL_SECONDS } from "../../../src/domain/egress/egress.type.ts"; +import { MAX_EGRESS_TARGETS } from "../../../src/domain/egress/egress.type.ts"; const validNotificationId = "64b7f0c2a1d2e3f4a5b6c7d8"; @@ -10,13 +10,11 @@ describe("settingsValidation", () => { const parsed = updateAppSettingsBodyValidation.parse({ egressCheckEnabled: true, egressCheckTargets: ["1.1.1.1", "dns.google:53", "https://example.com/health"], - egressPollIntervalSeconds: 30, egressNotifications: [validNotificationId], }); expect(parsed.egressCheckEnabled).toBe(true); expect(parsed.egressCheckTargets).toEqual(["1.1.1.1", "dns.google:53", "https://example.com/health"]); - expect(parsed.egressPollIntervalSeconds).toBe(30); expect(parsed.egressNotifications).toEqual([validNotificationId]); }); @@ -62,21 +60,6 @@ describe("settingsValidation", () => { } }); - it("accepts the poll interval boundaries", () => { - expect(updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: MIN_EGRESS_POLL_INTERVAL_SECONDS }).egressPollIntervalSeconds).toBe( - MIN_EGRESS_POLL_INTERVAL_SECONDS - ); - expect(updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: MAX_EGRESS_POLL_INTERVAL_SECONDS }).egressPollIntervalSeconds).toBe( - MAX_EGRESS_POLL_INTERVAL_SECONDS - ); - }); - - it("rejects out-of-range and non-integer poll intervals", () => { - for (const interval of [MIN_EGRESS_POLL_INTERVAL_SECONDS - 1, MAX_EGRESS_POLL_INTERVAL_SECONDS + 1, 0, -5, 30.5, "30"]) { - expect(() => updateAppSettingsBodyValidation.parse({ egressPollIntervalSeconds: interval })).toThrow(); - } - }); - it("accepts an empty notification list", () => { expect(updateAppSettingsBodyValidation.parse({ egressNotifications: [] }).egressNotifications).toEqual([]); }); From 4fcb13694b500289c38b7bbea59f90a830b8d4f0 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Thu, 17 Sep 2026 17:27:06 +0100 Subject: [PATCH 6/9] Exclude degraded checks from the docker and hardware aggregates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Those aggregations were reverted when docker and hardware could not carry the flag. Both types are now egress attributable, so their checks can be degraded and their totals, bucket counts and latest-check reads must exclude them like every other aggregate. 🔧 --- .../domain/checks/check.docker.aggregation.ts | 7 ++++++ .../checks/check.hardware.aggregations.ts | 4 +++ .../checksRepositoryEgressExclusion.test.ts | 25 ++++++++++++++----- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/server/src/domain/checks/check.docker.aggregation.ts b/server/src/domain/checks/check.docker.aggregation.ts index 37d6d8e87..225d9a68c 100644 --- a/server/src/domain/checks/check.docker.aggregation.ts +++ b/server/src/domain/checks/check.docker.aggregation.ts @@ -1,5 +1,6 @@ import CheckModel from "@/domain/checks/check.model.js"; import { DockerContainerStatsBucket, DockerStatsBucket } from "@/domain/checks/check.type.js"; +import { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.query.js"; import mongoose from "mongoose"; type DateRange = { start: Date; end: Date }; @@ -9,6 +10,7 @@ export const getDockerTotalChecks = async (monitorId: string, dates: DateRange): "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }); export const getDockerUpChecks = async (monitorId: string, dates: DateRange): Promise<{ totalChecks: number }> => { @@ -16,6 +18,7 @@ export const getDockerUpChecks = async (monitorId: string, dates: DateRange): Pr "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, status: true, }); return { totalChecks: count }; @@ -28,6 +31,7 @@ export const getDockerStats = async (monitorId: string, dates: DateRange, dateSt "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }, }, { @@ -48,6 +52,7 @@ export const getDockerLatestCheck = async (monitorId: string) => CheckModel.findOne({ "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }) .sort({ createdAt: -1 }) .select("containers containerSummary createdAt") @@ -65,6 +70,7 @@ export const getDockerContainerStats = async ( "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, "containers.name": containerName, }, }, @@ -88,6 +94,7 @@ export const getDockerContainerLatestCheck = async (monitorId: string, container "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "docker", "containers.name": containerName, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }) .sort({ createdAt: -1 }) .select("containers createdAt") diff --git a/server/src/domain/checks/check.hardware.aggregations.ts b/server/src/domain/checks/check.hardware.aggregations.ts index 6c672b630..3e0ee2bd7 100644 --- a/server/src/domain/checks/check.hardware.aggregations.ts +++ b/server/src/domain/checks/check.hardware.aggregations.ts @@ -1,6 +1,7 @@ import { CheckModel } from "@/domain/checks/check.model.js"; import mongoose from "mongoose"; import type { HardwareDiskStats, HardwareNetStats } from "@/domain/checks/check.type.js"; +import { EXCLUDE_DEGRADED_EGRESS_MATCH } from "@/domain/checks/check.query.js"; type DateRange = { start: Date; end: Date }; type HardwareUpChecks = { totalChecks: number }; @@ -70,6 +71,7 @@ export const getHardwareTotalChecks = async (monitorId: string, dates: DateRange "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "hardware", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }); }; @@ -78,6 +80,7 @@ export const getHardwareUpChecks = async (monitorId: string, dates: DateRange): "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "hardware", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, status: true, }); return { totalChecks: count }; @@ -95,6 +98,7 @@ export const getHardwareStats = async (monitorId: string, dates: DateRange, date "metadata.monitorId": new mongoose.Types.ObjectId(monitorId), "metadata.type": "hardware", createdAt: { $gte: dates.start, $lte: dates.end }, + ...EXCLUDE_DEGRADED_EGRESS_MATCH, }, }, { $sort: { createdAt: 1 } }, diff --git a/server/test/integration/checksRepositoryEgressExclusion.test.ts b/server/test/integration/checksRepositoryEgressExclusion.test.ts index 0f0950928..f7ac18daa 100644 --- a/server/test/integration/checksRepositoryEgressExclusion.test.ts +++ b/server/test/integration/checksRepositoryEgressExclusion.test.ts @@ -3,7 +3,13 @@ import mongoose from "mongoose"; import { MongoMemoryServer } from "mongodb-memory-server"; import MongoChecksRepository from "../../src/domain/checks/check.repository.mongo.ts"; import { CheckModel } from "../../src/domain/checks/check.model.ts"; -import type { Check, DockerChecksResult, PageSpeedChecksResult, UptimeChecksResult } from "../../src/domain/checks/check.type.ts"; +import type { + Check, + DockerChecksResult, + HardwareChecksResult, + PageSpeedChecksResult, + UptimeChecksResult, +} from "../../src/domain/checks/check.type.ts"; import { NETWORK_ERROR } from "../../src/types/network.ts"; import type { ILogger } from "../../src/utils/logger.ts"; import { createMockLogger } from "../helpers/createMockLogger.ts"; @@ -181,18 +187,25 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { expect(resolve.checks.map((check) => check.egressStatus).sort()).toEqual([undefined, "degraded"].sort()); }); - it("leaves the docker aggregates alone, since a docker check is never attributed to egress", async () => { + it("excludes degraded checks from the docker and hardware aggregates", async () => { const dockerMonitor = new mongoose.Types.ObjectId(); + const hardwareMonitor = new mongoose.Types.ObjectId(); const dockerMeta = { monitorId: dockerMonitor, teamId: TEAM_ID, type: "docker" }; + const hardwareMeta = { monitorId: hardwareMonitor, teamId: TEAM_ID, type: "hardware" }; - // The schema accepts the field on any type, so this pins the scope decision rather than the schema: - // the producer never sets it for docker or hardware, so their aggregations must not filter on it. + // A Capture agent is reached by an outbound HTTP request and a docker daemon can be reached over TCP, + // so both are egress attributable and their checks can carry the flag. await seedCheck({ metadata: dockerMeta }); await seedDegradedFailure({ metadata: dockerMeta }); + await seedCheck({ metadata: hardwareMeta }); + await seedDegradedFailure({ metadata: hardwareMeta }); const docker = (await repo.findByDateRangeAndMonitorId(dockerMonitor.toString(), "day", { type: "docker" })) as DockerChecksResult; + const hardware = (await repo.findByDateRangeAndMonitorId(hardwareMonitor.toString(), "day", { type: "hardware" })) as HardwareChecksResult; - expect(docker.aggregateData.totalChecks).toBe(2); - expect(docker.aggregate[0]).toMatchObject({ upCount: 1, totalCount: 2 }); + expect(docker.aggregateData.totalChecks).toBe(1); + expect(docker.aggregate[0]).toMatchObject({ upCount: 1, totalCount: 1 }); + expect(hardware.aggregateData.totalChecks).toBe(1); + expect(hardware.upChecks.totalChecks).toBe(1); }); }); From 13799af721ed4ffa206fad7948f77e11a13c9d23 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Thu, 17 Sep 2026 22:08:14 +0100 Subject: [PATCH 7/9] Read unevaluated checks by pending entry in the exclusion test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit findUnevaluatedByMonitorId now takes the job's pendingChecks rather than a timestamp, so the test builds those entries from the ids createChecks returns. 🔧 --- .../test/integration/checksRepositoryEgressExclusion.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/test/integration/checksRepositoryEgressExclusion.test.ts b/server/test/integration/checksRepositoryEgressExclusion.test.ts index f7ac18daa..c1f2376df 100644 --- a/server/test/integration/checksRepositoryEgressExclusion.test.ts +++ b/server/test/integration/checksRepositoryEgressExclusion.test.ts @@ -79,9 +79,10 @@ describe("MongoChecksRepository degraded-egress exclusion", () => { }); it("round-trips egressStatus through createChecks and the unevaluated-checks read, leaving it absent when unset", async () => { - await repo.createChecks([makeCheck({ egressStatus: "degraded" }), makeCheck({ egressStatus: "ok" }), makeCheck()]); + const created = await repo.createChecks([makeCheck({ egressStatus: "degraded" }), makeCheck({ egressStatus: "ok" }), makeCheck()]); + const pending = created.map((check) => ({ checkId: check.id as string, createdAt: new Date(check.createdAt as string).getTime() })); - const checks = await repo.findUnevaluatedByMonitorId(MONITOR_ID.toString(), BUCKET_TIME.getTime() - 1); + const checks = await repo.findUnevaluatedByMonitorId(MONITOR_ID.toString(), pending); // The evaluator runs from this read, so the flag must survive persistence for the short-circuit to fire. expect(checks.map((check) => check.egressStatus)).toEqual(["degraded", "ok", undefined]); From b55560e2f1db7b6c5cd618c384301e31f1e10c09 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Sat, 19 Sep 2026 14:43:11 +0100 Subject: [PATCH 8/9] Report whether the peer answered from every provider that can tell MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An egress spell masked a service on the surviving network going down: with `peerResponded` set only by the DNS and gRPC providers, every other failure fell back to the HTTP-status heuristic, which answers "nothing replied" for anything that is not speaking HTTP. A refused connection on the local network was read as the instance's own loss of egress, so the check was recorded as degraded and the monitor stayed up. `peerAnsweredFromError` holds the errno codes that require the target's own stack to have replied — a refusal, a reset, a TLS handshake that got as far as a certificate. Http, Port, WebSocket and Docker use it; Http also treats any status code as an answer, and Docker any status from the daemon. Hardware and PageSpeed go through Http and inherit it. `true` is the claim that costs something, since it blames the target, so it is only set on evidence. Two cases stay conservative and are commented as such: a ping that draws no echo reply, and a gamedig query that fails, neither of which can be told apart from the instance losing its egress. Nor can a proxied HTTP request that fails with no response, where the refusal may be the proxy's. 🔧 --- server/src/service/network/DockerProvider.ts | 18 +++- server/src/service/network/GameProvider.ts | 3 + server/src/service/network/HttpProvider.ts | 7 +- server/src/service/network/PingProvider.ts | 3 + server/src/service/network/PortProvider.ts | 6 +- .../src/service/network/WebSocketProvider.ts | 6 +- server/src/service/network/utils.ts | 24 +++++ .../providers/network/dockerProvider.test.ts | 39 ++++++++ .../providers/network/httpProvider.test.ts | 88 +++++++++++++++++++ .../providers/network/pingProvider.test.ts | 19 ++++ .../providers/network/portProvider.test.ts | 38 ++++++++ .../test/unit/providers/network/utils.test.ts | 36 +++++++- .../network/webSocketProvider.test.ts | 38 ++++++++ 13 files changed, 318 insertions(+), 7 deletions(-) diff --git a/server/src/service/network/DockerProvider.ts b/server/src/service/network/DockerProvider.ts index aa8070649..7ea35c13b 100644 --- a/server/src/service/network/DockerProvider.ts +++ b/server/src/service/network/DockerProvider.ts @@ -23,7 +23,7 @@ import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js"; import { ILogger } from "@/utils/logger.js"; import { AppError } from "@/utils/AppError.js"; import Dockerode from "dockerode"; -import { timeRequest } from "@/service/network/utils.js"; +import { timeRequest, peerAnsweredFromError } from "@/service/network/utils.js"; import { NETWORK_ERROR } from "@/types/network.js"; import { IEncryptionService } from "@/service/encryption/encryptionService.js"; import { DOCKER_TLS_URL, isDockerTlsUrl } from "@/utils/dockerHost.js"; @@ -315,12 +315,19 @@ export class DockerProvider implements IStatusProvider { return rawLogs.filter((log) => log !== null); }; - private failed = (monitor: Monitor, code: number, message: string, responseTime: number): MonitorStatusResponse => ({ + private failed = ( + monitor: Monitor, + code: number, + message: string, + responseTime: number, + peerResponded = false + ): MonitorStatusResponse => ({ monitorId: monitor.id, teamId: monitor.teamId, type: monitor.type, status: false, code, + peerResponded, message, responseTime, payload: null, @@ -337,13 +344,17 @@ export class DockerProvider implements IStatusProvider { if (error) { let message = "Docker host is unreachable"; let code = NETWORK_ERROR; + // A status code from the daemon is the daemon answering; otherwise only a refusal or a reset + // proves we reached the host, and anything else could be the instance's own loss of egress. + let peerResponded = peerAnsweredFromError(error); if (this.isDockerError(error)) { code = error.statusCode ?? NETWORK_ERROR; message = error.json?.message ?? error.reason ?? error.message; + peerResponded = peerResponded || error.statusCode !== undefined; } else if (error instanceof Error) { message = error.message; } - return this.failed(monitor, code, message, responseTime); + return this.failed(monitor, code, message, responseTime, peerResponded); } const summaries = await docker.listContainers({ all: true }); @@ -362,6 +373,7 @@ export class DockerProvider implements IStatusProvider { type: monitor.type, status: true, code: 200, + peerResponded: true, message: "Docker host is reachable", responseTime, payload: { containers, summary, logs }, diff --git a/server/src/service/network/GameProvider.ts b/server/src/service/network/GameProvider.ts index 5ccfaee6e..904f304f7 100644 --- a/server/src/service/network/GameProvider.ts +++ b/server/src/service/network/GameProvider.ts @@ -43,6 +43,9 @@ export class GameProvider implements IStatusProvider { }); if (!state) { + // gamedig reports a failed query the same way whichever leg failed, so `peerResponded` is left + // unset and a failure is put to the egress probe. A game server that stops answering during a + // spell is therefore recorded as degraded rather than down. return { monitorId: id, teamId: teamId, diff --git a/server/src/service/network/HttpProvider.ts b/server/src/service/network/HttpProvider.ts index 1ef35f474..9fa705b55 100644 --- a/server/src/service/network/HttpProvider.ts +++ b/server/src/service/network/HttpProvider.ts @@ -6,7 +6,7 @@ import { MonitorStatusResponse } from "@/types/network.js"; import { Agent as HttpsAgent } from "https"; import { Agent as HttpAgent } from "http"; import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js"; -import { isStatusUp } from "@/service/network/utils.js"; +import { isStatusUp, peerAnsweredFromError } from "@/service/network/utils.js"; import { NETWORK_ERROR } from "@/types/network.js"; import CacheableLookup from "cacheable-lookup"; import { HttpProxyAgent, HttpsProxyAgent } from "hpagent"; @@ -107,6 +107,7 @@ export class HttpProvider implements IStatusProvider { type: monitor.type, status: statusUp, code: statusCode, + peerResponded: true, message, responseTime, timings, @@ -123,6 +124,7 @@ export class HttpProvider implements IStatusProvider { type: monitor.type, status: false, code: statusCode, + peerResponded: true, message: "Response is not JSON", responseTime, timings, @@ -149,6 +151,7 @@ export class HttpProvider implements IStatusProvider { type: monitor.type, status: statusUp && matchResult.ok, code: statusCode, + peerResponded: true, message: matchResult.ok ? message : matchResult.message, responseTime, timings, @@ -172,6 +175,7 @@ export class HttpProvider implements IStatusProvider { type: monitor.type, status: false, code: statusCode ?? NETWORK_ERROR, + peerResponded: statusCode !== undefined || (!ctx?.proxyUrl && peerAnsweredFromError(error)), message: message, responseTime, timings: error.timings, @@ -196,6 +200,7 @@ export class HttpProvider implements IStatusProvider { type: monitor.type, status: false, code: NETWORK_ERROR, + peerResponded: !ctx?.proxyUrl && peerAnsweredFromError(error), message: error instanceof Error ? error.message : String(error), responseTime: 0, payload: null as T, diff --git a/server/src/service/network/PingProvider.ts b/server/src/service/network/PingProvider.ts index 7bcde8c7e..7a6abb33a 100644 --- a/server/src/service/network/PingProvider.ts +++ b/server/src/service/network/PingProvider.ts @@ -58,6 +58,9 @@ export class PingProvider implements IStatusProvider { type: monitor.type, status: response.alive ?? false, code: response.alive ? 200 : 5000, + // An echo reply is the host answering. Its absence is silence, which is indistinguishable + // from the instance having lost its egress, so the egress probe decides that case. + peerResponded: response.alive ?? false, message: response.alive ? "Success" : "Ping failed", responseTime, payload: response, diff --git a/server/src/service/network/PortProvider.ts b/server/src/service/network/PortProvider.ts index 692ca40af..bb21d0799 100644 --- a/server/src/service/network/PortProvider.ts +++ b/server/src/service/network/PortProvider.ts @@ -2,7 +2,7 @@ import { IStatusProvider } from "@/service/network/IStatusProvider.js"; import { PortStatusPayload, MonitorStatusResponse } from "@/types/network.js"; import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js"; import { AppError } from "@/utils/AppError.js"; -import { timeRequest } from "@/service/network/utils.js"; +import { timeRequest, peerAnsweredFromError } from "@/service/network/utils.js"; import { NETWORK_ERROR } from "@/types/network.js"; import * as net from "net"; type NetType = typeof net; @@ -59,6 +59,9 @@ export class PortProvider implements IStatusProvider { type: monitor.type, status: false, code: NETWORK_ERROR, + // A refused or reset connection came from the host itself; a timeout or an unreachable + // network did not, and could equally be the instance's own loss of egress. + peerResponded: peerAnsweredFromError(error), message: errorMessage, responseTime: responseTime, timings: undefined, @@ -72,6 +75,7 @@ export class PortProvider implements IStatusProvider { type: monitor.type, status: true, code: 200, + peerResponded: true, message: "Port check successful", responseTime: responseTime, timings: undefined, diff --git a/server/src/service/network/WebSocketProvider.ts b/server/src/service/network/WebSocketProvider.ts index d27106989..be9873d41 100644 --- a/server/src/service/network/WebSocketProvider.ts +++ b/server/src/service/network/WebSocketProvider.ts @@ -2,7 +2,7 @@ import { IStatusProvider } from "@/service/network/IStatusProvider.js"; import { WebSocketStatusPayload, MonitorStatusResponse } from "@/types/network.js"; import { Monitor, MonitorType } from "@/domain/monitors/monitor.type.js"; import { AppError } from "@/utils/AppError.js"; -import { timeRequest } from "@/service/network/utils.js"; +import { timeRequest, peerAnsweredFromError } from "@/service/network/utils.js"; import { NETWORK_ERROR } from "@/types/network.js"; import type WebSocket from "ws"; @@ -63,6 +63,9 @@ export class WebSocketProvider implements IStatusProvider= 200 && statusCode < 300) || customUpCodes.includes(statusCode); }; + +// Errno codes that can only arise once the target's own stack has answered: it refused the connection, reset +// an established one, or got far enough into a TLS handshake to present a certificate. Every other failure — +// a timeout, an unreachable network, a name that would not resolve — looks the same whether the target is +// down or the instance has lost its egress, so those are left for the egress probe to decide. +const PEER_ANSWERED_ERROR_CODES: ReadonlySet = new Set([ + "ECONNREFUSED", + "ECONNRESET", + "EPIPE", + "EPROTO", + "CERT_HAS_EXPIRED", + "CERT_NOT_YET_VALID", + "DEPTH_ZERO_SELF_SIGNED_CERT", + "SELF_SIGNED_CERT_IN_CHAIN", + "UNABLE_TO_GET_ISSUER_CERT_LOCALLY", + "UNABLE_TO_VERIFY_LEAF_SIGNATURE", + "ERR_TLS_CERT_ALTNAME_INVALID", + "ERR_SSL_WRONG_VERSION_NUMBER", +]); + +export const peerAnsweredFromError = (error: unknown): boolean => { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return typeof code === "string" && PEER_ANSWERED_ERROR_CODES.has(code); +}; diff --git a/server/test/unit/providers/network/dockerProvider.test.ts b/server/test/unit/providers/network/dockerProvider.test.ts index 58b2d7903..2259a1e96 100644 --- a/server/test/unit/providers/network/dockerProvider.test.ts +++ b/server/test/unit/providers/network/dockerProvider.test.ts @@ -697,4 +697,43 @@ describe("DockerProvider", () => { await expect(provider.handle(makeMonitor())).rejects.toThrow("Error performing Docker request"); }); }); + + // ── Peer reachability ──────────────────────────────────────────────────── + // A daemon that answers with an error has been reached; only silence can be the instance's own egress. + + describe("peer reachability", () => { + it("reports the peer as having answered when the daemon replies with a status code", async () => { + const dockerErr = Object.assign(new Error("base"), { statusCode: 500, json: { message: "engine exploded" } }); + const { provider } = setup({ ping: jest.fn().mockRejectedValue(dockerErr) }); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.peerResponded).toBe(true); + }); + + it.each([["ECONNREFUSED"], ["ECONNRESET"]])("reports the peer as having answered for %s", async (code) => { + const { provider } = setup({ ping: jest.fn().mockRejectedValue(Object.assign(new Error("connect failed"), { code })) }); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + + it.each([["ETIMEDOUT"], ["EHOSTUNREACH"], ["ENOTFOUND"]])("reports the peer as silent for %s", async (code) => { + const { provider } = setup({ ping: jest.fn().mockRejectedValue(Object.assign(new Error("connect failed"), { code })) }); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as having answered when the daemon is reachable", async () => { + const { provider } = setup({}); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + }); }); diff --git a/server/test/unit/providers/network/httpProvider.test.ts b/server/test/unit/providers/network/httpProvider.test.ts index 4e559ab06..928c46f43 100644 --- a/server/test/unit/providers/network/httpProvider.test.ts +++ b/server/test/unit/providers/network/httpProvider.test.ts @@ -643,4 +643,92 @@ describe("HttpProvider", () => { expect(result.message).toBe("Response code 500 (Internal Server Error)"); }); }); + + // ── Peer reachability ──────────────────────────────────────────────────── + // Any HTTP status proves the target answered. Without one, only an errno that requires the peer's own stack + // to have replied counts — and never through a proxy, where the refusal may be the proxy's rather than the + // target's, which is the instance's own egress path failing. + + describe("peer reachability", () => { + it("reports the peer as having answered on a successful response", async () => { + mockGot.mockResolvedValue(makeGotResponse()); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + + it("reports the peer as having answered for a failing status code", async () => { + const err = new HTTPError("Not Found", { statusCode: 404, body: "", headers: {} }, { phases: { total: 12 } }); + mockGot.mockRejectedValue(err); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(404); + expect(result.peerResponded).toBe(true); + }); + + it.each([["ECONNREFUSED"], ["ECONNRESET"], ["CERT_HAS_EXPIRED"], ["DEPTH_ZERO_SELF_SIGNED_CERT"]])( + "reports the peer as having answered for %s with no response", + async (code) => { + const err = new RequestError("request failed"); + (err as any).code = code; + mockGot.mockRejectedValue(err); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor()); + + expect(result.code).toBe(NETWORK_ERROR); + expect(result.peerResponded).toBe(true); + } + ); + + it.each([["ETIMEDOUT"], ["ENOTFOUND"], ["EAI_AGAIN"], ["ENETUNREACH"]])("reports the peer as silent for %s", async (code) => { + const err = new RequestError("request failed"); + (err as any).code = code; + mockGot.mockRejectedValue(err); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as silent when a proxy refuses the connection", async () => { + const err = new RequestError("connect ECONNREFUSED"); + (err as any).code = "ECONNREFUSED"; + mockGot.mockRejectedValue(err); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor(), { proxyUrl: "http://proxy.example.com:8080" }); + + // The refusal came from the proxy, which is part of this instance's route out. + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as having answered through a proxy when a status code came back", async () => { + const err = new RequestError("Response code 500 (Internal Server Error)"); + (err as any).code = "ECONNRESET"; + (err as any).response = { statusCode: 500, body: "", headers: {} }; + (err as any).timings = { phases: { total: 25 } }; + mockGot.mockRejectedValue(err); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor(), { proxyUrl: "http://proxy.example.com:8080" }); + + expect(result.peerResponded).toBe(true); + }); + + it("reports the peer as silent for a generic error carrying no code", async () => { + mockGot.mockRejectedValue(new Error("something broke")); + const { provider } = createProvider(); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + }); }); diff --git a/server/test/unit/providers/network/pingProvider.test.ts b/server/test/unit/providers/network/pingProvider.test.ts index 150472925..eaa6227b5 100644 --- a/server/test/unit/providers/network/pingProvider.test.ts +++ b/server/test/unit/providers/network/pingProvider.test.ts @@ -155,4 +155,23 @@ describe("PingProvider", () => { await expect(provider.handle(makeMonitor())).rejects.toThrow("42"); }); + + // ── Peer reachability ──────────────────────────────────────────────────── + + it("reports the peer as having answered when an echo reply comes back", async () => { + const provider = new PingProvider(createMockPing({ alive: true }) as any, net); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); + + it("reports the peer as silent when no echo reply comes back", async () => { + // Silence is indistinguishable from the instance losing its egress, so the egress probe decides. + const provider = new PingProvider(createMockPing({ alive: false }) as any, net); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); }); diff --git a/server/test/unit/providers/network/portProvider.test.ts b/server/test/unit/providers/network/portProvider.test.ts index fbbfb5767..caaf9ad3d 100644 --- a/server/test/unit/providers/network/portProvider.test.ts +++ b/server/test/unit/providers/network/portProvider.test.ts @@ -267,4 +267,42 @@ describe("PortProvider", () => { await expect(provider.handle(monitor)).rejects.toThrow("42"); }); + + // ── Peer reachability ──────────────────────────────────────────────────── + // A check is only recorded as degraded when nothing answered, so a service on the local network that + // refuses connections has to stay distinguishable from the instance losing its own egress. + + it.each([["ECONNREFUSED"], ["ECONNRESET"]])("reports the peer as having answered for %s", async (code) => { + const provider = new PortProvider(createErrorNet(Object.assign(new Error("connect failed"), { code }))); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(NETWORK_ERROR); + expect(result.peerResponded).toBe(true); + }); + + it.each([["ETIMEDOUT"], ["EHOSTUNREACH"], ["ENETUNREACH"], ["ENOTFOUND"]])("reports the peer as silent for %s", async (code) => { + const provider = new PortProvider(createErrorNet(Object.assign(new Error("connect failed"), { code }))); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as silent on a connection timeout", async () => { + const provider = new PortProvider(createTimeoutNet()); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as having answered when the port accepts the connection", async () => { + const provider = new PortProvider(createSuccessNet()); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); }); diff --git a/server/test/unit/providers/network/utils.test.ts b/server/test/unit/providers/network/utils.test.ts index a83e14cff..5a0d33972 100644 --- a/server/test/unit/providers/network/utils.test.ts +++ b/server/test/unit/providers/network/utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "@jest/globals"; -import { timeRequest, isStatusUp } from "../../../../src/service/network/utils.ts"; +import { timeRequest, isStatusUp, peerAnsweredFromError } from "../../../../src/service/network/utils.ts"; import { NETWORK_ERROR } from "../../../../src/types/network.ts"; describe("network utils", () => { @@ -39,6 +39,40 @@ describe("network utils", () => { }); }); + // The egress check blames the instance only when nothing answered, so this helper's `true` must never be a + // guess: every code in it requires the peer's own stack to have replied. + describe("peerAnsweredFromError", () => { + it.each([ + ["ECONNREFUSED", "the host refused the connection"], + ["ECONNRESET", "the host reset an established connection"], + ["EPIPE", "the host closed a connection we were writing to"], + ["EPROTO", "a protocol error on a connection that was made"], + ["CERT_HAS_EXPIRED", "the host presented a certificate"], + ["DEPTH_ZERO_SELF_SIGNED_CERT", "the host presented a certificate"], + ["SELF_SIGNED_CERT_IN_CHAIN", "the host presented a certificate"], + ["UNABLE_TO_GET_ISSUER_CERT_LOCALLY", "the host presented a certificate"], + ["UNABLE_TO_VERIFY_LEAF_SIGNATURE", "the host presented a certificate"], + ["ERR_TLS_CERT_ALTNAME_INVALID", "the host presented a certificate"], + ["CERT_NOT_YET_VALID", "the host presented a certificate"], + ["ERR_SSL_WRONG_VERSION_NUMBER", "the host spoke back, badly"], + ])("reports the peer as having answered for %s", (code) => { + expect(peerAnsweredFromError(Object.assign(new Error("failed"), { code }))).toBe(true); + }); + + it.each([["ETIMEDOUT"], ["ENOTFOUND"], ["EAI_AGAIN"], ["EHOSTUNREACH"], ["ENETUNREACH"], ["ECONNABORTED"], ["ENETDOWN"]])( + "reports the peer as silent for %s", + (code) => { + expect(peerAnsweredFromError(Object.assign(new Error("failed"), { code }))).toBe(false); + } + ); + + it("reports the peer as silent when the error carries no code", () => { + expect(peerAnsweredFromError(new Error("Connection timeout"))).toBe(false); + expect(peerAnsweredFromError(undefined)).toBe(false); + expect(peerAnsweredFromError("not an error")).toBe(false); + }); + }); + describe("isStatusUp", () => { it("returns true for 2xx status codes", () => { expect(isStatusUp(200)).toBe(true); diff --git a/server/test/unit/providers/network/webSocketProvider.test.ts b/server/test/unit/providers/network/webSocketProvider.test.ts index b765b2f74..13dc5190b 100644 --- a/server/test/unit/providers/network/webSocketProvider.test.ts +++ b/server/test/unit/providers/network/webSocketProvider.test.ts @@ -191,4 +191,42 @@ describe("WebSocketProvider", () => { ); }); }); + + // ── Peer reachability ──────────────────────────────────────────────────── + // `ws` rejects a non-101 handshake with a plain Error, so only the errno cases can be told apart from the + // instance losing its egress. That leaves a server answering the handshake with an HTTP error indistinct. + + const createErrorWS = (error: Error) => + jest.fn().mockImplementation(() => ({ + on: jest.fn((event: string, cb: Function) => { + if (event === "error") process.nextTick(() => cb(error)); + }), + close: jest.fn(), + })) as any; + + it.each([["ECONNREFUSED"], ["ECONNRESET"]])("reports the peer as having answered for %s", async (code) => { + const provider = new WebSocketProvider(createErrorWS(Object.assign(new Error("connect failed"), { code }))); + + const result = await provider.handle(makeMonitor()); + + expect(result.status).toBe(false); + expect(result.code).toBe(NETWORK_ERROR); + expect(result.peerResponded).toBe(true); + }); + + it.each([["ETIMEDOUT"], ["ENOTFOUND"], ["EHOSTUNREACH"]])("reports the peer as silent for %s", async (code) => { + const provider = new WebSocketProvider(createErrorWS(Object.assign(new Error("connect failed"), { code }))); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(false); + }); + + it("reports the peer as having answered when the socket opens", async () => { + const provider = new WebSocketProvider(createMockWS("open")); + + const result = await provider.handle(makeMonitor()); + + expect(result.peerResponded).toBe(true); + }); }); From 1b3b8e531245ea8b48cfd9b6ded167dae3492404 Mon Sep 17 00:00:00 2001 From: Craig Lawson Date: Sat, 19 Sep 2026 14:43:16 +0100 Subject: [PATCH 9/9] 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({