Skip to content

fix(grafana-alerting): adopt the MIT Learn synthetic checks and give them a window that can see a creep - #5400

Merged
blarghmatey merged 3 commits into
mainfrom
worktree-mitlearn-nextjs-probe-alert
Aug 13, 2026
Merged

fix(grafana-alerting): adopt the MIT Learn synthetic checks and give them a window that can see a creep#5400
blarghmatey merged 3 commits into
mainfrom
worktree-mitlearn-nextjs-probe-alert

Conversation

@blarghmatey

Copy link
Copy Markdown
Member

What are the relevant tickets?

No ol-infrastructure ticket. Prompted by the 2026-08-13 paging incident; the application-side cause is mitodl/mit-learn#3771

Description (What does it do?)

Brings the three hand-made MIT Learn Synthetic Monitoring rules under Pulumi and rewrites them. All three were created in the Grafana UI and unmanaged.

  • Adds metric_rules/synthetic_monitoring.py covering MIT Learn NextJS, MIT Learn API and MIT Learn Website. Rule groups are already imported into the Production stack, so this is ~ 3 to update, not a create.
  • Replaces the < 1 threshold, which paged on any one failed probe out of five from a single probe location. It fired twice within one hour on 2026-08-13 at A=0.75 and A=0.8, i.e. 3/4 and 4/5 probes succeeding.
  • Rewrites the annotations. All three carried a summary copied from a latency rule ("MIT Learn NextJS response times 30% Greater Than Normal") while the query measures availability.
  • Drops notification_settings.receiver="Rootly" from all three. That pointed at a second, UI-created contact point (uid eel3rjpiwahoge) distinct from Pulumi's rootly (uid bfsoqo63lsyrka) and bypassed the alertmanager.py policy tree entirely. They now route by severity label like everything else.
  • noDataState NoData -> OK. With the threshold baked into the PromQL a healthy check returns no series at all, so NoData would describe the healthy state on every evaluation.

Two things worth a reviewer's attention:

Each check gets two windows, not one. Threshold alone was not the fix. The incident that prompted this never produced a 5m window worse than 2 of 5 failures, held for ~2 minutes — short of for: 5m on a 300s interval. < 0.6 as proposed in docs/plans/grafana-alerting-remediation-spec.md would have been silent throughout, and so would the looser 2-of-5 used here. Over a 1h window the same period was unmistakable: flat 0 for the preceding ten hours, then a climb through 3%, 5%, 7% to 10%. So there is a fast rule for a cliff and a slow rule for the creep the fast one structurally cannot see, mirroring metric_rules/apisix_edge.py.

The slow rules ship unrouted. No severity label, so alertmanager.py's default route drops them in oblivion — evaluated and recorded in grafanacloud-alert-state-history, delivered nowhere. Measurement says both thresholds are still guesses: 4% over 1h would fire ~5 times per week on the API health endpoint alone, whose 1h failure ratio peaks at 8.3% in normal operation. Routing that at critical today would recreate the paging problem this is meant to end. Per-check 7d baselines are recorded in the module docstring so promotion is an evidence-based decision.

Also fixes a trap that the obvious translation walks into. avg_over_time(...) < 0.8 returns availability as its value, and base.py's pipeline fires on last(A) > 0. When every probe fails, availability is 0, the comparison passes, and the threshold stage reads 0 as "not firing" — so it would alert on a partial outage and go silent on a total one. The rules measure the failure ratio instead, whose value rises as the check gets worse.

How can this be tested?

Run from src/ol_infrastructure/infrastructure/grafana_alerting:

pulumi preview --stack Production   # ~ 3 to update, 26 unchanged, no replacements
pulumi preview --stack CI           # 26 unchanged
pulumi preview --stack QA           # see Additional Context re: 2 dashboard deletions

What I ran:

  • pulumi preview on all three stacks, results above. The Production diff shows the expression change, notificationSettings removed, severity added, noDataState corrected, and the new slow rule — with no resource replacement and the existing rule UIDs unchanged.
  • Every generated PromQL expression executed against the live grafanacloud-prom datasource, confirming it returns the labels the annotations template on (instance, probe) and returns nothing while healthy.
  • Both thresholds backtested over 7 days for all three checks. Fast > 0.3 for 5m: would not have fired once, on any check, including the 2026-08-10 moment when all 5 Next.js probes in a window failed. Slow > 0.04 for 30m: ~5 firings on the API endpoint, which is why it ships unrouted.
  • pre-commit run, ruff, mypy — all pass.

I have not run pulumi up. Nothing in Grafana has changed yet.

To validate after applying, confirm the three rules show the new expressions in the Grafana UI and that Learn NextJS Homepage (Bypass Fastly) - Check Failed still carries uid afjaps6wtn0n4a.

Additional Context

The Production import is already done. The three rule groups were adopted into Production Pulumi state with pulumi import before this PR. That is state-only and changed nothing in Grafana, but it is not undone by closing this PR. If this is rejected, run pulumi state delete on the three grafana:alerting/ruleGroup:RuleGroup URNs, otherwise the next pulumi up sees them declared-but-absent.

CLI pulumi import does not honour --provider here and fails against this program's explicit Grafana provider with "the Grafana client is required for this resource". It works if GRAFANA_URL / GRAFANA_AUTH are exported from src/bridge/secrets/grafana_cloud/api.production.yaml first. Noted in the directory's CLAUDE.md.

Production-only, deliberately. The Synthetic Monitoring folder UID is per-stack, unlike the grafanacloud-prom datasource UID that is uniform everywhere: Production is grafana-synthetic-monitoring-app, CI is ffqmgh1ukxam8a (plus a legacy 6GJToXwnz), QA has no such folder. Without the guard in create() this breaks at the pipeline's first stage. The probed hosts are production-only regardless.

Pre-existing QA drift, not from this branch. pulumi preview --stack QA wants to delete keycloak-overview-dashboard and keycloak-activity-dashboard. Neither name appears anywhere in the codebase and this branch does not touch dashboards/ — they are stale entries in QA state from an earlier version. Flagging so it is not mistaken for fallout from this change, but it means a QA apply will remove them.

The two sibling rules were included on purpose rather than left for later: they had the identical threshold, annotation and receiver-bypass defects, and leaving two unmanaged rules in a folder Pulumi now owns invites drift.

blarghmatey and others added 2 commits August 13, 2026 13:14
… them a window that can see a creep

The NextJS probe rule paged twice in an hour on 2026-08-13 for a single failed
probe out of five, from one probe location, with a summary ("response times 30%
Greater Than Normal") copy-pasted from a latency rule it has nothing to do with.
It and its two siblings were made in the UI and unmanaged.

Threshold alone was not the fix. The incident that prompted this -- a deploy that
put a jsdom parse on every SSR render, leaving the origin timing out at the
check's 15s ceiling for two hours -- never produced a 5m window worse than 2 of 5
failures held for ~2 minutes. The remediation spec's proposed `< 0.6` would have
been silent throughout, and so would the looser 2-of-5 used here. Over a 1h
window the same period was unmissable: flat 0 for ten hours, then 3% -> 10%.

So each check gets two rules: a fast one for a cliff, and a slow one for the
creep that the fast one structurally cannot see. The slow rules ship with no
severity label, into `oblivion`, because measurement says both thresholds are
still guesses -- 4% over 1h would fire ~5x/week on the API health endpoint alone,
whose baseline peaks at 8.3%. Their firing history is what should set the real
numbers, same as apisix_edge.py.

Also fixes a trap in the obvious translation: `avg_over_time(...) < 0.8` returns
the availability as its value, which base.py's `> 0` threshold stage reads as
"not firing" precisely when availability is 0. It would alert on a partial outage
and go quiet on a total one. Measuring the failure ratio inverts that.

Drops the `notification_settings.receiver="Rootly"` override on all three, which
pointed at a second UI-created contact point and bypassed the policy tree.

Rule UIDs are pinned to the imported values so alert history, live silences, and
the rule URLs in past Rootly alerts survive.

Refs mitodl/mit-learn#3771

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XBP7CuQXmxLnPNjQvk4aCz
The Synthetic Monitoring folder UID is per-stack, unlike the grafanacloud-prom
datasource UID that is uniform everywhere. Production is
`grafana-synthetic-monitoring-app`, CI is `ffqmgh1ukxam8a` (plus a legacy
`6GJToXwnz`), and QA has no such folder at all -- so registering these anywhere
but production targets a folder that does not exist, and the pipeline runs
CI -> QA -> Production.

The checks themselves are production-only anyway: learn.mit.edu,
api.learn.mit.edu and next.learn.mit.edu are production hosts and the probes
feeding probe_success are only configured on that stack.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XBP7CuQXmxLnPNjQvk4aCz
Copilot AI balanced review requested due to automatic review settings August 13, 2026 17:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adopts production MIT Learn synthetic checks into Pulumi and improves detection following mit-learn#3771.

Changes:

  • Adds fast and slow failure-ratio alerts for three endpoints.
  • Routes calibrated fast alerts by severity while leaving slow alerts unrouted.
  • Documents imported-rule handling and integrates the rules into Grafana alerting.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
metric_rules/synthetic_monitoring.py Defines production synthetic-monitoring rules.
metric_rules/base.py Registers the new rule module.
CLAUDE.md Documents synthetic rules and UI-rule imports.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ol_infrastructure/infrastructure/grafana_alerting/CLAUDE.md
…needs

pulumi import resolves against the CLI's default provider rather than the
explicit grafana.Provider __main__.py builds, and --provider does not override
it, so the documented recipe failed with "the Grafana client is required for
this resource" until GRAFANA_URL/GRAFANA_AUTH were exported.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XBP7CuQXmxLnPNjQvk4aCz
@blarghmatey
blarghmatey merged commit 583c58f into main Aug 13, 2026
7 checks passed
@blarghmatey
blarghmatey deleted the worktree-mitlearn-nextjs-probe-alert branch August 13, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants