Repository navigation
feat(observability): tenant and request id on every log record and span - #819
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzXgYrkbx4W91MUu18T2G7
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzXgYrkbx4W91MUu18T2G7
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Summary
Merge Risk: ⚪ Minimal · up to
|
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzXgYrkbx4W91MUu18T2G7
📚 Docs preview✅ Live: https://pr-819-wavehouse-docs.wave-rf.workers.dev (follows this PR)
Earlier commits (1)
|
Code Coverage OverviewLanguages: Go GoThe overall line coverage in commit 563a10d in the Show a line coverage summary of the most impacted files.
Updated |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
d15d028d-5354-433e-a7ab-484e9b0a34e1
📒 Files selected for processing (18)
AGENTS.mdCHANGELOG.mdcmd/wavehouse/main.godocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/reverse-proxy.mdxinternal/api/correlation.gointernal/api/correlation_test.gointernal/api/dlq.gointernal/api/router.gointernal/api/tenant.gointernal/auth/auth.gointernal/observability/logger.gointernal/observability/request.gointernal/observability/request_test.goscripts/ci/remote-ci.test.sh
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
📜 Review details
🧰 Additional context used
📚 Code guidelines (4)
docs/src/content/docs/deployment.md — auto-discovered
CONTRIBUTING.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
AGENTS.md — auto-discovered
📓 Path-based instructions (6)
Source excerpt: **Settings directory.**
📄 CodeRabbit inference engine (docs/src/content/docs/deployment.md)
Files:
docs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.md
Source excerpt: In a Docker / Podman / Kubernetes deployment, **`data_dir` must resolve to a host-backed volume**.
📄 CodeRabbit inference engine (docs/src/content/docs/deployment.md)
Files:
docs/src/content/docs/deployment.md
Source excerpt: Update the documentation and `CHANGELOG.md` (under `## Unreleased`) that your change affects.
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/api.mdCHANGELOG.mddocs/src/content/docs/architecture.md
See [AGENTS.md](../AGENTS.md) for project conventions, architecture notes, and AI agent instructions.
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
AGENTS.md
Source excerpt: **Docs prose**: never hard-wrap Markdown — one paragraph is one line.
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/api.mdCHANGELOG.mdAGENTS.mddocs/src/content/docs/architecture.md
Source excerpt: **In MDX, leave a blank line between a JSX tag and a code fence.**
📄 CodeRabbit inference engine (AGENTS.md)
Files:
docs/src/content/docs/reverse-proxy.mdxdocs/src/content/docs/configuration.mdx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-12T20:48:09.334Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
1. **Validate locally before every push** — run `make ci` the documented way, here or on a configured host with `make ci-remote` ([§Running `make ci`](#running-make-ci-for-agents)). Don't use CI as your first feedback loop.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-12T20:48:09.334Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Documentation Sync
Every code change should update the corresponding docs in the same PR. A code change without its doc update is incomplete.
| Change | Files to update |
| ------ | --------------- |
| Add/modify API endpoint | `docs/src/content/docs/api.md`, `README.md` (if user-facing) |
| Add/modify boot config option | `docs/src/content/docs/configuration.mdx`, `config.yaml`, `deployments/compose/*` env blocks, `docs/src/content/docs/deployment.md` |
| Add/modify a settings-directory key (`config.json`, `roles.json`, `policies.json`, `pipes.json`) | `docs/src/content/docs/settings-directory.mdx` (table **and** the example block), `internal/settings/seed/*.json`, `deployments/compose/settings/*.json`, `tests/e2e/fixtures/settings/*.json`, `config.yaml` (the `settings:` comment enumerates the keys) |
| Change architecture / add a package | `docs/src/content/docs/architecture.md`, `AGENTS.md` |
| Change ingest / e...
🪛 LanguageTool
docs/src/content/docs/deployment.md
[style] ~794-~794: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...e_id/span_idfor Loki-to-Tempo links whether or nototel.logs.enabled` is on. Leaving it o...
(WHETHER)
docs/src/content/docs/api.md
[style] ~83-~83: Since ownership is already implied, this phrasing may be redundant.
Context: ... ASCII characters, with no spaces, gets its own back; any other gets a fresh random 32-...
(PRP_OWN)
CHANGELOG.md
[typographical] ~19-~19: Consider using an em dash in dialogues and enumerations.
Context: - **Every request's log records and serve...
(DASH_RULE)
[style] ~19-~19: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: - Every request's log records and server span say which request and which tenant they belong to (internal/observability/{request,logger}.go (+ tests), internal/api/correlation.go (new, + tests), internal/api/{router,tenant,dlq}.go, cmd/wavehouse/main.go, a comment in internal/auth/auth.go, docs/src/content/docs/{api,architecture,deployment}.md, {configuration,reverse-proxy}.mdx, AGENTS.md): closes #771, part of the observability epic #777, and carries the request-id item of #333 that #25 asked for. chi's RequestID ran on every request,...
(TOO_LONG_SENTENCE)
[style] ~19-~19: Since ownership is already implied, this phrasing may be redundant.
Context: ...lution, has it too; a record that names its own tenant keeps it. The otelhttp server sp...
(PRP_OWN)
docs/src/content/docs/architecture.md
[style] ~192-~192: Consider shortening this phrase to just ‘whether’, unless you mean ‘regardless of whether’.
Context: ...ogger in it too, so stdout carries them whether or not OTLP logs are on. otlpSamplerFn is ex...
(WHETHER)
[style] ~193-~193: Since ownership is already implied, this phrasing may be redundant.
Context: ...ds and spans alike; a record that names its own tenant keeps it. They are never metric ...
(PRP_OWN)
🔇 Additional comments (17)
internal/observability/request.go (1)
34-35: LGTM!Also applies to: 53-58, 62-76, 80-98
internal/observability/logger.go (1)
15-16: LGTM!Also applies to: 21-22, 24-24
internal/observability/request_test.go (1)
42-95: LGTM!Also applies to: 97-117, 119-163
cmd/wavehouse/main.go (1)
16-16: LGTM!Also applies to: 164-167
AGENTS.md (1)
44-44: LGTM!docs/src/content/docs/architecture.md (1)
78-78: LGTM!Also applies to: 192-193
docs/src/content/docs/configuration.mdx (1)
298-298: LGTM!docs/src/content/docs/deployment.md (1)
747-750: LGTM!Also applies to: 794-794
internal/auth/auth.go (1)
500-504: LGTM!Also applies to: 526-527
internal/api/correlation.go (1)
13-58: LGTM!Also applies to: 78-85
internal/api/router.go (1)
123-123: LGTM!Also applies to: 167-167, 214-214, 262-262
internal/api/tenant.go (1)
10-10: LGTM!Also applies to: 102-102, 116-116, 157-157
internal/api/dlq.go (1)
10-10: LGTM!Also applies to: 37-37
internal/api/correlation_test.go (1)
26-96: LGTM!Also applies to: 98-209
CHANGELOG.md (1)
19-20: LGTM!docs/src/content/docs/api.md (1)
83-84: LGTM!docs/src/content/docs/reverse-proxy.mdx (1)
198-198: LGTM!
…ad naming none Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RzXgYrkbx4W91MUu18T2G7
…-request-id-gq494d
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Requested by Taite · project thread
Summary
Before: chi's
RequestIDran on every request but nothing read the id, so no log line, span or response carried it, and an incomingX-Request-Idwas dropped. The tenant was on no span and on few request-path log lines, so the authz-denied WARN, the operator-key audit line andbatch ingestedcould not be grouped by tenant.After: every record logged through a request's context carries
request_id, andtenantonce the tenant is resolved. The otelhttp server span carries the same two attributes. Every response returns the id inX-Request-Id. Neither field is a metric label.How:
internal/observability/request.go(new) adds a request scope in the context.TraceHandlerstamps it on records; a record that already names a tenant keeps its own. The tenant is filled in place, so the panic recoverer's record, logged through an earlier context, has it too.internal/api/correlation.go(new) replaces chi'sRequestID. It keeps an incomingX-Request-Idsent once with 1 to 128 printable ASCII characters and no spaces, otherwise it mints a random 32-hex id that, unlike chi's, names no host.annotateSpanruns after otelhttp and copies the scope onto the server span.TenantMWstamps theX-Tenant-IDtenant. On/v1/ops,opsRequestTenantruns ahead of authentication, so the operator-key audit line carries the tenant: a valid?tenant=, or tenant0for any ops route that names none (reads, raw SQL, the schema refresh). A settings reload naming none acts on every tenant and stamps nothing.cmd/wavehouse's boot logger is wrapped inTraceHandler, so stdout keeps these fields, andtrace_id/span_id, withotel.logs.enabledoff.Left for later:
internal/api/ingest.gois untouched because #712 rewrites it. Itsbatch ingestedand per-event lines already log through the request context, so they get both fields with no edit there. Itsingestchild span still carries onlytable; the tenant is on the server span above it. As asked, no wiring went intointernal/app/{wire,app}.go,internal/chconn/chconn.goorinternal/ingest/worker.go.The docs are updated in
api.md,architecture.md,deployment.md(a new "Following one request or one tenant" section),configuration.mdx,reverse-proxy.mdxandAGENTS.md, with a CHANGELOG entry under Unreleased.Test plan
make build,go vet ./...go test -race ./internal/api/ ./internal/observability/ ./internal/auth/ ./cmd/...tenantkey, tenant-route and ops-route lines (both routers) carrying the tenant from authentication on, and the ops and DLQ default tenant.ship_iton f70346fmake ci: not run in the cloud session, which has no Docker and cannot download golangci-lint; left for the owner to run locallyRelated Issues
Closes #771
Part of #777. Carries #333's request-id item. Related: #25.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RzXgYrkbx4W91MUu18T2G7