Repository navigation
fix(observability): continue ingest traces into the worker and insert - #842
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi
…ng on the ingest trace link Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi
📝 Summary
Merge Risk: 🔵 Low · up to
|
📚 Docs preview✅ Live: https://pr-842-wavehouse-docs.wave-rf.workers.dev (follows this PR)
Earlier commits (3)
|
…r spancheck Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi
…s, for spancheck Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi
Code coverage overviewLanguages: Go Go
View coverage of the 6 most impacted files.
ℹ️ Calculations represent line coverage. Results may change as calculations are updated. Updated |
…ce-continuity-6z0vca # Conflicts: # AGENTS.md # CHANGELOG.md # docs/src/content/docs/architecture.md
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: ASSERTIVE
- Plan: Advanced
- Run ID:
f017de32-fa60-4bc3-b90d-782d9d592581
📒 Files selected for processing: 14
AGENTS.mdCHANGELOG.mddocs/src/content/docs/architecture.mddocs/src/content/docs/deployment.mddocs/src/content/docs/ingest-pipeline.mdinternal/ingest/trace.gointernal/ingest/trace_test.gointernal/ingest/worker.gointernal/mq/embedded.gointernal/mq/external_consumer.gointernal/mq/mqtest/cases.gointernal/mq/mqtest/mqtest.gointernal/testutil/otlp.gotests/integration/otel_test.go
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: 5
.github/prompts/docs-review.md — auto-discovered
AGENTS.md — auto-discovered
docs/src/content/docs/development.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
CONTRIBUTING.md — auto-discovered
📓 Path-based instructions: 6
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Documentation *prose* — accuracy against the code, runnable examples, clarity, completeness — **and code↔docs sync** (code that changed but whose docs did...
📄 CodeRabbit inference engine (.github/prompts/docs-review.md)
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/architecture.mddocs/src/content/docs/ingest-pipeline.md
Source excerpt: **Sealed MQ boundary** — only `internal/mq` imports NATS/JetStream (`github.com/nats-io/…`), enforced by the `depguard` rule in `.golangci.yml`, so `make lint` fails on a leak in every package it builds, including the files...
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/mq/mqtest/mqtest.gointernal/mq/embedded.gointernal/mq/external_consumer.gointernal/mq/mqtest/cases.go
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse Tooling notes (the non-obvious bits `make help` won't tell you): **GNU Make 4+** required (uses `--output-sync=target`); macOS ships GNU Make 3.81, which...
📄 CodeRabbit inference engine (docs/src/content/docs/development.md)
Files:
internal/mq/mqtest/mqtest.gointernal/mq/embedded.gointernal/testutil/otlp.gointernal/mq/external_consumer.gointernal/mq/mqtest/cases.godocs/src/content/docs/deployment.mddocs/src/content/docs/architecture.mdinternal/ingest/worker.goAGENTS.mdCHANGELOG.mdinternal/ingest/trace.gotests/integration/otel_test.godocs/src/content/docs/ingest-pipeline.mdinternal/ingest/trace_test.go
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: Create the package under `internal/`.
📄 CodeRabbit inference engine (AGENTS.md)
Files:
internal/mq/mqtest/mqtest.gointernal/mq/embedded.gointernal/testutil/otlp.gointernal/mq/external_consumer.gointernal/mq/mqtest/cases.gointernal/ingest/worker.gointernal/ingest/trace.gointernal/ingest/trace_test.go
Applicability reference: AGENTS.md: # AGENTS.md — AI Agent Instructions for WaveHouse **Scope** is the canonical docs-prose set from `scripts/docs-prose.sh` — a *denylist*: every tracked `.md`/`.mdx` EXCEPT `.claude/**`, `.github/**`, `CHAN...
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/architecture.mdAGENTS.mdCHANGELOG.mddocs/src/content/docs/ingest-pipeline.md
🧠 Learnings: 1
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-10-14T20:22:40.258Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse
## Operating Rules
3. **Every code change updates its docs + `CHANGELOG.md` in the same PR** — a code change without its doc update is incomplete ([§Documentation Sync](#documentation-sync)).
🪛 LanguageTool
docs/src/content/docs/architecture.md
[style] ~192-~192: ‘out of reach’ might be wordy. Consider a shorter alternative.
Context: ...75% of the free disk by default) is set out of reach, so a budget is a cap and never a reser...
(EN_WORDINESS_PREMIUM_OUT_OF_REACH)
[style] ~192-~192: Consider using “who” when you are referring to people instead of objects.
Context: ... so the next reload tries the consumers that had not joined again. A publish or park...
(THAT_WHO)
[style] ~192-~192: The serial comma (Oxford comma, Harvard comma) is missing.
Context: ...oker's lock, which every tenant's open, resize and reload takes; a reload retries regardle...
(SERIAL_COMMA_ON)
[style] ~192-~192: For a more expressive style, consider rephrasing the sentence in the active voice.
Context: ...rts the budget last applied in full, so a failed resize is retried by the next reload. The consumers CreateConsumer and `Su...
(PASSIVE_VOICE_SIMPLE)
CHANGELOG.md
[typographical] ~37-~37: Consider using an em dash in dialogues and enumerations.
Context: - **An ingest request's trace continues p...
(DASH_RULE)
[style] ~37-~37: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: - An ingest request's trace continues past the queue to the worker's flush and the ClickHouse INSERT (internal/ingest/trace.go (new, + tests), a call site each in internal/ingest/worker.go's flushGroup and insertToClickHouse, one line each in internal/mq/{embedded,external_consumer}.go, internal/mq/mqtest, internal/testutil/otlp.go, tests/integration/otel_test.go, docs/src/content/docs/{architecture,deployment,ingest-pipeline}.md, AGENTS.md): closes #774, part of the observability epic #777. The publisher wrote the request's tra...
(TOO_LONG_SENTENCE)
docs/src/content/docs/ingest-pipeline.md
[style] ~178-~178: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...ism for resources.** ## Tracing a row The HTTP handler publishes each row under its request's span, and internal/mq writes that span's W3C trace context into the row's queue headers and reads it back onto the delivered message's Ctx, on the worker's consumer path as on the hub's. The worker starts no span per row: a...
(TOO_LONG_SENTENCE)
[grammar] ~178-~178: Ensure spelling is correct
Context: ...or ClickHouse unreachable or timed out, end their span with the error recorded. The...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments: 14
internal/mq/embedded.go: 1
982-982: LGTM!internal/mq/external_consumer.go: 1
15-15: LGTM!Also applies to: 562-562
internal/mq/mqtest/cases.go: 1
218-240: LGTM!internal/mq/mqtest/mqtest.go: 1
89-89: LGTM!docs/src/content/docs/architecture.md: 1
172-172: LGTM!Also applies to: 192-192, 204-204
AGENTS.md: 1
44-44: LGTM!internal/ingest/trace.go: 1
1-108: LGTM!internal/ingest/worker.go: 1
757-759: LGTM!Also applies to: 822-822, 845-847
internal/ingest/trace_test.go: 1
1-282: LGTM!internal/testutil/otlp.go: 1
203-216: LGTM!tests/integration/otel_test.go: 1
24-37: LGTM!Also applies to: 421-514
docs/src/content/docs/deployment.md: 1
768-769: LGTM!docs/src/content/docs/ingest-pipeline.md: 1
17-17: LGTM!Also applies to: 171-171, 176-179
CHANGELOG.md: 1
37-37: LGTM!
Requested by Taite · project thread
Summary
Before: an ingest request's trace ended at the queue publish. The publisher wrote the request's trace context into every row's queue headers, but the worker's consumer path never read it back (only the
Subscribepath did), and the worker started no span, so neither the flush nor the ClickHouseINSERTcould be tied to the request that produced the rows.After: the worker's consumer path extracts the trace context onto each delivered message, in both the embedded and the external NATS broker. Each flush of a batch opens
IngestWorker.Flush, a root span linked to the span of every request whose rows the batch holds (one batch holds many requests' rows, so links rather than a parent; at most 128 linked,producerscounting them all), withtenant,tableandrowsas attributes. EachINSERT, row-by-row isolation included, runs under aclickhouse.insertchild span with the standard database attributes, and the error is recorded when it fails.internal/ingest/trace.go;internal/ingest/worker.gogets only the two call sites (flushGroupandinsertToClickHouse, the latter now with a namederrreturn so the deferred span end sees it), since open PRs feat(typelayer)!: validate, coerce and filter through chtypes #712 and fix(chconn): release a tuple's HTTP clients with its pool #770 edit that file.internal/chconn/chconn.gois untouched.internal/mq/embedded.goandinternal/mq/external_consumer.gochange one line each; the broker conformance suite gainsConsumeCarriesTheTraceContextbeside the existingSubscribecase.tenantrides as a span attribute, never a metric label. The docs note that a flush is a trace of its own, sampled atotel.traces.sample_rateindependently of the requests it links to.Test plan
internal/ingest/trace_test.go: link per distinct producer, the 128 cap, the insert span as the flush span's child with its attributes, error recording, and an end-to-end test through the real embedded queue (publish under a span → consumer → flush links → insert child)mqtestconformance: the consumer path carries the trace context onCreateConsumer's ctx (embedded broker run here; external broker runs under the integration tag)tests/integration/otel_test.goTestOTel_IngestTraceContinuity: through the wired app with a caller'straceparent, the handler span is in the caller's trace, the flush links to it and the insert is the flush's childmake build,go vet ./..., gofumpt, goimports, markdownlint, misspell;-raceunit tests forinternal/ingest,internal/mq/...,internal/observability,internal/apiRelated Issues
Closes #774
🤖 Generated with Claude Code
https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi