DC-086 service-status flicker fix — asymmetric hysteresis
Dashboard badges perpetually flip green/red for a few seconds at a time, never stable. Root cause: health-checker emitted 'status-check' on every probe (every 30s) and dashboard-ws forwarded every one as 'status-change' to the browser with no diff; live-events.js then unconditionally called setBadge(). A single transient 5xx (Caddy reload, container restart, TLS handshake blip) flipped the badge and the next green probe flipped back. Fix: _computeDisplayedStatus applies asymmetric hysteresis — DOWN_THRESHOLD (default 2, env-tunable HEALTH_DOWN_THRESHOLD) consecutive probes that disagree with the displayed 'up' state flip to red; UP_THRESHOLD (default 1, HEALTH_UP_THRESHOLD) flips back to green. History and consecutiveFailures still record every raw probe so postmortem analysis is unchanged. Only the SSE broadcast is filtered. getCurrentStatus now returns the displayed status so a page reload shows the same badge as the live stream. 10 new tests cover first-emit, same-status-dedup, the actual flicker bug (one-down-then-up keeps green), two-down flips red, one-up recovers fast, long-steady-green produces exactly one emit, and env-var tuning. All 63 existing health-checker tests still pass. Full suite: 2484/2484.
This commit is contained in:
Hermes
@@ -33,12 +33,31 @@ const MAX_CHECK_INTERVAL = parseInt(process.env.HEALTH_CHECK_MAX_INTERVAL || '30
|
||||
const MAX_ENTRIES_PER_SERVICE = parseInt(process.env.HEALTH_MAX_ENTRIES || '500', 10); // Cap to prevent disk explosion
|
||||
const HISTORY_RETENTION_DAYS = parseInt(process.env.HEALTH_HISTORY_RETENTION || '30', 10);
|
||||
|
||||
// DC-086: hysteresis thresholds for badge display.
|
||||
// The raw probe result can flap on a single transient blip (Caddy reload,
|
||||
// container CPU steal, network hiccup, mid-flight TLS handshake). Showing
|
||||
// every probe result as-is to the dashboard creates the "perpetual flicker"
|
||||
// UX. Asymmetric thresholds: going red is slow (don't false-alarm), going
|
||||
// green is fast (don't keep showing red after recovery).
|
||||
// - DOWN_THRESHOLD = N consecutive "down" probes before the badge flips to red
|
||||
// - UP_THRESHOLD = N consecutive "up" probes before the badge flips back to green
|
||||
// Single probe flips to green on purpose — false-positive-green is much less
|
||||
// painful than perpetual-red (operators notice red, ignore green).
|
||||
const DOWN_THRESHOLD = Math.max(1, parseInt(process.env.HEALTH_DOWN_THRESHOLD || '2', 10));
|
||||
const UP_THRESHOLD = Math.max(1, parseInt(process.env.HEALTH_UP_THRESHOLD || '1', 10));
|
||||
|
||||
class HealthChecker extends EventEmitter {
|
||||
constructor() {
|
||||
super();
|
||||
this.config = this.loadConfig();
|
||||
this.history = this.loadHistory();
|
||||
this.currentStatus = new Map();
|
||||
// DC-086: the status the dashboard SHOULD display (post-hysteresis).
|
||||
// Distinct from currentStatus, which is the latest raw probe result.
|
||||
this.displayedStatus = new Map();
|
||||
// DC-086: counter of consecutive healthy/unhealthy probes since the
|
||||
// last displayed-status change. Reset to 0 whenever displayed status flips.
|
||||
this.consecutiveSinceChange = new Map();
|
||||
this.incidents = [];
|
||||
this.checking = false;
|
||||
this.checkInterval = null;
|
||||
@@ -273,27 +292,106 @@ class HealthChecker extends EventEmitter {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Compute the displayed status for a service given the latest raw probe
|
||||
* result. Applies asymmetric hysteresis:
|
||||
* - Going DOWN: requires DOWN_THRESHOLD (default 2) consecutive "down"
|
||||
* probes since the last display-state change. A single blip keeps the
|
||||
* badge green.
|
||||
* - Going UP: requires UP_THRESHOLD (default 1) consecutive "up" probes.
|
||||
* Any single "up" after a down streak flips back to green so the badge
|
||||
* doesn't linger red after the service has recovered.
|
||||
*
|
||||
* Returns the displayed status object (same shape as the raw status) so
|
||||
* recordStatus can use it both for the displayed map and as the broadcast
|
||||
* payload when the displayed status actually changes.
|
||||
*/
|
||||
_computeDisplayedStatus(serviceId, rawStatus) {
|
||||
const currentDisplayed = this.displayedStatus.get(serviceId);
|
||||
const previousStatus = currentDisplayed ? currentDisplayed.status : null;
|
||||
|
||||
// If no prior state, accept the raw probe as-is (first-check bootstrap).
|
||||
if (!previousStatus) {
|
||||
return rawStatus;
|
||||
}
|
||||
|
||||
// Probe agrees with current displayed → no change, reset the counter so
|
||||
// a brief blip doesn't accumulate against the displayed state.
|
||||
if (rawStatus.status === previousStatus) {
|
||||
this.consecutiveSinceChange.set(serviceId, 0);
|
||||
return currentDisplayed;
|
||||
}
|
||||
|
||||
// Probe disagrees with displayed. Bump the streak counter — this counts
|
||||
// CONSECUTIVE probes that disagree with what's shown, regardless of
|
||||
// whether the raw value itself changed between probes. That's what
|
||||
// makes "down, down" flip after threshold but "down, up, down" not flip.
|
||||
const prev = this.consecutiveSinceChange.get(serviceId) || 0;
|
||||
const next = prev + 1;
|
||||
|
||||
if (rawStatus.status === 'down') {
|
||||
// Going DOWN: need DOWN_THRESHOLD consecutive probes that disagree
|
||||
// with the displayed "up" state.
|
||||
if (previousStatus === 'up' && next < DOWN_THRESHOLD) {
|
||||
this.consecutiveSinceChange.set(serviceId, next);
|
||||
return currentDisplayed;
|
||||
}
|
||||
// Threshold met (or already down) — flip to red.
|
||||
this.consecutiveSinceChange.set(serviceId, 0);
|
||||
return rawStatus;
|
||||
}
|
||||
|
||||
// rawStatus.status === 'up' (must be — the equal-to-displayed case above
|
||||
// already returned). Going UP after a down streak: need UP_THRESHOLD.
|
||||
if (previousStatus === 'down' && next < UP_THRESHOLD) {
|
||||
this.consecutiveSinceChange.set(serviceId, next);
|
||||
return currentDisplayed;
|
||||
}
|
||||
this.consecutiveSinceChange.set(serviceId, 0);
|
||||
return rawStatus;
|
||||
}
|
||||
|
||||
/**
|
||||
* Record service status
|
||||
*
|
||||
* DC-086: history + consecutiveFailures are updated for EVERY probe
|
||||
* (operators want full probe history for postmortems). The dashboard's
|
||||
* `status-check` event is only emitted when the DISPLAYED status changes,
|
||||
* so the badge stops re-rendering on every probe.
|
||||
*/
|
||||
recordStatus(serviceId, status) {
|
||||
// Update current status
|
||||
// Update current (raw) status — used by checkForIncidents and history.
|
||||
this.currentStatus.set(serviceId, status);
|
||||
|
||||
// Add to history
|
||||
// Add raw probe to history (full fidelity — operators rely on this).
|
||||
if (!this.history[serviceId]) {
|
||||
this.history[serviceId] = [];
|
||||
}
|
||||
|
||||
this.history[serviceId].push(status);
|
||||
|
||||
|
||||
// Cap entries to prevent unbounded growth (disk explosion fix)
|
||||
if (this.history[serviceId].length > MAX_ENTRIES_PER_SERVICE) {
|
||||
this.history[serviceId] = this.history[serviceId].slice(-MAX_ENTRIES_PER_SERVICE);
|
||||
}
|
||||
|
||||
// Emit status event
|
||||
this.emit('status-check', status);
|
||||
// Compute the post-hysteresis displayed status; only emit when it changes.
|
||||
// _computeDisplayedStatus compares the raw probe against the DISPLAYED
|
||||
// status (not the previous raw status), so the "consecutive since
|
||||
// change" counter doesn't depend on the order of writes here.
|
||||
const displayed = this._computeDisplayedStatus(serviceId, status);
|
||||
const previousDisplayed = this.displayedStatus.get(serviceId);
|
||||
const displayChanged =
|
||||
!previousDisplayed || previousDisplayed.status !== displayed.status;
|
||||
|
||||
this.displayedStatus.set(serviceId, displayed);
|
||||
|
||||
if (displayChanged) {
|
||||
// Emit with the displayed status so the dashboard renders the same
|
||||
// state the hysteresis just decided. The raw probe result is still
|
||||
// in `history` and `currentStatus` for anyone who wants it.
|
||||
this.emit('status-check', displayed);
|
||||
}
|
||||
|
||||
// Save history periodically
|
||||
if (Math.random() < 0.05) { // 5% chance (every ~20 checks)
|
||||
@@ -445,19 +543,29 @@ class HealthChecker extends EventEmitter {
|
||||
}
|
||||
|
||||
/**
|
||||
* Get current status for all services
|
||||
* Get current status for all services.
|
||||
*
|
||||
* DC-086: returns the DISPLAYED status (post-hysteresis), not the latest
|
||||
* raw probe. A page reload should show the same badge state the live
|
||||
* SSE stream is currently showing — otherwise an operator who reloads
|
||||
* the page after a single blip sees red even though the hysteresis kept
|
||||
* the badge green for them.
|
||||
*/
|
||||
getCurrentStatus() {
|
||||
const result = {};
|
||||
|
||||
for (const [serviceId, status] of this.currentStatus.entries()) {
|
||||
|
||||
for (const [serviceId, rawStatus] of this.currentStatus.entries()) {
|
||||
const config = this.config.services[serviceId];
|
||||
const uptime24h = this.calculateUptime(serviceId, 24);
|
||||
const uptime7d = this.calculateUptime(serviceId, 168);
|
||||
const avgResponseTime = this.calculateAverageResponseTime(serviceId, 24);
|
||||
|
||||
|
||||
// Prefer the displayed status if we've already computed one; fall back
|
||||
// to the raw probe on the very first call (before recordStatus has run).
|
||||
const displayed = this.displayedStatus.get(serviceId) || rawStatus;
|
||||
|
||||
result[serviceId] = {
|
||||
...status,
|
||||
...displayed,
|
||||
name: config?.name || serviceId,
|
||||
uptime: {
|
||||
'24h': uptime24h,
|
||||
@@ -467,7 +575,7 @@ class HealthChecker extends EventEmitter {
|
||||
sla: config?.sla
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user