Full rebrand across cosmetic branding, code identifiers, and infrastructure/data-plane naming, using the supplied Cairn OBS logo package. Cosmetic: favicon/logo swap (also closes a stale license-audit finding -- the old favicon was SvelteKit's unreplaced scaffold logo), new centered welcome landing page, larger/legible sidebar logo, page titles, CLAUDE.md/README/docs prose. Code identifiers: Go module path github.com/sentry/sentry -> github.com/cairnobs/cairnobs across all 13 modules and ~91 files (protoc regenerated); Rust crates sentry-agent/sentry-parser/sentry-search -> cairnobs-*; CLI sentryctl -> cairnobsctl; Terraform provider fully renamed (sentry_dashboard etc. -> cairnobs_dashboard, provider type, env vars); every session/auth cookie name; agent config paths and Windows service identity. Deliberately preserved: the gRPC wire protocol's protobuf packages (sentry.logs.v1, sentry.agent.v1) and their Go import directory (proto/sentry/...) -- renaming the wire-level package would break every currently-deployed agent binary (confirmed two real hosts, including mail.inbuxa.com, are actively streaming through this exact contract) until rebuilt and redeployed in lockstep with an ingest cutover. Only the Go module path wrapping the generated code changes. Infrastructure: every docker-compose container name (root and three component-level compose files); the Helm chart (directory, Chart.yaml, named-template helpers, all templates, values.yaml image repos); Kubernetes Operator (CRD group sentry.io -> cairnobs.io, both CRD YAML files, Go identifiers, RBAC markers); the coupled enterprise/tenantcrd package. Caught and fixed real path-coupling bugs along the way: the Helm chart's search/ingest volume mounts and the dev-only-credential detection constant vs. docker-compose.yml's literal values had to move together or a security warning would have silently stopped firing. Data plane: Postgres database sentry_metadata -> cairnobs_metadata and role sentry -> cairnobs; ClickHouse database sentry -> cairnobs; Kafka topic sentry.logs.raw -> cairnobs.logs.raw and its consumer groups. Source-level defaults, docker-compose.yml, and every migrate.sh/ provision script default updated together; already-applied migration files left untouched per this repo's immutable-migration convention. Verified at every layer: all 13 Go modules build/vet/test clean, both Rust workspaces (agent, search) build/clippy/test clean, npm run check/ build clean, docker compose config validates on all four compose files. Live-verified against a real docker stack multiple times through this work, including a final fresh-volume run confirming the actual renamed Postgres database/role, ClickHouse database, and Kafka topic all work end to end with a real login and query, zero console errors.
359 lines
19 KiB
Markdown
359 lines
19 KiB
Markdown
# Alerting design
|
||
|
||
> **Status:** Design only — no code written against this yet. Task 4 of
|
||
> Phase 3. Per explicit instruction, execution stops here for review
|
||
> before task 5 (building `/alerting`) begins: the firing/resolved state
|
||
> machine and debounce behavior are easy to get subtly wrong, and this is
|
||
> the artifact to sign off on before anything is built against it. If
|
||
> implementation later reveals this design is wrong somewhere, fix this
|
||
> doc in the same change — same discipline as
|
||
> `/docs/query-language-design.md` and
|
||
> `/docs/phase-3-dashboard-design.md`.
|
||
|
||
## Why this design, in one paragraph
|
||
|
||
A rule is a saved Phase 2 query plus a condition (threshold or absence)
|
||
plus an evaluation interval plus a notification target. The genuinely
|
||
hard part isn't the data model — it's the firing/resolved state machine
|
||
under concurrent evaluation and its interaction with notification
|
||
delivery, where a naive implementation can plausibly double-fire, lose a
|
||
resolve, or silently misreport an infrastructure outage as "all clear."
|
||
This doc's state machine mirrors Prometheus Alertmanager's well-understood
|
||
`pending`/`firing` `for:` model, then adds four specific correctness
|
||
properties on top of it — not because the base model is wrong, but
|
||
because the *concurrent, at-least-once* environment a Go ticker-based
|
||
evaluator actually runs in exposes gaps a single-threaded description of
|
||
the model glosses over. Each fix below is stated as: the failure it
|
||
prevents, and the mechanism, so it's reviewable as a claim rather than
|
||
just an assertion.
|
||
|
||
## Data model
|
||
|
||
A rule is a saved query, evaluated on an interval, checked against a
|
||
condition, with a debounce before it's allowed to notify, and one
|
||
notification target it notifies. Two condition types:
|
||
|
||
- **`threshold`**: the query's result must resolve to exactly one row;
|
||
the value in that row's first column is compared against
|
||
`threshold_value` via `comparator`.
|
||
- **`absence`**: the query returned zero rows. The evaluation *window*
|
||
is not a separate rule field — it's whatever `earliest=`/`latest=` the
|
||
rule's own saved query already expresses (e.g. `service=payments
|
||
severity=ERROR earliest=-5m`), reusing Phase 2's time-range syntax
|
||
rather than inventing a second one.
|
||
|
||
```sql
|
||
CREATE TABLE notification_targets (
|
||
id UUID PRIMARY KEY,
|
||
tenant_id TEXT NOT NULL DEFAULT 'default',
|
||
name TEXT NOT NULL,
|
||
kind TEXT NOT NULL CHECK (kind IN ('webhook', 'slack', 'pagerduty')),
|
||
webhook_url TEXT NOT NULL,
|
||
payload_template TEXT, -- generic ("webhook") targets only; NULL for slack/pagerduty
|
||
headers JSONB NOT NULL DEFAULT '{}',
|
||
secret TEXT, -- PagerDuty routing key / generic-webhook HMAC secret -- plaintext, see "Known gaps" below
|
||
created_by TEXT NOT NULL DEFAULT 'anonymous',
|
||
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
||
);
|
||
|
||
CREATE TABLE alert_rules (
|
||
id UUID PRIMARY KEY,
|
||
tenant_id TEXT NOT NULL DEFAULT 'default',
|
||
name TEXT NOT NULL,
|
||
description TEXT NOT NULL DEFAULT '',
|
||
query TEXT NOT NULL,
|
||
query_language TEXT NOT NULL DEFAULT '',
|
||
condition_type TEXT NOT NULL CHECK (condition_type IN ('threshold', 'absence')),
|
||
comparator TEXT CHECK (comparator IN ('gt', 'gte', 'lt', 'lte', 'eq', 'ne')), -- NULL for absence
|
||
threshold_value DOUBLE PRECISION, -- NULL for absence
|
||
eval_interval_seconds INT NOT NULL CHECK (eval_interval_seconds >= 30),
|
||
for_minutes INT NOT NULL DEFAULT 0, -- 0 = fire on first true evaluation
|
||
renotify_interval_minutes INT, -- NULL = notify once per firing episode
|
||
notification_target_id UUID NOT NULL REFERENCES notification_targets(id),
|
||
enabled BOOLEAN NOT NULL DEFAULT true,
|
||
created_by TEXT NOT NULL DEFAULT 'anonymous',
|
||
created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
|
||
updated_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
||
);
|
||
|
||
CREATE TABLE alert_state (
|
||
rule_id UUID PRIMARY KEY REFERENCES alert_rules(id) ON DELETE CASCADE,
|
||
state TEXT NOT NULL DEFAULT 'ok' CHECK (state IN ('ok', 'pending', 'firing')),
|
||
condition_true_since TIMESTAMPTZ,
|
||
fired_at TIMESTAMPTZ,
|
||
last_notified_at TIMESTAMPTZ,
|
||
last_evaluated_at TIMESTAMPTZ,
|
||
last_eval_status TEXT NOT NULL DEFAULT 'ok' CHECK (last_eval_status IN ('ok', 'error')),
|
||
last_error TEXT,
|
||
last_value DOUBLE PRECISION,
|
||
consecutive_errors INT NOT NULL DEFAULT 0,
|
||
next_eval_at TIMESTAMPTZ NOT NULL, -- the claim column, see "Concurrency" below
|
||
claimed_at TIMESTAMPTZ
|
||
);
|
||
|
||
CREATE TABLE delivery_log (
|
||
id BIGSERIAL PRIMARY KEY,
|
||
rule_id UUID NOT NULL REFERENCES alert_rules(id) ON DELETE CASCADE,
|
||
notification_target_id UUID NOT NULL REFERENCES notification_targets(id),
|
||
event_type TEXT NOT NULL CHECK (event_type IN ('firing', 'resolved')),
|
||
status TEXT NOT NULL DEFAULT 'pending' CHECK (status IN ('pending', 'sent', 'failed', 'retrying')),
|
||
attempt_count INT NOT NULL DEFAULT 0,
|
||
max_attempts INT NOT NULL DEFAULT 5,
|
||
next_attempt_at TIMESTAMPTZ, -- the delivery worker's own claim key
|
||
last_attempt_at TIMESTAMPTZ,
|
||
last_error TEXT,
|
||
response_status INT,
|
||
payload JSONB NOT NULL, -- the actual rendered payload -- needed to debug template issues
|
||
created_at TIMESTAMPTZ NOT NULL DEFAULT now()
|
||
);
|
||
CREATE INDEX ON delivery_log (rule_id, created_at DESC); -- per-rule delivery log UI
|
||
CREATE INDEX ON delivery_log (status, next_attempt_at) WHERE status IN ('pending', 'retrying'); -- delivery worker's claim query
|
||
```
|
||
|
||
**`alert_state` must be inserted in the same transaction as its owning
|
||
`alert_rules` row**, `state = 'ok'`, `next_eval_at = now()` (evaluate
|
||
immediately on creation). A rule with no `alert_state` row is silently
|
||
never picked up by the claim query below — worth stating loudly since
|
||
it's the kind of thing that "just works" in every test that remembers to
|
||
seed both rows and then silently doesn't in production the one time it's
|
||
forgotten.
|
||
|
||
Single notification target per rule for MVP. Multi-target (e.g. page AND
|
||
Slack) is a disclosed, straightforward future extension — a join table,
|
||
not a redesign.
|
||
|
||
## Notification delivery: generic webhook as the base primitive
|
||
|
||
All three `notification_targets.kind` values ultimately do the same
|
||
thing — an HTTP POST to `webhook_url` — with kind-specific *payload
|
||
formatting only*, per the explicit requirement that this stays pluggable
|
||
rather than accumulating per-vendor delivery logic:
|
||
|
||
- **`webhook`**: `payload_template` is a Go `text/template` string,
|
||
rendered against the firing/resolved event (rule name, condition,
|
||
current value, timestamp). No template = a sane default JSON shape.
|
||
- **`slack`**: fixed formatter producing Slack's incoming-webhook shape
|
||
(`{"text": "..."}`), no user template. `payload_template` is ignored
|
||
for this kind (kept nullable in the schema rather than removed, so a
|
||
target's `kind` can be changed later without a payload column migration).
|
||
- **`pagerduty`**: fixed formatter producing PagerDuty's Events API v2
|
||
shape (`{"routing_key": secret, "event_action": "trigger"|"resolve",
|
||
"payload": {...}}`) — `secret` here is the PagerDuty integration
|
||
routing key, not a delivery credential in the auth sense.
|
||
|
||
All three go through the exact same HTTP POST + retry/backoff mechanism
|
||
in `internal/delivery/webhook.go`; `slack.go`/`pagerduty.go` are payload
|
||
builders only, never their own delivery path.
|
||
|
||
## The state machine
|
||
|
||
States per rule: `ok` → `pending` → `firing`, tracked in `alert_state`.
|
||
Every evaluation produces one of three outcomes — `condition_true`,
|
||
`condition_false`, or `error` — and error is handled entirely separately
|
||
from the other two (see fix 3).
|
||
|
||
**On `condition_true`:**
|
||
- `ok` → `pending`: set `condition_true_since = now()`. No notification.
|
||
- `pending` → `firing`, once `now() - condition_true_since >=
|
||
for_minutes`: send a **firing** notification, set `fired_at =
|
||
last_notified_at = now()`. `for_minutes = 0` means this transition
|
||
happens on the very first true evaluation.
|
||
- `pending`, not yet past `for_minutes`: no transition, no notification.
|
||
- `firing` → stays `firing`: silent, unless `renotify_interval_minutes`
|
||
is set and `now() - last_notified_at >= renotify_interval_minutes`, in
|
||
which case re-send **firing** and update `last_notified_at`. Default
|
||
(`NULL`) is "notify once per firing episode, stay silent until
|
||
resolved" — stated explicitly since it's exactly the kind of default an
|
||
implementer would otherwise have to guess.
|
||
|
||
**On `condition_false`:**
|
||
- `ok` → no-op.
|
||
- `pending` → `ok`: clear `condition_true_since`. **No notification** —
|
||
this was a blip inside the debounce window, not a real alert. This is
|
||
deliberate, not an oversight: it's the entire reason `for_minutes`
|
||
exists.
|
||
- `firing` → `ok`: clear `condition_true_since`/`fired_at`, send a
|
||
**resolved** notification.
|
||
|
||
`condition_true_since` is a **wall-clock timestamp**, not a
|
||
consecutive-evaluation counter. This is what makes the debounce survive
|
||
evaluator restarts/downtime correctly: if the evaluator is down for part
|
||
of a rule's `for_minutes` window and comes back, wall-clock math
|
||
correctly resumes toward firing where it left off, while a counter would
|
||
have silently lost that progress and restarted the count. Worth
|
||
defending explicitly here since a counter looks like the "simpler"
|
||
choice and a future contributor might "simplify" it into one.
|
||
|
||
**Disclosed non-goal: no debounce on the way down.** `firing` → `ok`
|
||
happens on a single false evaluation — there's no symmetric "stay firing
|
||
for N more minutes" hold (Grafana calls this "keep firing for"). A
|
||
condition that flickers right at the threshold produces a firing/resolved
|
||
notification pair per flicker. Future work, not solved in Phase 3.
|
||
|
||
### Four correctness properties, and the concrete failure each one prevents
|
||
|
||
**1. Concurrent evaluation of the same rule (claim-then-evaluate).**
|
||
A worker-pool evaluator is required at the scale task 8 targets (~500
|
||
rules @ 60s ≈ 8+ evaluations/sec sustained). If a single evaluation's
|
||
round-trip to `api`'s `/query` ever takes longer than the rule's own
|
||
interval, the next scheduler tick can pick the *same* rule again while
|
||
the first evaluation is still in flight — two goroutines then read-
|
||
modify-write the same `alert_state` row concurrently. Depending on
|
||
timing, that produces either a duplicated firing notification (both see
|
||
`pending`, both compute "elapsed ≥ for_minutes", both fire) or a lost
|
||
resolve (a slow evaluation's stale write clobbers a faster one).
|
||
|
||
Fix: atomically claim due rules **before** the slow network call starts:
|
||
|
||
```sql
|
||
UPDATE alert_state
|
||
SET next_eval_at = now() + (eval_interval_seconds || ' seconds')::interval,
|
||
claimed_at = now()
|
||
FROM alert_rules
|
||
WHERE alert_state.rule_id = alert_rules.id
|
||
AND alert_state.next_eval_at <= now()
|
||
AND alert_rules.enabled
|
||
LIMIT $batch_size
|
||
RETURNING alert_state.rule_id, alert_state.state, alert_state.condition_true_since, ...
|
||
```
|
||
|
||
`next_eval_at` is bumped *before* the `/query` HTTP call ever starts, so
|
||
a second scheduler tick can't re-select the same rule while the first is
|
||
still running. The `/query` call itself happens **outside** any database
|
||
transaction — never hold a Postgres connection open across a network
|
||
call to another service. A second, short transaction applies the
|
||
resulting state transition once the query result is known.
|
||
|
||
This same claim pattern is what makes horizontal evaluator replicas safe
|
||
to add later (a named Phase 4+ path, see task 8) without redesigning the
|
||
state model — each replica's claim query naturally excludes rows another
|
||
replica already claimed. Worth stating positively: this is the one place
|
||
in Phase 3 that's actively built *for* that future need, not just
|
||
avoiding a trap.
|
||
|
||
**2. Notification loss/duplication on crash (transactional outbox).**
|
||
If the state transition and the webhook POST happen as separate,
|
||
sequentially-ordered steps, a crash between them is unrecoverable in one
|
||
direction or the other: crash after a successful POST but before the DB
|
||
commit → next evaluation replays the transition and double-fires; crash
|
||
after commit but before the POST → the notification is silently owed
|
||
forever with no record that it was ever decided.
|
||
|
||
Fix: the state transition and `INSERT INTO delivery_log (..., status =
|
||
'pending')` happen in the **same database transaction** — "we decided to
|
||
notify" becomes durable exactly once, atomically with the state change
|
||
itself, before any network call to a notification target is attempted. A
|
||
**separate** delivery worker polls `delivery_log WHERE status IN
|
||
('pending', 'retrying') AND next_attempt_at <= now()` (same claim
|
||
pattern as fix 1), performs the actual HTTP POST, and updates
|
||
`status`/`attempt_count`/`last_error`. This decouples "did we decide to
|
||
notify" (transactionally certain) from "did the HTTP call succeed"
|
||
(best-effort with retries) — which is exactly what task 5's retry-with-
|
||
backoff requirement needs anyway, so this isn't extra machinery bolted
|
||
on for correctness's sake, it's the same piece of work.
|
||
|
||
**3. Query errors must never be treated as `condition_false`.**
|
||
The state machine above only describes `condition_true`/
|
||
`condition_false` — what happens when the `/query` call to `api` itself
|
||
fails (timeout, ClickHouse down, a 5xx)? Coercing an error to "false" is
|
||
the tempting default and the worst possible one: a `firing` alert would
|
||
silently auto-resolve and go quiet at precisely the moment something is
|
||
broken enough that the query can't even run. Coercing to "true" is
|
||
equally wrong the other way (spurious pages on transient infra hiccups).
|
||
|
||
Fix: evaluation outcome is modeled as a three-way result, and an `error`
|
||
outcome **never transitions `state`** at all. It only updates
|
||
`alert_state.last_evaluated_at`, `last_eval_status = 'error'`,
|
||
`last_error`, and increments `consecutive_errors`. This must be explicit
|
||
in the implementation, not left to the "obvious" path — the obvious
|
||
path (an error propagating into whatever boolean the rest of the
|
||
function expects) is also the wrong one.
|
||
|
||
**4. Zero rows on a `threshold` rule is an error, not a `0`.**
|
||
`stats count by host` can legitimately return zero rows for a threshold
|
||
rule (nothing matched in the window) — that's different in kind from an
|
||
`absence` rule, where zero rows *is* the signal being checked for. If a
|
||
threshold evaluation coerces "no rows" to a scalar `0` for comparison,
|
||
`count > 100` silently reports "definitely fine" in exactly the case
|
||
where the honest answer is "the query returned nothing, which might mean
|
||
nothing happened, or might mean something upstream is broken" — often
|
||
the more alarming possibility, not the safe one.
|
||
|
||
Fix, stated as an explicit rule rather than left implicit: `threshold`
|
||
evaluation requires **exactly one** result row (first row, first numeric
|
||
column, per the dashboard/query design's single-row precedent). Zero
|
||
rows — or more than one — is treated the same as fix 3's evaluation
|
||
error, not coerced to a value. Named non-goal alongside "no per-group
|
||
alerting": a threshold rule's query must resolve to a scalar.
|
||
|
||
## Evaluator architecture
|
||
|
||
A single Go process (`/alerting`), ticker-driven — **not** a workflow
|
||
engine, per the explicit instruction not to reach for one at this stage.
|
||
Loop shape:
|
||
|
||
1. Every few seconds, run the claim query (fix 1) for due, enabled rules
|
||
up to a bounded batch size.
|
||
2. Dispatch claimed rules to a bounded worker pool (goroutines).
|
||
3. Each worker: `POST /query` against `api` (reusing the existing
|
||
endpoint — the same precedent `cairnobsctl query` already set, never a
|
||
second query-execution path), evaluate the condition, run the state
|
||
transition (fix 3/4 aware) and, if applicable, the fix-2 transactional
|
||
outbox insert, in one short DB transaction.
|
||
4. A separate delivery-worker loop claims and sends `delivery_log` rows
|
||
independently of the evaluation loop.
|
||
|
||
`alerting` is therefore hard-dependent on `api` being reachable — if
|
||
`api`/ClickHouse is down, every due rule's evaluation records an error
|
||
(fix 3), not a false resolve. This is documented behavior, not an
|
||
accident: you can't trust a condition you can't evaluate. `alerting`'s
|
||
docker-compose entry depends on `api`'s healthcheck (added in task 3)
|
||
for this reason.
|
||
|
||
## Component boundary
|
||
|
||
`/alerting` (new top-level Go service, own `go.mod`) owns rule CRUD,
|
||
notification-target CRUD, the delivery-log read endpoint, the evaluator
|
||
loop, and the delivery worker — all four pieces of "alerting" in one
|
||
service, since they share the same Postgres tables and the same
|
||
claim-based concurrency pattern. It does **not** import `api`'s
|
||
`querylang` package or talk to ClickHouse/Tantivy directly; it only
|
||
calls `api`'s `POST /query` over HTTP, exactly like `cairnobsctl query`
|
||
and the web UI's dashboard panels already do. `web` gets a second
|
||
backend base URL (`alerting`'s) alongside the existing `api` one.
|
||
|
||
## Known gaps (named, not hidden)
|
||
|
||
- **`notification_targets.secret` is stored plaintext** in Postgres —
|
||
the same posture `dashboard-design.md` already named for that domain.
|
||
Becomes both an enterprise-tier (secrets/KMS) and a multi-tenancy
|
||
(per-tenant secret isolation) concern later; naming it now avoids it
|
||
being "discovered" as a surprise security-review finding during a
|
||
future push.
|
||
- **CORS on `alerting`'s HTTP API is wide open**, matching `api`'s
|
||
existing no-auth-system posture from Phases 0–2. Not a new problem
|
||
Phase 3 introduces, just a second surface that inherits the same one.
|
||
- **No per-group/multi-row threshold alerting** (fix 4's scope decision)
|
||
and **no resolved-side debounce** (state-machine section) are both
|
||
named future work.
|
||
- **Single shared Postgres role** for `api` and `alerting`, same as
|
||
`dashboard-design.md`'s note — fine for Phase 3, a named Phase 4 item
|
||
once real auth exists.
|
||
|
||
## Load-testing plan (task 8, not run yet)
|
||
|
||
Seed ~500 rules via `alerting`'s own create API (not a direct DB insert
|
||
— exercise the real code path) at 60-second intervals, against a real
|
||
ClickHouse dataset (reusing `hack/benchmark-fixture`'s generator).
|
||
Measure: drift between `alert_state.next_eval_at` and the actual claim
|
||
timestamp under sustained load, `consecutive_errors`/`last_eval_status`
|
||
distribution (did the evaluator start erroring under load, as opposed to
|
||
just running slow), and `delivery_log.attempt_count`/`status`
|
||
distribution. Document real numbers, not projections — matching every
|
||
prior phase's benchmark discipline — and name what would need to change
|
||
for materially larger rule counts (horizontal evaluator replicas
|
||
partitioning by rule-id hash, which fix 1's claim design already makes
|
||
safe to add without a state-model change; moving off a single-process
|
||
ticker) as explicit Phase 4+ scope, not solved here.
|