diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 93f7de8..2c23fe8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -112,9 +112,7 @@ jobs: # rules can each be individually correct and leave the state reported by # nobody. kubeconform SKIPs the Grafana CRDs and no gate executes the # PromQL, so an alert that can never fire looks exactly like one that has - # not fired yet. kubeconform SKIPs the Grafana CRDs and no gate - # executes the PromQL, so an alert that can never fire looks exactly like - # one that has not fired yet. + # not fired yet. - name: Excluded alert states are somebody's job run: ./scripts/check-alert-coverage.py @@ -128,6 +126,17 @@ jobs: - name: Every alert severity routes to a declared contact point run: ./scripts/check-alert-severity-routes.py + # A burn-rate summary is the alert TITLE — the sentence that reaches a + # human first and often the only one they read. The figure in it is prose + # that nothing parses, so a rule can name a budget its own expression does + # not spend and pass every rule-level validation, because the expression is + # correct and only the sentence is wrong. This derives the figure from the + # rule's factor and window against the objective the dashboards measure, so + # there is no constant here to agree with the standard today and be + # compared to nothing tomorrow. + - name: Burn-rate summaries state the budget their expression spends + run: ./scripts/check-burn-rate-budgets.py + # ── Kyverno policy unit tests ──────────────────────────────────────── # `kyverno test` proves each policy rule PASSES a compliant resource and FAILS # a violating one (policies/kyverno/tests). Without this, a policy could be diff --git a/CLAUDE.md b/CLAUDE.md index 054ca41..3ffd0ca 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -166,6 +166,17 @@ aggregate, so a clean `task validate` is necessary but not sufficient: `scripts/check-catalog-revision.py` (CI job `catalog-revision`) - **Alert coverage** — `scripts/check-alert-coverage.py`, which runs inside the `dashboards` job alongside the locally-available dashboard and Athena gates +- **Burn-rate budget claims** — `scripts/check-burn-rate-budgets.py`, in the same + `dashboards` job. A burn-rate summary is the alert title, and the budget figure + in it is prose nothing parses, so a rule can name a figure its own expression + does not spend while every rule-level validation passes. The figure is derived + rather than listed: burn factor times long window over the SLO window, with the + factor and window read from the rule's expression and the SLO window from the + dashboard panel measuring the same selector — out of the render, so it is a + panel a cluster receives rather than a file on disk. A rule whose selectors no + delivered panel measures is a finding, because a figure compared to nothing is + how the last wrong one survived. The figure has no independent existence: + there is no constant to correct and none in the summary that anything trusts - **Image vulnerabilities** — `scripts/check-image-vulnerabilities.py` (CI job `image-vulnerabilities`). The one with a target of its own, `task validate:image-vulnerabilities`, kept out of the aggregate because it diff --git a/dashboards/base/alerting/agent-operator.yaml b/dashboards/base/alerting/agent-operator.yaml index 3ac96dd..402d7ba 100644 --- a/dashboards/base/alerting/agent-operator.yaml +++ b/dashboards/base/alerting/agent-operator.yaml @@ -175,7 +175,7 @@ spec: service: eks-agent-platform component: operator annotations: - summary: operator reconcile latency budget burning (100% over 3d) + summary: operator reconcile latency budget burning slowest (10% in 3d) description: > The fraction of reconciles over 1s exceeds 1x the 99% latency objective over both the 3d and 6h windows — the budget is being spent at exactly the diff --git a/scripts/check-burn-rate-budgets.py b/scripts/check-burn-rate-budgets.py new file mode 100755 index 0000000..b8ad8bc --- /dev/null +++ b/scripts/check-burn-rate-budgets.py @@ -0,0 +1,405 @@ +#!/usr/bin/env python3 +"""The budget a burn-rate alert claims is the one its own expression spends. + +WHY THIS EXISTS + +A multi-window burn-rate rule fires on a rate of spend, and its summary is the +alert TITLE — the sentence that reaches a human first and often the only one +they read. One ticket-tier rule said: + + summary: operator reconcile latency budget burning (100% over 3d) + +for an expression comparing against a burn factor of 1 over a 3d window. Factor +1 spends the budget at exactly the rate that exhausts it over the SLO window, so +three days of it consumes three days' worth: 10% of a 30d budget, not 100%. The +title described total exhaustion — the most urgent condition an SLO has — on the +least urgent tier the file ships. + +Both readings of that title are damaging. Escalated, it is an outage that is not +happening. Recognised as wrong, it teaches on-call that these titles are not to +be trusted, which spends the credibility of the three tiers that are right. + +Nothing could see it. The expression is correct; only the sentence describing it +is wrong, so every rule-level validation passes. kubeconform SKIPs the +grafana.integreatly.org CRDs, no gate executes PromQL, and a summary is +free-text that nothing parses. + +WHAT IT CHECKS + +The claim is checked against arithmetic, not against a table of expected +figures. For a burn rule with factor `f` over long window `w`, against an SLO +window `W`: + + budget consumed = f * w / W + +Every term comes from the tree: + + * `f` and `w` from the rule's own expression — the `> bool f` threshold and + the longest range selector in it; + * `W` from the dashboard panel measuring the same metric selector over its + longest explicit range — read out of the RENDER, not off disk, so it is the + panel a cluster receives. That is the panel displaying the objective this + rule burns against, in a different file, edited by different work. + +The figure therefore has no independent existence. There is no number here to +correct, and none in the summary that anything trusts: the sentence is checked +against the arithmetic that produces it, every term of which is read from a +place that would have to be edited in step for a wrong figure to survive. + +So there is no constant here to agree with the standard today and be compared to +nothing tomorrow. A figure that drifts fails against the expression that +produces it; an SLO window that moves fails against the panel that measures it; +and a panel that stops being delivered stops anchoring anything, which is a +finding rather than a silent fallback to whatever the summary says. + +Every rule whose expression is a burn-rate expression must carry the claim. A +burn-rate title that does not say how much budget is at stake tells the reader +nothing they can act on, and leaving the claim out would otherwise remove the +rule from this gate's corpus without removing the rule. + +WHAT IT DOES NOT CHECK + + * Whether the SLO objective is the right one. `W` is read from the panel that + measures it; that the business wants 30d rather than 28d is not a fact in + this repository. + * Whether the factors implement the alerting policy correctly. That 14.4 over + 1h is the intended fast tier is a design decision; this asserts only that + the sentence and the expression describe the same spend. + * Whether the rule fires. The expression is read, never executed. + * The `description` under each summary, which states the same thing in prose + and in a different shape per file. The summary is checked because it is the + title — the sentence that reaches a human whether or not they open the rule. + * Agreement with the org SLO standard. The standard is the authority for these + figures and it is not in this repository, so what is asserted here is that + the catalog agrees with ITSELF: the sentence, the expression, and the panel + measuring the objective all describe one spend. A window that moved in the + standard and nowhere else would leave this green. +""" + +from __future__ import annotations + +import importlib.util +import json +import pathlib +import re +import subprocess +import sys +from fractions import Fraction + +import yaml + +_here = pathlib.Path(__file__).resolve().parent + +_gl = _here / "gatelib.py" +_gs = importlib.util.spec_from_file_location("gatelib", _gl) +assert _gs and _gs.loader, f"{_gl} is not loadable as a module" +gatelib = importlib.util.module_from_spec(_gs) +sys.modules["gatelib"] = gatelib +_gs.loader.exec_module(gatelib) + +ROOT = _here.parent +ALERT_DIR = ROOT / "dashboards" / "base" / "alerting" +# The kustomization delivering both the rules and the panels they burn against. +# Derived from the alerting directory rather than named again, so the rules and +# the objective cannot come to be read out of different trees. +KUSTOMIZE_ROOT = ALERT_DIR.parent +RULE_GROUP = "GrafanaAlertRuleGroup" +DASHBOARD = "GrafanaDashboard" + +UNIT_SECONDS = {"s": 1, "m": 60, "h": 3600, "d": 86400, "w": 604800} + +# `metric{labels}[range]` — the labels are kept because they are part of what a +# panel has to match to be measuring the same thing. +SELECTOR = re.compile( + r"(?P[a-zA-Z_:][a-zA-Z0-9_:]*(?:\{[^}]*\})?)\[(?P\d+[smhdw])\]") +# The burn factor, and what separates it from every other `> bool` in a rule. +# A burn comparison is made against the error ratio NORMALISED by the budget: +# ` / > bool `. Matching `> bool` alone also +# matches the traffic guard these rules carry — `sum(rate(...)) > bool 0.0167`, +# roughly one request a minute, there to stop an idle service alerting on a +# ratio computed from nothing. Read as a factor it makes a rule look like two +# contradictory claims about one rate of spend. +FACTOR = re.compile( + r"/\s*(?P0\.\d+)\s*>\s*bool\s+(?P\d+(?:\.\d+)?)") +# The claim in the summary: a percentage of budget over a window. +CLAIM = re.compile( + r"(?P\d+(?:\.(?P\d+))?)%\s+(?:in|over)\s+(?P\d+[smhdw])") + + +def under_root(path: pathlib.Path) -> str: + """`path` relative to the repository when it is inside it, else as given.""" + try: + return str(path.relative_to(ROOT)) + except ValueError: + return str(path) + + +def seconds(duration: str) -> int: + """A PromQL duration in seconds. The grammar here is a single unit, which + is what every range in this catalog uses; a compound `1h30m` would not match + SELECTOR and so would never reach this.""" + return int(duration[:-1]) * UNIT_SECONDS[duration[-1]] + + +def human(total: int) -> str: + """Seconds as the largest whole unit that divides them, for messages that a + reader compares against a duration written in the file.""" + for unit in ("w", "d", "h", "m"): + size = UNIT_SECONDS[unit] + if total >= size and total % size == 0: + return f"{total // size}{unit}" + return f"{total}s" + + +def rule_expr(rule: dict) -> str: + """The rule's query, as one string. + + A Grafana rule carries a list of `data` stages; the burn arithmetic is in + the datasource query, and the later stages are threshold expressions over + its result. + """ + for stage in rule.get("data") or []: + if not isinstance(stage, dict): + continue + expr = (stage.get("model") or {}).get("expr") + if isinstance(expr, str) and expr.strip(): + return expr + return "" + + +def burn_rules(alert_dir: pathlib.Path): + """(path, group, rule, expr) for every rule whose query is a burn-rate one. + + Selected on the EXPRESSION rather than on the summary. Selecting on the + sentence would let a rule leave this gate's corpus by having its claim + deleted, which is the one edit most likely to accompany a wrong figure. + """ + for path in sorted(alert_dir.glob("*.yaml")): + for doc in gatelib.read_yaml_all(path): + if not isinstance(doc, dict) or doc.get("kind") != RULE_GROUP: + continue + group = (doc.get("metadata") or {}).get("name", path.stem) + for rule in (doc.get("spec") or {}).get("rules") or []: + if not isinstance(rule, dict): + continue + expr = rule_expr(rule) + if FACTOR.search(expr) and SELECTOR.search(expr): + yield path, str(group), rule, expr + + +def long_window(expr: str) -> tuple[int, set[str]]: + """The longest range in `expr`, and the selectors carrying it. + + A dual-window burn rule ANDs a long window against a short confirmation + window. The long one is the period the claim is about; the short one exists + to stop the alert firing on a spike that has already stopped. + """ + ranges: dict[int, set[str]] = {} + for m in SELECTOR.finditer(expr): + ranges.setdefault(seconds(m.group("range")), set()).add( + re.sub(r"\s+", "", m.group("sel"))) + longest = max(ranges) + return longest, ranges[longest] + + +def factors(expr: str) -> set[Fraction]: + """Every burn factor the expression compares against. + + A set, because a dual-window rule states the same factor twice and a rule + stating two different ones is not a single claim about a rate of spend. + """ + return {Fraction(m.group("factor")) for m in FACTOR.finditer(expr)} + + +def delivered_dashboards(root: pathlib.Path) -> list[dict]: + """The dashboard JSON of every GrafanaDashboard `root` renders. + + Read from the render rather than off disk, because `resources` is an + explicit list and a panel can stop being delivered without moving, being + edited, or failing to render. A dashboard the catalog no longer ships is + not a second source: the rules would keep burning against an objective + nobody can see, and this gate would keep certifying their figures against + a document no cluster receives. + """ + gatelib.require("kustomize") + proc = subprocess.run(["kustomize", "build", str(root)], + capture_output=True, text=True) + if proc.returncode != 0: + print(f"Cannot run: kustomize build {under_root(root)} failed, so " + f"which panels this catalog delivers could not be read. The " + f"dashboards on disk say nothing about that on their own.") + print(proc.stderr.strip()[:500]) + sys.exit(gatelib.CANNOT_RUN) + out: list[dict] = [] + for doc in yaml.safe_load_all(proc.stdout): + if not isinstance(doc, dict) or doc.get("kind") != DASHBOARD: + continue + raw = (doc.get("spec") or {}).get("json") + if not isinstance(raw, str): + continue + try: + out.append(json.loads(raw)) + except json.JSONDecodeError as err: + # Refusing to skip a dashboard that will not parse. A gate that + # silently drops its subject reports success over nothing. + name = (doc.get("metadata") or {}).get("name", "") + print(f"Cannot run: {DASHBOARD}/{name} renders spec.json that is not " + f"valid JSON, so the objective it measures could not be read: " + f"{err}") + sys.exit(gatelib.CANNOT_RUN) + return out + + +def objective_windows() -> dict[str, int]: + """selector -> the longest explicit range a DELIVERED panel applies to it. + + The SLO window, read from the panel that displays the objective. A burn + window is by construction shorter than the window it burns against, so the + longest range a panel measures this selector over is the objective's. + """ + out: dict[str, int] = {} + for dash in delivered_dashboards(KUSTOMIZE_ROOT): + for expr in panel_exprs(dash): + for m in SELECTOR.finditer(expr): + sel = re.sub(r"\s+", "", m.group("sel")) + out[sel] = max(out.get(sel, 0), seconds(m.group("range"))) + return out + + +def panel_exprs(node) -> list[str]: + """Every `expr` under a dashboard, rows and nested panels included.""" + found: list[str] = [] + if isinstance(node, dict): + expr = node.get("expr") + if isinstance(expr, str) and expr.strip(): + found.append(expr) + for value in node.values(): + found.extend(panel_exprs(value)) + elif isinstance(node, list): + for item in node: + found.extend(panel_exprs(item)) + return found + + +def decimal(value: Fraction) -> str: + """A Fraction as the decimal a reader will find in the expression. + + Exact arithmetic is what lets the comparison be an equality rather than a + tolerance, but `72/5` is not the string anyone can search the file for. + """ + if value.denominator == 1: + return str(value.numerator) + return f"{float(value):g}" + + +def as_percent(value: Fraction, places: int) -> str: + """`value` as a percentage string with `places` decimals, no float.""" + scaled = value * 100 + if places == 0: + return str(round(scaled)) + quantum = Fraction(10) ** places + return f"{float(round(scaled * quantum) / quantum):.{places}f}" + + +def main() -> int: + if not ALERT_DIR.is_dir(): + print(f"Cannot run: {ALERT_DIR.relative_to(ROOT)} does not exist, so the " + f"burn-rate rules whose claims this gate checks are not there.") + print("This gate examined nothing, which is not the same as finding nothing.") + return gatelib.CANNOT_RUN + + rules = list(burn_rules(ALERT_DIR)) + if not rules: + print(f"Cannot run: no burn-rate rule found under " + f"{ALERT_DIR.relative_to(ROOT)}. A run over no rule reports what a " + f"catalog of correct claims reports.") + return gatelib.CANNOT_RUN + + windows = objective_windows() + failures: list[str] = [] + checked = 0 + + for path, group, rule, expr in rules: + title = str(rule.get("title") or rule.get("uid") or "") + where = f"{path.name}: {group}/{title}" + + seen = factors(expr) + if len(seen) != 1: + failures.append( + f"{where} compares against {len(seen)} different burn factors " + f"({', '.join(decimal(f) for f in sorted(seen))}), so there is no one " + f"rate of spend for its summary to be describing.") + continue + factor = seen.pop() + + window, selectors = long_window(expr) + + summary = str((rule.get("annotations") or {}).get("summary") or "") + claim = CLAIM.search(summary) + if not claim: + failures.append( + f"{where} is a burn-rate rule and its summary states no budget " + f"figure: {summary!r}. The summary is the alert title, and a burn " + f"title that does not say how much budget is at stake gives the " + f"reader nothing to act on.") + continue + + stated_window = seconds(claim.group("window")) + if stated_window != window: + failures.append( + f"{where} says {claim.group(0)!r} and its expression measures over " + f"{human(window)}. The sentence and the query describe different " + f"periods, so one of them is about an alert that does not exist.") + continue + + objective = {windows[sel] for sel in selectors if sel in windows} + if not objective: + failures.append( + f"{where} claims {claim.group(0)!r} and no dashboard delivered " + f"by {under_root(KUSTOMIZE_ROOT)} measures " + f"{' or '.join(sorted(selectors))} over an explicit range. The " + f"figure is compared to nothing, which is how the last wrong one " + f"survived.") + continue + if len(objective) > 1: + failures.append( + f"{where} burns against selectors whose panels measure them over " + f"{', '.join(human(w) for w in sorted(objective))}. Which of those " + f"is the SLO window decides the figure, and the tree states both.") + continue + slo = objective.pop() + + places = len(claim.group("frac") or "") + derived = factor * Fraction(window, slo) + stated = Fraction(claim.group("pct")) / 100 + if as_percent(derived, places) != as_percent(stated, places): + failures.append( + f"{where} claims {claim.group(0)!r} and its expression spends " + f"{as_percent(derived, places)}% over {human(window)}: burn factor " + f"{decimal(factor)} against a {human(slo)} objective is " + f"{decimal(factor)} x {human(window)} / {human(slo)}. This " + f"sentence is the alert title.") + continue + checked += 1 + + if failures: + print(f"{len(failures)} burn-rate budget claim(s) the expression does not " + f"support:\n") + for f in failures: + print(f" {f}") + return 1 + + objectives = sorted({human(windows[sel]) + for _p, _g, _r, e in rules + for sel in long_window(e)[1] if sel in windows}) + print(f"✓ every burn-rate summary states the budget its expression spends: " + f"{checked} rule(s) across {len({g for _p, g, _r, _e in rules})} group(s), " + f"each derived from its own factor and window against the " + f"{', '.join(objectives)} objective(s) the dashboards measure") + print(" the objective itself, the choice of factors, and whether any rule " + "fires are outside this claim") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/tests/controls.py b/scripts/tests/controls.py index f4ea12b..198396c 100755 --- a/scripts/tests/controls.py +++ b/scripts/tests/controls.py @@ -504,6 +504,21 @@ def m_alert_severity_routes(root): marker=f"severity: {MARKER}-urgent") +def m_burn_rate_budgets(root): + """Put back the figure this gate was written for. + + A ticket-tier rule whose expression compares against a burn factor of 1 over + 3d, claiming the budget is fully consumed. Factor 1 spends at exactly the + rate that exhausts the budget over the SLO window, so 3d of it is 10% of a + 30d budget. The expression stays correct and only the sentence changes, + which is what every rule-level validation passes. + """ + return _sub(root, "dashboards/base/alerting/agent-operator.yaml", + "budget burning slowest (10% in 3d)", + "budget burning (100% over 3d)", + marker="100% over 3d") + + def m_chart_deprecation(root): """A recorded chart that nothing pins, which the offline gate must reject.""" import json @@ -579,6 +594,8 @@ def m_env_coverage(root): "check-alert-coverage.py": ("an alert on an unexported KSM field", m_alert_coverage), "check-alert-severity-routes.py": ("a severity label that routes nowhere", m_alert_severity_routes), + "check-burn-rate-budgets.py": ("a summary claiming a budget its expression " + "does not spend", m_burn_rate_budgets), } diff --git a/scripts/tests/reverify-gates.sh b/scripts/tests/reverify-gates.sh index 04fcc63..42b708c 100755 --- a/scripts/tests/reverify-gates.sh +++ b/scripts/tests/reverify-gates.sh @@ -158,6 +158,7 @@ run 0 "check-renovate-coverage.py" ./scripts/check-renovate-coverage.py run 0 "check-ai-config.py" ./scripts/check-ai-config.py run 0 "check-alert-severity-routes.py" ./scripts/check-alert-severity-routes.py run 0 "check-env-coverage.py" ./scripts/check-env-coverage.py +run 0 "check-burn-rate-budgets.py" ./scripts/check-burn-rate-budgets.py run 0 "check-named-things.py" ./scripts/check-named-things.py run 0 "check-policy-validity.py" ./scripts/check-policy-validity.py run 0 "no-placeholders.sh" ./scripts/no-placeholders.sh @@ -403,6 +404,39 @@ run nonzero "alert-severity-routes: the routing tree stops shipping" \ ./scripts/check-alert-severity-routes.py res $F +# The number an on-call reads first. The expression stays correct and only the +# sentence changes, which is every rule-level validation's blind spot. +F=dashboards/base/alerting/agent-operator.yaml; mut $F +python3 - "$F" <<'PYX' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace("budget burning slowest (10% in 3d)", "budget burning (100% over 3d)", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" claimed the 3d tier consumes the whole budget") +PYX +run nonzero "burn-rate-budgets: a summary claiming a budget its expression does not spend" \ + ./scripts/check-burn-rate-budgets.py +res $F + +# The other term, and the one that lives outside the alerting directory. The +# dashboard is unedited and still renders; it is simply no longer in the +# kustomization's resources. The rules ship and burn, the panel measuring what +# they burn against does not, and every figure is then anchored to a document no +# cluster receives. +F=dashboards/base/kustomization.yaml; mut $F +python3 - "$F" <<'PYZ' +import pathlib,sys +p=pathlib.Path(sys.argv[1]); s=p.read_text() +m=s.replace(" - platform/agent-operator.yaml\n", "", 1) +assert m!=s, "mutation did not land" +p.write_text(m) +print(" dropped the panel measuring the objective from the kustomization") +PYZ +run nonzero "burn-rate-budgets: the panel measuring the objective stops shipping" \ + ./scripts/check-burn-rate-budgets.py +res $F + F=addons/bootstrap/cert-manager/values-hub.yaml; mut $F; rm -f $F run nonzero "env-coverage: deleted hub delta" ./scripts/check-env-coverage.py res $F @@ -515,7 +549,7 @@ echo "RESULT pass=$pass fail=$fail" # The harness owes the same assertion it demands of the gates: with every `run` # line deleted it would report pass=0 fail=0 and exit 0, which is a green run # over nothing checked. -MIN_CHECKS=41 +MIN_CHECKS=44 total=$((pass + fail)) if [ "$total" -lt "$MIN_CHECKS" ]; then echo "FAIL ran $total check(s), under the floor of $MIN_CHECKS — this harness" diff --git a/scripts/tests/run.py b/scripts/tests/run.py index c950e81..959549a 100755 --- a/scripts/tests/run.py +++ b/scripts/tests/run.py @@ -62,6 +62,9 @@ # The floors that keep an emptied corpus from reading as a clean one, # asserted apart from the gates that carry them. "test_corpus_floors", + # The arithmetic behind a figure an on-call reads at the worst moment, and + # the anchoring that keeps it from being a constant compared to nothing. + "test_burn_rate_budgets", ) # A floor well under the real count. It catches "discovery found almost nothing", @@ -154,7 +157,12 @@ # ratchet that declines to move is one nobody can regress against — but it is # the weaker half of this file's claim, and the per-gate floors and the controls # in scripts/tests/controls.py are where the real one lives. -MAX_UNCOVERED_GATES = 11 +# +# So this number moves by what a change actually covered, not by what the run +# reports. A gate arriving with its own tests lowers it; a gate that starts +# reading as covered because something new imports it does not, and an import +# removed later would give that coverage back with nothing lost. +MAX_UNCOVERED_GATES = 10 PER_GATE_FLOORS = { "scripts/check-named-things.py": 35, diff --git a/scripts/tests/test_burn_rate_budgets.py b/scripts/tests/test_burn_rate_budgets.py new file mode 100644 index 0000000..55b770f --- /dev/null +++ b/scripts/tests/test_burn_rate_budgets.py @@ -0,0 +1,375 @@ +"""Unit tests for the burn-rate budget gate. + +The defect this gate exists for was a number that agreed with nothing: a +summary claiming 100% budget consumed where its own expression spends 10%. So +the cases here concentrate on the two ways a gate like this reports a false +pass — reading the figure from somewhere that cannot disagree with it, and +quietly dropping a rule out of the corpus so its claim is never read. + +Every term of the arithmetic comes from a different place in the tree, and each +of those places gets a fixture that moves it. +""" + +from __future__ import annotations + +import contextlib +import io +import json +import pathlib +import shutil +import tempfile +import unittest +from fractions import Fraction + +import yaml +from gateloader import load + +gate = load("check-burn-rate-budgets") + +ROOT = pathlib.Path(__file__).resolve().parent.parent.parent + +# The objective is read out of the render, so the planted fixtures are rendered +# too rather than compared against a stub. The job running these installs +# kustomize; a checkout without it skips rather than aborting the runner on the +# gate's exit-2 refusal. +HAS_KUSTOMIZE = shutil.which("kustomize") is not None + + +def burn_expr(budget, factor, long_window, short_window, + guard=None, numerator=None, denominator=None): + """A dual-window burn expression in the shape this catalog writes them. + + Two metric selectors by default, because the real rules divide one by + another and a rule whose two selectors disagree about the objective is one + of the cases below. + """ + num = numerator or NUMERATOR + den = denominator or DENOMINATOR + + def half(window): + return (f"(sum(rate({num}[{window}])) / " + f"clamp_min(sum(rate({den}[{window}])), 0.001) / " + f"{budget} > bool {factor})") + + expr = half(long_window) + " * " + half(short_window) + if guard is not None: + expr += f" * (sum(rate({den}[{long_window}])) > bool {guard})" + return expr + + +def rule(title, summary, expr): + return {"uid": title.lower(), "title": title, + "annotations": {"summary": summary}, + "data": [{"refId": "A", "model": {"refId": "A", "expr": expr}}, + {"refId": "B", "model": {"refId": "B", "expression": "A"}}]} + + +def group(name, *rules): + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaAlertRuleGroup", + "metadata": {"name": name}, + "spec": {"rules": list(rules)}} + + +def dashboard(name, *exprs): + return {"apiVersion": "grafana.integreatly.org/v1beta1", + "kind": "GrafanaDashboard", + "metadata": {"name": name}, + "spec": {"json": json.dumps( + {"title": name, + "panels": [{"type": "timeseries", "title": f"p{i}", + "targets": [{"expr": e}]} + for i, e in enumerate(exprs)]})}} + + +NUMERATOR = 'svc_request_duration_seconds_bucket{job="svc",le="1"}' +DENOMINATOR = 'svc_request_duration_seconds_count{job="svc"}' +PANELS = (f"sum(rate({NUMERATOR}[30d])) / sum(rate({DENOMINATOR}[30d]))",) + + +class ReadingTheExpression(unittest.TestCase): + """What the rule itself states, before anything is compared to it.""" + + def test_the_burn_factor_is_the_one_divided_by_the_budget(self): + expr = burn_expr("0.01", "14.4", "1h", "5m") + self.assertEqual(gate.factors(expr), {Fraction("14.4")}) + + def test_a_traffic_guard_is_not_a_burn_factor(self): + """`sum(rate(...)) > bool 0.0167` keeps an idle service from alerting on + a ratio computed from no requests. Counted as a factor it makes a rule + look like two contradictory claims about one rate of spend, and the + rules carrying one are the majority of this catalog.""" + expr = burn_expr("0.001", "14.4", "1h", "5m", guard="0.0167") + self.assertEqual(gate.factors(expr), {Fraction("14.4")}) + + def test_the_long_window_is_the_period_the_claim_is_about(self): + expr = burn_expr("0.01", "6", "6h", "30m") + window, selectors = gate.long_window(expr) + self.assertEqual(window, 6 * 3600) + self.assertIn(NUMERATOR, selectors) + + def test_the_short_confirmation_window_is_not_taken_for_it(self): + """The short window exists to stop a spike that has already stopped from + firing; a claim about it would be a claim about a different alert.""" + expr = burn_expr("0.01", "14.4", "1h", "5m") + self.assertEqual(gate.long_window(expr)[0], 3600) + + def test_a_rule_with_no_burn_comparison_is_not_in_the_corpus(self): + found = list(gate.burn_rules(self.planted())) + self.assertEqual([r["title"] for _p, _g, r, _e in found], ["Burn"]) + + def planted(self): + d = pathlib.Path(tempfile.mkdtemp()) + (d / "g.yaml").write_text(yaml.safe_dump(group( + "g", + rule("Burn", "burning (2% in 1h)", + burn_expr("0.01", "14.4", "1h", "5m")), + rule("Rate", "error rate above 5%", + f"sum(rate({DENOMINATOR}[5m])) > 0.05")))) + return d + + +class TheDurationGrammar(unittest.TestCase): + def test_units_convert(self): + for text, want in (("5m", 300), ("1h", 3600), ("1d", 86400), + ("3d", 259200), ("30d", 2592000), ("1w", 604800)): + with self.subTest(duration=text): + self.assertEqual(gate.seconds(text), want) + + def test_seconds_render_as_the_unit_a_file_would_write(self): + for total, want in ((2592000, "30d"), (259200, "3d"), (3600, "1h"), + (1800, "30m"), (604800, "1w")): + with self.subTest(seconds=total): + self.assertEqual(gate.human(total), want) + + def test_a_factor_renders_as_the_decimal_in_the_expression(self): + """Exact arithmetic is what makes the comparison an equality rather than + a tolerance, but `72/5` is not a string anyone can search the file for.""" + self.assertEqual(gate.decimal(Fraction("14.4")), "14.4") + self.assertEqual(gate.decimal(Fraction(1)), "1") + self.assertEqual(gate.decimal(Fraction(6)), "6") + + +@unittest.skipUnless(HAS_KUSTOMIZE, "kustomize is not on PATH") +class TheVerdict(unittest.TestCase): + """main() over a planted alerting directory and dashboard corpus.""" + + def verdict(self, rules, panel_exprs=PANELS, ship_dashboard=True): + """main() over a planted tree, rendered the way the real one is. + + `ship_dashboard=False` writes the dashboard and leaves it out of the + kustomization's `resources`, which is what a panel that stops being + delivered looks like from disk. + """ + root = pathlib.Path(tempfile.mkdtemp()) + base = root / "dashboards" / "base" + alerts = base / "alerting" + boards = base / "platform" + alerts.mkdir(parents=True) + boards.mkdir(parents=True) + (alerts / "g.yaml").write_text(yaml.safe_dump(group("slo", *rules))) + (boards / "d.yaml").write_text( + yaml.safe_dump(dashboard("d", *panel_exprs))) + resources = [" - alerting/g.yaml"] + if ship_dashboard: + resources.append(" - platform/d.yaml") + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n" + "\n".join(resources) + "\n") + saved = (gate.ROOT, gate.ALERT_DIR, gate.KUSTOMIZE_ROOT) + gate.ROOT, gate.ALERT_DIR, gate.KUSTOMIZE_ROOT = root, alerts, base + try: + with contextlib.redirect_stdout(io.StringIO()) as out: + rc = gate.main() + finally: + gate.ROOT, gate.ALERT_DIR, gate.KUSTOMIZE_ROOT = saved + return rc, out.getvalue() + + def tiers(self): + """The four-tier ladder, each claim the one its expression spends + against a 30d objective.""" + return [ + rule("Fast", "burning fast (2% in 1h)", + burn_expr("0.01", "14.4", "1h", "5m")), + rule("Slow", "burning (5% in 6h)", + burn_expr("0.01", "6", "6h", "30m")), + rule("Ticket1d", "burning slowly (10% in 1d)", + burn_expr("0.01", "3", "1d", "2h")), + rule("Ticket3d", "burning slowest (10% in 3d)", + burn_expr("0.01", "1", "3d", "6h")), + ] + + def test_a_consistent_ladder_passes(self): + """The control. Without it every case below could be failing for a + reason the case did not plant.""" + rc, out = self.verdict(self.tiers()) + self.assertEqual(rc, 0, out) + + def test_the_defect_this_gate_was_written_for(self): + """Factor 1 over 3d against a 30d objective spends 10%, not 100%. The + expression is right and only the sentence is wrong, which is why every + rule-level validation passed it.""" + tiers = self.tiers() + tiers[3]["annotations"]["summary"] = "burning (100% over 3d)" + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("claims '100% over 3d'", out) + self.assertIn("spends 10% over 3d", out) + self.assertIn("1 x 3d / 30d", out) + + def test_a_factor_that_drifts_from_its_claim_is_reported(self): + tiers = self.tiers() + tiers[0]["data"][0]["model"]["expr"] = burn_expr( + "0.01", "7.2", "1h", "5m") + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("burn factor 7.2", out) + + def test_an_objective_window_that_moves_on_the_dashboard_is_reported(self): + """The term that lives in another file. Nothing in the alerting + directory changes here, and every claim in it becomes wrong.""" + rc, out = self.verdict( + self.tiers(), + panel_exprs=(f"sum(rate({NUMERATOR}[60d])) / " + f"sum(rate({DENOMINATOR}[60d]))",)) + self.assertEqual(rc, 1, out) + self.assertIn("60d objective", out) + self.assertEqual(out.count("claims"), 4) + + def test_a_claim_compared_to_nothing_is_reported(self): + """No panel measures what the rule burns against, so the figure agrees + with whatever it says. That is the state the defect survived in.""" + rc, out = self.verdict( + self.tiers(), panel_exprs=('sum(rate(other_metric{job="x"}[30d]))',)) + self.assertEqual(rc, 1, out) + self.assertIn("compared to nothing", out) + + def test_a_burn_rule_stating_no_figure_is_reported(self): + """Otherwise deleting the claim removes the rule from this gate's + corpus, and that is the edit most likely to accompany a wrong one.""" + tiers = self.tiers() + tiers[0]["annotations"]["summary"] = "budget burning fast" + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("states no budget figure", out) + + def test_a_summary_naming_another_window_is_reported(self): + tiers = self.tiers() + tiers[0]["annotations"]["summary"] = "burning fast (2% in 2h)" + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("measures over 1h", out) + + def test_two_burn_factors_in_one_rule_are_reported(self): + tiers = self.tiers() + tiers[0]["data"][0]["model"]["expr"] = ( + f"(sum(rate({NUMERATOR}[1h])) / 0.01 > bool 14.4)" + f" * (sum(rate({NUMERATOR}[5m])) / 0.01 > bool 6)") + rc, out = self.verdict(tiers) + self.assertEqual(rc, 1, out) + self.assertIn("2 different burn factors", out) + + def test_two_objective_windows_for_one_rule_are_reported(self): + """Which panel states the objective decides the figure. A tree stating + both has not decided, and picking one here would be this gate choosing + the answer it checks against.""" + rc, out = self.verdict(self.tiers(), panel_exprs=( + f"sum(rate({NUMERATOR}[30d]))", + f"sum(rate({DENOMINATOR}[7d]))", + )) + self.assertEqual(rc, 1, out) + self.assertIn("which of those", out.lower()) + + def test_a_panel_that_stops_being_delivered_is_reported(self): + """The dashboard is on disk, unedited, and renders fine — it is simply + no longer in the kustomization's `resources`. The rules still ship and + still burn; the panel measuring what they burn against does not. A + figure anchored to a document no cluster receives is anchored to + nothing.""" + rc, out = self.verdict(self.tiers(), ship_dashboard=False) + self.assertEqual(rc, 1, out) + self.assertIn("no dashboard delivered by", out) + + def test_a_render_that_fails_cannot_run(self): + """Exit 2. A kustomization that does not build says nothing about which + panels a cluster receives, and the dashboards on disk say nothing about + it either.""" + root = pathlib.Path(tempfile.mkdtemp()) + base = root / "dashboards" / "base" + base.mkdir(parents=True) + (base / "kustomization.yaml").write_text( + "apiVersion: kustomize.config.k8s.io/v1beta1\n" + "kind: Kustomization\nresources:\n - platform/gone.yaml\n") + with self.assertRaises(SystemExit) as caught, \ + contextlib.redirect_stdout(io.StringIO()) as out: + gate.delivered_dashboards(base) + self.assertEqual(caught.exception.code, gate.gatelib.CANNOT_RUN) + self.assertIn("could not be read", out.getvalue()) + + def test_no_burn_rule_at_all_cannot_run(self): + """Exit 2. A directory with no burn rule reports what a directory of + correct claims reports.""" + rc, out = self.verdict([rule("Rate", "error rate above 5%", + f"sum(rate({DENOMINATOR}[5m])) > 0.05")]) + self.assertEqual(rc, gate.gatelib.CANNOT_RUN, out) + self.assertIn("no burn-rate rule", out) + + +class TheShippedCatalog(unittest.TestCase): + """Over the tree, so a rule added with a wrong figure fails here.""" + + def test_the_catalog_passes(self): + with contextlib.redirect_stdout(io.StringIO()) as out: + self.assertEqual(gate.main(), 0, out.getvalue()) + + def test_the_ladder_carries_more_than_one_factor(self): + """A catalog whose tiers all shared a factor would pass a gate that + derived the figure from any one of them, so the corpus this runs over is + asserted rather than assumed.""" + seen = set() + for _p, _g, _r, expr in gate.burn_rules(gate.ALERT_DIR): + seen |= gate.factors(expr) + self.assertGreaterEqual(len(seen), 3, seen) + + def test_every_burn_rule_is_anchored_to_a_measured_objective(self): + """The figure is only as good as the thing it is compared against.""" + windows = gate.objective_windows() + for path, group_name, r, expr in gate.burn_rules(gate.ALERT_DIR): + with self.subTest(rule=f"{path.name}:{r.get('title')}"): + selectors = gate.long_window(expr)[1] + self.assertTrue( + [s for s in selectors if s in windows], + f"{group_name}/{r.get('title')} burns against " + f"{selectors} and no dashboard panel measures any of them") + + def test_the_rules_this_reads_are_rules_the_catalog_delivers(self): + """This gate reads the alerting directory; a cluster receives what the + kustomization renders. It does not take the second reading itself, + because check-alert-severity-routes.py already refuses a rule group on + disk that the same root does not render — so the corpus here is the + delivered one by way of that gate. + + Asserted rather than assumed. The two gates have to agree on the root + and on the kind, and a narrowing on either side would otherwise leave + this one reading files no cluster has, silently. + """ + routes = load("check-alert-severity-routes") + self.assertEqual(routes.KUSTOMIZE_ROOT, gate.KUSTOMIZE_ROOT, + "the two alerting gates render different roots, so one " + "of them is asserting over a tree the other does not") + self.assertIn(gate.RULE_GROUP, routes.ALERTING_KINDS, + f"{routes.__name__} no longer refuses an unrendered " + f"{gate.RULE_GROUP}, so this gate's corpus is files on " + f"disk rather than what a cluster receives") + + def test_the_objective_is_read_from_the_dashboards_not_declared_here(self): + """A constant agreeing with the standard today and compared to nothing + tomorrow is the defect this gate was written for, one level up.""" + source = (ROOT / "scripts" / "check-burn-rate-budgets.py").read_text() + self.assertNotIn("2592000", source) + self.assertNotIn('"30d"', source) + self.assertIn('subprocess.run(["kustomize", "build"', source) + + +if __name__ == "__main__": + unittest.main()