Skip to content

feat: structured Cloud Logging access logs, trace correlation and Error Reporting events - #164

Merged
balebbae merged 2 commits into
mainfrom
devin/1789082976-gcp-observability
Sep 11, 2026
Merged

balebbae merged 2 commits into
mainfrom
devin/1789082976-gcp-observability

Conversation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

Summary

Audit of how the Go API's logs reach GCP found the Zap JSON encoder was already Cloud-Logging-shaped (severity/message/timestamp), but everything around it leaked out of that pipeline or lacked the fields Cloud Logging / Error Reporting key on:

  • chi/middleware.Logger and middleware.Recoverer wrote plain-text lines to stdout, so every access log and every panic showed up in Cloud Logging as textPayload with default severity — invisible to jsonPayload.* log-based metrics and never grouped by Error Reporting.
  • log.Println / log.Fatal in main.go and health.go did the same.
  • No trace correlation: application entries couldn't be nested under the Cloud Run request log in Logs Explorer.
  • zap.NewProductionConfig() samples Info/Warn entries (100/s then 1-in-100), which silently undercounts any log-based metric.
  • Push dispatcher logged full Web Push subscription endpoint URLs (they are capability URLs).
  • /v1/health returned 200 regardless of DB state, so Cloud Run readiness and the uptime check couldn't see a dead database.
  • conflictResponse (409) logged at ERROR, polluting the 5xx/Error Reporting view with client-side conflicts.

What changed

internal/logger

  • NewProductionConfig(): sampling disabled, RFC3339Nano timestamps, Cloud Logging severity names (incl. WARNING, CRITICAL, EMERGENCY).
  • New gcp.go: ParseTraceContext(http.Header) (W3C traceparent first, then X-Cloud-Trace-Context, decimal span → 16-hex), TraceFields(projectID, tc) emitting logging.googleapis.com/{trace,spanId,trace_sampled}, and ErrorReportFields(...) emitting @type, serviceContext, context.{httpRequest,reportLocation}, stack_trace.

cmd/api

  • requestLoggingMiddleware replaces middleware.Logger + middleware.Recoverer:
    reqLogger = app.logger.With(request_id, <trace fields>)
    ctx       = withLogger(ctx, reqLogger)
    defer: recover() -> logPanic(...)  (CRITICAL, Error Reporting event with debug.Stack()) -> 500
    defer: access log "request completed" {httpRequest:{requestMethod,requestUrl(no query),status,responseSize,latency,remoteIp,userAgent,protocol}, route:"/v1/foo/{id}"}
           severity: INFO <400 / WARNING 4xx / ERROR 5xx; 2xx/3xx static assets skipped
    
    http.ErrAbortHandler is re-panicked as net/http expects.
  • app.requestLogger(r) / app.loggerFromContext(ctx) return the request-scoped logger (fallback: process logger). All handler-path app.logger.* calls switched over; background/dispatcher logs stay on the process logger.
  • internalServerError now logs at ERROR with Error Reporting fields + reportLocation of the caller; conflictResponse → WARNING.
  • observabilityConfig{projectID, service, version}: GOOGLE_CLOUD_PROJECT → else GCE metadata server (2s timeout, no-op off GCP); service = K_SERVICESERVICE_NAME. Trace fields are omitted when no project ID is known (Cloud Logging requires the projects/<id>/traces/<id> form).
  • main.go: no more std log; missing .env is not logged in non-dev; startup line logs version/env/service/project/go version; all init failures use logger.Fatalw.
  • /v1/health pings the DB (2s timeout) → 200 {status:ok,database:ok,version,env} or 503 {status:degraded,database:unreachable}. Still Basic-Auth-protected.
  • Push dispatcher logs endpoint_host instead of the endpoint URL; pushSendError unwraps *url.Error so the URL doesn't leak via the error string.

Config / docs: .env.example documents GOOGLE_CLOUD_PROJECT and SERVICE_NAME; swagger regenerated for the health 503.

Not touched / for you to verify in the console

  • No GCP resources were inspected or changed (dashboard, alert policies, uptime check, Cloud Run settings).
  • With this PR you can build log-based metrics on jsonPayload.httpRequest.status / jsonPayload.route, and Error Reporting will start grouping panics and 500s by stack. If the Cloud Run service's health probe or uptime check hits /v1/health, note it now returns 503 when Postgres is unreachable — confirm that's the desired probe semantics (it will restart/flag the instance on DB outages).

Verification

gofmt -l . clean, go vet ./..., staticcheck ./... (v0.8.1, as in CI), go mod verify, go test -race ./... all pass. New tests: internal/logger/gcp_test.go, cmd/api/observability_test.go (access-log shape, request-id/trace propagation into handler logs, panic → 500 + Error Reporting entry, ErrAbortHandler passthrough, 500 report payload, static-asset filtering, health DB probe, push URL redaction).

Link to Devin session: https://app.devin.ai/sessions/4f1e2c60bdb1404692edbce1079e0ac6
Open in Devin Desktop: https://app.devin.ai/desktop/session/4f1e2c60bdb1404692edbce1079e0ac6?variant=devin
Requested by: @balebbae

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@balebbae
balebbae merged commit 69b1c73 into main Sep 11, 2026
3 checks passed
balebbae added a commit to hackutd/hackutd-harp that referenced this pull request Sep 11, 2026
…or Reporting events (hackutd#164)

Cherry-picked from upstream hackutd/harp. Two conflicts were resolved in
favour of this fork's diverged code:

- cmd/api/emails.go: kept per-recipient marking after delivery (660fbbf);
  upstream still marks all recipients before dispatch.
- cmd/api/main.go: kept SuperTokens init after the mailer, which this fork
  needs for themed passwordless email (InitSuperTokens takes mailClient).
  Its Fatal call moved to the sugared Fatalw style the commit introduces,
  since the zap import is gone.

(cherry picked from commit 69b1c73)

Co-authored-by: Caleb Bae <baecal000@gmail.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012CcXtZJizDrfbeYZ551eZ5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant