diff --git a/README.md b/README.md index 6a8cf9b..227ffa3 100644 --- a/README.md +++ b/README.md @@ -1786,7 +1786,7 @@ retries) is individually logged for full observability. #### EventTotals and TargetTotals Running counts in each event database, read by the statistics pane at the -top of the webhook page. `EventTotals` is one row: +top of the webhook page and by the webhook list. `EventTotals` is one row: | Field | Type | Description | | ---------------- | --------- | ----------- | @@ -1818,6 +1818,14 @@ target. Its failure percentage for a window is the deliveries that became `failed` in it out of all that became `delivered` or `failed` in it, and a dash when none did. +The webhook list at `/hooks` shows three of the pane's figures for each +webhook: its events within retention and its last event, both from +`EventTotals`, and its deliveries that failed in the last 24 hours, +counted with the pane's query. It opens each webhook's event database once +(the handle stays open) and runs those two reads there, so its cost grows +with the number of webhooks and, for each, with the deliveries that +finished in the last 24 hours, never with the events stored. + #### Event-tier indexes These indexes on the per-webhook event databases are declared in the model @@ -1825,7 +1833,7 @@ tags, so `AutoMigrate` creates them on a fresh database: | Table | Columns | Serves | | ------------------ | --------------------------- | ------ | -| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, and the webhook page's statistics, which count each target's deliveries by status and when they finished | +| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, and the webhook page's statistics and the webhook list, which count each target's deliveries by status and when they finished | | `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which counts and deletes the deliveries of expired events | | `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events | | `events` | `deleted_at`, `created_at` | The webhook page's statistics, which count recent events | diff --git a/internal/handlers/source_list_test.go b/internal/handlers/source_list_test.go new file mode 100644 index 0000000..29050f6 --- /dev/null +++ b/internal/handlers/source_list_test.go @@ -0,0 +1,467 @@ +package handlers_test + +import ( + "net/http" + "net/http/httptest" + "regexp" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + "gorm.io/gorm/clause" + "sneak.berlin/go/webhooker/internal/database" + "sneak.berlin/go/webhooker/internal/handlers" + "sneak.berlin/go/webhooker/internal/logger" + "sneak.berlin/go/webhooker/internal/session" +) + +// failedHighlight is how the list marks a number of failed deliveries +// that is not zero. +const failedHighlight = `class="font-medium text-red-600"` + +// listWebhook adds a webhook with the given name, owned by the test +// user. +func listWebhook( + t *testing.T, db *database.Database, name string, +) *database.Webhook { + t.Helper() + + wh := &database.Webhook{UserID: deleteTestUserID, Name: name} + require.NoError(t, db.DB().Omit(clause.Associations).Create(wh).Error) + + return wh +} + +// addEntrypoints adds the given number of entrypoints, all active or +// all inactive, to a webhook and returns their paths. +func addEntrypoints( + t *testing.T, db *database.Database, webhookID string, + count int, active bool, +) []string { + t.Helper() + + paths := make([]string, count) + for i := range paths { + paths[i] = statsEntrypoint(t, db, webhookID, active) + } + + return paths +} + +// addTargets adds the given number of targets, all active or all +// inactive, to a webhook and returns them. +func addTargets( + t *testing.T, db *database.Database, webhookID string, + count int, active bool, +) []*database.Target { + t.Helper() + + targets := make([]*database.Target, count) + for i := range targets { + targets[i] = seedTarget(t, db, webhookID, database.TargetTypeLog) + require.NoError(t, db.DB().Model(targets[i]). + Update("active", active).Error) + } + + return targets +} + +// renderWebhookList runs the real webhook list handler as the test user +// and returns the rendered page. +func renderWebhookList( + t *testing.T, h *handlers.Handlers, sess *session.Session, +) string { + t.Helper() + + cookies := authenticatedCookies( + t, sess, deleteTestUserID, deleteTestUsername, + ) + + w := httptest.NewRecorder() + h.HandleSourceList().ServeHTTP( + w, getRequest(t, "/hooks", cookies, nil), + ) + require.Equal(t, http.StatusOK, w.Code) + + return w.Body.String() +} + +// listCard returns one webhook's entry in a rendered webhook list, its +// markup as rendered and its text with the markup taken out and each +// run of space made one space. +func listCard(t *testing.T, page, webhookID string) (string, string) { + t.Helper() + + _, card, found := strings.Cut(page, `href="/hook/`+webhookID+`"`) + require.True(t, found, "the list has no entry for %s", webhookID) + + card, _, _ = strings.Cut(card, "") + text := regexp.MustCompile(`<[^>]*>`).ReplaceAllString(card, " ") + + return card, strings.Join(strings.Fields(text), " ") +} + +// receiveEvents posts the given number of events to an entrypoint +// through the real receiver, and returns the webhook's event database +// and its events, oldest first. +func receiveEvents( + t *testing.T, + h *handlers.Handlers, + dbMgr *database.WebhookDBManager, + webhookID, path string, + count int, +) (*gorm.DB, []database.Event) { + t.Helper() + + router := receiverRouter(h) + + for range count { + require.Equal(t, http.StatusOK, postReceiver(t, router, path)) + } + + webhookDB, err := dbMgr.GetDB(webhookID) + require.NoError(t, err) + + events := listEvents(t, webhookDB) + require.Len(t, events, count) + + return webhookDB, events +} + +// seedFailingWebhook adds a webhook with six entrypoints, two of them +// inactive, and seven targets, five of them inactive. Four events reach +// its two active targets, arriving 31, 5, 4 and 3 hours ago, and its +// event totals row records the last one. Three deliveries failed in the +// last 24 hours, two to the first target and one to the second, one +// failed 30 hours ago, two were delivered, and two are still pending. +// It returns the webhook and when its last event arrived. +func seedFailingWebhook( + t *testing.T, + h *handlers.Handlers, + db *database.Database, + dbMgr *database.WebhookDBManager, +) (*database.Webhook, time.Time) { + t.Helper() + + wh := listWebhook(t, db, "failing") + paths := addEntrypoints(t, db, wh.ID, 4, true) + addEntrypoints(t, db, wh.ID, 2, false) + + active := addTargets(t, db, wh.ID, 2, true) + first, second := active[0], active[1] + + addTargets(t, db, wh.ID, 5, false) + + webhookDB, events := receiveEvents(t, h, dbMgr, wh.ID, paths[0], 4) + now := time.Now() + lastEventAt := now.Add(-3 * time.Hour) + + statsAge(t, webhookDB, events[0].ID, now.Add(-31*time.Hour)) + statsAge(t, webhookDB, events[1].ID, now.Add(-5*time.Hour)) + statsAge(t, webhookDB, events[2].ID, now.Add(-4*time.Hour)) + statsAge(t, webhookDB, events[3].ID, lastEventAt) + require.NoError(t, database.AddEventTotals(webhookDB, + database.EventTotals{LastEventAt: &lastEventAt})) + + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[0].ID, first.ID), + database.DeliveryStatusFailed, now.Add(-30*time.Hour)) + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[0].ID, second.ID), + database.DeliveryStatusDelivered, now.Add(-30*time.Hour)) + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[1].ID, first.ID), + database.DeliveryStatusFailed, now.Add(-time.Hour)) + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[2].ID, first.ID), + database.DeliveryStatusFailed, now.Add(-time.Minute)) + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[2].ID, second.ID), + database.DeliveryStatusFailed, now.Add(-time.Minute)) + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[3].ID, second.ID), + database.DeliveryStatusDelivered, now.Add(-time.Minute)) + + return wh, lastEventAt +} + +// seedHealthyWebhook adds a webhook with four entrypoints and two +// targets, all active, and three events, arriving 8, 7 and 6 hours ago +// and each delivered to both targets. Its event totals row records the +// last event. It returns the webhook and when its last event arrived. +func seedHealthyWebhook( + t *testing.T, + h *handlers.Handlers, + db *database.Database, + dbMgr *database.WebhookDBManager, +) (*database.Webhook, time.Time) { + t.Helper() + + wh := listWebhook(t, db, "healthy") + paths := addEntrypoints(t, db, wh.ID, 4, true) + targets := addTargets(t, db, wh.ID, 2, true) + + webhookDB, events := receiveEvents(t, h, dbMgr, wh.ID, paths[0], 3) + now := time.Now() + lastEventAt := now.Add(-6 * time.Hour) + + statsAge(t, webhookDB, events[0].ID, now.Add(-8*time.Hour)) + statsAge(t, webhookDB, events[1].ID, now.Add(-7*time.Hour)) + statsAge(t, webhookDB, events[2].ID, lastEventAt) + require.NoError(t, database.AddEventTotals(webhookDB, + database.EventTotals{LastEventAt: &lastEventAt})) + + for _, ev := range events { + for _, target := range targets { + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, ev.ID, target.ID), + database.DeliveryStatusDelivered, now) + } + } + + return wh, lastEventAt +} + +// lastEventText is how the list shows when the last event arrived. +func lastEventText(at time.Time) string { + return at.UTC().Format("2006-01-02 15:04:05 UTC") +} + +// TestSourceList_ShowsActivityOfEachWebhook checks the figures the list +// shows for a webhook with recent failures, a healthy one, a new one +// that has received no event, and one without an event database. +func TestSourceList_ShowsActivityOfEachWebhook(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + failing, failingLastEvent := seedFailingWebhook(t, h, db, dbMgr) + healthy, healthyLastEvent := seedHealthyWebhook(t, h, db, dbMgr) + + // Creating a webhook creates its event database. + fresh := listWebhook(t, db, "fresh") + require.NoError(t, dbMgr.CreateDB(fresh.ID)) + addEntrypoints(t, db, fresh.ID, 2, true) + addTargets(t, db, fresh.ID, 3, true) + + quiet := listWebhook(t, db, "quiet") + addEntrypoints(t, db, quiet.ID, 2, true) + addTargets(t, db, quiet.ID, 3, true) + + page := renderWebhookList(t, h, sess) + + card, text := listCard(t, page, failing.ID) + assert.Contains(t, text, "6 entrypoints, 2 inactive") + assert.Contains(t, text, "7 targets, 5 inactive") + assert.Contains(t, text, "4 events within retention") + assert.Contains(t, text, "Last event "+lastEventText(failingLastEvent)) + assert.Contains(t, card, + failedHighlight+">3 failed deliveries in the last 24 hours<") + + card, text = listCard(t, page, healthy.ID) + assert.Contains(t, text, "4 entrypoints") + assert.Contains(t, text, "2 targets") + assert.Contains(t, text, "3 events within retention") + assert.Contains(t, text, "Last event "+lastEventText(healthyLastEvent)) + assert.Contains(t, text, "0 failed deliveries in the last 24 hours") + assert.NotContains(t, text, "inactive") + assert.NotContains(t, card, failedHighlight) + + card, text = listCard(t, page, fresh.ID) + assert.Contains(t, text, "2 entrypoints") + assert.Contains(t, text, "3 targets") + assert.Contains(t, text, "0 events within retention") + assert.Contains(t, text, "No events yet") + assert.Contains(t, text, "0 failed deliveries in the last 24 hours") + assert.NotContains(t, card, failedHighlight) + + card, text = listCard(t, page, quiet.ID) + assert.Contains(t, text, "2 entrypoints") + assert.Contains(t, text, "3 targets") + assert.Contains(t, text, "0 events within retention") + assert.Contains(t, text, "No events yet") + assert.Contains(t, text, "0 failed deliveries in the last 24 hours") + assert.NotContains(t, card, failedHighlight) + assert.False(t, dbMgr.DBExists(quiet.ID), + "showing the list must not create an event database") +} + +// TestSourceList_CountsOnlyEventsWithinRetention checks that once +// retention has removed one of a webhook's three events, the list +// counts the two still stored. +func TestSourceList_CountsOnlyEventsWithinRetention(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + log *logger.Logger + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr, &log) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := &database.Webhook{ + UserID: deleteTestUserID, Name: "pruned", RetentionDays: 14, + } + require.NoError(t, db.DB().Omit(clause.Associations).Create(wh).Error) + + paths := addEntrypoints(t, db, wh.ID, 3, true) + addTargets(t, db, wh.ID, 4, true) + + webhookDB, events := receiveEvents(t, h, dbMgr, wh.ID, paths[0], 3) + + statsAge(t, webhookDB, events[0].ID, time.Now().Add(-15*24*time.Hour)) + statsPrune(t, db, dbMgr, log, webhookDB) + require.Len(t, listEvents(t, webhookDB), 2) + + _, text := listCard(t, renderWebhookList(t, h, sess), wh.ID) + assert.Contains(t, text, "3 entrypoints") + assert.Contains(t, text, "4 targets") + assert.Contains(t, text, "2 events within retention") +} + +// TestSourceList_LastEventSurvivesPruningEveryEvent checks that once +// retention has removed every event of a webhook, the list still shows +// when the last one arrived rather than "No events yet". +func TestSourceList_LastEventSurvivesPruningEveryEvent(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + log *logger.Logger + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr, &log) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := &database.Webhook{ + UserID: deleteTestUserID, Name: "emptied", RetentionDays: 1, + } + require.NoError(t, db.DB().Omit(clause.Associations).Create(wh).Error) + + paths := addEntrypoints(t, db, wh.ID, 2, true) + addTargets(t, db, wh.ID, 3, true) + + webhookDB, events := receiveEvents(t, h, dbMgr, wh.ID, paths[0], 1) + lastEventAt := time.Now().Add(-50 * time.Hour) + + statsAge(t, webhookDB, events[0].ID, lastEventAt) + require.NoError(t, database.AddEventTotals(webhookDB, + database.EventTotals{LastEventAt: &lastEventAt})) + statsPrune(t, db, dbMgr, log, webhookDB) + require.Empty(t, listEvents(t, webhookDB)) + + _, text := listCard(t, renderWebhookList(t, h, sess), wh.ID) + assert.Contains(t, text, "0 events within retention") + assert.Contains(t, text, "Last event "+lastEventText(lastEventAt)) + assert.NotContains(t, text, "No events yet") +} + +// TestSourceList_CountsOfOneInSingular checks that a webhook with one +// entrypoint, one target, one event within retention and one failed +// delivery in the last 24 hours has each written in the singular. +func TestSourceList_CountsOfOneInSingular(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + wh := listWebhook(t, db, "single") + paths := addEntrypoints(t, db, wh.ID, 1, true) + targets := addTargets(t, db, wh.ID, 1, true) + + webhookDB, events := receiveEvents(t, h, dbMgr, wh.ID, paths[0], 1) + now := time.Now() + lastEventAt := now.Add(-9 * time.Hour) + + statsAge(t, webhookDB, events[0].ID, lastEventAt) + require.NoError(t, database.AddEventTotals(webhookDB, + database.EventTotals{LastEventAt: &lastEventAt})) + statsFinish(t, webhookDB, + statsDelivery(t, webhookDB, events[0].ID, targets[0].ID), + database.DeliveryStatusFailed, now.Add(-time.Hour)) + + card, text := listCard(t, renderWebhookList(t, h, sess), wh.ID) + assert.Contains(t, card, ">1 entrypoint<") + assert.Contains(t, card, ">1 target<") + assert.Contains(t, card, ">1 event within retention<") + assert.Contains(t, text, "Last event "+lastEventText(lastEventAt)) + assert.Contains(t, card, + failedHighlight+">1 failed delivery in the last 24 hours<") +} + +// TestSourceList_UnreadableEventDatabase checks that a webhook whose +// event database cannot be read says so in its entry instead of +// showing zeros, and that the rest of the list is still shown. +func TestSourceList_UnreadableEventDatabase(t *testing.T) { + t.Parallel() + + var ( + h *handlers.Handlers + sess *session.Session + db *database.Database + dbMgr *database.WebhookDBManager + ) + + app := newTestApp(t, &h, &sess, &db, &dbMgr) + app.RequireStart() + + t.Cleanup(app.RequireStop) + + broken := listWebhook(t, db, "broken") + addEntrypoints(t, db, broken.ID, 2, true) + addTargets(t, db, broken.ID, 3, true) + + brokenDB, err := dbMgr.GetDB(broken.ID) + require.NoError(t, err) + require.NoError(t, + brokenDB.Migrator().DropTable(&database.EventTotals{})) + + quiet := listWebhook(t, db, "quiet") + addEntrypoints(t, db, quiet.ID, 2, true) + addTargets(t, db, quiet.ID, 3, true) + + page := renderWebhookList(t, h, sess) + + _, text := listCard(t, page, broken.ID) + assert.Contains(t, text, "2 entrypoints") + assert.Contains(t, text, "3 targets") + assert.Contains(t, text, "The event figures could not be read.") + assert.NotContains(t, text, "events") + assert.NotContains(t, text, "failed") + + _, text = listCard(t, page, quiet.ID) + assert.Contains(t, text, "No events yet") +} diff --git a/internal/handlers/source_management.go b/internal/handlers/source_management.go index cd1bb14..0e8aa1b 100644 --- a/internal/handlers/source_management.go +++ b/internal/handlers/source_management.go @@ -3,10 +3,12 @@ package handlers import ( "encoding/json" "errors" + "fmt" "net/http" "slices" "strconv" "strings" + "time" "github.com/go-chi/chi" "github.com/google/uuid" @@ -20,9 +22,20 @@ import ( type WebhookListItem struct { database.Webhook - EntrypointCount int64 - TargetCount int64 - EventCount int64 + EntrypointCount int + InactiveEntrypointCount int + TargetCount int + InactiveTargetCount int + + // EventCount is how many events the webhook holds, LastEventAt + // when the newest arrived (nil before the first), and + // FailedLast24Hours how many of its deliveries failed in the last + // 24 hours. When the webhook's event database could not be read, + // EventsUnreadable is set and these three are not known. + EventCount int64 + LastEventAt *time.Time + FailedLast24Hours int64 + EventsUnreadable bool } // errMissingURL signals that a required URL was not provided. @@ -154,7 +167,12 @@ func (h *Handlers) HandleSourceList() http.HandlerFunc { return } - items := h.buildWebhookListItems(webhooks) + items, err := h.buildWebhookListItems(webhooks) + if err != nil { + h.serverError(w, r, "failed to list webhooks", err) + + return + } data := map[string]any{ "Webhooks": items, @@ -164,36 +182,115 @@ func (h *Handlers) HandleSourceList() http.HandlerFunc { } } -// buildWebhookListItems builds list items with counts. +// buildWebhookListItems builds the list's entry for each webhook. It +// fails when the main database cannot be read. A webhook whose event +// database cannot be read is marked on its own entry, and the error is +// logged. func (h *Handlers) buildWebhookListItems( webhooks []database.Webhook, -) []WebhookListItem { +) ([]WebhookListItem, error) { items := make([]WebhookListItem, len(webhooks)) + since := time.Now().Add(-longWindow) for i := range webhooks { - items[i].Webhook = webhooks[i] + item := &items[i] + item.Webhook = webhooks[i] - h.db.DB().Model(&database.Entrypoint{}).Where( - "webhook_id = ?", webhooks[i].ID, - ).Count(&items[i].EntrypointCount) + var err error - h.db.DB().Model(&database.Target{}).Where( - "webhook_id = ?", webhooks[i].ID, - ).Count(&items[i].TargetCount) + item.EntrypointCount, item.InactiveEntrypointCount, err = + h.countWithInactive(&database.Entrypoint{}, item.ID) + if err != nil { + return nil, err + } - if h.dbMgr.DBExists(webhooks[i].ID) { - webhookDB, err := h.dbMgr.GetDB( - webhooks[i].ID, + item.TargetCount, item.InactiveTargetCount, err = + h.countWithInactive(&database.Target{}, item.ID) + if err != nil { + return nil, err + } + + // Opening an event database that does not exist would create + // it, and it would hold nothing to count. + if !h.dbMgr.DBExists(item.ID) { + continue + } + + err = h.readListEventFigures(item, since) + if err != nil { + h.log.Error( + "failed to read webhook list figures", + "webhook_id", item.ID, + "error", err, ) - if err == nil { - webhookDB.Model( - &database.Event{}, - ).Count(&items[i].EventCount) - } + + item.EventsUnreadable = true } } - return items + return items, nil +} + +// countWithInactive returns how many entrypoints or targets, as model +// says, a webhook has, and how many of them are inactive. +func (h *Handlers) countWithInactive( + model any, webhookID string, +) (int, int, error) { + var active []bool + + err := h.db.DB().Model(model). + Where("webhook_id = ?", webhookID). + Pluck("active", &active).Error + if err != nil { + return 0, 0, fmt.Errorf( + "reading active flags of webhook %s: %w", webhookID, err, + ) + } + + inactive := 0 + + for _, a := range active { + if !a { + inactive++ + } + } + + return len(active), inactive, nil +} + +// readListEventFigures fills in the figures the list shows from the +// webhook's event database, with the statistics pane's own queries: +// the event count and last arrival from the event totals row, and the +// deliveries that failed since the given time from the deliveries' +// status index. +func (h *Handlers) readListEventFigures( + item *WebhookListItem, since time.Time, +) error { + webhookDB, err := h.dbMgr.GetDB(item.ID) + if err != nil { + return err + } + + var totals database.EventTotals + + err = webhookDB.Take(&totals).Error + if err != nil { + return fmt.Errorf("reading event totals: %w", err) + } + + item.EventCount = totals.Events - totals.EventsRemoved + item.LastEventAt = totals.LastEventAt + + byTarget, err := finishedByTarget(webhookDB, since) + if err != nil { + return err + } + + for _, f := range byTarget { + item.FailedLast24Hours += f.Failed + } + + return nil } // HandleSourceCreate shows the form to create a new webhook. diff --git a/templates/sources_list.html b/templates/sources_list.html index 44fae4f..18ce43d 100644 --- a/templates/sources_list.html +++ b/templates/sources_list.html @@ -27,10 +27,16 @@ Retention: {{.RetentionLabel}} -