observability: 28 alert rules in this estate cannot fire - #1113
Conversation
KubePersistentVolumeFillingUp was loaded with health=ok, lastError=none,
state=inactive, querying kubelet_volume_stats_available_bytes{job="kubelet"}
against a Prometheus with no kubelet job. Zero series. It evaluated an
empty set forever and read as "the volumes are fine" while the disk hit
100%.
That generalises: a control that cannot fail is not a control. A first
scan found 28 alerting rules in that state.
Adds an hourly stdlib-Python guard that parses every alerting rule into
its vector selectors and asks Prometheus whether each matched anything.
"The rule returns nothing" is the wrong test — up == 0 correctly returns
nothing when everything is up. The question is whether the INPUTS exist.
Three causes, three verdicts, because collapsing them makes false
positives:
DEAD-NO-TARGET job="X" named but up{job="X"} has no series
DEAD-LABEL-MISMATCH metric exists, label matchers exclude everything
DORMANT target up, metric never created (lazy counters)
Exemptions live in policy.json and must carry a reason, an expiry, and —
where the hazard is still real — a compensating_control. Expired
exemptions alert. Two entries record that they have NO compensating
control yet (inode exhaustion, CPU throttling) and carry short expiries.
The guard does not exempt itself.
Also fixes two of this estate's own dead rules:
HellGraphDown was up{job=~".*hellgraph.*"} == 0, matching zero series.
Two causes: the ServiceMonitor selects `app: hellgraph` but the chart
renders only app.kubernetes.io/* labels, AND hellgraph-service returns
404 on /metrics. Fixing only the selector would have produced a
permanently-firing false alarm, which is not an improvement on a
permanently-silent one. Now reads kube-state-metrics, which genuinely
observes the deployment.
CanaryGateBlind is new. The Argo Rollouts slo-gate AnalysisTemplate
gates every canary on job:request_error_ratio:rate5m, which derives
from http_server_request_duration_seconds_count — zero series. An SLO
gate with no input is not a gate, and whether Rollouts treats the
empty result as pass or error is not something to discover during a
bad deploy. Built on absent(), so the missing input is itself loud.
Proof it fires — a probe rule with two deliberately-dead rules and one
healthy control:
red checked=137 live=101 DEAD=9 both probes flagged, control not
green checked=137 live=103 DEAD=7 probes cleared after fixing
Two bugs the proof caught:
1. All nine findings were emitted as AlertRuleDead, and Alertmanager
suppressed six of them — the default inhibit_rules drop warning when
a critical shares (namespace, alertname). Six real findings swallowed
by a control reporting success: this guard reproducing, in its own
delivery path, the exact defect it detects. Fixed with distinct
alertnames per severity.
2. The first in-cluster run exited 1 on findings, tripping backoffLimit
into BackoffLimitExceeded — a permanently-red hourly Job. Findings
now travel by alert; non-zero exit is reserved for the guard being
broken.
In-cluster run completes and delivers: checked=141 live=105 dead=7,
8 alerts delivered. It also caught its own RuleLivenessGuardFailing as
DEAD-LABEL-MISMATCH before any CronJob-named Job existed, then cleared it
once one did.
The absent() branch of RuleLivenessGuardNotRunning produces no labels of its own, so the alert would not match the AlertmanagerConfig sub-route (operator-scoped OnNamespace) and the alert reporting that dead-rule detection is down would itself be routed to null. Same defect found in the companion alert-delivery rules.
…wns them Found a parallel branch, fix/observability-delivery-plane (pushed, no PR yet), that treats the same two dead rules more thoroughly than this PR did and would have conflicted on prometheusrule-slos.yaml. It reaches the same conclusion on HellGraphDown independently — repoint to kube_deployment_status_replicas_available, because the ServiceMonitor selected app: hellgraph against a chart that renders only app.kubernetes.io/* labels. It goes further by deleting the dead ServiceMonitor outright rather than leaving it selecting nothing. On the request-SLO group it is better evidenced than this PR was. It establishes that http_server_request_duration_seconds_* has no emitter anywhere in the estate — no OTel SDK, no prometheus_client, no promhttp in any requirements.txt or any of the four go.mod files, and /metrics 404s on every service probed — and therefore deletes the recording rules and HighErrorRatio rather than alerting on their absence. That matters, because the CanaryGateBlind alert added here would have fired permanently until roughly 47 services gained HTTP middleware. A permanently-firing critical is alert fatigue, which is the objection this wave raised against fixing HellGraphDown's selector alone. Deleting the rules and re-expressing the gate is the better remedy. So this PR keeps only what is genuinely additive: the guard that detects the whole CLASS of unfireable rules. That branch fixes specific rules; this one finds the next ones. The guard flagged both of those rules on its first live run, which is the evidence they are complementary rather than duplicative.
Deconflicted with
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Introduces a “rule-liveness guard” CronJob and supporting policy/docs to detect Prometheus alerting rules whose input selectors match zero series (rules that cannot fire), and to alert on dead/dormant/unscannable rules with expiring exemptions.
Changes:
- Adds a Kubernetes CronJob + ConfigMap-packaged Python guard that scans Prometheus rules and posts findings to Alertmanager with EvidenceReceipt logging.
- Adds meta-monitoring PrometheusRule to alert when the guard is not running or repeatedly failing.
- Introduces
policy.jsonfor expiring, justified exemptions (with compensating controls) and documents operation/runbook.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/k8s/rule-liveness-guard/base/prometheusrule-guard.yaml | Adds self-monitoring alerts for guard execution and failures. |
| infra/k8s/rule-liveness-guard/base/policy.json | Defines expiring exemptions to prevent known-absent metrics from being treated as dead. |
| infra/k8s/rule-liveness-guard/base/kustomization.yaml | Wires CronJob, PrometheusRule, and ConfigMap generation for guard + policy. |
| infra/k8s/rule-liveness-guard/base/guard.py | Implements PromQL selector extraction, series existence checks, classification, alert delivery, and receipts. |
| infra/k8s/rule-liveness-guard/base/cronjob.yaml | Deploys the guard as an hourly CronJob with hardening and pinned image digest. |
| infra/k8s/rule-liveness-guard/README.md | Documents the defect, approach, verdicts, exemptions, evidence receipts, and runbook. |
| deploy/argocd/rule-liveness-guard.yaml | Adds Argo CD Application for deploying the guard after monitoring CRDs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| volumeMounts: | ||
| - name: guard | ||
| mountPath: /opt/guard/guard.py | ||
| subPath: guard.py | ||
| readOnly: true | ||
| - name: guard | ||
| mountPath: /etc/guard/policy.json | ||
| subPath: policy.json | ||
| readOnly: true |
| def split_selector(selector): | ||
| """('metric_name', 'job="x"') -> bare name and the job matcher value if any.""" | ||
| name = selector.split("{", 1)[0].strip() | ||
| job = None | ||
| if "{" in selector: | ||
| inner = selector[selector.index("{") + 1: selector.rindex("}")] | ||
| for part in inner.split(","): | ||
| part = part.strip() | ||
| if part.startswith("job=") and "=~" not in part and "!=" not in part: | ||
| job = part[4:].strip().strip('"').strip("'") | ||
| return name, job |
| # Every input is empty. WHY it is empty decides what to do about it. | ||
| kind, why = diagnose(empty[0], start, now, cache) | ||
| ex = exempt.get(name) or exempt.get(key) | ||
| if ex: | ||
| if ex["expires"] < utc_now()[:10]: | ||
| expired.append((key, ex)) | ||
| else: | ||
| exempted.append((key, ex)) | ||
| else: | ||
| dead.append((key, name, empty, kind, why)) |
| for: 30m | ||
| labels: { severity: warning } | ||
| annotations: |
| def selector_series(selector, start, end): | ||
| """How many series this selector matched over the window. -1 on error.""" | ||
| url = PROM + "/api/v1/series?" + urllib.parse.urlencode( | ||
| {"match[]": selector, "start": start, "end": end} | ||
| ) | ||
| try: | ||
| d = http_json(url) | ||
| except (urllib.error.URLError, urllib.error.HTTPError, ValueError): | ||
| return -1 | ||
| if d.get("status") != "success": | ||
| return -1 | ||
| return len(d.get("data") or []) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
infra/k8s/rule-liveness-guard/base/guard.py:320
- Selectors of the form
{__name__=\"foo\",...}(whichextract_selectorsexplicitly supports) producename == \"\"insplit_selector. Indiagnose(), that leads toselector_series(name, ...)with an emptymatch[]value, which is invalid and can misclassify the rule as unscannable or produce incorrect diagnosis. Fix by (a) extracting__name__from bare{...}selectors insplit_selector, or (b) guardingdiagnose()so the "bare metric exists" check is skipped whennameis empty.
def split_selector(selector):
"""('metric_name', 'job="x"') -> bare name and the job matcher value if any."""
name = selector.split("{", 1)[0].strip()
job = None
if "{" in selector:
inner = selector[selector.index("{") + 1: selector.rindex("}")]
for part in inner.split(","):
part = part.strip()
if part.startswith("job=") and "=~" not in part and "!=" not in part:
job = part[4:].strip().strip('"').strip("'")
return name, job
def diagnose(selector, start, end, cache):
"""Why does this selector match nothing? The cause changes what to do.
DEAD-NO-TARGET the job it names is not scraped at all, so no scraper will
ever produce this series. Structural. Critical.
DEAD-LABEL-MISMATCH the bare metric DOES exist but the label matchers exclude
everything. Almost always a real bug -- a renamed label or
a selector copied from a different topology. Critical.
DORMANT the metric name has never appeared, but its target is up.
Usually a counter that is only created on first occurrence
(e.g. *_failures_total), or a disabled collector. Warning:
needs a human decision, not an automatic verdict.
"""
name, job = split_selector(selector)
if job is not None:
key = 'up{job="%s"}' % job
if key not in cache:
cache[key] = selector_series(key, start, end)
if cache[key] == 0:
return "DEAD-NO-TARGET", "no scrape target with job=%r" % job
if "{" in selector:
if name not in cache:
cache[name] = selector_series(name, start, end)
if cache[name] > 0:
return "DEAD-LABEL-MISMATCH", (
"metric %s has %d series but the label matchers exclude all of them"
% (name, cache[name])
)
return "DORMANT", "metric %s has never been reported" % name
infra/k8s/rule-liveness-guard/base/guard.py:273
- Using
/api/v1/seriesreturns the full labelset for every matching series, which can be large and expensive for high-cardinality selectors (and you only need an existence/count signal). This can put unnecessary load on Prometheus during scans. Consider switching existence checks to/api/v1/querywith an aggregated expression that returns a small result (e.g.,sum(count_over_time(<selector>[<lookback>]))or similar) so the response size is bounded and scanning cost is reduced.
def selector_series(selector, start, end):
"""How many series this selector matched over the window. -1 on error."""
url = PROM + "/api/v1/series?" + urllib.parse.urlencode(
{"match[]": selector, "start": start, "end": end}
)
try:
d = http_json(url)
except (urllib.error.URLError, urllib.error.HTTPError, ValueError):
return -1
if d.get("status") != "success":
return -1
return len(d.get("data") or [])
| def extract_selectors(expr): | ||
| """Return (selectors, uses_absence_func). Raises ParseError on anything odd.""" | ||
| selectors = [] | ||
| uses_absence = False | ||
| i, n = 0, len(expr) | ||
| while i < n: | ||
| c = expr[i] | ||
| if c == "#": | ||
| while i < n and expr[i] != "\n": | ||
| i += 1 | ||
| continue | ||
| if c in "\"'`": | ||
| i = _skip_string(expr, i) | ||
| continue | ||
| if c == "[": # range / subquery: durations, not metrics | ||
| i = _match(expr, i, "[", "]") | ||
| continue | ||
| if c == "{": # bare {__name__="x"} selector | ||
| end = _match(expr, i, "{", "}") | ||
| selectors.append(expr[i:end]) | ||
| i = end | ||
| continue | ||
| if c in IDENT_START: | ||
| j = i | ||
| while j < n and expr[j] in IDENT_CHAR: | ||
| j += 1 | ||
| word = expr[i:j] | ||
| nxt = _peek(expr, j) | ||
| if word in ABSENCE_FUNCS: | ||
| uses_absence = True | ||
| if word in FUNCTIONS or word in AGGREGATIONS: | ||
| i = j | ||
| continue | ||
| if word in KEYWORDS: | ||
| if word in LABEL_LIST_KEYWORDS and nxt < n and expr[nxt] == "(": | ||
| i = _match(expr, nxt, "(", ")") | ||
| continue | ||
| i = j | ||
| continue | ||
| if nxt < n and expr[nxt] == "(": | ||
| # Unknown identifier used as a function. Do not guess. | ||
| raise ParseError("unknown function %r" % word) | ||
| sel, k = word, j | ||
| nk = _peek(expr, k) | ||
| if nk < n and expr[nk] == "{": | ||
| end = _match(expr, nk, "{", "}") | ||
| sel, k = word + expr[nk:end], end | ||
| selectors.append(sel) | ||
| i = k | ||
| continue | ||
| i += 1 | ||
| return selectors, uses_absence |
| volumeMounts: | ||
| - name: guard | ||
| mountPath: /opt/guard/guard.py | ||
| subPath: guard.py | ||
| readOnly: true | ||
| - name: guard | ||
| mountPath: /etc/guard/policy.json | ||
| subPath: policy.json | ||
| readOnly: true |
|
Merge-gate disposition (Copilot 3-channel + adversarial pass) The guard fails loud/closed as designed (verified): Prometheus unreachable → non-zero exit → Rejected — subPath parent-dir finding ( Acknowledged, non-blocking (safe-direction / low-impact): Verdict: MERGE-READY. |
The defect, generalised
KubePersistentVolumeFillingUpwas loaded withhealth=ok,lastError=none,state=inactive, queryingkubelet_volume_stats_available_bytes{job="kubelet"}against a Prometheus with no kubelet job. Zero series. It evaluated an empty set forever and read as "the volumes are fine" while the disk hit 100%.That is not one bad rule. A control that cannot fail is not a control. A first scan of this estate found 28 alerting rules in that state.
Why the obvious test is wrong
"The rule returns nothing" would flag
up == 0, which returns nothing precisely when everything is healthy. The question is whether the rule's inputs exist:So the guard parses each rule into its vector selectors and asks Prometheus whether each matched anything.
absent()/absent_over_time()rules are exempt by construction.Three causes, three verdicts
Collapsing them produces false positives:
job="X", butup{job="X"}has no series*_failures_total)Exemptions expire and must name a compensating control
policy.jsonentries needreason,expires, and — where the hazard is real —compensating_control.KubePersistentVolumeFillingUpis exempted only becausepvc-capacity-guard(#1105) reads the same metrics from GMP. Two entries record they have no compensating control yet and carry short expiries:KubePersistentVolumeInodesFillingUp(inode exhaustion uncovered) andCPUThrottlingHigh(throttling unobserved).The guard does not exempt itself — its own meta-rules are scanned like everything else.
Two of the estate's own dead rules, fixed
HellGraphDownwasup{job=~".*hellgraph.*"} == 0, matching zero series. Two independent causes: thehellgraphServiceMonitor selectsapp: hellgraphbut the shared chart renders onlyapp.kubernetes.io/*labels, andhellgraph-servicereturns HTTP 404 on/metrics. Fixing only the selector would have produced a permanently-firing false alarm, which is not an improvement on a permanently-silent one. Now reads kube-state-metrics, which genuinely observes the deployment (3 live series). Giving hellgraph a real/metricsis app work, not this lane.CanaryGateBlindis new, and is the sharpest finding here.infra/k8s/rollouts/base/analysistemplate-slo.yamlgates every canary promotion onjob:request_error_ratio:rate5m, which derives fromhttp_server_request_duration_seconds_count— zero series. An SLO gate with no input is not a gate, and whether Argo Rollouts treats the empty result as pass or error is not something to discover during a bad deploy. Built onabsent(), so the missing input is itself critical.Proof it fires
A probe rule with two deliberately-dead rules and one healthy control:
Red
ProbeLivewas not flagged — the false-positive control held.Green, after pointing both probes at metrics that exist:
Probe rule deleted afterwards.
The proof caught two bugs in this guard
1. Six findings were silently suppressed. All nine were emitted as
AlertRuleDead; the stack's defaultinhibit_rulesdropseverity=warningwhen acriticalshares(namespace, alertname), and every finding carriesnamespace=observability:This guard reproducing, in its own delivery path, the exact defect it exists to detect. Fixed with distinct alertnames per severity (
AlertRuleDead/AlertRuleDormant); re-run confirms all ninestate=active.2. Exit 1 on findings made the Job permanently red. The first in-cluster run tripped
backoffLimitintoBackoffLimitExceeded— an hourly Job failing for as long as any dead rule existed, the kind everyone learns to ignore, andRuleLivenessGuardFailingrendered meaningless. Findings now travel by alert; non-zero exit is reserved for the guard itself being broken.In-cluster
It also flagged its own
RuleLivenessGuardFailingas DEAD-LABEL-MISMATCH before any CronJob-named Job existed, then cleared it once one ran — the self-scan working.Receipts
One
EvidenceReceiptper scan to stdout → Cloud Logging.statusissucceededon a clean scan,partialwith findings;hashcovers the canonical finding set.Coordination
New paths (
infra/k8s/rule-liveness-guard/**,deploy/argocd/rule-liveness-guard.yaml) plusinfra/k8s/observability/base/prometheusrule-slos.yaml. Depends conceptually on #1112 (alert delivery) but has no textual overlap. No overlap with #1091, #1105,images.yml,infra/k8s/zot/**,apps/health-twin/**,tools/.