Record notification delivery on the incident timeline
Release / test (push) Failing after 8s
Release / build (amd64, darwin) (push) Has been skipped
Release / build (amd64, linux) (push) Has been skipped
Release / build (arm64, darwin) (push) Has been skipped
Release / build (arm64, linux) (push) Has been skipped
Release / docker (push) Has been skipped
Release / chart (push) Has been skipped
Release / release (push) Has been skipped
Release / test (push) Failing after 8s
Release / build (amd64, darwin) (push) Has been skipped
Release / build (amd64, linux) (push) Has been skipped
Release / build (arm64, darwin) (push) Has been skipped
Release / build (arm64, linux) (push) Has been skipped
Release / docker (push) Has been skipped
Release / chart (push) Has been skipped
Release / release (push) Has been skipped
An incident's history went quiet after "Incident opened": nothing said that anybody had been paged, reminded, or told it resolved. Delivery lived only in the notifications outbox, which no API exposes, so when a page failed to arrive there was nothing in the product that said whether it had been sent. The notifier now writes two event types. A notified event once ntfy accepts the publish, carrying the kind in detail and the paged user in user_id — absent when the page went to the shared fallback topic, which belongs to nobody. And a notify_failed event when a notification exhausts its retries, which is the one worth having: without it a page that never landed leaves the timeline identical to one that did. Both are written from the delivery result rather than at enqueue. A queued notification is an intention, and the timeline is append-only, so claiming somebody was told before ntfy accepted it would be a lie that stays there. A failed timeline write is logged rather than returned, so it cannot make a delivered row look unsent and send the page twice. The topic is deliberately in neither: it is a shared secret with the ntfy server, and every API key can read the timeline. No migration — incident_events.type is free text, unlike notifications.kind.
This commit is contained in:
@@ -235,6 +235,12 @@ Delivery is a queue, not an inline call: the webhook writes a row and a
|
|||||||
background notifier sends it within 30 seconds, retrying with exponential
|
background notifier sends it within 30 seconds, retrying with exponential
|
||||||
backoff up to 8 attempts. Nothing about ingestion blocks on ntfy being reachable.
|
backoff up to 8 attempts. Nothing about ingestion blocks on ntfy being reachable.
|
||||||
|
|
||||||
|
Every delivery is recorded on the incident's timeline: a `notified` event once
|
||||||
|
ntfy accepts the publish, and a `notify_failed` event when a notification
|
||||||
|
exhausts its retries. Written from the result rather than at enqueue, so the
|
||||||
|
timeline says what actually happened — and a page that never landed is visible
|
||||||
|
instead of looking the same as one that did.
|
||||||
|
|
||||||
### Stale alert expiry
|
### Stale alert expiry
|
||||||
|
|
||||||
A resolved webhook is the only signal that an alert has stopped firing, so a
|
A resolved webhook is the only signal that an alert has stopped firing, so a
|
||||||
@@ -353,8 +359,16 @@ name: degrade unknown values to "resolved, reason unknown".
|
|||||||
|
|
||||||
Types written today: `triggered`, `alert_added`, `alert_resolved`,
|
Types written today: `triggered`, `alert_added`, `alert_resolved`,
|
||||||
`acknowledged`, `unacknowledged`, `assigned`, `snoozed`, `unsnoozed`, `resolved`,
|
`acknowledged`, `unacknowledged`, `assigned`, `snoozed`, `unsnoozed`, `resolved`,
|
||||||
`note`. On an `assigned` event `user_id` is the **assignee**, not the actor. New
|
`note`, `notified`, `notify_failed`. On an `assigned` event `user_id` is the
|
||||||
types may be added; render unknown ones generically rather than dropping them.
|
**assignee**, not the actor. New types may be added; render unknown ones
|
||||||
|
generically rather than dropping them.
|
||||||
|
|
||||||
|
On `notified` and `notify_failed`, `detail` carries the notification kind
|
||||||
|
(`triggered` | `reminder` | `resolved`), and on a failure the reason after it.
|
||||||
|
`user_id` is who was paged — absent means the page went to the shared fallback
|
||||||
|
topic and so belongs to nobody. The topic itself is never written to the
|
||||||
|
timeline: it is a shared secret with the ntfy server, and every API key can read
|
||||||
|
this.
|
||||||
|
|
||||||
### Alerts
|
### Alerts
|
||||||
|
|
||||||
|
|||||||
@@ -45,6 +45,19 @@ const (
|
|||||||
notifyResolved = "resolved"
|
notifyResolved = "resolved"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// Timeline event types the notifier writes, so an incident's history says who
|
||||||
|
// was paged and whether the page landed. Written from the delivery result
|
||||||
|
// rather than at enqueue: a queued notification is an intention, and claiming
|
||||||
|
// somebody was told before ntfy accepted it would be a lie the timeline keeps.
|
||||||
|
//
|
||||||
|
// The topic is deliberately absent from both. It is a shared secret with the
|
||||||
|
// ntfy server — anyone holding it can publish to it — and the timeline is
|
||||||
|
// readable by every API key.
|
||||||
|
const (
|
||||||
|
eventNotified = "notified"
|
||||||
|
eventNotifyFailed = "notify_failed"
|
||||||
|
)
|
||||||
|
|
||||||
// NotifyConfig is everything the notifier needs to reach ntfy and to build URLs
|
// NotifyConfig is everything the notifier needs to reach ntfy and to build URLs
|
||||||
// a phone can follow back to this server.
|
// a phone can follow back to this server.
|
||||||
type NotifyConfig struct {
|
type NotifyConfig struct {
|
||||||
@@ -208,6 +221,11 @@ func deliverPending(ctx context.Context, db *sql.DB, cfg NotifyConfig) {
|
|||||||
time.Now().Unix(), n.id); err != nil {
|
time.Now().Unix(), n.id); err != nil {
|
||||||
log.Printf("notifier: mark sent %d: %v", n.id, err)
|
log.Printf("notifier: mark sent %d: %v", n.id, err)
|
||||||
}
|
}
|
||||||
|
// Logged, not returned: the page has already gone out, and treating a
|
||||||
|
// failed timeline write as a failed delivery would send it again.
|
||||||
|
if err := logEvent(ctx, db, n.incidentID, eventNotified, n.userID, nil, &n.kind); err != nil {
|
||||||
|
log.Printf("notifier: log delivery of %d: %v", n.id, err)
|
||||||
|
}
|
||||||
sent++
|
sent++
|
||||||
}
|
}
|
||||||
if sent > 0 {
|
if sent > 0 {
|
||||||
@@ -243,6 +261,11 @@ func pendingNotifications(ctx context.Context, db *sql.DB) ([]outboxRow, error)
|
|||||||
}
|
}
|
||||||
|
|
||||||
// markFailed bumps the attempt count and pushes the row out to its next retry.
|
// markFailed bumps the attempt count and pushes the row out to its next retry.
|
||||||
|
//
|
||||||
|
// The attempt that exhausts the budget also writes a timeline event. Without it
|
||||||
|
// a page that never landed leaves the incident's history identical to one that
|
||||||
|
// did, which is the failure most worth seeing: nobody was told, and nothing
|
||||||
|
// says so.
|
||||||
func markFailed(ctx context.Context, db *sql.DB, n outboxRow, cause error) {
|
func markFailed(ctx context.Context, db *sql.DB, n outboxRow, cause error) {
|
||||||
next := time.Now().Add(retryDelay(n.attempts)).Unix()
|
next := time.Now().Add(retryDelay(n.attempts)).Unix()
|
||||||
if _, err := db.ExecContext(ctx,
|
if _, err := db.ExecContext(ctx,
|
||||||
@@ -250,6 +273,14 @@ func markFailed(ctx context.Context, db *sql.DB, n outboxRow, cause error) {
|
|||||||
next, cause.Error(), n.id); err != nil {
|
next, cause.Error(), n.id); err != nil {
|
||||||
log.Printf("notifier: mark failed %d: %v", n.id, err)
|
log.Printf("notifier: mark failed %d: %v", n.id, err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if n.attempts+1 < notifyMaxAttempts {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
detail := fmt.Sprintf("%s: %s", n.kind, cause)
|
||||||
|
if err := logEvent(ctx, db, n.incidentID, eventNotifyFailed, n.userID, nil, &detail); err != nil {
|
||||||
|
log.Printf("notifier: log failure of %d: %v", n.id, err)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// retryDelay doubles the wait per attempt, up to notifyRetryMax.
|
// retryDelay doubles the wait per attempt, up to notifyRetryMax.
|
||||||
|
|||||||
@@ -199,6 +199,150 @@ func TestNotify_DeliveredOnlyOnce(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Delivery on the timeline
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
// notifyEvents picks the notifier's entries out of an incident's timeline.
|
||||||
|
// Asserted through the API rather than the table: the timeline is what the
|
||||||
|
// clients read, so its shape is the contract worth covering.
|
||||||
|
func notifyEvents(t *testing.T, s *ts, id int) []map[string]any {
|
||||||
|
t.Helper()
|
||||||
|
var out []map[string]any
|
||||||
|
for _, e := range timeline(t, s, id) {
|
||||||
|
if e["type"] == "notified" || e["type"] == "notify_failed" {
|
||||||
|
out = append(out, e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNotify_DeliveryIsRecordedOnTheTimeline(t *testing.T) {
|
||||||
|
s, _ := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
|
||||||
|
|
||||||
|
fireCritical(t, s)
|
||||||
|
// Queued is not notified: nothing is on the timeline until ntfy accepts it.
|
||||||
|
if got := notifyEvents(t, s, 1); len(got) != 0 {
|
||||||
|
t.Fatalf("expected no event before delivery, got %v", got)
|
||||||
|
}
|
||||||
|
|
||||||
|
s.sweepNotify(t)
|
||||||
|
|
||||||
|
events := notifyEvents(t, s, 1)
|
||||||
|
if len(events) != 1 {
|
||||||
|
t.Fatalf("expected 1 notification event, got %v", events)
|
||||||
|
}
|
||||||
|
e := events[0]
|
||||||
|
if e["type"] != "notified" {
|
||||||
|
t.Errorf("expected a notified event, got %v", e["type"])
|
||||||
|
}
|
||||||
|
if e["detail"] != "triggered" {
|
||||||
|
t.Errorf("expected the kind in detail, got %v", e["detail"])
|
||||||
|
}
|
||||||
|
if e["username"] != "admin" {
|
||||||
|
t.Errorf("expected the paged user attached, got %v", e["username"])
|
||||||
|
}
|
||||||
|
// The topic is a shared secret with ntfy; the timeline is not the place for it.
|
||||||
|
for _, v := range e {
|
||||||
|
if s, ok := v.(string); ok && strings.Contains(s, "terdut-admin") {
|
||||||
|
t.Errorf("expected the topic kept out of the timeline, found it in %v", e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A redelivery-free pass must not double-log either.
|
||||||
|
func TestNotify_TimelineRecordsOneEventPerDelivery(t *testing.T) {
|
||||||
|
s, _ := notifyTS(t, api.NotifyConfig{
|
||||||
|
PublicURL: "https://terdut.example.com",
|
||||||
|
RepeatEvery: 15 * time.Minute,
|
||||||
|
})
|
||||||
|
|
||||||
|
fireCritical(t, s)
|
||||||
|
s.sweepNotify(t)
|
||||||
|
s.sweepNotify(t)
|
||||||
|
s.ageNotifications(t, 20*time.Minute)
|
||||||
|
s.sweepNotify(t)
|
||||||
|
|
||||||
|
events := notifyEvents(t, s, 1)
|
||||||
|
if len(events) != 2 {
|
||||||
|
t.Fatalf("expected one event per delivery, got %v", events)
|
||||||
|
}
|
||||||
|
if events[0]["detail"] != "triggered" || events[1]["detail"] != "reminder" {
|
||||||
|
t.Errorf("expected triggered then reminder, got %v and %v",
|
||||||
|
events[0]["detail"], events[1]["detail"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestNotify_AllClearIsRecordedOnTheTimeline(t *testing.T) {
|
||||||
|
s, _ := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
|
||||||
|
|
||||||
|
fireCritical(t, s)
|
||||||
|
s.sweepNotify(t)
|
||||||
|
|
||||||
|
postWebhook(t, s, []map[string]any{
|
||||||
|
amAlert("fp-notify", "DiskFull", "resolved", "2026-05-20T10:00:00Z",
|
||||||
|
"2026-05-20T11:00:00Z", map[string]string{"severity": "critical"}),
|
||||||
|
}, "{}:{alertname=\"DiskFull\"}")
|
||||||
|
s.sweepNotify(t)
|
||||||
|
|
||||||
|
events := notifyEvents(t, s, 1)
|
||||||
|
if len(events) != 2 {
|
||||||
|
t.Fatalf("expected the all-clear recorded, got %v", events)
|
||||||
|
}
|
||||||
|
if events[1]["detail"] != "resolved" {
|
||||||
|
t.Errorf("expected a resolved event, got %v", events[1]["detail"])
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A page to the shared fallback belongs to nobody, and the timeline has to say
|
||||||
|
// so rather than attributing it to whoever happens to be on call now.
|
||||||
|
func TestNotify_FallbackDeliveryHasNoUser(t *testing.T) {
|
||||||
|
f := newFakeNtfy(t)
|
||||||
|
s := newTS(t, api.NotifyConfig{
|
||||||
|
BaseURL: f.URL,
|
||||||
|
FallbackTopic: "terdut-oncall",
|
||||||
|
PublicURL: "https://terdut.example.com",
|
||||||
|
})
|
||||||
|
|
||||||
|
fireCritical(t, s)
|
||||||
|
s.sweepNotify(t)
|
||||||
|
|
||||||
|
events := notifyEvents(t, s, 1)
|
||||||
|
if len(events) != 1 {
|
||||||
|
t.Fatalf("expected 1 notification event, got %v", events)
|
||||||
|
}
|
||||||
|
if got, ok := events[0]["username"]; ok && got != nil && got != "" {
|
||||||
|
t.Errorf("expected no user on a fallback-topic page, got %v", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The failure worth seeing: nobody was paged, and the timeline says so instead
|
||||||
|
// of looking exactly like a delivery that worked.
|
||||||
|
func TestNotify_ExhaustedRetriesAreRecordedOnce(t *testing.T) {
|
||||||
|
s, f := notifyTS(t, api.NotifyConfig{PublicURL: "https://terdut.example.com"})
|
||||||
|
f.failWith(http.StatusInternalServerError)
|
||||||
|
|
||||||
|
fireCritical(t, s)
|
||||||
|
// One pass per attempt, each made due by clearing the backoff the last one set.
|
||||||
|
for i := 0; i < 10; i++ {
|
||||||
|
s.sweepNotify(t)
|
||||||
|
s.exec(t, "UPDATE notifications SET send_after = ? WHERE sent_at IS NULL",
|
||||||
|
time.Now().Add(-time.Second).Unix())
|
||||||
|
}
|
||||||
|
|
||||||
|
events := notifyEvents(t, s, 1)
|
||||||
|
if len(events) != 1 {
|
||||||
|
t.Fatalf("expected exactly one failure event, got %v", events)
|
||||||
|
}
|
||||||
|
if events[0]["type"] != "notify_failed" {
|
||||||
|
t.Errorf("expected notify_failed, got %v", events[0]["type"])
|
||||||
|
}
|
||||||
|
detail, _ := events[0]["detail"].(string)
|
||||||
|
if !strings.HasPrefix(detail, "triggered: ") || !strings.Contains(detail, "500") {
|
||||||
|
t.Errorf("expected the kind and the reason in %q", detail)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
// Acknowledging from the notification
|
// Acknowledging from the notification
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user