Author SHA1 Message Date
clawbot ef8e8cf398 Serve /metrics from a registry of its own (closes #227)
check / check (push) Successful in 4m2s
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.
The scrape keeps the same series and labels, including go_*,
process_* and promhttp_metric_handler_*.

Model: opus-5-5
2026-09-29 09:20:29 +00:00
18 changed files with 221 additions and 131 deletions
+2 -9
View File
@@ -88,9 +88,7 @@ RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static
# alpine:3.21, 2026-03-17 # alpine:3.21, 2026-03-17
FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709 FROM alpine:3.21@sha256:c3f8e73fdb79deaebaa2037150150191b9dcbfba68b4a46d70103204c53f4709
# su-exec 0.2-r3 (Alpine 3.21), 2026-09-29: the entrypoint runs the app RUN apk --no-cache add ca-certificates
# as webhooker with it.
RUN apk --no-cache add ca-certificates su-exec=0.2-r3
# Create non-root user # Create non-root user
RUN addgroup -g 1000 -S webhooker && \ RUN addgroup -g 1000 -S webhooker && \
@@ -101,17 +99,13 @@ WORKDIR /app
# Copy binary from builder # Copy binary from builder
COPY --from=builder /build/bin/webhooker /app/webhooker COPY --from=builder /build/bin/webhooker /app/webhooker
# Not under /app, which belongs to webhooker: this script runs as root.
COPY deploy/docker-entrypoint.sh /usr/local/bin/docker-entrypoint.sh
# Create data directory for all SQLite databases (main app DB + # Create data directory for all SQLite databases (main app DB +
# per-webhook event DBs). DATA_DIR defaults to /var/lib/webhooker. # per-webhook event DBs). DATA_DIR defaults to /var/lib/webhooker.
RUN mkdir -p /var/lib/webhooker RUN mkdir -p /var/lib/webhooker
RUN chown -R webhooker:webhooker /app /var/lib/webhooker RUN chown -R webhooker:webhooker /app /var/lib/webhooker
# No USER: the entrypoint starts as root to make the data directory USER webhooker
# webhooker's, then runs the app as webhooker.
EXPOSE 8080 EXPOSE 8080
@@ -130,5 +124,4 @@ ENV BIND_ADDRESS=0.0.0.0
HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \
CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1 CMD wget --no-verbose --tries=1 --spider http://localhost:8080/.well-known/healthcheck || exit 1
ENTRYPOINT ["/usr/local/bin/docker-entrypoint.sh"]
CMD ["/app/webhooker"] CMD ["/app/webhooker"]
+77 -43
View File
@@ -552,9 +552,8 @@ printf '%s' "$NEW_PASSWORD" | \
DATA_DIR=/var/lib/webhooker webhooker resetpw admin DATA_DIR=/var/lib/webhooker webhooker resetpw admin
``` ```
In a container it is the same binary. The image's `CMD` is In a container it is the same binary, which the image sets as `CMD`
`/app/webhooker`, and a command given to `docker run` replaces all of rather than `ENTRYPOINT`, so the whole command has to be given:
it, so the whole command has to be given:
```bash ```bash
docker run --rm -v webhooker-data:/var/lib/webhooker \ docker run --rm -v webhooker-data:/var/lib/webhooker \
@@ -691,22 +690,38 @@ those three values rather than trusting the figure. Measured at 65s on
Docker 29.7.2.) A container `unhealthy` with `connection refused` in Docker 29.7.2.) A container `unhealthy` with `connection refused` in
its health log, or a published port that resets connections, is this. its health log, or a published port that resets connections, is this.
The app runs as a non-root user (`webhooker`, UID 1000), exposes port The container runs as a non-root user (`webhooker`, UID 1000), exposes
8080, and includes a health check against `/.well-known/healthcheck`. port 8080, and includes a health check against
The `/var/lib/webhooker` volume holds all SQLite databases: the main `/.well-known/healthcheck`. The `/var/lib/webhooker` volume holds all
application database (`webhooker.db`), the per-webhook event databases SQLite databases: the main application database (`webhooker.db`), the
(`events-{uuid}.db`), and any archive databases written by `database` per-webhook event databases (`events-{uuid}.db`), and any archive
targets (`archive-{uuid}.db`). Mount this as a persistent volume to databases written by `database` targets (`archive-{uuid}.db`). Mount
preserve data across container restarts. this as a persistent volume to preserve data across container
restarts.
**The container sets its data directory's owner and mode itself **The bind-mounted directory must be owned by UID 1000, or the
before the app starts**, so a host directory can be mounted as it is, container does not start.** Docker creates a `-v` source path that
whoever owns it. The image's `ENTRYPOINT`, does not exist yet as `root:root`, and the process runs as UID 1000,
`deploy/docker-entrypoint.sh`, starts as root, creates `DATA_DIR` if so it cannot take its `DATA_DIR` lock:
it is missing, gives the directory and anything in it that belongs to
another user to `webhooker`, sets the directory to `0750`, and only ```
then runs the app as `webhooker`. Started with `--user`, it changes webhooker: locking data directory /var/lib/webhooker: open
nothing and runs the app as that user. /var/lib/webhooker/webhooker.lock: permission denied
```
It exits non-zero at that point, before opening any database. Create
the directory ahead of the first `docker run`:
```bash
mkdir -p /path/to/data
chown 1000:1000 /path/to/data
chmod 750 /path/to/data
```
The same `chown` is what a restore needs — see step 4 of
[Restore](#restore). A **named volume** does not have this problem:
Docker copies the image's ownership onto a volume it initializes, and
the image creates `/var/lib/webhooker` owned by `webhooker`.
**The file modes are not yours to set, and do not depend on the **The file modes are not yours to set, and do not depend on the
directory.** `webhooker.db` holds target configuration in plaintext — directory.** `webhooker.db` holds target configuration in plaintext —
@@ -714,10 +729,13 @@ bearer tokens, API keys, Slack webhook URLs — along with the session
encryption key, so webhooker creates every SQLite file it owns `0600`: encryption key, so webhooker creates every SQLite file it owns `0600`:
each database and both of its `-wal` and `-shm` sidecars, across all each database and both of its `-wal` and `-shm` sidecars, across all
three tiers. Files an earlier build left `0644` are tightened when three tiers. Files an earlier build left `0644` are tightened when
they are opened. The directory's `0750` is defence in depth — it stops they are opened. A `DATA_DIR` webhooker creates itself is `0750`, but
other local users listing the directory and learning your webhook a bind mount supplies its own directory and Docker's default for one
UUIDs from the `events-{uuid}.db` filenames — not the barrier it creates is `0755`; the `0600` files hold there regardless. The
protecting the credentials. `chmod 750` above is defence in depth — it stops other local users
listing the directory and learning your webhook UUIDs from the
`events-{uuid}.db` filenames — not the barrier protecting the
credentials.
### Running under upaas ### Running under upaas
@@ -733,6 +751,17 @@ repository's `Dockerfile` and runs it. The app needs:
app name, port `8080`. Leave `PORT` unset: the image's health check app name, port `8080`. Leave `PORT` unset: the image's health check
probes `8080`. probes `8080`.
- **Volume:** one host directory mounted at `/var/lib/webhooker`. - **Volume:** one host directory mounted at `/var/lib/webhooker`.
upaas bind-mounts the host path it is given and does not create it,
and the container does not start unless UID 1000 owns it (see
[Running with Docker](#running-with-docker)). Create it before the
first deploy:
```bash
mkdir -p /path/to/data
chown 1000:1000 /path/to/data
chmod 750 /path/to/data
```
- **Environment variables:** - **Environment variables:**
- `WEBHOOKER_ENVIRONMENT=prod` - `WEBHOOKER_ENVIRONMENT=prod`
- `TRUSTED_PROXIES`: your reverse proxy's address on that Docker - `TRUSTED_PROXIES`: your reverse proxy's address on that Docker
@@ -991,12 +1020,12 @@ done
`.backup` reads through the WAL and writes a single consistent file with `.backup` reads through the WAL and writes a single consistent file with
no sidecars of its own, so the destination is complete as it stands. no sidecars of its own, so the destination is complete as it stands.
Two caveats. First, the runtime image is `alpine:3.21` with only Two caveats. First, the runtime image is `alpine:3.21` with only
`ca-certificates` and `su-exec` added — the `sqlite3` CLI is **not** in `ca-certificates` added — the `sqlite3` CLI is **not** in it, so run
it, so run this on the host against the volume path, or from a this on the host against the volume path, or from a throwaway container
throwaway container that mounts the volume. Second, each file is that mounts the volume. Second, each file is captured at its own
captured at its own instant, so a webhook created or an event delivered instant, so a webhook created or an event delivered between two files
between two files being copied lands in one and not the other. If you being copied lands in one and not the other. If you need the whole set
need the whole set coherent as of a single moment, stop the service. coherent as of a single moment, stop the service.
Note that `sqlite3 <db> .dump` is **not** one of these procedures: it is Note that `sqlite3 <db> .dump` is **not** one of these procedures: it is
an export, it holds a read transaction open for as long as it runs, and an export, it holds a read transaction open for as long as it runs, and
@@ -1050,11 +1079,21 @@ with any `-wal`/`-shm` beside it, or wait until there are none.
archive not opened since a crash. A copy salvaged from a crashed archive not opened since a crash. A copy salvaged from a crashed
instance has them for everything, and needs all of them. instance has them for everything, and needs all of them.
4. Start the service. The container gives the directory and the 4. **Fix ownership.** The container runs as the non-root `webhooker`
restored files to the `webhooker` user before the app starts, user, UID 1000 / GID 1000. Restored files must be owned by (or
whoever restored them (see writable by) that UID, and so must the directory itself — SQLite
[Running with Docker](#running-with-docker)). `AutoMigrate` runs creates the `-wal` and `-shm` sidecars beside the database, so a
against each restored database as it is opened. writable file inside a directory it cannot write is not enough:
```bash
chown -R 1000:1000 /path/to/data
```
Restoring as `root` on the host and forgetting this step is the
usual way a restore fails.
5. Start the service. `AutoMigrate` runs against each restored database
as it is opened.
### Upgrades ### Upgrades
@@ -3032,9 +3071,7 @@ check, see [The login endpoint](#the-login-endpoint).
- Prometheus metrics behind basic auth - Prometheus metrics behind basic auth
- Static assets embedded in binary (no filesystem access needed at - Static assets embedded in binary (no filesystem access needed at
runtime) runtime)
- The app runs as a non-root user (UID 1000) in the container; only - Container runs as non-root user (UID 1000)
the `ENTRYPOINT` script that sets the data directory's owner runs as
root, before the app starts
- GORM soft deletes on every entity that carries `BaseModel`, which is - GORM soft deletes on every entity that carries `BaseModel`, which is
all of them but `Setting` (data preserved for audit) all of them but `Setting` (data preserved for audit)
@@ -3161,13 +3198,10 @@ version is fixed independently of the compiler's:
`GO_LDFLAGS`, so neither can drop the `-X` that stamps the version. `GO_LDFLAGS`, so neither can drop the `-X` that stamps the version.
The version arrives as the `VERSION` build arg, since the context The version arrives as the `VERSION` build arg, since the context
has no `.git` (see [Version stamping](#version-stamping)). has no `.git` (see [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,
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` creates the `/var/lib/webhooker` directory for all SQLite databases,
directory for all SQLite databases, exposes port 8080, and includes runs as the non-root `webhooker` user (UID 1000), exposes port 8080,
a health check against `/.well-known/healthcheck`. It sets no and includes a health check against `/.well-known/healthcheck`.
`USER`: the `ENTRYPOINT` script starts as root, sets the data
directory's owner and mode, and runs the app as the non-root
`webhooker` user (UID 1000) through `su-exec`.
The lint stage invokes `golangci-lint` directly rather than `make lint`: The lint stage invokes `golangci-lint` directly rather than `make lint`:
it is already the pinned linter image, and `make lint` builds it is already the pinned linter image, and `make lint` builds
+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
-22
View File
@@ -1,22 +0,0 @@
#!/bin/sh
# deploy/docker-entrypoint.sh: the image's ENTRYPOINT. A bind-mounted
# data directory keeps its owner from the host, often root, and the app
# could not write to it. Started as root, this creates DATA_DIR if
# needed, gives it and everything in it to webhooker, sets its mode, and
# runs the command as webhooker, so the app never runs as root. Started
# as another user, it only runs the command.
set -eu
main() {
if [ "$(id -u)" != 0 ]; then
exec "$@"
fi
dir="${DATA_DIR:-/var/lib/webhooker}"
mkdir -p "$dir"
find "$dir" ! -user webhooker -exec chown -h webhooker:webhooker {} +
chmod 750 "$dir"
exec su-exec webhooker "$@"
}
main "$@"
+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,
+20
View File
@@ -0,0 +1,20 @@
package handlers
import (
"net/http"
"github.com/prometheus/client_golang/prometheus/promhttp"
)
// HandleMetrics returns the Prometheus scrape handler for the
// registry every collector in this process registers 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
}
+26 -20
View File
@@ -3,17 +3,17 @@
// 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. These collectors, the inbound HTTP metrics recorded in
// collectors register there too, so both surfaces are gathered by the // internal/middleware, and the Go runtime and process collectors all
// one promhttp handler mounted on the authenticated /metrics route. // 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 +57,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 +99,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 {
+4 -7
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"
) )
@@ -152,16 +151,14 @@ 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 on
// the default registry, which is the one the /metrics route gathers. // the registry the /metrics route serves. Every call shares the one
// recorder New built, 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 {
+2 -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,
+13
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"
@@ -152,6 +155,7 @@ type MiddlewareParams struct {
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,12 @@ type Middleware struct {
params *MiddlewareParams params *MiddlewareParams
session *session.Session session *session.Session
// metricsRecorder records the inbound HTTP metrics on the
// registry /metrics serves. It is built once, in New, because
// building it registers its collectors, and a second
// registration on the same registry panics; see Metrics.
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 +189,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
} }
+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,
),
)
}) })
} }
+43
View File
@@ -23,6 +23,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"
@@ -112,6 +113,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,
@@ -961,3 +964,43 @@ 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 and Go runtime series.
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",
} {
assert.Contains(t, scrape.Body.String(), series)
}
}
}