Serve /metrics from a registry of its own (closes #227)

A second metrics-enabled router in one process panicked on a duplicate collector registration, because every collector registered on Prometheus's global default registry. metrics.NewRegistry now builds one registry with the Go runtime and process collectors; fx provides it and the delivery metric set built on it. The middleware builds its HTTP recorder once on that registry (NewForTest on a fresh one), the engine and handlers take the metric set from fx, and nothing registers on the global default any more.

/metrics is served from the new registry with the same series names, labels and auth. A test builds two metrics-enabled routers in one process.

Model: opus-5-5

README.md

@@ -2951,13 +2951,15 @@ Components are wired via Uber fx in this order:
 7. `healthcheck.New` — Health check service
 8. `session.New` — Cookie-based session manager (key from database)
 9. `handlers.New` — HTTP handlers
-10. `middleware.New` — HTTP middleware
-11. `delivery.New` — Event-driven delivery engine
-12. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
-13. `delivery.Engine` → `delivery.Notifier` — interface bridge
-14. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so
+10. `metrics.NewRegistry` — The registry `/metrics` serves
+11. `metrics.New` — The delivery collectors, registered on that registry
+12. `middleware.New` — HTTP middleware
+13. `delivery.New` — Event-driven delivery engine
+14. `delivery.NewArchiveSweeper` — Periodic pruning of idle archives
+15. `delivery.Engine` → `delivery.Notifier` — interface bridge
+16. `delivery.Engine` → `delivery.WebhookEvictor` — interface bridge so
     deleting a webhook releases its archive writer
-15. `server.New` — HTTP server and router
+17. `server.New` — HTTP server and router
 
 The server starts via `fx.Invoke(func(*server.Server, *delivery.Engine,
 *database.RetentionReaper, *delivery.ArchiveSweeper) {})`, which

cmd/webhooker/main.go

@@ -16,6 +16,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/resetpw"
 	"sneak.berlin/go/webhooker/internal/server"
@@ -177,6 +178,10 @@ func newApp() *fx.App {
 			healthcheck.New,
 			session.New,
 			handlers.New,
+			// The registry /metrics serves, and the delivery
+			// collectors registered on it.
+			metrics.NewRegistry,
+			metrics.New,
 			middleware.New,
 			// The one SSRF guard both target-creation validation
 			// and the delivery dialer consult, so they cannot

internal/delivery/engine.go

@@ -148,6 +148,7 @@ type EngineParams struct {
 	DBManager *database.WebhookDBManager
 	Logger    *logger.Logger
 	SSRFGuard *Guard
+	Metrics   *metrics.Set
 }
 
 // Engine processes queued deliveries in the background
@@ -167,10 +168,10 @@ type Engine struct {
 	retryCh    chan Task
 	workers    int
 
-	// mtr is the delivery metric set. Production wires the
-	// process-wide one; a test can substitute a set registered on
-	// a private registry so its assertions are not disturbed by
-	// deliveries other tests are making at the same time.
+	// mtr is the delivery metric set. Production wires the one
+	// registered on the registry /metrics serves; a test can
+	// substitute a set registered on a registry it holds, so it can
+	// gather what its own deliveries recorded.
 	mtr *metrics.Set
 
 	// targets maps each target type to its implementation.
@@ -204,7 +205,7 @@ func New(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    defaultWorkers,
-		mtr:        metrics.Default(),
+		mtr:        params.Metrics,
 	}
 
 	e.initTargets(&http.Client{

internal/delivery/export_test.go

@@ -9,6 +9,7 @@ import (
 	"net/url"
 	"time"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"gorm.io/gorm"
 	"sneak.berlin/go/webhooker/internal/database"
@@ -399,7 +400,7 @@ func NewTestEngine(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    workers,
-		mtr:        metrics.Default(),
+		mtr:        metrics.New(prometheus.NewRegistry()),
 	}
 	e.initTargets(client)
 
@@ -414,7 +415,7 @@ func NewTestEngineSmallRetry(
 	e := &Engine{
 		log:     log,
 		retryCh: make(chan Task, 1),
-		mtr:     metrics.Default(),
+		mtr:     metrics.New(prometheus.NewRegistry()),
 	}
 	e.initTargets(nil)
 
@@ -437,7 +438,7 @@ func NewTestEngineWithDB(
 		deliveryCh: make(chan Task, deliveryChannelSize),
 		retryCh:    make(chan Task, retryChannelSize),
 		workers:    workers,
-		mtr:        metrics.Default(),
+		mtr:        metrics.New(prometheus.NewRegistry()),
 	}
 	e.initTargets(client)
 
@@ -445,8 +446,7 @@ func NewTestEngineWithDB(
 }
 
 // ExportSetMetrics substitutes the engine's metric set, so a test can
-// assert on collectors registered on a private registry instead of
-// the process-wide ones every other test is also moving.
+// assert on collectors registered on a registry it holds.
 func (e *Engine) ExportSetMetrics(mtr *metrics.Set) {
 	e.mtr = mtr
 }

internal/delivery/metrics_test.go

@@ -35,9 +35,8 @@ const (
 )
 
 // mIsolate gives the setup's engine a metric set registered on a
-// private registry. The process-wide collectors are moved by every
-// other delivery test running in parallel, so exact assertions are
-// only possible against a registry this test owns.
+// registry this test holds, so its exact assertions can gather from
+// it.
 func mIsolate(
 	t *testing.T, s iSetup,
 ) *prometheus.Registry {

internal/handlers/auth.go

@@ -333,9 +333,7 @@ func (h *Handlers) HandleLogout() http.HandlerFunc {
 			)
 		}
 
-		http.Redirect(
-			w, r, withNotice("/pages/login", signedOut),
-			http.StatusSeeOther,
-		)
+		// Redirect to login page
+		http.Redirect(w, r, "/pages/login", http.StatusSeeOther)
 	}
 }

internal/handlers/delivery_replay.go

@@ -11,37 +11,72 @@ import (
 	"sneak.berlin/go/webhooker/internal/delivery"
 )
 
-// The outcomes of a replay POST, as the notice codes its redirect
-// carries. noticeFor holds the line each one shows.
+// replayOutcomeParam is the query parameter the replay POST redirects
+// with and the event log page reads its banner from.
+const replayOutcomeParam = "replay"
+
+// replayOutcomeCode is the outcome of a replay POST. The redirect
+// carries one of these fixed codes rather than a message, so nothing a
+// client submits can reach the rendered page through it.
+type replayOutcomeCode string
+
 const (
 	// replayQueued reports that a new delivery was created and handed
 	// to the delivery engine.
-	replayQueued noticeCode = "replay-queued"
+	replayQueued replayOutcomeCode = "queued"
 
 	// replayTargetDeleted reports a target that once existed and has
 	// since been deleted. Deletes are soft and deliveries carry no
 	// foreign key to the target row, so the history survives its
 	// target and this is the ordinary case for an old event.
-	replayTargetDeleted noticeCode = "replay-target-deleted"
+	replayTargetDeleted replayOutcomeCode = "target-deleted"
 
 	// replayTargetMissing reports a target id that names no row at
 	// all, deleted or otherwise.
-	replayTargetMissing noticeCode = "replay-target-missing"
+	replayTargetMissing replayOutcomeCode = "target-missing"
 
 	// replayTargetInactive reports a target the operator has
 	// deactivated. A deactivated target receives no new deliveries, so
 	// a replay to it would be a delivery they switched off.
-	replayTargetInactive noticeCode = "replay-target-inactive"
+	replayTargetInactive replayOutcomeCode = "target-inactive"
 
 	// replayNotTerminal reports a delivery the engine has not finished
 	// with.
-	replayNotTerminal noticeCode = "replay-not-terminal"
+	replayNotTerminal replayOutcomeCode = "not-terminal"
 
 	// replayInFlight reports that an earlier replay of this event to
 	// this target is still running.
-	replayInFlight noticeCode = "replay-in-flight"
+	replayInFlight replayOutcomeCode = "in-flight"
 )
 
+// replayOutcome returns the banner the event log page shows for an
+// outcome code, and whether the replay was queued. An unrecognised
+// code yields no banner.
+func replayOutcome(code string) (string, bool) {
+	switch replayOutcomeCode(code) {
+	case replayQueued:
+		return "Replay queued: a new delivery was created against " +
+			"the target's current configuration.", true
+	case replayTargetDeleted:
+		return "Not replayed: the target this delivery was for has " +
+			"been deleted. Recreate the target, then replay.", false
+	case replayTargetMissing:
+		return "Not replayed: the target this delivery was for no " +
+			"longer exists.", false
+	case replayTargetInactive:
+		return "Not replayed: the target this delivery was for is " +
+			"deactivated. Activate it, then replay.", false
+	case replayNotTerminal:
+		return "Not replayed: this delivery has not finished yet.",
+			false
+	case replayInFlight:
+		return "Not replayed: a delivery of this event to this " +
+			"target is already in flight.", false
+	default:
+		return "", false
+	}
+}
+
 // HandleDeliveryReplay re-sends a finished delivery's event to its
 // target.
 //
@@ -105,14 +140,14 @@ func (h *Handlers) replayDelivery(
 	}
 
 	if !original.Status.Terminal() {
-		redirectToEventLog(w, r, webhook, replayNotTerminal)
+		h.finishReplay(w, r, webhook, replayNotTerminal)
 
 		return
 	}
 
 	target, code := h.replayTarget(webhook.ID, original.TargetID)
 	if target == nil {
-		redirectToEventLog(w, r, webhook, code)
+		h.finishReplay(w, r, webhook, code)
 
 		return
 	}
@@ -165,7 +200,7 @@ func (h *Handlers) queueReplay(
 	}
 
 	if inFlight > 0 {
-		redirectToEventLog(w, r, webhook, replayInFlight)
+		h.finishReplay(w, r, webhook, replayInFlight)
 
 		return
 	}
@@ -203,7 +238,7 @@ func (h *Handlers) queueReplay(
 		"delivery_id", task.DeliveryID,
 	)
 
-	redirectToEventLog(w, r, webhook, replayQueued)
+	h.finishReplay(w, r, webhook, replayQueued)
 }
 
 // replayTarget loads the delivery's target as it stands now.
@@ -216,7 +251,7 @@ func (h *Handlers) queueReplay(
 // with the returned code saying why.
 func (h *Handlers) replayTarget(
 	webhookID, targetID string,
-) (*database.Target, noticeCode) {
+) (*database.Target, replayOutcomeCode) {
 	var target database.Target
 
 	err := h.db.DB().Unscoped().Where(
@@ -326,16 +361,17 @@ func replayBody(body string) *string {
 	return &body
 }
 
-// redirectToEventLog redirects a replay or resubmit back to the event
-// log it was triggered from, carrying the outcome as its notice and
-// the page number the form submitted.
-func redirectToEventLog(
+// finishReplay redirects back to the event log the replay was
+// triggered from, carrying the outcome code the page turns into a
+// banner and the page number the form submitted.
+func (h *Handlers) finishReplay(
 	w http.ResponseWriter,
 	r *http.Request,
 	webhook database.Webhook,
-	code noticeCode,
+	code replayOutcomeCode,
 ) {
-	dest := withNotice("/hook/"+webhook.ID+"/events", code)
+	dest := "/hook/" + webhook.ID + "/events?" +
+		replayOutcomeParam + "=" + string(code)
 
 	// The page is read from the form rather than the query string:
 	// this is a POST, and its query string is what logs and Referer

internal/handlers/delivery_replay_test.go

@@ -212,7 +212,7 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-queued",
+		"/hook/"+wh.ID+"/events?replay=queued",
 		w.Header().Get("Location"),
 	)
 
@@ -362,7 +362,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-target-deleted",
+		"/hook/"+wh.ID+"/events?replay=target-deleted",
 		w.Header().Get("Location"),
 	)
 
@@ -390,7 +390,7 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, missing.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-target-missing",
+		"/hook/"+wh.ID+"/events?replay=target-missing",
 		missing.Header().Get("Location"),
 	)
 }
@@ -431,7 +431,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, first.Code)
 	require.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-queued",
+		"/hook/"+wh.ID+"/events?replay=queued",
 		first.Header().Get("Location"),
 	)
 
@@ -439,7 +439,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, second.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-in-flight",
+		"/hook/"+wh.ID+"/events?replay=in-flight",
 		second.Header().Get("Location"),
 	)
 
@@ -465,7 +465,7 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
 	require.Equal(t, http.StatusSeeOther, pending.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=replay-not-terminal",
+		"/hook/"+wh.ID+"/events?replay=not-terminal",
 		pending.Header().Get("Location"),
 	)
 }
@@ -509,7 +509,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
 	assert.Contains(t, body, ">Replay<")
 
 	refused := renderSourceLogsPageWithQuery(
-		t, h, sess, wh.ID, "?notice=replay-target-deleted",
+		t, h, sess, wh.ID, "?replay=target-deleted",
 	)
 
 	assert.Contains(t, refused, "alert-error")
@@ -517,7 +517,7 @@ func TestHandleSourceLogs_RendersReplayControlAndBanner(t *testing.T) {
 
 	// An outcome code nobody issued renders no banner at all.
 	unknown := renderSourceLogsPageWithQuery(
-		t, h, sess, wh.ID, "?notice=made-up",
+		t, h, sess, wh.ID, "?replay=made-up",
 	)
 
 	assert.NotContains(t, unknown, "alert-error")

internal/handlers/event_resubmit.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"errors"
 	"net/http"
+	"strconv"
 
 	"github.com/go-chi/chi"
 	"github.com/google/uuid"
@@ -10,19 +11,43 @@ import (
 	"sneak.berlin/go/webhooker/internal/database"
 )
 
-// The outcomes of a resubmit POST, as the notice codes its redirect
-// carries. noticeFor holds the line each one shows.
+// resubmitOutcomeParam is the query parameter the resubmit POST
+// redirects with and the event log page reads its banner from.
+const resubmitOutcomeParam = "resubmit"
+
+// resubmitOutcomeCode is the outcome of a resubmit POST. The redirect
+// carries one of these fixed codes rather than a message, so nothing a
+// client submits can reach the rendered page through it.
+type resubmitOutcomeCode string
+
 const (
 	// resubmitQueued reports that a new event was stored and its
 	// deliveries handed to the delivery engine.
-	resubmitQueued noticeCode = "resubmit-queued"
+	resubmitQueued resubmitOutcomeCode = "queued"
 
 	// resubmitNoTargets reports a source with no active targets. The
 	// new event is stored either way, exactly as a received event
 	// with no targets is.
-	resubmitNoTargets noticeCode = "resubmit-no-targets"
+	resubmitNoTargets resubmitOutcomeCode = "no-targets"
 )
 
+// resubmitOutcome returns the banner the event log page shows for an
+// outcome code, and whether the resubmit was queued. An unrecognised
+// code yields no banner.
+func resubmitOutcome(code string) (string, bool) {
+	switch resubmitOutcomeCode(code) {
+	case resubmitQueued:
+		return "Resubmitted: a new event was created from the stored " +
+			"one and queued to every active target.", true
+	case resubmitNoTargets:
+		return "Resubmitted: a new event was created, but this " +
+				"source has no active targets, so nothing was queued.",
+			true
+	default:
+		return "", false
+	}
+}
+
 // resubmitSource is the stored event a resubmit copies. Its body is
 // read as bytes rather than as a string so the copy is byte-identical
 // to what was received, whatever the payload's encoding.
@@ -220,5 +245,29 @@ func (h *Handlers) queueResubmit(
 		code = resubmitNoTargets
 	}
 
-	redirectToEventLog(w, r, webhook, code)
+	h.finishResubmit(w, r, webhook, code)
+}
+
+// finishResubmit redirects back to the event log the resubmit was
+// triggered from, carrying the outcome code the page turns into a
+// banner and the page number the form submitted.
+func (h *Handlers) finishResubmit(
+	w http.ResponseWriter,
+	r *http.Request,
+	webhook database.Webhook,
+	code resubmitOutcomeCode,
+) {
+	dest := "/hook/" + webhook.ID + "/events?" +
+		resubmitOutcomeParam + "=" + string(code)
+
+	// The page is read from the form rather than the query string:
+	// this is a POST, and its query string is what logs and Referer
+	// headers record.
+	if page := pageOrFirst(
+		r.PostFormValue("page"),
+	); page > 1 {
+		dest += "&page=" + strconv.Itoa(page)
+	}
+
+	http.Redirect(w, r, dest, http.StatusSeeOther)
 }

internal/handlers/event_resubmit_test.go

@@ -154,7 +154,7 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+		"/hook/"+wh.ID+"/events?resubmit=queued",
 		w.Header().Get("Location"),
 	)
 
@@ -282,7 +282,7 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
 		require.Equal(t, http.StatusSeeOther, w.Code)
 		assert.Equal(
 			t,
-			"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+			"/hook/"+wh.ID+"/events?resubmit=queued",
 			w.Header().Get("Location"),
 			"a resubmit must not be refused while an earlier "+
 				"one is in flight",
@@ -436,7 +436,7 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-queued",
+		"/hook/"+wh.ID+"/events?resubmit=queued",
 		w.Header().Get("Location"),
 		"an inactive target is skipped, not an error",
 	)
@@ -482,7 +482,7 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
 	require.Equal(t, http.StatusSeeOther, w.Code)
 	assert.Equal(
 		t,
-		"/hook/"+wh.ID+"/events?notice=resubmit-no-targets",
+		"/hook/"+wh.ID+"/events?resubmit=no-targets",
 		w.Header().Get("Location"),
 	)
 

internal/handlers/handlers.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"sync/atomic"
 
+	"github.com/prometheus/client_golang/prometheus"
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/database"
 	"sneak.berlin/go/webhooker/internal/delivery"
@@ -64,6 +65,8 @@ type HandlersParams struct {
 	Notifier     delivery.Notifier
 	Evictor      delivery.WebhookEvictor
 	SSRFGuard    *delivery.Guard
+	Metrics      *metrics.Set
+	Registry     *prometheus.Registry
 }
 
 // Handlers provides HTTP handler methods for all application
@@ -94,10 +97,10 @@ type Handlers struct {
 
 // parsePageTemplate parses a page-specific template set from the
 // embedded FS. Each page template is combined with the shared
-// base, htmlheader, navbar and notice templates, and with any further
-// files the page includes. The page file must be listed first so that
-// its root action ({{template "base" .}}) becomes the template set's
-// entry point.
+// base, htmlheader, and navbar templates, and with any further files
+// the page includes. The page file must be listed first so that its
+// root action ({{template "base" .}}) becomes the template set's entry
+// point.
 func parsePageTemplate(
 	pageFile string, included ...string,
 ) *template.Template {
@@ -106,7 +109,6 @@ func parsePageTemplate(
 		"base.html",
 		"htmlheader.html",
 		"navbar.html",
-		"notice.html",
 	}, included...)
 
 	return template.Must(
@@ -130,7 +132,7 @@ func New(
 	s.mw = params.Middleware
 	s.notifier = params.Notifier
 	s.evictor = params.Evictor
-	s.mtr = metrics.Default()
+	s.mtr = params.Metrics
 	s.ssrf = params.SSRFGuard
 
 	// Parse all page templates once at startup
@@ -207,13 +209,11 @@ func (s *Handlers) renderError(
 	// served outside the routes where NoCache runs.
 	w.Header().Set("Cache-Control", "no-store")
 
-	// No notice: one would say an action worked above a page saying
-	// the request failed.
 	data := s.pageData(r, map[string]any{
 		"Status":     status,
 		"StatusText": http.StatusText(status),
 		"Message":    errorPageText(status),
-	}, nil)
+	})
 
 	var buf bytes.Buffer
 
@@ -267,7 +267,6 @@ type templateDataWrapper struct {
 	User      *UserInfo
 	CSRFToken string
 	Version   string
-	Notice    *notice
 	Data      any
 }
 
@@ -312,15 +311,12 @@ func (s *Handlers) renderTemplate(
 		return
 	}
 
-	s.executeTemplate(w, r, tmpl, s.pageData(r, data, noticeFor(r)))
+	s.executeTemplate(w, r, tmpl, s.pageData(r, data))
 }
 
 // pageData adds the fields the shared layout renders to a page's own
-// data. The layout shows the notice, when there is one, above the
-// page.
-func (s *Handlers) pageData(
-	r *http.Request, data any, pageNotice *notice,
-) any {
+// data.
+func (s *Handlers) pageData(r *http.Request, data any) any {
 	userInfo := s.getUserInfo(r)
 	csrfToken := middleware.CSRFToken(r)
 
@@ -334,7 +330,6 @@ func (s *Handlers) pageData(
 		m["User"] = userInfo
 		m["CSRFToken"] = csrfToken
 		m["Version"] = version
-		m["Notice"] = pageNotice
 
 		return m
 	}
@@ -343,7 +338,6 @@ func (s *Handlers) pageData(
 		User:      userInfo,
 		CSRFToken: csrfToken,
 		Version:   version,
-		Notice:    pageNotice,
 		Data:      data,
 	}
 }

internal/handlers/handlers_test.go

@@ -20,6 +20,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/session"
 )
@@ -109,6 +110,8 @@ func newTestApp(
 			func(r *recordingEvictor) delivery.WebhookEvictor {
 				return r
 			},
+			metrics.NewRegistry,
+			metrics.New,
 			middleware.New,
 			delivery.NewGuard,
 			handlers.New,

internal/handlers/metrics.go

@@ -0,0 +1,21 @@
+package handlers
+
+import (
+	"net/http"
+
+	"github.com/prometheus/client_golang/prometheus/promhttp"
+)
+
+// HandleMetrics returns the Prometheus scrape handler for the
+// registry built by metrics.NewRegistry, which the HTTP, delivery, Go
+// runtime and process collectors register on. It is what
+// promhttp.Handler builds for the global default registry, including
+// the promhttp_metric_handler_* series that count scrapes, pointed at
+// that registry instead.
+func (s *Handlers) HandleMetrics() http.HandlerFunc {
+	reg := s.params.Registry
+
+	return promhttp.InstrumentMetricHandler(
+		reg, promhttp.HandlerFor(reg, promhttp.HandlerOpts{}),
+	).ServeHTTP
+}

internal/handlers/notice.go

@@ -1,109 +0,0 @@
-package handlers
-
-import "net/http"
-
-// noticeParam is the query parameter an action's redirect carries its
-// notice code in.
-const noticeParam = "notice"
-
-// noticeCode names one of the fixed lines noticeFor knows. An action
-// redirects with the code rather than the line, so nothing a client
-// puts in the URL reaches the page: a code noticeFor does not know
-// shows nothing.
-type noticeCode string
-
-// The codes of the actions on the webhook pages and of signing out.
-// Replay's and resubmit's are beside those actions, with the reasons
-// each can be refused.
-const (
-	webhookCreated        noticeCode = "webhook-created"
-	webhookSaved          noticeCode = "webhook-saved"
-	webhookDeleted        noticeCode = "webhook-deleted"
-	entrypointAdded       noticeCode = "entrypoint-added"
-	entrypointDeleted     noticeCode = "entrypoint-deleted"
-	entrypointActivated   noticeCode = "entrypoint-activated"
-	entrypointDeactivated noticeCode = "entrypoint-deactivated"
-	targetAdded           noticeCode = "target-added"
-	targetSaved           noticeCode = "target-saved"
-	targetDeleted         noticeCode = "target-deleted"
-	targetActivated       noticeCode = "target-activated"
-	targetDeactivated     noticeCode = "target-deactivated"
-	signedOut             noticeCode = "signed-out"
-)
-
-// notice is the line templates/notice.html shows above a page to say
-// what an action did.
-type notice struct {
-	Text string
-
-	// Failed shows the line as an error: the action was refused.
-	Failed bool
-}
-
-// noticeFor returns the notice the request's URL names, or nil when it
-// names none or an unknown code.
-func noticeFor(r *http.Request) *notice {
-	n, ok := map[noticeCode]notice{
-		webhookCreated:        {Text: "Webhook created."},
-		webhookSaved:          {Text: "Webhook saved."},
-		webhookDeleted:        {Text: "Webhook deleted."},
-		entrypointAdded:       {Text: "Entrypoint added."},
-		entrypointDeleted:     {Text: "Entrypoint deleted."},
-		entrypointActivated:   {Text: "Entrypoint activated."},
-		entrypointDeactivated: {Text: "Entrypoint deactivated."},
-		targetAdded:           {Text: "Target added."},
-		targetSaved:           {Text: "Target saved."},
-		targetDeleted:         {Text: "Target deleted."},
-		targetActivated:       {Text: "Target activated."},
-		targetDeactivated:     {Text: "Target deactivated."},
-		signedOut:             {Text: "Signed out."},
-
-		replayQueued: {
-			Text: "Replay queued: a new delivery was created " +
-				"against the target's current configuration.",
-		},
-		replayTargetDeleted: {
-			Text: "Not replayed: the target this delivery was for " +
-				"has been deleted. Recreate the target, then replay.",
-			Failed: true,
-		},
-		replayTargetMissing: {
-			Text: "Not replayed: the target this delivery was for " +
-				"no longer exists.",
-			Failed: true,
-		},
-		replayTargetInactive: {
-			Text: "Not replayed: the target this delivery was for " +
-				"is deactivated. Activate it, then replay.",
-			Failed: true,
-		},
-		replayNotTerminal: {
-			Text:   "Not replayed: this delivery has not finished yet.",
-			Failed: true,
-		},
-		replayInFlight: {
-			Text: "Not replayed: a delivery of this event to this " +
-				"target is already in flight.",
-			Failed: true,
-		},
-
-		resubmitQueued: {
-			Text: "Resubmitted: a new event was created from the " +
-				"stored one and queued to every active target.",
-		},
-		resubmitNoTargets: {
-			Text: "Resubmitted: a new event was created, but this " +
-				"source has no active targets, so nothing was queued.",
-		},
-	}[noticeCode(r.URL.Query().Get(noticeParam))]
-	if !ok {
-		return nil
-	}
-
-	return &n
-}
-
-// withNotice returns path with code added as its notice.
-func withNotice(path string, code noticeCode) string {
-	return path + "?" + noticeParam + "=" + string(code)
-}

internal/handlers/source_delete_test.go

@@ -411,9 +411,7 @@ func TestHandleSourceDelete_RemovesConfigAndEventDatabase(
 	h.HandleSourceDelete().ServeHTTP(w, req)
 
 	require.Equal(t, http.StatusSeeOther, w.Code)
-	assert.Equal(
-		t, "/hooks?notice=webhook-deleted", w.Header().Get("Location"),
-	)
+	assert.Equal(t, "/hooks", w.Header().Get("Location"))
 
 	assert.Equal(
 		t, int64(0),

internal/handlers/source_management.go

@@ -322,8 +322,7 @@ func (h *Handlers) createWebhookWithEntrypoint(
 	)
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, webhookCreated),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -580,8 +579,7 @@ func (h *Handlers) applyWebhookEdit(
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, webhookSaved),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -664,9 +662,7 @@ func (h *Handlers) deleteWebhookResources(
 		return
 	}
 
-	http.Redirect(
-		w, r, withNotice("/hooks", webhookDeleted), http.StatusSeeOther,
-	)
+	http.Redirect(w, r, "/hooks", http.StatusSeeOther)
 }
 
 // commitWebhookDeletion soft-deletes a webhook's entrypoints,
@@ -845,16 +841,31 @@ func (h *Handlers) HandleSourceLogs() http.HandlerFunc {
 			totalPages++
 		}
 
+		// The banner a replay or resubmit POST redirected back
+		// with. The message comes from a fixed set keyed by the
+		// outcome code, never from the query string itself.
+		replayMsg, replayOK := replayOutcome(
+			r.URL.Query().Get(replayOutcomeParam),
+		)
+
+		resubmitMsg, resubmitOK := resubmitOutcome(
+			r.URL.Query().Get(resubmitOutcomeParam),
+		)
+
 		data := map[string]any{
-			tmplKeyWebhook: &webhook,
-			"Events":       evts,
-			"Page":         page,
-			"TotalPages":   totalPages,
-			"TotalEvents":  total,
-			"HasPrev":      page > 1,
-			"HasNext":      page < totalPages,
-			"PrevPage":     page - 1,
-			"NextPage":     page + 1,
+			tmplKeyWebhook:    &webhook,
+			"Events":          evts,
+			"ReplayMessage":   replayMsg,
+			"ReplayQueued":    replayOK,
+			"ResubmitMessage": resubmitMsg,
+			"ResubmitQueued":  resubmitOK,
+			"Page":            page,
+			"TotalPages":      totalPages,
+			"TotalEvents":     total,
+			"HasPrev":         page > 1,
+			"HasNext":         page < totalPages,
+			"PrevPage":        page - 1,
+			"NextPage":        page + 1,
 		}
 
 		h.renderTemplate(w, r, "source_logs.html", data)
@@ -1243,8 +1254,7 @@ func (h *Handlers) HandleEntrypointCreate() http.HandlerFunc {
 		}
 
 		http.Redirect(
-			w, r, withNotice("/hook/"+webhook.ID, entrypointAdded),
-			http.StatusSeeOther,
+			w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 		)
 	}
 }
@@ -1355,8 +1365,7 @@ func (h *Handlers) processTargetCreate(
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, targetAdded),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 
@@ -1634,7 +1643,6 @@ func (h *Handlers) HandleEntrypointDelete() http.HandlerFunc {
 		"entrypointID", &database.Entrypoint{},
 		"failed to delete entrypoint",
 		nil,
-		entrypointDeleted,
 	)
 }
 
@@ -1647,21 +1655,18 @@ func (h *Handlers) HandleTargetDelete() http.HandlerFunc {
 		"targetID", &database.Target{},
 		"failed to delete target",
 		h.evictArchiveWriterIfUnused,
-		targetDeleted,
 	)
 }
 
 // deleteChildResource returns a handler that deletes a child
 // resource (entrypoint or target) belonging to a webhook. The
 // optional afterDelete hook runs with the webhook's id once the
-// delete has succeeded, before the redirect, which carries done as
-// its notice.
+// delete has succeeded, before the redirect.
 func (h *Handlers) deleteChildResource(
 	idParam string,
 	model any,
 	errMsg string,
 	afterDelete func(webhookID string),
-	done noticeCode,
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
@@ -1703,7 +1708,7 @@ func (h *Handlers) deleteChildResource(
 
 		http.Redirect(
 			w, r,
-			withNotice("/hook/"+webhook.ID, done),
+			"/hook/"+webhook.ID,
 			http.StatusSeeOther,
 		)
 	}
@@ -1714,7 +1719,7 @@ func (h *Handlers) deleteChildResource(
 func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 	return h.toggleChildResource(
 		"entrypointID",
-		func(webhookID, childID string) (bool, error) {
+		func(webhookID, childID string) error {
 			var ep database.Entrypoint
 
 			err := h.db.DB().Where(
@@ -1722,15 +1727,14 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 				childID, webhookID,
 			).First(&ep).Error
 			if err != nil {
-				return false, err
+				return err
 			}
 
 			ep.Active = !ep.Active
 
-			return ep.Active, h.db.DB().Save(&ep).Error
+			return h.db.DB().Save(&ep).Error
 		},
 		"failed to toggle entrypoint",
-		entrypointActivated, entrypointDeactivated,
 	)
 }
 
@@ -1738,7 +1742,7 @@ func (h *Handlers) HandleEntrypointToggle() http.HandlerFunc {
 func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 	return h.toggleChildResource(
 		"targetID",
-		func(webhookID, childID string) (bool, error) {
+		func(webhookID, childID string) error {
 			var tgt database.Target
 
 			err := h.db.DB().Where(
@@ -1746,27 +1750,23 @@ func (h *Handlers) HandleTargetToggle() http.HandlerFunc {
 				childID, webhookID,
 			).First(&tgt).Error
 			if err != nil {
-				return false, err
+				return err
 			}
 
 			tgt.Active = !tgt.Active
 
-			return tgt.Active, h.db.DB().Save(&tgt).Error
+			return h.db.DB().Save(&tgt).Error
 		},
 		"failed to toggle target",
-		targetActivated, targetDeactivated,
 	)
 }
 
 // toggleChildResource returns a handler that toggles the active
-// state of a child resource belonging to a webhook. toggleFn returns
-// the new state, and the redirect carries activated or deactivated as
-// its notice to match.
+// state of a child resource belonging to a webhook.
 func (h *Handlers) toggleChildResource(
 	idParam string,
-	toggleFn func(webhookID, childID string) (bool, error),
+	toggleFn func(webhookID, childID string) error,
 	errMsg string,
-	activated, deactivated noticeCode,
 ) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		userID, ok := h.getUserID(r)
@@ -1792,21 +1792,16 @@ func (h *Handlers) toggleChildResource(
 			return
 		}
 
-		active, err := toggleFn(webhook.ID, childID)
+		err = toggleFn(webhook.ID, childID)
 		if err != nil {
 			h.serverError(w, r, errMsg, err)
 
 			return
 		}
 
-		done := deactivated
-		if active {
-			done = activated
-		}
-
 		http.Redirect(
 			w, r,
-			withNotice("/hook/"+webhook.ID, done),
+			"/hook/"+webhook.ID,
 			http.StatusSeeOther,
 		)
 	}

internal/handlers/target_edit.go

@@ -161,8 +161,7 @@ func (h *Handlers) applyTargetEdit(
 	}
 
 	http.Redirect(
-		w, r, withNotice("/hook/"+webhook.ID, targetSaved),
-		http.StatusSeeOther,
+		w, r, "/hook/"+webhook.ID, http.StatusSeeOther,
 	)
 }
 

internal/metrics/metrics.go

@@ -3,17 +3,18 @@
 // deliveries are attempted, how they end, how long they take, how
 // deep the queues are, and how many circuit breakers are open.
 //
-// The inbound HTTP metrics come from the go-http-metrics recorder in
-// internal/middleware and land on prometheus.DefaultRegisterer. These
-// collectors register there too, so both surfaces are gathered by the
-// one promhttp handler mounted on the authenticated /metrics route.
+// It also builds the registry the authenticated /metrics route
+// serves. In production, these collectors, the inbound HTTP metrics
+// recorded in internal/middleware, and the Go runtime and process
+// collectors all register on that one registry, never on Prometheus's
+// global default.
 package metrics
 
 import (
-	"sync"
 	"time"
 
 	"github.com/prometheus/client_golang/prometheus"
+	"github.com/prometheus/client_golang/prometheus/collectors"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"sneak.berlin/go/webhooker/internal/database"
 )
@@ -57,25 +58,31 @@ var knownTargetTypes = []database.TargetType{
 	database.TargetTypeSlack,
 }
 
-// defaultSet is the process-wide metric set, registered on the same
-// registry the HTTP middleware and the /metrics handler already use.
-// It is built on first use rather than in an init so that a test
-// binary that never touches metrics never registers them.
+// NewRegistry returns the registry /metrics serves, carrying the Go
+// runtime and process collectors that Prometheus's global default
+// registry carries, so the go_* and process_* series stay in the
+// scrape.
 //
-//nolint:gochecknoglobals // one process-wide registration, by design
-var defaultSet = sync.OnceValue(func() *Set {
-	return New(prometheus.DefaultRegisterer)
-})
+// A registry of its own, rather than the global default, is what lets
+// two dependency graphs in one process — two tests, say — each
+// register their collectors without the second registration
+// panicking.
+func NewRegistry() *prometheus.Registry {
+	reg := prometheus.NewRegistry()
+	reg.MustRegister(
+		collectors.NewGoCollector(),
+		collectors.NewProcessCollector(
+			collectors.ProcessCollectorOpts{},
+		),
+	)
 
-// Default returns the process-wide metric set.
-func Default() *Set {
-	return defaultSet()
+	return reg
 }
 
 // Set is one registered group of webhooker's delivery collectors.
-// Production uses the single Default set; tests build their own
-// against a private registry so assertions are not disturbed by
-// deliveries other tests are making concurrently.
+// Production builds one on the registry /metrics serves; tests build
+// one on a registry of their own so they can gather what their own
+// deliveries recorded.
 type Set struct {
 	eventsReceived      prometheus.Counter
 	deliveryAttempts    *prometheus.CounterVec
@@ -93,7 +100,7 @@ type Set struct {
 // New registers a full set of delivery collectors on reg and returns
 // it. It panics if reg already holds them, which is the intended
 // behaviour for a duplicate registration.
-func New(reg prometheus.Registerer) *Set {
+func New(reg *prometheus.Registry) *Set {
 	factory := promauto.With(reg)
 
 	s := &Set{

internal/middleware/export_test.go

@@ -10,8 +10,7 @@ import (
 
 // MetricsMiddlewareForTest builds the metrics recording middleware
 // against a caller-supplied recorder, so a test can gather from its
-// own Prometheus registry rather than the process-wide default one
-// that Middleware.Metrics uses.
+// own Prometheus registry without building a whole Middleware.
 func MetricsMiddlewareForTest(
 	rec httpmetrics.Recorder,
 ) func(http.Handler) http.Handler {

internal/middleware/metrics.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/go-chi/chi"
 	httpmetrics "github.com/slok/go-http-metrics/metrics"
-	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	ghmm "github.com/slok/go-http-metrics/middleware"
 	"github.com/slok/go-http-metrics/middleware/std"
 )
@@ -151,17 +150,17 @@ func (r boundedLabelRecorder) AddInflightRequests(
 
 var _ httpmetrics.Recorder = boundedLabelRecorder{}
 
-// Metrics returns middleware that records Prometheus HTTP metrics on
-// the default registry, which is the one the /metrics route gathers.
+// Metrics returns middleware that records Prometheus HTTP metrics
+// with the Middleware's one recorder, which New builds on the registry
+// the /metrics route serves and NewForTest on a registry of its own.
+// Every call reuses that recorder, so any number of routers can
+// install it.
 func (s *Middleware) Metrics() func(http.Handler) http.Handler {
-	return metricsMiddleware(
-		prommetrics.NewRecorder(prommetrics.Config{}),
-	)
+	return metricsMiddleware(s.metricsRecorder)
 }
 
 // metricsMiddleware builds the recording middleware against a given
-// recorder, so tests can gather from a registry of their own instead
-// of the process-wide default.
+// recorder, so tests can gather from a registry of their own.
 func metricsMiddleware(
 	rec httpmetrics.Recorder,
 ) func(http.Handler) http.Handler {

internal/middleware/metrics_test.go

@@ -57,9 +57,8 @@ const (
 // Server.setupWebhookRoutes inside it. That ordering is the whole
 // defect, so a test that flattens it would prove nothing.
 //
-// The recorder writes to a registry of the test's own rather than the
-// process-wide default one, so each test observes only its own
-// traffic.
+// The recorder writes to a registry of the test's own, so each test
+// observes only its own traffic.
 func metricsTestRouter(
 	t *testing.T,
 	receiverLimit int,
@@ -455,3 +454,29 @@ func TestMetrics_StatusAndSizeStillRecorded(t *testing.T) {
 		"the interceptor must still count written bytes",
 	)
 }
+
+// TestMetrics_WorksOnNewForTestMiddleware pins that a Middleware built
+// by NewForTest has a recorder of its own: its Metrics() serves a
+// request instead of panicking, and a second one does not collide
+// with the first.
+func TestMetrics_WorksOnNewForTestMiddleware(t *testing.T) {
+	t.Parallel()
+
+	log := slog.New(slog.DiscardHandler)
+	cfg := &config.Config{Environment: "prod"}
+	ok := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte(okBody))
+	})
+
+	for range 2 {
+		h := middleware.NewForTest(log, cfg, nil).Metrics()(ok)
+
+		req := httptest.NewRequestWithContext(
+			t.Context(), http.MethodGet, okRoute, nil,
+		)
+		w := httptest.NewRecorder()
+		h.ServeHTTP(w, req)
+
+		assert.Equal(t, http.StatusOK, w.Code)
+	}
+}

internal/middleware/middleware.go

@@ -14,6 +14,9 @@ import (
 	"github.com/go-chi/chi"
 	"github.com/go-chi/chi/middleware"
 	"github.com/go-chi/cors"
+	"github.com/prometheus/client_golang/prometheus"
+	httpmetrics "github.com/slok/go-http-metrics/metrics"
+	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	"go.uber.org/fx"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/globals"
@@ -149,10 +152,11 @@ const (
 type MiddlewareParams struct {
 	fx.In
 
-	Logger  *logger.Logger
-	Globals *globals.Globals
-	Config  *config.Config
-	Session *session.Session
+	Logger   *logger.Logger
+	Globals  *globals.Globals
+	Config   *config.Config
+	Session  *session.Session
+	Registry *prometheus.Registry
 }
 
 // Middleware provides HTTP middleware for logging, CORS, auth, and
@@ -162,6 +166,14 @@ type Middleware struct {
 	params  *MiddlewareParams
 	session *session.Session
 
+	// metricsRecorder records the inbound HTTP metrics. New builds
+	// it on the registry /metrics serves, NewForTest on a registry
+	// of its own. Either way it is built once per Middleware and
+	// Metrics reuses it, because building it registers its
+	// collectors, and a second registration on the same registry
+	// panics.
+	metricsRecorder httpmetrics.Recorder
+
 	// loginGuard counts failed credential verifications and bounds
 	// concurrent password hashing. It is built on first use so that
 	// every construction path gets one; see guard().
@@ -180,6 +192,9 @@ func New(
 	s.params = &params
 	s.log = params.Logger.Get()
 	s.session = params.Session
+	s.metricsRecorder = prommetrics.NewRecorder(
+		prommetrics.Config{Registry: params.Registry},
+	)
 
 	return s, nil
 }

internal/middleware/testing.go

@@ -3,12 +3,17 @@ package middleware
 import (
 	"log/slog"
 
+	"github.com/prometheus/client_golang/prometheus"
+	prommetrics "github.com/slok/go-http-metrics/metrics/prometheus"
 	"sneak.berlin/go/webhooker/internal/config"
 	"sneak.berlin/go/webhooker/internal/session"
 )
 
 // NewForTest creates a Middleware with the minimum dependencies
 // needed for testing. This bypasses the fx lifecycle.
+//
+// Its metrics recorder writes to a fresh registry of its own, so
+// Metrics() works on it and two of them never collide.
 func NewForTest(
 	log *slog.Logger,
 	cfg *config.Config,
@@ -20,5 +25,8 @@ func NewForTest(
 			Config: cfg,
 		},
 		session: sess,
+		metricsRecorder: prommetrics.NewRecorder(
+			prommetrics.Config{Registry: prometheus.NewRegistry()},
+		),
 	}
 }

internal/resetpw/resetpw_test.go

@@ -24,6 +24,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/resetpw"
 	"sneak.berlin/go/webhooker/internal/session"
@@ -163,6 +164,8 @@ func newServerApp(
 			session.New,
 			func() delivery.Notifier { return &noopNotifier{} },
 			func() delivery.WebhookEvictor { return &noopEvictor{} },
+			metrics.NewRegistry,
+			metrics.New,
 			middleware.New,
 			delivery.NewGuard,
 			handlers.New,

internal/server/error_page_test.go

@@ -83,22 +83,6 @@ func TestErrorPage_DeletedTarget(t *testing.T) {
 	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
 }
 
-// TestErrorPage_ShowsNoNotice pins that a notice code in the URL of a
-// page that fails is not shown above the error.
-func TestErrorPage_ShowsNoNotice(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "owner", "somepassword")
-	cookies := env.authCookies(t, userID, "owner")
-
-	w := env.get("/hook/no-such-webhook?notice=webhook-saved", cookies)
-
-	assertErrorPage(t, w, http.StatusNotFound, backToWebhooks)
-	assert.NotContains(t, w.Body.String(), "Webhook saved.")
-}
-
 func TestErrorPage_UnknownPath(t *testing.T) {
 	t.Parallel()
 

internal/server/routes.go

@@ -7,7 +7,6 @@ import (
 	sentryhttp "github.com/getsentry/sentry-go/http"
 	"github.com/go-chi/chi"
 	"github.com/go-chi/chi/middleware"
-	"github.com/prometheus/client_golang/prometheus/promhttp"
 	"sneak.berlin/go/webhooker/static"
 )
 
@@ -149,12 +148,7 @@ func (s *Server) setupRoutes() {
 	if s.params.Config.MetricsAuthEnabled() {
 		s.router.Group(func(r chi.Router) {
 			r.Use(s.mw.MetricsAuth())
-			r.Get(
-				"/metrics",
-				http.HandlerFunc(
-					promhttp.Handler().ServeHTTP,
-				),
-			)
+			r.Get("/metrics", s.h.HandleMetrics())
 		})
 	}
 

internal/server/routes_test.go

@@ -24,6 +24,7 @@ import (
 	"sneak.berlin/go/webhooker/internal/handlers"
 	"sneak.berlin/go/webhooker/internal/healthcheck"
 	"sneak.berlin/go/webhooker/internal/logger"
+	"sneak.berlin/go/webhooker/internal/metrics"
 	"sneak.berlin/go/webhooker/internal/middleware"
 	"sneak.berlin/go/webhooker/internal/server"
 	"sneak.berlin/go/webhooker/internal/session"
@@ -113,6 +114,8 @@ func newTestEnvWithConfig(
 			session.New,
 			func() delivery.Notifier { return &noopNotifier{} },
 			func() delivery.WebhookEvictor { return &noopEvictor{} },
+			metrics.NewRegistry,
+			metrics.New,
 			middleware.New,
 			delivery.NewGuard,
 			handlers.New,
@@ -260,24 +263,6 @@ func (e *testEnv) urlFrom(
 	return html.UnescapeString(match[1])
 }
 
-// requireNotice requires w to redirect to dest carrying the notice
-// code, then renders that page and requires it to show text.
-func (e *testEnv) requireNotice(
-	t *testing.T,
-	w *httptest.ResponseRecorder,
-	dest, code, text string,
-	cookies []*http.Cookie,
-) {
-	t.Helper()
-
-	require.Equal(t, http.StatusSeeOther, w.Code)
-	require.Equal(t, dest+"?notice="+code, w.Header().Get("Location"))
-
-	page := e.get(w.Header().Get("Location"), cookies)
-	require.Equal(t, http.StatusOK, page.Code)
-	assert.Contains(t, page.Body.String(), text)
-}
-
 // authCookies forges an authenticated session for the given user.
 func (e *testEnv) authCookies(
 	t *testing.T,
@@ -756,31 +741,6 @@ func TestPagesLogin_ReturnsToTheRequestedPage(t *testing.T) {
 	assert.Equal(t, asked, w.Header().Get("Location"))
 }
 
-// TestPagesLogout_SaysSignedOut signs out with the navbar's form and
-// lands on the sign-in page, which says so.
-func TestPagesLogout_SaysSignedOut(t *testing.T) {
-	t.Parallel()
-
-	env := newTestEnv(t)
-
-	userID, _ := env.seedUser(t, "leaver", "somepassword")
-	token, cookies := env.csrfFrom(
-		t, "/hooks", env.authCookies(t, userID, "leaver"),
-	)
-
-	form := url.Values{}
-	form.Set("csrf_token", token)
-
-	w := env.post(
-		env.urlFrom(t, "/hooks", `action="(/pages/logout)"`, cookies),
-		form, cookies,
-	)
-
-	// The sign-in page is requested without the session cookie, which
-	// the logout told the browser to delete.
-	env.requireNotice(t, w, "/pages/login", "signed-out", "Signed out.", nil)
-}
-
 // --- /user/{username} group ---
 
 // TestPasswordChange_OversizeBody_RejectedAndPasswordUnchanged
@@ -898,9 +858,9 @@ func TestHooks_ListAndNewWebhookForm(t *testing.T) {
 	require.NoError(t,
 		env.db.DB().Where("name = ?", "created").First(&created).Error,
 	)
-	env.requireNotice(
-		t, w, "/hook/"+created.ID, "webhook-created", "Webhook created.",
-		cookies,
+	assert.Equal(
+		t, "/hook/"+created.ID, w.Header().Get("Location"),
+		"creating a webhook should redirect to its page",
 	)
 }
 
@@ -931,7 +891,8 @@ func TestHook_EditFormAndDelete(t *testing.T) {
 		env.urlFrom(t, editPage, `action="(/hook/[^/"]+/edit)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(t, w, page, "webhook-saved", "Webhook saved.", cookies)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, page, w.Header().Get("Location"))
 
 	var edited database.Webhook
 
@@ -945,17 +906,16 @@ func TestHook_EditFormAndDelete(t *testing.T) {
 		env.urlFrom(t, page, `action="(/hook/[^/"]+/delete)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(
-		t, w, "/hooks", "webhook-deleted", "Webhook deleted.", cookies,
-	)
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(t, "/hooks", w.Header().Get("Location"))
 	assert.Equal(
 		t, http.StatusNotFound, env.get(page, cookies).Code,
 		"a deleted webhook's page should be gone",
 	)
 }
 
-// TestHook_EntrypointActions adds, deactivates, activates and deletes
-// an entrypoint with the forms on the webhook page, each submitted to
+// TestHook_EntrypointActions adds, deactivates and deletes an
+// entrypoint with the forms on the webhook page, each submitted to
 // the action and with the token the page rendered.
 func TestHook_EntrypointActions(t *testing.T) {
 	t.Parallel()
@@ -973,19 +933,16 @@ func TestHook_EntrypointActions(t *testing.T) {
 	form.Set("csrf_token", token)
 
 	// submit posts the webhook page's form whose action pattern
-	// captures, and requires the redirect back to that page with the
-	// notice code, and the page to show text.
-	submit := func(pattern, code, text string) {
+	// captures, and requires the redirect back to that page.
+	submit := func(pattern string) {
 		t.Helper()
 
 		w := env.post(env.urlFrom(t, page, pattern, cookies), form, cookies)
-		env.requireNotice(t, w, page, code, text, cookies)
+		require.Equal(t, http.StatusSeeOther, w.Code)
+		require.Equal(t, page, w.Header().Get("Location"))
 	}
 
-	toggle := `action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`
-
-	submit(`action="(/hook/[^/"]+/entrypoints)"`,
-		"entrypoint-added", "Entrypoint added.")
+	submit(`action="(/hook/[^/"]+/entrypoints)"`)
 
 	var added database.Entrypoint
 
@@ -994,7 +951,7 @@ func TestHook_EntrypointActions(t *testing.T) {
 	)
 	require.True(t, added.Active)
 
-	submit(toggle, "entrypoint-deactivated", "Entrypoint deactivated.")
+	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/toggle)"`)
 
 	var toggled database.Entrypoint
 
@@ -1003,10 +960,7 @@ func TestHook_EntrypointActions(t *testing.T) {
 	)
 	assert.False(t, toggled.Active, "the toggle should deactivate it")
 
-	submit(toggle, "entrypoint-activated", "Entrypoint activated.")
-
-	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`,
-		"entrypoint-deleted", "Entrypoint deleted.")
+	submit(`action="(/hook/[^/"]+/entrypoints/[^/"]+/delete)"`)
 
 	var left int64
 
@@ -1017,8 +971,8 @@ func TestHook_EntrypointActions(t *testing.T) {
 
 // TestHook_TargetActions adds a target with the form on the webhook
 // page, follows its Edit link to the target edit form and submits
-// it, then deactivates, activates and deletes it, every URL and token
-// taken from the rendered pages.
+// it, then deactivates and deletes it, every URL and token taken from
+// the rendered pages.
 func TestHook_TargetActions(t *testing.T) {
 	t.Parallel()
 
@@ -1033,29 +987,27 @@ func TestHook_TargetActions(t *testing.T) {
 
 	// submit posts form, with the token, to the action pattern
 	// captures on the page at from, and requires the redirect back to
-	// the webhook page with the notice code, and that page to show
-	// text.
-	submit := func(from, pattern string, form url.Values, code, text string) {
+	// the webhook page.
+	submit := func(from, pattern string, form url.Values) {
 		t.Helper()
 
 		form.Set("csrf_token", token)
 
 		w := env.post(env.urlFrom(t, from, pattern, cookies), form, cookies)
-		env.requireNotice(t, w, page, code, text, cookies)
+		require.Equal(t, http.StatusSeeOther, w.Code)
+		require.Equal(t, page, w.Header().Get("Location"))
 	}
 
-	toggle := `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`
-
 	submit(page, `action="(/hook/[^/"]+/targets)"`, url.Values{
 		"name": {"added"},
 		"type": {string(database.TargetTypeLog)},
-	}, "target-added", "Target added.")
+	})
 
 	editPage := env.urlFrom(
 		t, page, `href="(/hook/[^/"]+/targets/[^/"]+/edit)"`, cookies,
 	)
 	submit(editPage, `action="(/hook/[^/"]+/targets/[^/"]+/edit)"`,
-		url.Values{"name": {"renamed"}}, "target-saved", "Target saved.")
+		url.Values{"name": {"renamed"}})
 
 	var edited database.Target
 
@@ -1065,8 +1017,8 @@ func TestHook_TargetActions(t *testing.T) {
 	assert.Equal(t, "renamed", edited.Name)
 	require.True(t, edited.Active)
 
-	submit(page, toggle, url.Values{},
-		"target-deactivated", "Target deactivated.")
+	submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/toggle)"`,
+		url.Values{})
 
 	var toggled database.Target
 
@@ -1075,11 +1027,8 @@ func TestHook_TargetActions(t *testing.T) {
 	)
 	assert.False(t, toggled.Active, "the toggle should deactivate it")
 
-	submit(page, toggle, url.Values{},
-		"target-activated", "Target activated.")
-
 	submit(page, `action="(/hook/[^/"]+/targets/[^/"]+/delete)"`,
-		url.Values{}, "target-deleted", "Target deleted.")
+		url.Values{})
 
 	var left int64
 
@@ -1115,9 +1064,9 @@ func TestHook_ResubmitFromEventLog(t *testing.T) {
 		env.urlFrom(t, logsPath, `action="(/hook/[^"]+/resubmit)"`, cookies),
 		form, cookies,
 	)
-	env.requireNotice(
-		t, w, logsPath, "resubmit-no-targets",
-		"this source has no active targets", cookies,
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, logsPath+"?resubmit=no-targets", w.Header().Get("Location"),
 	)
 
 	webhookDB, err := env.dbMgr.GetDB(wh.ID)
@@ -1353,8 +1302,10 @@ func TestDeliveryReplay_PostOnlyAndCSRFProtected(t *testing.T) {
 		html.UnescapeString(action[1]), form, cookies,
 	)
 
-	env.requireNotice(
-		t, w, logsPath, "replay-queued", "Replay queued:", cookies,
+	require.Equal(t, http.StatusSeeOther, w.Code)
+	assert.Equal(
+		t, logsPath+"?replay=queued",
+		w.Header().Get("Location"),
 	)
 	assert.Equal(
 		t, int64(2), env.countDeliveries(t, wh.ID),
@@ -1530,3 +1481,46 @@ func TestMetricsRouteUnmountedOnHalfSetConfig(t *testing.T) {
 		})
 	}
 }
+
+// TestTwoMetricsRoutersInOneProcess pins
+// https://git.eeqj.de/sneak/webhooker/issues/227: a second
+// metrics-enabled router in one process used to panic, because the
+// HTTP metrics registered on Prometheus's global default registry.
+// Two routers are built over separate dependency graphs and a third
+// over the first graph again, and each must still serve the HTTP,
+// delivery, Go runtime and process series, and the series counting
+// scrapes of /metrics itself.
+func TestTwoMetricsRoutersInOneProcess(t *testing.T) {
+	t.Parallel()
+
+	first := newTestEnvWithConfig(
+		t, metricsConfig(t, metricsUser, metricsAuthValue),
+	)
+	second := newTestEnvWithConfig(
+		t, metricsConfig(t, metricsUser, metricsAuthValue),
+	)
+	third := &testEnv{
+		router: server.NewRouterForTest(
+			first.log.Get(), first.cfg, first.mw, first.hnd,
+		),
+	}
+
+	for _, env := range []*testEnv{first, second, third} {
+		env.get("/", nil)
+
+		scrape := env.metricsRequest(metricsUser, metricsAuthValue)
+		require.Equal(t, http.StatusOK, scrape.Code)
+
+		for _, series := range []string{
+			"http_request_duration_seconds",
+			"http_response_size_bytes",
+			"http_requests_inflight",
+			"webhooker_events_received_total",
+			"go_goroutines",
+			"process_start_time_seconds",
+			"promhttp_metric_handler_requests_total",
+		} {
+			assert.Contains(t, scrape.Body.String(), series)
+		}
+	}
+}

templates/base.html

@@ -7,7 +7,6 @@
 <body class="bg-gray-50 min-h-screen flex flex-col">
     <div class="flex-grow">
         {{template "navbar" .}}
-        {{template "notice" .}}
         {{block "content" .}}{{end}}
     </div>
     {{template "footer" .}}

templates/notice.html

@@ -1,7 +0,0 @@
-{{define "notice"}}
-{{with .Notice}}
-<div class="max-w-6xl mx-auto px-6 pt-4">
-    <div class="{{if .Failed}}alert-error{{else}}alert-success{{end}}">{{.Text}}</div>
-</div>
-{{end}}
-{{end}}

templates/source_logs.html

@@ -12,6 +12,14 @@
         </div>
     </div>
 
+    {{if .ReplayMessage}}
+    <div class="{{if .ReplayQueued}}alert-success{{else}}alert-error{{end}}">{{.ReplayMessage}}</div>
+    {{end}}
+
+    {{if .ResubmitMessage}}
+    <div class="{{if .ResubmitQueued}}alert-success{{else}}alert-error{{end}}">{{.ResubmitMessage}}</div>
+    {{end}}
+
     <div class="card">
         <div class="divide-y divide-gray-100">
             {{range .Events}}