Author SHA1 Message Date
clawbot 4a24b5f889 Serve /metrics from a registry of its own (closes #227)
check / check (push) Waiting to run
The HTTP metrics recorder, the delivery collectors and the Go and
process collectors now register on one prometheus.Registry that fx
provides, instead of Prometheus's global default registry, and
/metrics serves that registry. A second metrics-enabled router in one
process, or the server tests run with -count=2, no longer panics on a
duplicate registration.

The middleware builds its recorder once, in New, so installing
Metrics() on more than one router over the same graph is also safe;
NewForTest gives its Middleware a recorder on a fresh registry. The
scrape keeps the same series and labels, including go_*, process_*
and promhttp_metric_handler_*.

Model: opus-5-5
2026-10-01 21:00:29 +00:00
clawbot 9d29baaa2d Commit the Alpine.js tarball in 3p/ and extract it at build time (closes #345)
check / check (push) Waiting to run
The build no longer downloads Alpine.js. Its npm package tarball is committed as 3p/alpinejs-3.14.9.tgz, byte for byte the file script/fetch-assets downloaded, with the sha256 that script pinned. script/assets (make assets) extracts package/dist/cdn.min.js to the ignored static/js/alpine.min.js; script/test runs it, so make test, make check, the pre-commit hook and the Dockerfile get the file with no network access, and make build, run and dev run it too.

Removed: script/fetch-assets, its Dockerfile step, static/vendor.sha256 and static/vendor_test.go. The README describes the new flow.

Model: opus-5-5
2026-10-01 22:44:41 +02:00
30 changed files with 261 additions and 331 deletions
+2 -4
View File
@@ -3,10 +3,8 @@
# stage of the Dockerfile. # stage of the Dockerfile.
.git/ .git/
bin/ bin/
# Third-party browser assets are fetched and hash-verified inside the build by # Extracted from 3p/ by `make assets` inside the build; a host copy is not
# script/fetch-assets. Excluding any host copy keeps a developer's working tree # needed. The tarball in 3p/ must stay in the context.
# from supplying the bytes that get shipped. The script and its
# static/vendor.sha256 manifest stay in the context.
static/js/alpine.min.js static/js/alpine.min.js
*.md *.md
LICENSE LICENSE
+2 -3
View File
@@ -46,7 +46,6 @@ temp/
# CI cache barrier, written into the build context by the check workflow # CI cache barrier, written into the build context by the check workflow
.ci-fingerprint .ci-fingerprint
# Third-party browser assets, fetched and hash-verified by # Alpine.js, extracted by `make assets` from its tarball in 3p/, which is
# script/fetch-assets against static/vendor.sha256. Not committed: # what is committed.
# REPO_POLICIES.md forbids minified bundles in version control.
/static/js/alpine.min.js /static/js/alpine.min.js
Binary file not shown.
+4 -11
View File
@@ -51,15 +51,8 @@ RUN go mod download
# the lint stage above. # the lint stage above.
COPY . . COPY . .
# Fetch the third-party browser assets the UI serves. They are not committed # Run tests and build. Both first run script/assets, which extracts Alpine.js
# (REPO_POLICIES.md forbids minified bundles in version control) and # from its tarball in 3p/.
# .dockerignore keeps any host copy out of the build context, so this step is
# the only way they enter the image. Each download is checked against a
# hardcoded sha256 and the build fails on mismatch; make test re-checks the
# hashes against the bytes go:embed actually put in the binary.
RUN script/fetch-assets
# Run tests and build
RUN make test RUN make test
# Version stamped into the binary. .dockerignore excludes .git/, so # Version stamped into the binary. .dockerignore excludes .git/, so
@@ -67,8 +60,8 @@ RUN make test
# host and passes it in. The default is what a bare `docker build .` # host and passes it in. The default is what a bare `docker build .`
# with no --build-arg gets, and it names no tag the tree may not be at. # with no --build-arg gets, and it names no tag the tree may not be at.
# #
# Declared here, below the test and asset steps, so a changed version # Declared here, below the test step, so a changed version does not
# does not invalidate their cached layers. # invalidate its cached layer.
ARG VERSION=unknown ARG VERSION=unknown
RUN make build VERSION="$VERSION" RUN make build VERSION="$VERSION"
+3 -3
View File
@@ -28,7 +28,7 @@ setup:
@script/setup @script/setup
assets: assets:
@script/fetch-assets @script/assets
test: test:
@script/test @script/test
@@ -45,13 +45,13 @@ fmt-check:
check: check:
@script/check @script/check
build: build: assets
go build -ldflags '$(strip -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker go build -ldflags '$(strip -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
run: build run: build
./bin/webhooker ./bin/webhooker
dev: dev: assets
go run ./cmd/webhooker go run ./cmd/webhooker
deps: deps:
+40 -42
View File
@@ -21,9 +21,6 @@ before deploying one.
- Go 1.26.1+ (the version in `go.mod`) - Go 1.26.1+ (the version in `go.mod`)
- Docker (for linting, for the test stage of the CI gate, and for - Docker (for linting, for the test stage of the CI gate, and for
containerized deployment) containerized deployment)
- `curl`, used by `script/fetch-assets` to download the third-party
browser assets, which are not committed (`make bootstrap` installs
it if missing)
golangci-lint is not a prerequisite and must not be installed on the golangci-lint is not a prerequisite and must not be installed on the
host: `script/bootstrap` does not install it, and `make lint` runs the host: `script/bootstrap` does not install it, and `make lint` runs the
@@ -36,9 +33,7 @@ digest-pinned linter image via `Dockerfile.lint`.
git clone https://git.eeqj.de/sneak/webhooker.git git clone https://git.eeqj.de/sneak/webhooker.git
cd webhooker cd webhooker
# Install Go dependencies and the third-party browser assets. # Install the Go toolchain if missing, and the Go dependencies
# `make deps` alone is not enough: it only runs go mod download/tidy,
# and the checks below need the fetched assets.
make bootstrap make bootstrap
# Run all checks (test, lint, format check) # Run all checks (test, lint, format check)
@@ -58,7 +53,7 @@ make docker
```bash ```bash
make bootstrap # Install all dependencies (idempotent) make bootstrap # Install all dependencies (idempotent)
make setup # Bootstrap + install git pre-commit hook make setup # Bootstrap + install git pre-commit hook
make assets # Fetch + verify third-party browser assets make assets # Extract Alpine.js from 3p/ (test, check, build, dev run it)
make fmt # Format code (gofmt + goimports) make fmt # Format code (gofmt + goimports)
make fmt-check # Fail if gofmt would change anything (writes nothing) make fmt-check # Fail if gofmt would change anything (writes nothing)
make lint # Run golangci-lint in Docker (Dockerfile.lint) make lint # Run golangci-lint in Docker (Dockerfile.lint)
@@ -1221,14 +1216,15 @@ This repository adheres to the
standard: normalized scripts in `script/` are the entrypoints for the standard: normalized scripts in `script/` are the entrypoints for the
development workflow. Ten of the Makefile's seventeen targets are thin development workflow. Ten of the Makefile's seventeen targets are thin
shims that call them; `build`, `run`, `dev`, `deps`, `clean`, `css` and shims that call them; `build`, `run`, `dev`, `deps`, `clean`, `css` and
`version` are inline commands with no script behind them, though `version` are inline commands with no script behind them, though `build`,
`build` and `version` both take their value from `script/version`. `run` and `dev` first run `script/assets`, and `build` and `version` both
take their value from `script/version`.
`make check` needs the third-party browser assets in `static/`, which `script/test`, `make build` and `make dev` each run `script/assets`
are not committed, so run `make bootstrap` (or just `make assets`) once first, which writes the ignored `static/js/alpine.min.js` (see
after cloning. Without them the tests fail with a message naming that [Third-party browser assets](#third-party-browser-assets)), so
remedy. `make check` does not fetch them itself because it must not `make test`, `make check` and the pre-commit hook work on a fresh clone
change any files in the repo. without a separate step.
We provide: We provide:
@@ -1236,8 +1232,8 @@ We provide:
- `script/setup` — make a fresh clone ready for development - `script/setup` — make a fresh clone ready for development
(bootstrap, then install-precommit) (bootstrap, then install-precommit)
- `script/projectname` — output the project name ("webhooker") - `script/projectname` — output the project name ("webhooker")
- `script/fetch-assets` — download the third-party browser assets into - `script/assets` — extract Alpine.js from its tarball in `3p/` (see
`static/`, verifying each against its pinned sha256 [Third-party browser assets](#third-party-browser-assets))
- `script/test` — run the test suite - `script/test` — run the test suite
- `script/lint` — run golangci-lint in Docker (see Linting below) - `script/lint` — run golangci-lint in Docker (see Linting below)
- `script/fmt` — format all code (writes) - `script/fmt` — format all code (writes)
@@ -1259,24 +1255,25 @@ We provide:
## Third-party browser assets ## Third-party browser assets
The web UI serves one third-party script, Alpine.js. It is **not** committed: The web UI serves one third-party script, Alpine.js. Its npm package tarball
a minified bundle in the tree is unreviewable, and `REPO_POLICIES.md` bars is committed as `3p/alpinejs-3.14.9.tgz`, byte for byte as the npm registry
both committed build artifacts and unpinned external references. publishes it. It is a dependency, not this repo's build output, so
`REPO_POLICIES.md`'s rule against committed build artifacts does not apply.
The directory is `3p/` rather than `vendor/` because Go treats a root
`vendor/` directory as its module vendor directory.
Instead `script/fetch-assets` downloads it from a pinned URL, checks the `script/assets` (`make assets`) extracts the browser build,
download against a hardcoded sha256, and installs it under `static/`. The `package/dist/cdn.min.js`, from the tarball to `static/js/alpine.min.js`,
sha256 of every installed asset is recorded in `static/vendor.sha256`, and where `go:embed` picks it up. `script/test`, `make build` and `make dev` run
`static/vendor_test.go` re-hashes the bytes `go:embed` put in the binary it first, and the Dockerfile builds through `make test` and `make build`, so
against that manifest — so the pin is enforced on what actually ships, not nothing downloads Alpine.js. The extracted file is not committed, and
merely written down. Any mismatch fails the build. `.dockerignore` keeps any host copy out of the build context.
`make bootstrap` runs the fetch for local development, and the Dockerfile To move to a new version: download
runs it in the build stage; `.gitignore` and `.dockerignore` keep the `https://registry.npmjs.org/alpinejs/-/alpinejs-<version>.tgz`, check it
artifact out of both the repo and the build context. against the `dist.integrity` hash listed at
`https://registry.npmjs.org/alpinejs/<version>`, replace the tarball in `3p/`
To move to a new version: update the version, URL, and tarball sha256 in with it, update its file name in `script/assets`, and run `make check`.
`script/fetch-assets` and the asset sha256 in `static/vendor.sha256`, then
run `make assets && make check`.
## Rationale ## Rationale
@@ -2755,6 +2752,8 @@ imports. The entry point is `cmd/webhooker/main.go`.
``` ```
webhooker/ webhooker/
├── 3p/
│ └── alpinejs-3.14.9.tgz # Alpine.js npm package, extracted by make assets
├── cmd/webhooker/ ├── cmd/webhooker/
│ └── main.go # Entry point: subcommand dispatch; no args locks DATA_DIR and wires fx │ └── main.go # Entry point: subcommand dispatch; no args locks DATA_DIR and wires fx
├── internal/ ├── internal/
@@ -2848,8 +2847,7 @@ webhooker/
│ ├── css/tailwind.css # Generated stylesheet the pages load │ ├── css/tailwind.css # Generated stylesheet the pages load
│ ├── css/style.css # Older hand-written stylesheet, no longer loaded │ ├── css/style.css # Older hand-written stylesheet, no longer loaded
│ ├── js/app.js # Progressive-enhancement copy-to-clipboard │ ├── js/app.js # Progressive-enhancement copy-to-clipboard
│ ├── js/alpine.min.js # Alpine.js, fetched by script/fetch-assets, not committed │ └── js/alpine.min.js # Alpine.js, extracted from 3p/ by make assets, not committed
│ └── vendor.sha256 # Pinned hashes the fetched assets are verified against
├── templates/ # Go HTML templates (base, login, sources, etc.) ├── templates/ # Go HTML templates (base, login, sources, etc.)
├── script/ # Scripts to Rule Them All entrypoints ├── script/ # Scripts to Rule Them All entrypoints
├── Dockerfile # Three stages: lint, test+build, Alpine runtime ├── Dockerfile # Three stages: lint, test+build, Alpine runtime
@@ -3161,14 +3159,14 @@ version is fixed independently of the compiler's:
`make fmt-check`, then `golangci-lint config verify` and `make fmt-check`, then `golangci-lint config verify` and
`golangci-lint run`, both with `--network=none`. `golangci-lint run`, both with `--network=none`.
2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint 2. **Builder stage** (`golang:1.26.1-bookworm`) — depends on the lint
stage passing (it copies a file from it), runs `script/fetch-assets` stage passing (it copies a file from it), runs `make test` and
to download and verify the third-party browser assets, then runs `make build` (both extract Alpine.js from `3p/` first), and finally
`make test` and `make build`, and finally rebuilds the binary with rebuilds the binary with `CGO_ENABLED=1` and static linking so it
`CGO_ENABLED=1` and static linking so it runs on musl. Both builds runs on musl. Both builds go through `make build`, the relink adding
go through `make build`, the relink adding its `-extldflags` via its `-extldflags` via `GO_LDFLAGS`, so neither can drop the `-X` that
`GO_LDFLAGS`, so neither can drop the `-X` that stamps the version. stamps the version. The version arrives as the `VERSION` build arg,
The version arrives as the `VERSION` build arg, since the context since the context has no `.git` (see
has no `.git` (see [Version stamping](#version-stamping)). [Version stamping](#version-stamping)).
3. **Runtime stage** (`alpine:3.21`) — copies the static binary and 3. **Runtime stage** (`alpine:3.21`) — copies the static binary and
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` `deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker`
directory for all SQLite databases, exposes port 8080, and includes directory for all SQLite databases, exposes port 8080, and includes
+5
View File
@@ -16,6 +16,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/resetpw" "sneak.berlin/go/webhooker/internal/resetpw"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
@@ -177,6 +178,10 @@ func newApp() *fx.App {
healthcheck.New, healthcheck.New,
session.New, session.New,
handlers.New, handlers.New,
// The registry /metrics serves, and the delivery
// collectors registered on it.
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
// The one SSRF guard both target-creation validation // The one SSRF guard both target-creation validation
// and the delivery dialer consult, so they cannot // and the delivery dialer consult, so they cannot
+6 -5
View File
@@ -148,6 +148,7 @@ type EngineParams struct {
DBManager *database.WebhookDBManager DBManager *database.WebhookDBManager
Logger *logger.Logger Logger *logger.Logger
SSRFGuard *Guard SSRFGuard *Guard
Metrics *metrics.Set
} }
// Engine processes queued deliveries in the background // Engine processes queued deliveries in the background
@@ -167,10 +168,10 @@ type Engine struct {
retryCh chan Task retryCh chan Task
workers int workers int
// mtr is the delivery metric set. Production wires the // mtr is the delivery metric set. Production wires the one
// process-wide one; a test can substitute a set registered on // registered on the registry /metrics serves; a test can
// a private registry so its assertions are not disturbed by // substitute a set registered on a registry it holds, so it can
// deliveries other tests are making at the same time. // gather what its own deliveries recorded.
mtr *metrics.Set mtr *metrics.Set
// targets maps each target type to its implementation. // targets maps each target type to its implementation.
@@ -204,7 +205,7 @@ func New(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: defaultWorkers, workers: defaultWorkers,
mtr: metrics.Default(), mtr: params.Metrics,
} }
e.initTargets(&http.Client{ e.initTargets(&http.Client{
+5 -5
View File
@@ -9,6 +9,7 @@ import (
"net/url" "net/url"
"time" "time"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx" "go.uber.org/fx"
"gorm.io/gorm" "gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
@@ -389,7 +390,7 @@ func NewTestEngine(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: workers, workers: workers,
mtr: metrics.Default(), mtr: metrics.New(prometheus.NewRegistry()),
} }
e.initTargets(client) e.initTargets(client)
@@ -404,7 +405,7 @@ func NewTestEngineSmallRetry(
e := &Engine{ e := &Engine{
log: log, log: log,
retryCh: make(chan Task, 1), retryCh: make(chan Task, 1),
mtr: metrics.Default(), mtr: metrics.New(prometheus.NewRegistry()),
} }
e.initTargets(nil) e.initTargets(nil)
@@ -427,7 +428,7 @@ func NewTestEngineWithDB(
deliveryCh: make(chan Task, deliveryChannelSize), deliveryCh: make(chan Task, deliveryChannelSize),
retryCh: make(chan Task, retryChannelSize), retryCh: make(chan Task, retryChannelSize),
workers: workers, workers: workers,
mtr: metrics.Default(), mtr: metrics.New(prometheus.NewRegistry()),
} }
e.initTargets(client) e.initTargets(client)
@@ -435,8 +436,7 @@ func NewTestEngineWithDB(
} }
// ExportSetMetrics substitutes the engine's metric set, so a test can // ExportSetMetrics substitutes the engine's metric set, so a test can
// assert on collectors registered on a private registry instead of // assert on collectors registered on a registry it holds.
// the process-wide ones every other test is also moving.
func (e *Engine) ExportSetMetrics(mtr *metrics.Set) { func (e *Engine) ExportSetMetrics(mtr *metrics.Set) {
e.mtr = mtr e.mtr = mtr
} }
+2 -3
View File
@@ -35,9 +35,8 @@ const (
) )
// mIsolate gives the setup's engine a metric set registered on a // mIsolate gives the setup's engine a metric set registered on a
// private registry. The process-wide collectors are moved by every // registry this test holds, so its exact assertions can gather from
// other delivery test running in parallel, so exact assertions are // it.
// only possible against a registry this test owns.
func mIsolate( func mIsolate(
t *testing.T, s iSetup, t *testing.T, s iSetup,
) *prometheus.Registry { ) *prometheus.Registry {
+4 -1
View File
@@ -12,6 +12,7 @@ import (
"net/http" "net/http"
"sync/atomic" "sync/atomic"
"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx" "go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
@@ -61,6 +62,8 @@ type HandlersParams struct {
Notifier delivery.Notifier Notifier delivery.Notifier
Evictor delivery.WebhookEvictor Evictor delivery.WebhookEvictor
SSRFGuard *delivery.Guard SSRFGuard *delivery.Guard
Metrics *metrics.Set
Registry *prometheus.Registry
} }
// Handlers provides HTTP handler methods for all application // Handlers provides HTTP handler methods for all application
@@ -122,7 +125,7 @@ func New(
s.mw = params.Middleware s.mw = params.Middleware
s.notifier = params.Notifier s.notifier = params.Notifier
s.evictor = params.Evictor s.evictor = params.Evictor
s.mtr = metrics.Default() s.mtr = params.Metrics
s.ssrf = params.SSRFGuard s.ssrf = params.SSRFGuard
// Parse all page templates once at startup // Parse all page templates once at startup
+3
View File
@@ -20,6 +20,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
@@ -109,6 +110,8 @@ func newTestApp(
func(r *recordingEvictor) delivery.WebhookEvictor { func(r *recordingEvictor) delivery.WebhookEvictor {
return r return r
}, },
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
delivery.NewGuard, delivery.NewGuard,
handlers.New, handlers.New,
+21
View File
@@ -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
}
+27 -20
View File
@@ -3,17 +3,18 @@
// deliveries are attempted, how they end, how long they take, how // deliveries are attempted, how they end, how long they take, how
// deep the queues are, and how many circuit breakers are open. // deep the queues are, and how many circuit breakers are open.
// //
// The inbound HTTP metrics come from the go-http-metrics recorder in // It also builds the registry the authenticated /metrics route
// internal/middleware and land on prometheus.DefaultRegisterer. These // serves. In production, these collectors, the inbound HTTP metrics
// collectors register there too, so both surfaces are gathered by the // recorded in internal/middleware, and the Go runtime and process
// one promhttp handler mounted on the authenticated /metrics route. // collectors all register on that one registry, never on Prometheus's
// global default.
package metrics package metrics
import ( import (
"sync"
"time" "time"
"github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/collectors"
"github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/client_golang/prometheus/promauto"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
) )
@@ -57,25 +58,31 @@ var knownTargetTypes = []database.TargetType{
database.TargetTypeSlack, database.TargetTypeSlack,
} }
// defaultSet is the process-wide metric set, registered on the same // NewRegistry returns the registry /metrics serves, carrying the Go
// registry the HTTP middleware and the /metrics handler already use. // runtime and process collectors that Prometheus's global default
// It is built on first use rather than in an init so that a test // registry carries, so the go_* and process_* series stay in the
// binary that never touches metrics never registers them. // scrape.
// //
//nolint:gochecknoglobals // one process-wide registration, by design // A registry of its own, rather than the global default, is what lets
var defaultSet = sync.OnceValue(func() *Set { // two dependency graphs in one process — two tests, say — each
return New(prometheus.DefaultRegisterer) // 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. return reg
func Default() *Set {
return defaultSet()
} }
// Set is one registered group of webhooker's delivery collectors. // Set is one registered group of webhooker's delivery collectors.
// Production uses the single Default set; tests build their own // Production builds one on the registry /metrics serves; tests build
// against a private registry so assertions are not disturbed by // their own against a private registry so assertions are not
// deliveries other tests are making concurrently. // disturbed by deliveries other tests are making concurrently.
type Set struct { type Set struct {
eventsReceived prometheus.Counter eventsReceived prometheus.Counter
deliveryAttempts *prometheus.CounterVec deliveryAttempts *prometheus.CounterVec
@@ -93,7 +100,7 @@ type Set struct {
// New registers a full set of delivery collectors on reg and returns // New registers a full set of delivery collectors on reg and returns
// it. It panics if reg already holds them, which is the intended // it. It panics if reg already holds them, which is the intended
// behaviour for a duplicate registration. // behaviour for a duplicate registration.
func New(reg prometheus.Registerer) *Set { func New(reg *prometheus.Registry) *Set {
factory := promauto.With(reg) factory := promauto.With(reg)
s := &Set{ s := &Set{
+1 -2
View File
@@ -10,8 +10,7 @@ import (
// MetricsMiddlewareForTest builds the metrics recording middleware // MetricsMiddlewareForTest builds the metrics recording middleware
// against a caller-supplied recorder, so a test can gather from its // against a caller-supplied recorder, so a test can gather from its
// own Prometheus registry rather than the process-wide default one // own Prometheus registry without building a whole Middleware.
// that Middleware.Metrics uses.
func MetricsMiddlewareForTest( func MetricsMiddlewareForTest(
rec httpmetrics.Recorder, rec httpmetrics.Recorder,
) func(http.Handler) http.Handler { ) func(http.Handler) http.Handler {
+7 -8
View File
@@ -7,7 +7,6 @@ import (
"github.com/go-chi/chi" "github.com/go-chi/chi"
httpmetrics "github.com/slok/go-http-metrics/metrics" 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" ghmm "github.com/slok/go-http-metrics/middleware"
"github.com/slok/go-http-metrics/middleware/std" "github.com/slok/go-http-metrics/middleware/std"
) )
@@ -151,17 +150,17 @@ func (r boundedLabelRecorder) AddInflightRequests(
var _ httpmetrics.Recorder = boundedLabelRecorder{} var _ httpmetrics.Recorder = boundedLabelRecorder{}
// Metrics returns middleware that records Prometheus HTTP metrics on // Metrics returns middleware that records Prometheus HTTP metrics
// the default registry, which is the one the /metrics route gathers. // 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 { func (s *Middleware) Metrics() func(http.Handler) http.Handler {
return metricsMiddleware( return metricsMiddleware(s.metricsRecorder)
prommetrics.NewRecorder(prommetrics.Config{}),
)
} }
// metricsMiddleware builds the recording middleware against a given // metricsMiddleware builds the recording middleware against a given
// recorder, so tests can gather from a registry of their own instead // recorder, so tests can gather from a registry of their own.
// of the process-wide default.
func metricsMiddleware( func metricsMiddleware(
rec httpmetrics.Recorder, rec httpmetrics.Recorder,
) func(http.Handler) http.Handler { ) func(http.Handler) http.Handler {
+28 -3
View File
@@ -57,9 +57,8 @@ const (
// Server.setupWebhookRoutes inside it. That ordering is the whole // Server.setupWebhookRoutes inside it. That ordering is the whole
// defect, so a test that flattens it would prove nothing. // defect, so a test that flattens it would prove nothing.
// //
// The recorder writes to a registry of the test's own rather than the // The recorder writes to a registry of the test's own, so each test
// process-wide default one, so each test observes only its own // observes only its own traffic.
// traffic.
func metricsTestRouter( func metricsTestRouter(
t *testing.T, t *testing.T,
receiverLimit int, receiverLimit int,
@@ -455,3 +454,29 @@ func TestMetrics_StatusAndSizeStillRecorded(t *testing.T) {
"the interceptor must still count written bytes", "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)
}
}
+19 -4
View File
@@ -13,6 +13,9 @@ import (
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/go-chi/chi/middleware" "github.com/go-chi/chi/middleware"
"github.com/go-chi/cors" "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" "go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/config" "sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/globals" "sneak.berlin/go/webhooker/internal/globals"
@@ -148,10 +151,11 @@ const (
type MiddlewareParams struct { type MiddlewareParams struct {
fx.In fx.In
Logger *logger.Logger Logger *logger.Logger
Globals *globals.Globals Globals *globals.Globals
Config *config.Config Config *config.Config
Session *session.Session Session *session.Session
Registry *prometheus.Registry
} }
// Middleware provides HTTP middleware for logging, CORS, auth, and // Middleware provides HTTP middleware for logging, CORS, auth, and
@@ -161,6 +165,14 @@ type Middleware struct {
params *MiddlewareParams params *MiddlewareParams
session *session.Session 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 // loginGuard counts failed credential verifications and bounds
// concurrent password hashing. It is built on first use so that // concurrent password hashing. It is built on first use so that
// every construction path gets one; see guard(). // every construction path gets one; see guard().
@@ -179,6 +191,9 @@ func New(
s.params = &params s.params = &params
s.log = params.Logger.Get() s.log = params.Logger.Get()
s.session = params.Session s.session = params.Session
s.metricsRecorder = prommetrics.NewRecorder(
prommetrics.Config{Registry: params.Registry},
)
return s, nil return s, nil
} }
+8
View File
@@ -3,12 +3,17 @@ package middleware
import ( import (
"log/slog" "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/config"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
// NewForTest creates a Middleware with the minimum dependencies // NewForTest creates a Middleware with the minimum dependencies
// needed for testing. This bypasses the fx lifecycle. // 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( func NewForTest(
log *slog.Logger, log *slog.Logger,
cfg *config.Config, cfg *config.Config,
@@ -20,5 +25,8 @@ func NewForTest(
Config: cfg, Config: cfg,
}, },
session: sess, session: sess,
metricsRecorder: prommetrics.NewRecorder(
prommetrics.Config{Registry: prometheus.NewRegistry()},
),
} }
} }
+3
View File
@@ -24,6 +24,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/resetpw" "sneak.berlin/go/webhooker/internal/resetpw"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
@@ -163,6 +164,8 @@ func newServerApp(
session.New, session.New,
func() delivery.Notifier { return &noopNotifier{} }, func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} }, func() delivery.WebhookEvictor { return &noopEvictor{} },
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
delivery.NewGuard, delivery.NewGuard,
handlers.New, handlers.New,
+1 -7
View File
@@ -7,7 +7,6 @@ import (
sentryhttp "github.com/getsentry/sentry-go/http" sentryhttp "github.com/getsentry/sentry-go/http"
"github.com/go-chi/chi" "github.com/go-chi/chi"
"github.com/go-chi/chi/middleware" "github.com/go-chi/chi/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/webhooker/static" "sneak.berlin/go/webhooker/static"
) )
@@ -130,12 +129,7 @@ func (s *Server) setupRoutes() {
if s.params.Config.MetricsAuthEnabled() { if s.params.Config.MetricsAuthEnabled() {
s.router.Group(func(r chi.Router) { s.router.Group(func(r chi.Router) {
r.Use(s.mw.MetricsAuth()) r.Use(s.mw.MetricsAuth())
r.Get( r.Get("/metrics", s.h.HandleMetrics())
"/metrics",
http.HandlerFunc(
promhttp.Handler().ServeHTTP,
),
)
}) })
} }
+46
View File
@@ -24,6 +24,7 @@ import (
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/healthcheck" "sneak.berlin/go/webhooker/internal/healthcheck"
"sneak.berlin/go/webhooker/internal/logger" "sneak.berlin/go/webhooker/internal/logger"
"sneak.berlin/go/webhooker/internal/metrics"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
@@ -113,6 +114,8 @@ func newTestEnvWithConfig(
session.New, session.New,
func() delivery.Notifier { return &noopNotifier{} }, func() delivery.Notifier { return &noopNotifier{} },
func() delivery.WebhookEvictor { return &noopEvictor{} }, func() delivery.WebhookEvictor { return &noopEvictor{} },
metrics.NewRegistry,
metrics.New,
middleware.New, middleware.New,
delivery.NewGuard, delivery.NewGuard,
handlers.New, handlers.New,
@@ -1027,3 +1030,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)
}
}
}
+3 -3
View File
@@ -13,9 +13,9 @@ import (
// TestBaseTemplateScriptsAreServed walks every /s/ script the base // TestBaseTemplateScriptsAreServed walks every /s/ script the base
// template loads on each page and fetches it through the real router. // template loads on each page and fetches it through the real router.
// Alpine.js is fetched at build time rather than committed, so nothing // Alpine.js is extracted from its tarball in 3p/ at build time, so the
// in the repo guarantees it is present: this is the check that the page // file is not in the tree: this is the check that the page still gets
// still gets the JavaScript it asks for. // the JavaScript it asks for.
func TestBaseTemplateScriptsAreServed(t *testing.T) { func TestBaseTemplateScriptsAreServed(t *testing.T) {
t.Parallel() t.Parallel()
Executable
+16
View File
@@ -0,0 +1,16 @@
#!/bin/sh
# script/assets: extract Alpine.js from its npm package tarball, committed
# in 3p/, to static/js/alpine.min.js, where go:embed reads it. The
# extracted file is not committed. script/test, make build and make dev run
# this first.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() {
cd "$ROOT"
tar -xzOf 3p/alpinejs-3.14.9.tgz package/dist/cdn.min.js \
>static/js/alpine.min.js
}
main "$@"
+1 -8
View File
@@ -4,9 +4,7 @@
# installed tools are skipped. Base tooling comes from nix, apt, brew, # installed tools are skipped. Base tooling comes from nix, apt, brew,
# or apk (detected in that order); assumes NOTHING is present (not git, # or apk (detected in that order); assumes NOTHING is present (not git,
# make, or go). golangci-lint is deliberately not installed: linting runs # make, or go). golangci-lint is deliberately not installed: linting runs
# only in docker, via script/lint and Dockerfile.lint. Finishes by running # only in docker, via script/lint and Dockerfile.lint.
# script/fetch-assets, which installs the hash-pinned third-party browser
# assets the repo does not commit.
set -eu set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)" ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
@@ -69,11 +67,6 @@ main() {
go mod download go mod download
# Third-party browser assets are not committed; fetch and verify them
# so a fresh clone can build and test.
if missing curl; then pkg_install curl curl curl curl; fi
"$ROOT/script/fetch-assets"
echo "bootstrap complete" echo "bootstrap complete"
} }
+2 -1
View File
@@ -1,6 +1,7 @@
#!/bin/sh #!/bin/sh
# script/check: run all checks (test, lint, fmt-check). Our own # script/check: run all checks (test, lint, fmt-check). Our own
# extension to scripts-to-rule-them-all. Must not modify any files. # extension to scripts-to-rule-them-all.
# Writes only the ignored static/js/alpine.min.js, through script/test.
# Generic: usually needs no adaptation. # Generic: usually needs no adaptation.
set -eu set -eu
-104
View File
@@ -1,104 +0,0 @@
#!/bin/sh
# script/fetch-assets: download the third-party browser assets the web UI
# ships and install them under static/. Minified bundles are not committed
# (REPO_POLICIES.md: no build artifacts in version control), so the build
# fetches them here. Every download is verified against a hardcoded sha256
# before it is installed, and any mismatch aborts. Idempotent: an asset
# already present with its pinned hash is left alone.
set -eu
ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
# The sha256 of each installed asset lives in static/vendor.sha256, in
# sha256sum(1) format, with paths relative to static/. That file is the
# single source of truth: this script verifies against it, and
# static/vendor_test.go asserts the bytes embedded into the binary match
# it, so the hash cannot rot into a value nothing checks.
MANIFEST="static/vendor.sha256"
# Alpine.js 3.14.9, 2026-08-17. Fetched from registry.npmjs.org, the
# publisher of record; the jsDelivr and unpkg copies are mirrors of this
# same tarball. dist/cdn.min.js is the browser build Alpine publishes for
# a <script> tag.
ALPINE_VERSION="3.14.9"
ALPINE_URL="https://registry.npmjs.org/alpinejs/-/alpinejs-${ALPINE_VERSION}.tgz"
# sha256 of alpinejs-3.14.9.tgz
ALPINE_TARBALL_SHA256="97dad7c0c81e659cfc8e7700055da9770f8186187cb9a8a76efb57e00d5ce52a"
ALPINE_MEMBER="package/dist/cdn.min.js"
ALPINE_DEST="js/alpine.min.js"
sha256_of() {
if command -v sha256sum >/dev/null 2>&1; then
sha256sum "$1" | cut -d' ' -f1
else
shasum -a 256 "$1" | cut -d' ' -f1
fi
}
# expected_sha256 <path-relative-to-static>
expected_sha256() {
awk -v want="$1" '$2 == want { print $1; found = 1 }
END { if (!found) exit 1 }' "$ROOT/$MANIFEST"
}
# verify <file> <expected-sha256> <what>
verify() {
actual="$(sha256_of "$1")"
if [ "$actual" != "$2" ]; then
echo "fetch-assets: sha256 mismatch for $3" >&2
echo " expected: $2" >&2
echo " actual: $actual" >&2
exit 1
fi
}
# up_to_date <path-relative-to-static> <expected-sha256>
up_to_date() {
[ -f "$ROOT/static/$1" ] || return 1
[ "$(sha256_of "$ROOT/static/$1")" = "$2" ]
}
fetch_alpine() {
want="$(expected_sha256 "$ALPINE_DEST")"
if up_to_date "$ALPINE_DEST" "$want"; then
echo "fetch-assets: static/$ALPINE_DEST already at $want"
return 0
fi
echo "fetch-assets: fetching Alpine.js $ALPINE_VERSION from $ALPINE_URL"
tmp="$(mktemp -d)"
trap 'rm -rf "$tmp"' EXIT INT TERM
curl -fsSL -o "$tmp/alpine.tgz" "$ALPINE_URL"
verify "$tmp/alpine.tgz" "$ALPINE_TARBALL_SHA256" "alpinejs-${ALPINE_VERSION}.tgz"
tar -xzOf "$tmp/alpine.tgz" "$ALPINE_MEMBER" >"$tmp/alpine.min.js"
verify "$tmp/alpine.min.js" "$want" "$ALPINE_MEMBER from alpinejs-${ALPINE_VERSION}.tgz"
mkdir -p "$(dirname "$ROOT/static/$ALPINE_DEST")"
cp "$tmp/alpine.min.js" "$ROOT/static/$ALPINE_DEST"
rm -rf "$tmp"
trap - EXIT INT TERM
echo "fetch-assets: installed static/$ALPINE_DEST ($want)"
}
# Re-check every manifest entry against what is now on disk, so an entry
# no script installs fails loudly instead of passing silently.
verify_manifest() {
while read -r want path; do
case "$want" in '' | '#'*) continue ;; esac
if [ ! -f "$ROOT/static/$path" ]; then
echo "fetch-assets: $MANIFEST lists static/$path, which is missing" >&2
exit 1
fi
verify "$ROOT/static/$path" "$want" "static/$path"
done <"$ROOT/$MANIFEST"
}
main() {
cd "$ROOT"
fetch_alpine
verify_manifest
echo "fetch-assets: all assets in $MANIFEST verified"
}
main "$@"
+1
View File
@@ -28,6 +28,7 @@ ROOT="$(cd "$(dirname "$0")/.." && pwd -P)"
main() { main() {
cd "$ROOT" cd "$ROOT"
"$ROOT/script/assets"
go test -v -race -timeout 90s ./... go test -v -race -timeout 90s ./...
} }
-1
View File
@@ -1 +0,0 @@
3ed1eed252488921df65e363d6715deb04d7f92aaedb9e52199fdf73cb1e0ad3 js/alpine.min.js
-92
View File
@@ -1,92 +0,0 @@
package static_test
import (
"bufio"
"crypto/sha256"
"encoding/hex"
"os"
"strings"
"testing"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/static"
)
const manifestPath = "vendor.sha256"
// fetchHint is appended to every failure here: the assets the manifest
// covers are fetched by the build, not committed, so a fresh clone that
// has not run script/fetch-assets fails this test and should be told why.
const fetchHint = "run `script/fetch-assets` (or `make assets`) to install " +
"the pinned third-party assets"
// TestVendoredAssetsMatchManifest asserts that every asset listed in
// static/vendor.sha256 is embedded in the binary with exactly the pinned
// bytes. script/fetch-assets verifies the same hashes at download time;
// this test verifies them again on what actually ships, so a build that
// skipped, cached, or subverted the fetch cannot produce a binary serving
// unpinned third-party JavaScript.
func TestVendoredAssetsMatchManifest(t *testing.T) {
t.Parallel()
entries := readManifest(t)
require.NotEmpty(t, entries, "%s lists no assets", manifestPath)
for path, want := range entries {
t.Run(path, func(t *testing.T) {
t.Parallel()
data, err := static.Static.ReadFile(path)
require.NoErrorf(
t, err,
"%s is listed in %s but is not embedded; %s",
path, manifestPath, fetchHint,
)
sum := sha256.Sum256(data)
got := hex.EncodeToString(sum[:])
require.Equalf(
t, want, got,
"embedded %s does not match its pinned sha256 in %s; %s",
path, manifestPath, fetchHint,
)
})
}
}
// readManifest parses static/vendor.sha256, which is in sha256sum(1)
// format with paths relative to static/.
func readManifest(t *testing.T) map[string]string {
t.Helper()
f, err := os.Open(manifestPath)
require.NoError(t, err, "opening %s", manifestPath)
defer func() { require.NoError(t, f.Close()) }()
entries := make(map[string]string)
scanner := bufio.NewScanner(f)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
if line == "" || strings.HasPrefix(line, "#") {
continue
}
fields := strings.Fields(line)
require.Lenf(
t, fields, 2,
"%s: malformed entry %q, want \"<sha256> <path>\"",
manifestPath, line,
)
sum, path := fields[0], fields[1]
require.Lenf(t, sum, 64, "%s: %q is not a sha256", manifestPath, sum)
entries[path] = sum
}
require.NoError(t, scanner.Err(), "reading %s", manifestPath)
return entries
}