Conversation
|
I’m addressing the two CodeQL log-input comments before the next roadmap feature. The limit-clamp record can use a constant message and numeric fields; it does not need to repeat the raw URL/client address already covered by access logging. The request logger will use a fixed unmatched-route label and remove CR/LF from request-supplied identity fields before handing them to slog. The built-in text/JSON handlers already escape records, but these changes also keep arbitrary unmatched paths out of the logs and make the field boundary explicit. I’ll add text/JSON regressions for hostile paths/request IDs, retain trusted client identity, rerun CI/native checks, and rebuild the combined preview with the existing HTTP/WS protections. |
|
@MrAlders0n please review this change with your Claude agent as part of the September 13 Beacon review batch. The current candidate is Please check log-level/format behavior, failure severity, redaction and the documented #140/#141 integration adjustments. Issue #51 requires both this PR and cache diagnostics #145. Preferred sequence: merge #145 first, then add the closing reference for #51 here once the combined scope is accepted. Neither individual PR currently carries that closing link. |
What this PR does
Beacon can now filter runtime logs with
log.level/LOG_LEVELand select text or JSON withlog.format/LOG_FORMAT. Defaults are info/text; nonempty environment settings override YAML. Invalid options fail startup with an actionable message.Related to #51. This covers the logging controls and runtime migration; cache diagnostics are in independent PR #145. It uses standard-library
slog, adds component/broker fields, moves routine ingest chatter to debug and keeps operational failures at error. HTTP requests retain chi's streaming/WebSocket wrapper and log status-based severity with the trusted client address, route pattern and request ID. Query strings and the old WebSocket hello payload are omitted. A malformed PostgreSQL DSN cannot leak its password through the startup parse error.The call-site migration is included with the controls so selecting warn/error does not silently hide failures still routed through legacy info logging. Build-time IATA generation keeps its existing command-line logger. No dependency, schema, rotation or per-component-level change.
Type of change
Checklist
go build ./...passesgofmt -l .is emptygo vet ./...passesgo test ./...passesTesting notes
Full build/vet/tests pass locally and on the Pi 5 with PostgreSQL integration tests enabled. Focused Windows race checks pass for logging, request middleware, ingestion and background tasks. Tests cover all four thresholds, text/JSON output, YAML/environment precedence, invalid settings, real startup failures, HTTP 200/429/500 and panic reporting, trusted client identity and background failure semantics. The startup tests use synthetic credentials and assert they are absent from output.
CI and CodeQL pass. The combined Pi preview runs JSON/info logging with both MQTT feeds advancing; public API, WebSocket upgrade/hello/ping and matching corresponding source are verified. The preview's source page identifies the exact combined build.
Integration note for pending #140/#141: keep #141's attempt check before WebSocket acceptance and its close-1013 concurrent-limit signal afterward when resolving the handler conflict. #140's access-log test must capture slog and assert structured status/client/route fields. The combined preview includes these adjustments and passes the existing HTTP/WS regressions; this PR's source remains based directly on current dev.
The CodeQL review follow-up in
0aef4e9removes raw path/client values from the clamp record, omits unmatched request paths and strips CR/LF from request-derived log fields. New text/JSON regressions fail on the prior head and pass afterward; native tests and the updated CI/CodeQL run pass. This update is included in combined preview4e729ff1.AI-assisted implementation and testing under the contributor's standing authorization. This is submitted for maintainer review; no claim of a separate human review of this final diff is implied.