feat(observability): self-hosted log aggregation (loki + structured logs) - #84
Conversation
0xfandom
left a comment
There was a problem hiding this comment.
Review — PR2 of 3 for #69 (WS-7.5 log aggregation)
Scoped to what PR2 adds on top of #83. Config is sound, the LOG_FORMAT=json gate is correct, and the Loki/Promtail wiring is consistent with the Grafana datasource + dashboard panel query.
Acceptance criteria
| # | Criterion | Evidence | Verdict |
|---|---|---|---|
| 1 | Loki 168h retention | deploy/docker/loki/loki-config.yml:31 retention_period: 168h + reject_old_samples_max_age: 168h |
Met |
| 2 | Loki schema/storage current | loki-config.yml:20-28 TSDB + schema v13 + 24h index period; allow_structured_metadata: true supported in Loki 2.9.4 (pinned at docker-compose.yml:108) |
Met |
| 3 | Single-host auth off | loki-config.yml:1 auth_enabled: false |
Met |
| 4 | Loki filesystem paths align with volume | chunks_directory: /loki/chunks, path_prefix: /loki, compactor working_directory: /loki/compactor all under mount loki-data:/loki (docker-compose.yml:115) |
Met |
| 5 | Promtail labels match dashboard query | promtail-config.yml:21-27 emits service + container_name; dashboard query overview.json:184 is {service=~"aether-.*"} | json | level=~"(?i)(error|warn)" which matches compose service names aether-rust / aether-go |
Met |
| 6 | Promtail clients → loki DNS | promtail-config.yml:10 http://loki:3100/loki/api/v1/push (not localhost) |
Met |
| 7 | Promtail positions persisted | promtail-config.yml:7 → /var/lib/promtail/positions.yaml, mounted via promtail-data:/var/lib/promtail in docker-compose.yml:127 |
Met |
| 8 | Docker socket mounted for SD | docker-compose.yml:126 /var/run/docker.sock:/var/run/docker.sock:ro |
Met |
| 9 | depends_on ordering |
docker-compose.yml:128 promtail depends on loki; no cycles |
Met |
| 10 | Grafana Loki datasource | deploy/docker/grafana/provisioning/datasources/loki.yml uid: loki, access: proxy, url: http://loki:3100 — matches dashboard panel datasource.uid at overview.json:179 |
Met |
| 11 | LOG_FORMAT=json gate correct |
crates/grpc-server/src/main.rs:40-52 matches only exact "json"; any other value (including JSON, empty string, unicode, unset) falls through to the human-readable fmt() branch. Wildcard arm is the default |
Met |
| 12 | tracing-subscriber json feature |
Cargo.toml:43 features = ["env-filter", "json"] — added cleanly, no other deps touched |
Met |
| 13 | Sensible span defaults | main.rs:44-45 with_current_span(true).with_span_list(false) — avoids per-event span-list inflation (important for Loki ingestion volume) |
Met |
| 14 | slog handler set before any log line | cmd/executor/main.go:93, cmd/monitor/metrics.go:99, cmd/pooldiscovery/main.go:277 — slog.SetDefault(...) is the first statement in each main(). No init-order hazard |
Met |
| 15 | risk package inherits the JSON handler | internal/risk/manager.go:149,209,318,331,353,402 uses package-level slog (= slog.Default()). Executor sets the default before constructing RiskManager at cmd/executor/main.go:168, so risk lines ship as JSON with the same aether-go Docker label the dashboard query needs |
Met |
| 16 | Level regex handles slog casing | slog's JSONHandler emits "level":"INFO"/"WARN"/"ERROR" uppercase; panel regex uses (?i) at overview.json:184 |
Met |
| 17 | No Slack-only policy regression | PR2 does not touch alertmanager.yml or reintroduce PagerDuty/Telegram/Discord in any new config. The legacy enum in cmd/monitor/alerter.go is pre-PR2 and only logs — not a submission path |
Met |
| 18 | No secrets leaked | .env.example adds only LOG_FORMAT doc comment (lines 12-15); SLACK_WEBHOOK_URL was pre-existing from PR1 and is still blank |
Met |
| 19 | Backward-compatible local dev | Rust: unset LOG_FORMAT → pretty fmt branch (main.rs:49-51). Go: running go test does not invoke any main(), so the JSON handler is not installed in tests — slog.Default() stays stdlib text. Confirmed by reviewer-reported clean test runs |
Met |
Findings
LOW — dashboard panel coverage gap from deliberately-untouched call sites
The "Recent errors" panel filter is {service=~"aether-.*"} | json | level=~"(?i)(error|warn)". The | json parser silently drops non-JSON lines, so every remaining log.Printf in cmd/executor/submitter.go:138,141, cmd/executor/gas_oracle.go:130,159,163, cmd/executor/nonce.go:55,96,102, cmd/executor/metrics.go:252, and cmd/risk/state.go:16-17 (i.e. the sites the PR body explicitly leaves as stdlib log) will be invisible on the new panel. A few of these are exactly the signals an on-call would want to see — Bundle rejected by %s, eth_getBalance failed, Nonce sync failed. Not a blocker since you called it out as scoped-out, but worth tracking as PR3 scope or a follow-up ticket; otherwise the panel will under-report real incidents.
Also note that slog.SetDefault does not redirect the stdlib log package. A one-liner log.SetOutput(...) bridge or slog.NewLogLogger(...) would at least funnel those lines into the same JSON stream without touching every call site — consider for the next pass.
LOW — fmt.Println banners emit non-JSON after slog.SetDefault
cmd/executor/main.go:95, cmd/monitor/metrics.go:101, cmd/pooldiscovery/main.go:293 print a plain-text banner to stdout immediately after installing the JSON handler. Cosmetically inconsistent and will not parse through | json. Cheap fix: swap for slog.Info("service starting", ...) or delete.
LOW — aether-monitor is built but not started by compose
deploy/docker/go.Dockerfile:8,13 builds the aether-monitor binary but ENTRYPOINT at line 14 is aether-executor, and the aether-go compose service has no sidecar or override to launch monitor. This is pre-PR2, but it means the PR's slog conversion in cmd/monitor/*.go has no effect on Loki ingestion today. Worth noting so expectations are set; ship monitor as a separate compose service in a follow-up.
NIT — PR body lacks a linking line
Per convention the last line should reference the tracking issue. Since PR3 remains, Refs #69 (not Closes) is appropriate.
Verdict
Config is correct, the JSON gate is defensive, the Loki/Promtail/Grafana wiring lines up end-to-end, and no init-order or secret-leak hazards were found. The partial scope is a known tradeoff and properly flagged. Approving — please track the dashboard coverage gap and the monitor-container launch as follow-ups.
Self-hosted log aggregation stack wired into the existing docker-compose so container stdout/stderr is scraped by promtail and pushed to loki with 7 day retention. Labels attached per stream: service (from the compose service name) and container_name. Rust service gets LOG_FORMAT=json set on the container so its tracing-subscriber emits structured JSON.
Mirror the prometheus datasource file so grafana auto-provisions loki alongside it, pointing at the in-network loki service. Pinned uid so dashboards can reference it stably.
Enable the tracing-subscriber json feature and branch on LOG_FORMAT at startup: LOG_FORMAT=json installs a structured json layer (current span included, span list omitted to keep lines compact) while any other value keeps the existing pretty text output. Production docker runs with LOG_FORMAT=json so promtail can ship structured records to loki; local cargo run stays human readable.
Install a json slog handler as the default logger in each binary entry point (executor, monitor, pooldiscovery) and convert log.Printf call sites at the module boundaries to slog.Info/Warn/Error with structured key/value fields. Boundary scope only: cmd/executor/main.go, cmd/monitor/*, cmd/pooldiscovery/main.go, and internal/risk/manager.go. Deeper utility loggers in gas_oracle, nonce, submitter, bundle, and state files keep their stdlib log for now and can be migrated later. Fatal exits are converted to slog.Error + os.Exit(1) since slog has no Fatal equivalent. Context-aware InfoContext/ErrorContext is used on the arb processing hot path so future tracing work can attach span ids.
Append a loki-backed logs panel below the existing 7 panels. Filters to aether-* services and level=(error|warn) after parsing the structured log lines, sorted newest first, with details expanded on click. The query works against both the rust tracing json layer and the go slog json handler because both emit a level field.
- migrate bundle/nonce/gas_oracle/submitter/state.go to slog so loki errors-panel stops silently dropping log.Printf / log.Fatalf - flatten_event(true) on rust json tracing so fields land at root, not under fields.* - loki healthcheck + promtail service_healthy dep to avoid cold-boot 503s - loki limits: max_streams_per_user 500, ingestion_rate 8MB, burst 16MB
bc19ae0 to
33de991
Compare
2198463 to
6edd8b3
Compare
Re-review round 2 — coverage gap closedRebased onto current High (the one that actually mattered)
Medium
Deliberately deferred (noise for this PR)
Validation
Note on diffThe rebase onto the updated PR #83 surfaced a few whitespace-only gofmt drifts in unrelated files (struct field alignment). Those are in the diff but are pure cosmetic — they unblock the Ready for re-review. |
0xfandom
left a comment
There was a problem hiding this comment.
Summary
PR #84 adds self-hosted Loki + Promtail to the compose stack, wires Grafana to query it with a pinned datasource UID, migrates the Go services to log/slog JSON output, and gates a Rust tracing-subscriber JSON layer behind LOG_FORMAT=json. Round-2 fixes landed cleanly: bundle.go:99 converted to slog.Error + os.Exit(1) (no more invisible log.Fatalf), slog migration completed in bundle/nonce/gas_oracle/submitter/state, Loki has /ready healthcheck with promtail.depends_on.loki.condition: service_healthy, and limits_config is populated with max_streams_per_user, ingestion_rate_mb, and ingestion_burst_size_mb. .flatten_event(true) is correctly in the LOG_FORMAT=json branch. The known-deferred stdlib log.Printf in cmd/executor/metrics.go is the only real gap and is called out explicitly as follow-up.
Issue #69 WS-7.5 AC table
| Criterion | Status | Evidence |
|---|---|---|
| Self-hosted Loki | Met | deploy/docker/docker-compose.yml:107-123 — grafana/loki:2.9.4, volume-mounted config, /ready healthcheck |
| Promtail shipping logs | Met | docker-compose.yml:125-138; promtail-config.yml:12-27 docker SD with com.docker.compose.project filter |
Rust tracing-subscriber JSON |
Met | crates/grpc-server/src/main.rs:39-52 — LOG_FORMAT=json branch, .json().flatten_event(true).with_current_span(true).with_span_list(false) |
| Cargo feature flag | Met | Cargo.toml:43 — tracing-subscriber = { version = "0.3", features = ["env-filter", "json"] } |
| Go slog JSON output | Met (1 known gap) | cmd/executor/main.go:91, cmd/monitor/metrics.go:99, cmd/pooldiscovery/main.go:277 — slog.SetDefault(slog.New(slog.NewJSONHandler(os.Stderr, ...))) as first line of each main |
| Grafana Loki datasource | Met | grafana/provisioning/datasources/loki.yml:5 — uid: loki pinned, url: http://loki:3100 |
| Errors panel on Overview | Met | overview.json:174-198 — type: "logs", {service=~"aether-.*"} | json | level=~"(?i)(error|warn)", maxLines: 200 |
| Compose integration | Met | docker-compose.yml:107-138 — Loki and Promtail on aether-net, named volumes loki-data/promtail-data declared at :159-163 |
Round-2 fix verification
| Item | Status | Evidence |
|---|---|---|
| slog migration complete in bundle/nonce/gas_oracle/submitter/state | Verified | bundle.go:7,9, nonce.go:5, gas_oracle.go:5, submitter.go:10, internal/risk/state.go:5 all import log/slog only; no "log" imports remain |
bundle.go:99 log.Fatalf → slog.Error + os.Exit(1) |
Verified | cmd/executor/bundle.go:99-101 — slog.Error("crypto/rand failure", "err", err); os.Exit(1); os imported at :9 |
.flatten_event(true) on JSON builder |
Verified | crates/grpc-server/src/main.rs:43 — inside Ok("json") match arm, chained on .json() |
| Loki healthcheck | Verified | docker-compose.yml:116-120 — wget -qO- http://localhost:3100/ready | grep -q ready, 10s interval, 10 retries |
Promtail depends_on long-form map |
Verified | docker-compose.yml:133-135 — depends_on: loki: condition: service_healthy (map form) |
| Loki limits | Verified | loki-config.yml:36-38 — max_streams_per_user: 500, ingestion_rate_mb: 8, ingestion_burst_size_mb: 16 |
Original-scope fresh pass
- LogQL panel (
overview.json:175-197): paneltype: "logs"(not deprecated), datasource{type: loki, uid: loki}matches provisioning,maxLines: 200,sortOrder: "Descending",showTime: true. Query shape correct for slog +flatten_event(true)output (both emitlevelat root). - Promtail labels (
promtail-config.yml:20-27): onlyservice,container_name,compose_project. All bounded by container count (~7-8). Nojson/regexpipeline stage. No cardinality bomb. - Loki config:
auth_enabled: false(single-tenant OK), filesystem paths under/lokimounted volume,retention_period: 168h+compactor.retention_enabled: true+retention_delete_delay: 2h+delete_request_store: filesystem— retention actually enforced, not just declared.reject_old_samples: truematches window — no silent drops. - Rust JSON output: with
flatten_event(true), emits{"timestamp", "level", "target", "message", <event fields at root>, "span":{"name"}}. Matches panel query. - Stdlib
log.*in runtime path: onlycmd/executor/metrics.go:5,108,110,168,299remain (the deliberately-deferred scope).:168fires on big.Int→float64 precision loss in everyaddBigIntCounter,:299fires on everyeth_getBalanceRPC failure. These bypass slog and will NOT appear in the "Recent errors" panel — known gap, acknowledged.
Must-fix blockers
None.
Should-fix nits
- S1 —
cmd/monitor/alerter.go:79-102severity mismatch. Critical/Error-class alerts are still logged atslog.Inforegardless ofalert.Severity. The Recent Errors panel filterslevel=~(error|warn)and will silently miss every dispatched critical alert. Fix:switch alert.Severity { case SeverityCritical, SeverityError: slog.Error("alert dispatched", ...) case SeverityWarning: slog.Warn("alert dispatched", ...) default: slog.Info("alert dispatched", ...) }
- S2 —
docker-compose.yml:16still hasversion: "3.8". Compose v2 warns on this; harmless noise on everydocker compose up. Cheap delete. - S3 —
cmd/monitor/metrics.go:132-133usesfmt.Sprintfto build a URL and pass as singleurlkey. Preserves structure better to loghost/port/endpointseparately. - S4 —
loki-config.yml:48ruler.alertmanager_urlwires Loki's ruler to shared Alertmanager despite no Loki rules existing yet. If someone later adds Loki alert rules, they'll fire to the same Alertmanager as Prom rules without anyreceiverrouting to distinguish. Leaverulerunset or add a comment.
Can-defer (follow-up)
- D1 — Migrate
cmd/executor/metrics.gooff stdliblog(L108, L110, L168, L299). Precision-loss warning at L168 is especially worth capturing in Loki: silent big.Int→float64 truncation on profit/gas counters is money-adjacent signal that should be queryable. - D2 — Remove obsolete
version: "3.8"(see S2). - D3 — Flashbots signer auth-key redaction audit.
submitter.go:143logserrfrom rejected bundles — if a builder echoes auth headers back in an error body, those could reach Loki. - D4 — Consider
pipeline_stages: - match:in promtail to drop debug-level lines whenslog.Debug/tracing::debug!fires in prod.
Verdict
APPROVE — ship it.
Round-2 blockers are fully addressed. The one remaining stdlib log site in cmd/executor/metrics.go is scoped as a known follow-up and does not block production use of the Loki pipeline for 95%+ of runtime logs. Loki retention is actually enforced, Promtail labels are bounded, datasource UID is pinned, and the Recent Errors panel will correctly parse output from both the Go slog JSON handler and the Rust flatten_event(true) layer.
Summary
PR2 of the observability series for issue #69 (WS-7.5). Stacked on top of #83. Adds self-hosted log aggregation so we can answer "why did it revert?" without ssh'ing in, and swaps Rust and Go logging to structured JSON at the entry points Promtail sees.
What's in
deploy/docker/docker-compose.yml, both self-hosted onaether-net. Loki runs single-binary with filesystem storage and 168h retention; Promtail scrapes the Docker socket and attachesservice+container_namelabels.lokiso dashboards can reference it stably.tracing-subscriberJSON layer gated byLOG_FORMAT=json(set on theaether-rustcontainer). Unset / any other value keeps the existing pretty output for localcargo run.log/slogJSON handler installed as the default logger in each binary entry (cmd/executor,cmd/monitor,cmd/pooldiscovery).log.Printfcall sites at the module boundaries are converted toslog.Info/Warn/Errorwith structured key/value fields.internal/risk/manager.goboundary logs go throughslogtoo (using the default logger). Deeper utility loggers ingas_oracle.go,nonce.go,submitter.go,bundle.go,state.goare intentionally left as stdliblogfor now to keep the diff scoped.{service=~\"aether-.*\"} | json | level=~\"(?i)(error|warn)\"with a 200-line cap, newest first.What's out (stacked PR3 — WS-7.6 / WS-7.7)
scripts/canary.pyTest plan
go build ./...cleango vet ./...cleango test ./cmd/executor/... ./internal/risk/... ./cmd/monitor/... ./cmd/pooldiscovery/...greencargo check -p aether-grpc-servercleancargo test -p aether-grpc-server --libgreendocker-compose.yml,loki-config.yml,promtail-config.yml,loki.yml,overview.jsonall parse as valid YAML/JSONdocker compose -f deploy/docker/docker-compose.yml up -d), confirmcurl http://localhost:3100/readyreturnsready, confirm Grafana shows both datasources healthy and the Overview dashboard renders the new panel.Notes
feat/observability-pr1(PR feat(observability): Prometheus alerts, Grafana dashboards, Slack routing #83). Once that merges, GitHub will rebase this ontomainautomatically.SLACK_WEBHOOK_URLenv substitution for Alertmanager introduced in PR1 is left untouched; no new secrets are required for PR2.