Backend observability: fx logs bypass slog, Identify() is dead, Sentry/Prometheus config is parsed but unwired #27

New Issue

clawbot · 2026-08-09T03:45:07+02:00

clawbot commented

2026-08-09 03:45:07 +02:00

Problem

The backend parses configuration for three observability features that do not exist, and part of its own logging escapes the structured logger. Verified on main at fbfe1df.

1. fx's own logging bypasses slog entirely

backend/cmd/netwatch-server/main.go:29-41 calls fx.New(...) with no fx.WithLogger(...). fx therefore falls back to its built-in console logger, writing unstructured plain text to stderr for every provide, invoke, and lifecycle hook, plus all shutdown output.

CODE_STYLEGUIDE.md: "For services/servers, log JSON to stdout... Use structured logging whenever possible." Today a production deployment's log stream is a mix of clean JSON from internal/logger and unparseable plain text from fx.

Side effect: go.uber.org/zap v1.26.0 (go.mod:26) is linked into the binary purely as fx's default logger backend. No application code imports it. Bridging fx to slog removes zap's runtime role.

2. `logger.Identify()` is dead code

internal/logger/logger.go:79-85 defines Identify(), which logs appname, version, and buildarch. Grepping the backend finds only the definition — no call site.

CODE_STYLEGUIDE_GO.md: "Embed the git commit hash into the binary and include it in startup logs and in health check output." The version does reach the logs by accident via internal/server/http.go:29-33 ("http begin listen" carries version and buildarch), but appname never does, and the intended identification line never fires.

3. Three config values are parsed and never read

internal/config/config.go loads SENTRY_DSN, METRICS_USERNAME, and METRICS_PASSWORD into the Config struct. Nothing reads any of them.

GO_HTTP_SERVER_CONVENTIONS.md lists these as mandatory libraries and specifies the wiring:

github.com/getsentry/sentry-go — conditional init on SENTRY_DSN, sentryhttp middleware with Repanic: true, sentry.Flush(2*time.Second) on shutdown.
github.com/prometheus/client_golang + github.com/slok/go-http-metrics + github.com/99designs/basicauth-go — Metrics() middleware and a basic-auth-protected GET /metrics.

None are in go.mod. internal/middleware/middleware.go has no Metrics() or MetricsAuth(). internal/server/routes.go registers no /metrics route. The Server struct (server.go:37-47) omits the sentryEnabled field the conventions doc specifies.

backend/README.md:61 already lists Prometheus metrics as an open TODO.

4. Healthcheck JSON keys violate the snake_case rule

internal/healthcheck/healthcheck.go:39-40 emits uptimeHuman and uptimeSeconds.

CODE_STYLEGUIDE_GO.md: "Use snake_case for JSON keys." GO_HTTP_SERVER_CONVENTIONS.md independently specifies uptime_seconds and uptime_human. Both authorities agree; the code disagrees with both.

Also divergent, same file: the type is Response rather than HealthcheckResponse, the method is Check() rather than Healthcheck(), and maintenance_mode is absent (there is no MAINTENANCE_MODE config at all).

Correct and not to be regressed: the endpoint path /.well-known/healthcheck, Content-Type: application/json; charset=utf-8, "status":"ok", HTTP 200, and the git hash reaching the output via Version.

Definition of done

fx.WithLogger bridges fx's lifecycle logging into the existing slog logger, so the entire process emits one coherent structured stream. Verify by running the binary non-TTY and confirming every startup and shutdown line is valid JSON.
logger.Identify() is called at startup (from main.go or an fx OnStart), or deleted if genuinely unwanted. Do not leave it dead.
Healthcheck JSON keys are snake_case: uptime_seconds, uptime_human. Rename the type and method to match the conventions doc while you are in the file.
For Sentry and Prometheus, choose one and apply it consistently: either wire them up per the conventions doc, or remove the dead SENTRY_DSN / METRICS_USERNAME / METRICS_PASSWORD config entries so the config surface stops advertising features that do not exist.
- Config that is parsed, documented, and ignored is worse than either extreme — it tells an operator the feature is available when it is not.
- My recommendation: wire up Prometheus, drop Sentry. /metrics behind basic auth is cheap, self-hosted, and immediately useful for a monitoring tool. Sentry adds an external dependency and an outbound network egress to a third party for a service with no users yet — if you want it later it is a small, isolated addition.
- This is a judgement call. State what you chose and why in the PR; if you would rather I escalate it to @sneak first, say so on this issue instead of guessing.
backend/README.md's config table lists exactly the variables that are actually honoured — no more, no fewer. It currently documents 3 of the 6 defined.
Any new dependency is checked against the Go package defaults before being added.
cd backend && make check passes; root make check passes.
TODO.md updated in the same commit.
Commit title ends with (closes #N).

Notes on scope and sequencing

The healthcheck key rename is a breaking change to the response shape. Nothing currently consumes it (see #25 — the frontend does not call the backend at all), so now is the cheapest possible moment to make it.
This issue is deliberately not on the 1.0.0 milestone. It is real compliance debt, but it does not block a first tag, and #25 may reduce its urgency further. Attach it to the milestone if #25 resolves toward shipping the backend.
Do not fold in the Recoverer panic-logging fix — that belongs to #23, which is already in that file.

Implementation requirements

Follow GO_HTTP_SERVER_CONVENTIONS.md for fx wiring, middleware shape, and the metrics route pattern.
make targets only; never raw go invocations.
No attribution trailers in the commit message.

## Problem The backend parses configuration for three observability features that do not exist, and part of its own logging escapes the structured logger. Verified on `main` at `fbfe1df`. ### 1. fx's own logging bypasses slog entirely `backend/cmd/netwatch-server/main.go:29-41` calls `fx.New(...)` with no `fx.WithLogger(...)`. fx therefore falls back to its built-in console logger, writing **unstructured plain text to stderr** for every provide, invoke, and lifecycle hook, plus all shutdown output. `CODE_STYLEGUIDE.md`: "For services/servers, log JSON to stdout... Use structured logging whenever possible." Today a production deployment's log stream is a mix of clean JSON from `internal/logger` and unparseable plain text from fx. Side effect: `go.uber.org/zap v1.26.0` (`go.mod:26`) is linked into the binary purely as fx's default logger backend. No application code imports it. Bridging fx to slog removes zap's runtime role. ### 2. `logger.Identify()` is dead code `internal/logger/logger.go:79-85` defines `Identify()`, which logs appname, version, and buildarch. Grepping the backend finds **only the definition** — no call site. `CODE_STYLEGUIDE_GO.md`: "Embed the git commit hash into the binary and include it **in startup logs** and in health check output." The version does reach the logs by accident via `internal/server/http.go:29-33` ("http begin listen" carries `version` and `buildarch`), but `appname` never does, and the intended identification line never fires. ### 3. Three config values are parsed and never read `internal/config/config.go` loads `SENTRY_DSN`, `METRICS_USERNAME`, and `METRICS_PASSWORD` into the `Config` struct. Nothing reads any of them. `GO_HTTP_SERVER_CONVENTIONS.md` lists these as **mandatory** libraries and specifies the wiring: - `github.com/getsentry/sentry-go` — conditional init on `SENTRY_DSN`, `sentryhttp` middleware with `Repanic: true`, `sentry.Flush(2*time.Second)` on shutdown. - `github.com/prometheus/client_golang` + `github.com/slok/go-http-metrics` + `github.com/99designs/basicauth-go` — `Metrics()` middleware and a basic-auth-protected `GET /metrics`. None are in `go.mod`. `internal/middleware/middleware.go` has no `Metrics()` or `MetricsAuth()`. `internal/server/routes.go` registers no `/metrics` route. The `Server` struct (`server.go:37-47`) omits the `sentryEnabled` field the conventions doc specifies. `backend/README.md:61` already lists Prometheus metrics as an open TODO. ### 4. Healthcheck JSON keys violate the snake_case rule `internal/healthcheck/healthcheck.go:39-40` emits `uptimeHuman` and `uptimeSeconds`. `CODE_STYLEGUIDE_GO.md`: "**Use snake_case for JSON keys.**" `GO_HTTP_SERVER_CONVENTIONS.md` independently specifies `uptime_seconds` and `uptime_human`. Both authorities agree; the code disagrees with both. Also divergent, same file: the type is `Response` rather than `HealthcheckResponse`, the method is `Check()` rather than `Healthcheck()`, and `maintenance_mode` is absent (there is no `MAINTENANCE_MODE` config at all). Correct and not to be regressed: the endpoint path `/.well-known/healthcheck`, `Content-Type: application/json; charset=utf-8`, `"status":"ok"`, HTTP 200, and the git hash reaching the output via `Version`. ## Definition of done - [ ] `fx.WithLogger` bridges fx's lifecycle logging into the existing slog logger, so the entire process emits one coherent structured stream. Verify by running the binary non-TTY and confirming every startup and shutdown line is valid JSON. - [ ] `logger.Identify()` is called at startup (from `main.go` or an fx `OnStart`), or deleted if genuinely unwanted. Do not leave it dead. - [ ] Healthcheck JSON keys are snake_case: `uptime_seconds`, `uptime_human`. Rename the type and method to match the conventions doc while you are in the file. - [ ] For Sentry and Prometheus, choose one and apply it consistently: **either** wire them up per the conventions doc, **or** remove the dead `SENTRY_DSN` / `METRICS_USERNAME` / `METRICS_PASSWORD` config entries so the config surface stops advertising features that do not exist. - Config that is parsed, documented, and ignored is worse than either extreme — it tells an operator the feature is available when it is not. - **My recommendation: wire up Prometheus, drop Sentry.** `/metrics` behind basic auth is cheap, self-hosted, and immediately useful for a monitoring tool. Sentry adds an external dependency and an outbound network egress to a third party for a service with no users yet — if you want it later it is a small, isolated addition. - This is a judgement call. State what you chose and why in the PR; if you would rather I escalate it to @sneak first, say so on this issue instead of guessing. - [ ] `backend/README.md`'s config table lists exactly the variables that are actually honoured — no more, no fewer. It currently documents 3 of the 6 defined. - [ ] Any new dependency is checked against the Go package defaults before being added. - [ ] `cd backend && make check` passes; root `make check` passes. - [ ] `TODO.md` updated in the same commit. - [ ] Commit title ends with ` (closes #N)`. ## Notes on scope and sequencing - The healthcheck key rename is a **breaking change** to the response shape. Nothing currently consumes it (see #25 — the frontend does not call the backend at all), so now is the cheapest possible moment to make it. - This issue is deliberately **not** on the `1.0.0` milestone. It is real compliance debt, but it does not block a first tag, and #25 may reduce its urgency further. Attach it to the milestone if #25 resolves toward shipping the backend. - Do not fold in the `Recoverer` panic-logging fix — that belongs to #23, which is already in that file. ## Implementation requirements - Follow `GO_HTTP_SERVER_CONVENTIONS.md` for fx wiring, middleware shape, and the metrics route pattern. - `make` targets only; never raw `go` invocations. - No attribution trailers in the commit message.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/netwatch#27

Backend observability: fx logs bypass slog, Identify() is dead, Sentry/Prometheus config is parsed but unwired #27