feat(logging): wire LOG_LEVEL into slog, add LOG_FORMAT=json, correlate requests - #37
Merged
Merged
Conversation
…te requests
LOG_LEVEL was dead configuration. config.Load declared it, defaulted it, read
the env var and logged it back out at startup, and nothing consumed it:
grepping the non-test backend for slog.SetDefault, NewTextHandler and
NewJSONHandler returned zero hits. The process ran on slog's default handler
at the default level for its entire life.
Setting LOG_LEVEL=debug in production therefore did nothing at all, which is
worse than not offering the knob: an operator turns it up during an incident,
sees no change, and concludes the problem is elsewhere. What it suppressed was
exactly the diagnostic layer -- the slog.Debug calls in the WebSocket, terminal
and git paths, which are the entire evidence base when a user reports that
their terminal dropped or the logs pane went blank.
Three changes, cheapest to make together:
internal/logging installs the process-wide handler. main() bootstraps it from
the environment BEFORE config.Load, so config's own startup lines -- including
the volume-path-identity warning -- go through the configured handler rather
than slog's default, then re-installs it from the validated config, which
stays authoritative. An unrecognised LOG_LEVEL or LOG_FORMAT stops the process
at startup instead of silently becoming info.
LOG_FORMAT selects text (default, so existing deployments are unchanged) or
json. The codebase already logs structured key-value pairs throughout; only
the encoder was missing.
middleware.RequestID assigns each request a UUID, exposes it on the gin
context, returns it in X-Request-ID and puts it in the HTTP log line.
ActionLogger.LogWithRequest carries it onto the audit row (migration 10 adds
a nullable action_log.request_id), so a 500 in the HTTP log can be joined to
the action it produced. An inbound X-Request-ID is honoured only when it
parses as a UUID -- the value lands in log lines and audit rows, where an
arbitrary caller-supplied string could forge entries or bloat the database.
Handler helpers now take the gin context rather than a pre-extracted userID,
which also replaces eleven userID.(string) type assertions with userIDFrom's
"anonymous" fallback.
Verified against the running binary, not just in unit tests:
LOG_LEVEL=bogus -> exits 1, 'unrecognised log level "bogus" (want one of
debug, error, info, warn)'
LOG_LEVEL=warn -> 0 INFO lines, WARN lines still present
LOG_LEVEL=debug -> 'Pulling git changes (CLI)' appears, one of the lines
the bead names as permanently invisible
LOG_FORMAT=json -> 19 of 19 startup lines parse as JSON
request id -> X-Request-Id: 6d4ac13a-... matched the HTTP log line's
request_id and the action_log row for that same pull
agent-os-7li
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes
agent-os-7li. First of the fourmain.gobeads (7li→0jp→r7e→69a), which are sequential because they land in overlapping regions of the same file.The bug
LOG_LEVELwas dead configuration:config.godeclared the field, defaulted it toinfo, read the env var and logged it back out at startup. Nothing consumed it. The process ran on slog's default handler at the default level for its entire life.Setting
LOG_LEVEL=debugin production did nothing at all — worse than not offering the knob, because an operator turns it up during an incident, sees no change, and concludes the problem is elsewhere. What it suppressed is exactly the diagnostic layer: theslog.Debugcalls inhandlers/terminal.go,handlers/ws.go,services/terminal.goandservices/git.go. When a user reports a dropped terminal or a blank logs pane, those lines are the whole evidence base.What changed
internal/logginginstalls the process-wide handler.main()bootstraps it from the environment beforeconfig.Load, then re-installs it from the validated config. The acceptance criterion says "immediately afterconfig.Load()"; installing it before as well is what makes the rest of that sentence — "and before any other logging" — actually true.config.Loademits two lines of its own, one of them the volume-path-identity warning, and with the naive ordering those would still bypass the handler. With the bootstrap, all 19 startup lines are JSON in JSON mode.cfgremains the authoritative source.LOG_FORMATselectstext(default, so existing deployments see no change) orjson. The codebase already logs structured key-value pairs throughout; only the encoder was missing.middleware.RequestIDassigns each request a UUID, exposes it on the gin context, returns it inX-Request-ID, and puts it in the HTTP log line.ActionLogger.LogWithRequestcarries it onto the audit row (migration 10 adds a nullableaction_log.request_id). There was previously no way to join a 500 in the HTTP log to the row it produced inaction_log.An inbound
X-Request-IDis honoured only when it parses as a UUID. The value ends up in log lines and audit rows, so an arbitrary caller-supplied string could forge log entries or bloat the database — there is a test forabc" msg="user deleted everything.Handler helpers now take the gin context instead of a pre-extracted userID, which also replaces eleven
userID.(string)type assertions withuserIDFrom's"anonymous"fallback.Verified against the running binary
Unit tests prove the level gating in isolation; these are the same claims made against the real process, which is where the bug lived.
LOG_LEVEL=bogusunrecognised log level "bogus" (want one of debug, error, info, warn)LOG_LEVEL=warnlevel=INFOlines, WARN lines still presentLOG_LEVEL=debuglevel=DEBUG msg="Pulling git changes (CLI)"appears — one of the lines the bead names as permanently invisibleLOG_FORMAT=jsonX-Request-Id: 6d4ac13a-2baa-454f-b61f-e26eb0b7d767matchedrequest_id=6d4ac13a-…in the HTTP log line and theaction_logrow for that same pullThe last one, end to end:
Gates
Frontend untouched.
Deliberately not done
The bead's step 5 ("consider promoting a few WebSocket
slog.Debugcalls toWarn") is not in the acceptance criteria and is a judgement call about each individual call site. Left out to keep this reviewable; the debug lines are now reachable viaLOG_LEVEL=debug, which was the actual blocker.PORTturns out to have the same shape of problem asLOG_LEVELdid —.env.exampledocuments it,config.Loaddefaults it to5001, and nothing readsos.Getenv("PORT"), so the server ignores it. Out of scope here; noticed while testing and worth its own issue.