[add] alert feed paging, filters, ack attribution and delivery records - #110
Merged
Conversation
The feed rendered every event on one page with no filters, which stops being usable exactly when alerting starts being useful. It now pages 50 at a time and filters by severity, lifecycle kind, rule, container and message text — in SQL, so totals and paging describe the whole result set. Host scoping moved into the query for the same reason: filtering a page after fetching it gives short pages and a total that counts events the viewer may not see. Every webhook call and email send is recorded against its alert with the outcome, status and a response excerpt. A 500, a refused SMTP connection or a rule with email ticked and no recipient anywhere all used to fail silently. Stores the webhook's name and host, never its URL — those carry tokens and this table is readable with the alerts section. Ack records who and when. A toast raises new alerts while the app is open, detected by highest event id rather than unread count, since the count also moves when somebody acknowledges something.
The tone was applied as bg-danger/10, which replaced card's opaque bg-panel with a translucent colour — so the page behind showed through and the text was hard to read over anything busy. Tone now colours the left edge and the bar; the panel stays opaque. Adds a countdown bar so the disappearance isn't a surprise, and pauses it on hover: a toast that vanishes mid-sentence is worse than one that lingers. The CSS animation and the JS timer pause together, so the bar never disagrees with when the toast actually goes.
Four follow-ups from using the feed: - Host was filterable in the API but had no control in the UI. - The toast arrived seconds AFTER the row it announced, because the Shell polled every 8s for the badge and the feed polled every 5s for the table. One shared poll drives the badge, the toasts and the table, so the skew is gone by construction rather than by matching intervals. - Ack all, behind a danger confirm that says whether it is about to clear the current filter or the whole feed. It reuses the SAME where-clause builder as the list, so it can never touch a row the caller could not see; the store test fails if that filter is dropped. - Toasts can be turned off per account under Profile > Preferences. Server-side prefs, so the choice follows the account. Also fixes a self-inflicted type bug: ackAllAlerts first referred to Parameters<typeof api.alerts>, a circular reference inside the same object literal that silently degraded the whole api object to any and produced implicit-any errors across unrelated files.
It counted every unacknowledged event, so a condition RESOLVING pushed the number up — the badge grew as things got better, which is how people learn to stop reading it. It now counts unacknowledged warnings and criticals. That needs no separate rule about resolved events: a resolution is emitted as info, so filtering by severity covers it, and an informational alert of any kind is by definition not an outstanding problem. Info and resolved events still appear in the feed and still count in its totals.
Your suggestion, and better than what I had: instead of teaching the badge, the outstanding filter and bulk acknowledge each to skip resolutions, mark one at the point of writing. There is nothing to do about a condition that ended, so it is never outstanding anywhere. Keeps the data honest if the Acknowledge action ever comes back on those rows, and leaves acknowledged_by empty because no person did it — the feed renders an em dash rather than claiming someone looked. Replaces the three ExcludeKind special cases with one rule in InsertAlertEvent. The badge still filters to warnings and criticals, so a plain info alert does not raise an alarm either.
There was no way to see an alert properly: the row truncates, and only the delivery cell expanded — and only when an attempt had been made. Clicking a row now opens the full message, the measured value, how long the condition lasted, the host, a link to the container it is about, who acknowledged it and when, and every delivery attempt with the endpoint's own response. Acknowledging is available from there, so reading it and dealing with it are not two trips.
/metrics never learned about the alert engine. It now exposes dockercmd_alert_firing (one series per live condition), a firing count and the outstanding warning/critical count. The firing gauge is what you would actually page on: it vanishes when the condition resolves. Also corrects the cpu_percent help text, which said 'host-relative'. It is the docker-stats per-core figure, so a dashboard built on that description read 4x high on a four-core host. Adds cpu_cores so it can be normalised. Sorting is server-side over the whole result set, not the visible page, and the key goes through a fixed whitelist since ORDER BY cannot be parameterised. Severity sorts by importance, not alphabetically — the latter would put warning above info and look almost right. Ack all moves to the PageHeader actions slot, contextual to the feed tab, per the repo's own UI convention.
Two findings from reading the branch back: - A failed webhook wrote its own URL into the delivery record. target was carefully limited to a name and a host because webhook URLs carry tokens, but net/http wraps transport errors in *url.Error whose message embeds the full URL — so a refused connection stored the secret next to the field that avoided it. redactURL keeps the cause and drops the URL. - The ack-all confirm quoted the filtered TOTAL while promising to acknowledge only the unacknowledged ones, overstating what the button would do. The list response now carries an exact outstanding count. Plus a duplicated doc comment on InsertAlertEvent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four gaps found while actually using the alert feed, all of the same shape: the
feed was fine as a demo and unusable as an operational tool.
Paging and filtering. Every event rendered on one page, no filters at all —
which stops working at the point alerting starts being useful. Now 50 per page,
filterable by severity, lifecycle kind, rule, container and message text, plus an
unacknowledged only toggle.
Delivery is now checkable rather than assumed. Every webhook call and e-mail
send is recorded against its alert with the outcome, HTTP status and a response
excerpt. Before this, a webhook returning 500, an SMTP server refusing the
connection, or a rule with e-mail ticked while no recipient is configured
anywhere all failed silently: the alert appeared in the feed and looked handled
while nothing had left the building.
Acknowledging records who and when. "Someone dealt with this" is only
actionable if you can go and ask them.
A toast when an alert arrives while the app is open, so it reaches you
without sitting on the Alerts page.
Type of change
Checklist
go test -short ./...andgo vet ./...passgofmtgate is clean (gofmt -l $(git ls-files '*.go')after staging)cd web && npx tsc --noEmit)web/distdocs/and added aCHANGELOG.mdentryNotes for reviewers
Paging forced a security change, and that is the part worth reviewing. The old
handler fetched everything and dropped other hosts' events in Go. That was safe,
but it cannot survive paging: a page of 50 filtered down to 6 gives short pages
and a total that counts rows the caller may not see. Host scoping therefore moved
into the SQL, with
AlertQuery.HostIDswherenilmeans unrestricted and anempty non-nil slice means nothing — the fail-closed direction. There is a test
asserting exactly that distinction, and it fails when the guard is removed.
An explicit
?host=is re-checked against the caller's scope and 403s rather thanbeing silently ignored.
Two things stored deliberately narrowly. The delivery record keeps the
webhook's name and host, never its full URL — webhook URLs routinely carry a
token in the path or query, and this table is readable by anyone holding the
alerts section. Response bodies are truncated to ~500 characters so a remote
endpoint cannot write unbounded text into the database. Both have tests.
LIKE wildcards are escaped, with the matching
ESCAPE '\'clause — SQLite hasno default escape character, so escaping without it would silently search for the
wrong string. Searching for
100%finds the row containing it rather thaneverything.
A test that passed by luck, and how it was caught. The wildcard test
originally had one row containing
100%and no other row containing100, so anunescaped search (
%100%%) also matched exactly one row and the mutation ranclean. The fixture now has a
1004 restartsrow, so the unescaped version matchestwo and the test fails as it should. Three guards were mutation-verified this way:
LIKE escaping, the empty-scope fail-closed branch, and the detail truncation.
Toast detection is by highest event id, not by unread count. The count also
moves when somebody acknowledges something, which is not news. The first poll only
establishes a baseline, so opening the app doesn't announce the entire backlog.
Not included: delivery retry. Failures are recorded, not re-attempted —
retry needs a queue and a backoff policy, and quietly retrying a webhook that
returns 500 for a good reason is its own hazard. Worth doing deliberately, not as
a side effect of this.