observability: nothing detects an alert rule that can never fire #444

Öppen
öppnade 2026-08-19 06:33:32 +00:00 av supernaut · 0 kommentarer
Ägare

Follow-up to #442, which fixed one vacuous alert. This is about detecting the next one.

The gap

scripts/check-alert-rules.py (#439) proves the rule set renders and parses. It cannot prove a
rule can fire. A selector matching zero series does both cleanly, and StrictUndefined is
satisfied whenever the Jinja variables resolve.

That is not hypothetical. UnitMetricStale shipped in that state and stayed there until #442. It was
found by hand, by someone querying its matcher out of curiosity. Nothing would have found it
otherwise, and its stated job was watching the watchdog.

The signal already exists

vmalert publishes vmalert_alerting_rules_last_evaluation_series_fetched per rule. == 0 means that
rule's selectors matched nothing on the last evaluation.

Measured on the monitoring VM, before and after #442's apply:

When Rules loaded series_fetched == 0
Before 49 ["UnitMetricStale"]
After 51 none

So the metric found exactly the rule that was broken, and no others. It also confirms the blast
radius was one rule rather than a class of them.

#442's issue body was wrong about the cost of this. It said a check would need datasource access
from CI and would be its own design problem. It does not. The measurement above is a single instant
query, and the guard can be an ordinary alert rule.

Why a runtime alert beats a CI stage

A CI stage would only check at merge time. The runtime alert catches a strictly larger set, because a
rule does not have to ship broken to become broken:

  • A metric gets renamed and every matcher naming it silently stops selecting. That is the
    bitborg_* / gitborg_* rename hazard, already recorded in ADR 0039 §7b, where two textfile
    writers were half-renamed and dual-brand matchers were the only reason nothing blanked.
  • A textfile writer is removed or retired and its consumers go quiet rather than red.
  • A label's meaning changes upstream, as file= did here.

None of those are visible at merge time. All of them are visible the moment the rule next evaluates.

flowchart TB
  pr["PR touching alert-rules.yml.j2"]
  ci["check-alert-rules.py (#439)<br/>renders + parses"]
  prod["vmalert evaluating in production"]
  new["proposed: VacuousAlertRule<br/>series_fetched == 0"]

  pr --> ci --> prod
  prod --> new

  ci -. "cannot see:<br/>selector matches nothing" .-> new
  drift["later drift:<br/>metric renamed, writer retired,<br/>label meaning changed"] --> prod

  classDef gap stroke-dasharray: 4 3;
  class new gap;

The two layers are complementary, not alternatives. Keep both.

Proposed rule

- alert: VacuousAlertRule
  expr: vmalert_alerting_rules_last_evaluation_series_fetched{alertname!~"{{ alert_vacuity_exempt | join('|') }}"} == 0
  for: 30m
  labels:
    severity: warning
  annotations:
    summary: "Alert rule {{ $labels.alertname }} matches no series"
    description: "Its selectors fetched 0 series, so it cannot fire whatever happens. Usually a renamed metric or a wrong label matcher."

With alert_vacuity_exempt as a role default in roles/monitoring/defaults/main.yml.

Three things were checked before proposing this, because each could have made it unworkable:

  • All 51 rules emit the metric (count(...) == 51), so == 0 has no absence blind spot. A rule
    cannot hide by not reporting.
  • vector(1) reports 1, not 0. The Watchdog rule has no selectors at all and was the obvious
    false-positive candidate. It is not one.
  • The exempt list therefore starts EMPTY. Nothing needs exempting today. A rule that legitimately
    selects nothing is possible in principle (one watching for a metric that only appears during a
    failure), which is why the list exists, but it should stay empty until something earns a place and
    each entry should carry a comment saying why.

for: 30m rather than 5m: after a vmalert restart the first evaluation has not happened yet, and
this must not page on every apply.

Known limitation, stated rather than hidden

This rule is evaluated by vmalert, so it cannot report on a rule set that is not being evaluated at
all. That case is covered separately by the cross-probe and, since #442, by AlertingHealthMetricStale.
It also detects vacuity only after the rule reaches production, which is exactly why #439's check
stays.

Definition of done

  • VacuousAlertRule added, with alert_vacuity_exempt defaulting to an empty list
  • Shown able to FIRE, not merely to parse, by temporarily pointing a rule at a nonexistent metric
    and observing the alert, per the discipline the template comment now requires
  • Confirmed it does NOT fire on the current 51 rules
  • scripts/check-alert-rules.py still passes, self-test included
  • docs/runbook.md observability section says what to do when it fires
  • #442's body corrected where it overstates the cost of this work
Follow-up to #442, which fixed one vacuous alert. This is about detecting the next one. ## The gap `scripts/check-alert-rules.py` (#439) proves the rule set **renders and parses**. It cannot prove a rule can **fire**. A selector matching zero series does both cleanly, and `StrictUndefined` is satisfied whenever the Jinja variables resolve. That is not hypothetical. `UnitMetricStale` shipped in that state and stayed there until #442. It was found by hand, by someone querying its matcher out of curiosity. Nothing would have found it otherwise, and its stated job was watching the watchdog. ## The signal already exists vmalert publishes `vmalert_alerting_rules_last_evaluation_series_fetched` per rule. `== 0` means that rule's selectors matched nothing on the last evaluation. Measured on the monitoring VM, before and after #442's apply: | When | Rules loaded | `series_fetched == 0` | | --- | --- | --- | | Before | 49 | `["UnitMetricStale"]` | | After | 51 | none | So the metric found exactly the rule that was broken, and no others. It also confirms the blast radius was one rule rather than a class of them. **#442's issue body was wrong about the cost of this.** It said a check would need datasource access from CI and would be its own design problem. It does not. The measurement above is a single instant query, and the guard can be an ordinary alert rule. ## Why a runtime alert beats a CI stage A CI stage would only check at merge time. The runtime alert catches a strictly larger set, because a rule does not have to ship broken to become broken: - A metric gets renamed and every matcher naming it silently stops selecting. That is the `bitborg_*` / `gitborg_*` rename hazard, already recorded in ADR 0039 §7b, where two textfile writers were half-renamed and dual-brand matchers were the only reason nothing blanked. - A textfile writer is removed or retired and its consumers go quiet rather than red. - A label's meaning changes upstream, as `file=` did here. None of those are visible at merge time. All of them are visible the moment the rule next evaluates. ```mermaid flowchart TB pr["PR touching alert-rules.yml.j2"] ci["check-alert-rules.py (#439)<br/>renders + parses"] prod["vmalert evaluating in production"] new["proposed: VacuousAlertRule<br/>series_fetched == 0"] pr --> ci --> prod prod --> new ci -. "cannot see:<br/>selector matches nothing" .-> new drift["later drift:<br/>metric renamed, writer retired,<br/>label meaning changed"] --> prod classDef gap stroke-dasharray: 4 3; class new gap; ``` The two layers are complementary, not alternatives. Keep both. ## Proposed rule ```yaml - alert: VacuousAlertRule expr: vmalert_alerting_rules_last_evaluation_series_fetched{alertname!~"{{ alert_vacuity_exempt | join('|') }}"} == 0 for: 30m labels: severity: warning annotations: summary: "Alert rule {{ $labels.alertname }} matches no series" description: "Its selectors fetched 0 series, so it cannot fire whatever happens. Usually a renamed metric or a wrong label matcher." ``` With `alert_vacuity_exempt` as a role default in `roles/monitoring/defaults/main.yml`. Three things were checked before proposing this, because each could have made it unworkable: - **All 51 rules emit the metric** (`count(...) == 51`), so `== 0` has no absence blind spot. A rule cannot hide by not reporting. - **`vector(1)` reports 1, not 0.** The `Watchdog` rule has no selectors at all and was the obvious false-positive candidate. It is not one. - **The exempt list therefore starts EMPTY.** Nothing needs exempting today. A rule that legitimately selects nothing is possible in principle (one watching for a metric that only appears during a failure), which is why the list exists, but it should stay empty until something earns a place and each entry should carry a comment saying why. `for: 30m` rather than `5m`: after a vmalert restart the first evaluation has not happened yet, and this must not page on every apply. ## Known limitation, stated rather than hidden This rule is evaluated by vmalert, so it cannot report on a rule set that is not being evaluated at all. That case is covered separately by the cross-probe and, since #442, by `AlertingHealthMetricStale`. It also detects vacuity only after the rule reaches production, which is exactly why #439's check stays. ## Definition of done - [ ] `VacuousAlertRule` added, with `alert_vacuity_exempt` defaulting to an empty list - [ ] Shown able to FIRE, not merely to parse, by temporarily pointing a rule at a nonexistent metric and observing the alert, per the discipline the template comment now requires - [ ] Confirmed it does NOT fire on the current 51 rules - [ ] `scripts/check-alert-rules.py` still passes, self-test included - [ ] `docs/runbook.md` observability section says what to do when it fires - [ ] #442's body corrected where it overstates the cost of this work
Logga in för att delta i denna konversation.
Ingen milstolpe
Inga tilldelade
1 deltagare
Notiser
Förfallodatum
Förfallodatumet är ogiltigt eller utanför gränserna. Använd formatet "åååå-mm-dd".

Inget förfallodatum satt.

Beroenden

Inga beroenden satta

Referens
bitborg/bitborg-infra#444
Ingen beskrivning angiven.