Skip to content

fix(observability): continue ingest traces into the worker and insert - #842

Merged
taitelee merged 6 commits into
mainfrom
claude/774-ingest-trace-continuity-6z0vca
Oct 14, 2026
Merged

taitelee merged 6 commits into
mainfrom
claude/774-ingest-trace-continuity-6z0vca

Conversation

@taitelee

@taitelee taitelee commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

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 Subscribe path did), and the worker started no span, so neither the flush nor the ClickHouse INSERT could 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, producers counting them all), with tenant, table and rows as attributes. Each INSERT, row-by-row isolation included, runs under a clickhouse.insert child span with the standard database attributes, and the error is recorded when it fails.

  • New code lives in internal/ingest/trace.go; internal/ingest/worker.go gets only the two call sites (flushGroup and insertToClickHouse, the latter now with a named err return 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.go is untouched.
  • internal/mq/embedded.go and internal/mq/external_consumer.go change one line each; the broker conformance suite gains ConsumeCarriesTheTraceContext beside the existing Subscribe case.
  • tenant rides as a span attribute, never a metric label. The docs note that a flush is a trace of its own, sampled at otel.traces.sample_rate independently of the requests it links to.
  • Query, pipe and ops ClickHouse spans mentioned in the issue are out of scope here.

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)
  • mqtest conformance: the consumer path carries the trace context on CreateConsumer's ctx (embedded broker run here; external broker runs under the integration tag)
  • tests/integration/otel_test.go TestOTel_IngestTraceContinuity: through the wired app with a caller's traceparent, the handler span is in the caller's trace, the flush links to it and the insert is the flush's child
  • make build, go vet ./..., gofumpt, goimports, markdownlint, misspell; -race unit tests for internal/ingest, internal/mq/..., internal/observability, internal/api
  • GitHub CI green on the head commit: Lint, unit, integration (app and backends), e2e, coverage, docs build

Related Issues

Closes #774

🤖 Generated with Claude Code

https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi

taitelee and others added 2 commits October 13, 2026 19:10
…ng on the ingest trace link

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011Da6nRMNGVuCzVmpf2G2Pi
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

📝 Summary

Summary by CodeRabbit

  • New Features
    • Distributed traces now follow ingest requests through queued messages, worker processing, and ClickHouse inserts, making it easier to investigate a request across the pipeline.
    • Worker flushes are linked to the requests that contributed data, including batches from multiple requests. Traces also include details about the tenant, table, and row counts, and record insert failures.
  • Documentation
    • Updated observability and architecture guides with details on tracing across the ingest pipeline, including sampling behavior.

The change extracts trace context on worker message-consumption paths. Ingest workers create flush spans linked to producer spans and child spans for ClickHouse inserts. Tests and documentation cover context propagation, span attributes, error recording, and trace continuity.

Changes: Ingest trace continuity

Layer / File(s) Summary
Extract trace context from consumed messages
internal/mq/embedded.go, internal/mq/external_consumer.go, internal/mq/mqtest/*, docs/src/content/docs/architecture.md, AGENTS.md
Both consumer implementations extract trace context from message headers. MQ conformance coverage checks that the delivered context retains the trace identifiers and worker context value.
Create worker flush and insert spans
internal/ingest/trace.go, internal/ingest/worker.go, internal/ingest/trace_test.go, docs/src/content/docs/ingest-pipeline.md
Flush spans link to distinct producer contexts, up to 128 links, and report the producer count. ClickHouse insert spans are children of flush spans. Both span types record errors. Tests cover span attributes, links, parentage, errors, and queue-to-worker tracing.
Verify and document trace continuity
internal/testutil/otlp.go, tests/integration/otel_test.go, docs/src/content/docs/architecture.md, docs/src/content/docs/deployment.md, CHANGELOG.md
The integration test checks the handler span, root flush span, producer link, and child insert span. Documentation and the changelog describe the propagation path, span attributes, sampling, and error recording.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant IngestHandler.Handle
  participant MessageBroker
  participant workerConsumer
  participant IngestWorker.Flush
  participant ClickHouse
  IngestHandler.Handle->>MessageBroker: Publish rows with trace headers
  MessageBroker->>workerConsumer: Deliver messages with headers
  workerConsumer->>IngestWorker.Flush: Extract trace context onto message contexts
  IngestWorker.Flush->>IngestWorker.Flush: Start root span with producer links
  IngestWorker.Flush->>ClickHouse: Execute INSERT with child clickhouse.insert span
Loading
Merge Risk: 🔵 Low · up to b5907

A narrow settings-reload race can leave a failed insert without its expected insert span. The failure remains visible on the flush span, so this is mergeable with a targeted fix or accepted follow-up.

Pre-merge checks: Passed 2 · Failed 3

Failed Failed checks: 3 warnings

Check name Status Explanation Resolution
Linked Issues check Warning Issue #774 is open and directly linked. The PR implements the ingest path: queue consumer extraction, linked IngestWorker.Flush spans, child clickhouse.insert spans, error recording, and ingest co… Implement query and pipe tracing for their ClickHouse calls. Add automated continuity tests that fail when the query or pipe trace stops before the ClickHouse call.
Out of Scope Changes check Warning The ingest code, broker conformance tests, OTLP test helper, integration test, changelog, and ingest documentation support the stated ingest-tracing scope. AGENTS.md also changes the observability d… Revert the unrelated PprofHandler profiling-listener change in AGENTS.md. Keep only the trace-context documentation that supports the ingest tracing objective.
Docstring Coverage Warning Docstring coverage is 73.68% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 9 files. (5 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 2 passed
Check name Status Explanation
Title check Passed The title clearly and concisely summarizes the main change: continuing ingest traces through the worker and ClickHouse insert.
Description check Passed The description directly explains the trace propagation changes, span behavior, scope, and test coverage.
Full details: Linked Issues check

Explanation

Issue #774 is open and directly linked. The PR implements the ingest path: queue consumer extraction, linked IngestWorker.Flush spans, child clickhouse.insert spans, error recording, and ingest continuity tests. Issue #774 also requires query or pipe traces to include their ClickHouse call. The PR explicitly excludes query and pipe tracing, and the changed-file summary shows no implementation or tests for that requirement.

Full details: Out of Scope Changes check

Explanation

The ingest code, broker conformance tests, OTLP test helper, integration test, changelog, and ingest documentation support the stated ingest-tracing scope. AGENTS.md also changes the observability description to add PprofHandler on a dedicated profiling listener. That profiling-listener change has no demonstrated connection to issue #774 or ingest trace continuity.

Full details: Docstring Coverage

Explanation

Docstring coverage is 73.68% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 9 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added documentation Improvements or additions to documentation go Pull requests that update go code area/ingest Ingest pipeline (Bento, batching, DLQ) area/docs Documentation, site/, README labels Oct 13, 2026
@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

📚 Docs preview

✅ Live: https://pr-842-wavehouse-docs.wave-rf.workers.dev (follows this PR)
This commit (b59076f): https://381eecaf-wavehouse-docs.wave-rf.workers.dev (exactly this commit; open it if the link above looks stale)

  • Commit: b59076f test(ingest): import tenant from pkg/tenant after its move on main
  • Author: @taitelee, Claude Fable 5.1
  • Committed: 2026-10-14 16:04 EDT
  • Uploaded: 2026-10-14 16:16 EDT (run)
Earlier commits (3)
Commit Preview (that commit only) Uploaded
c093b48 https://d98c9d0b-wavehouse-docs.wave-rf.workers.dev 2026-10-13 15:28 EDT
5de953f https://cb5ffb1c-wavehouse-docs.wave-rf.workers.dev 2026-10-13 15:22 EDT
7d39933 https://77e189fa-wavehouse-docs.wave-rf.workers.dev 2026-10-13 15:18 EDT

taitelee and others added 2 commits October 13, 2026 19:18
…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
@github-code-quality

github-code-quality Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Go

Go

main 731d54e claude/774-ingest-tr... b59076f +/-
93% 93% 0%
View coverage of the 6 most impacted files.
File main 731d54e claude/774-ingest-tr... b59076f +/-
internal/mq/embedded.go 90% 90% 0%
internal/ingest/worker.go 97% 97% 0%
internal/mq/mqtest/cases.go 96% 96% 0%
internal/mq/mqtest/mqtest.go 100% 100% 0%
internal/mq/nats_topology.go 91% 92% +1%
internal/ingest/trace.go 0% 100% +100%

ℹ️ Calculations represent line coverage. Results may change as calculations are updated.


Updated October 14, 2026 20:17 UTC

taitelee and others added 2 commits October 14, 2026 19:59
…ce-continuity-6z0vca

# Conflicts:
#	AGENTS.md
#	CHANGELOG.md
#	docs/src/content/docs/architecture.md
@taitelee
taitelee marked this pull request as ready for review October 14, 2026 20:13
@taitelee
taitelee requested review from a team and EricAndrechek October 14, 2026 20:13
@taitelee

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@taitelee

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: f017de32-fa60-4bc3-b90d-782d9d592581
📥 Commits

Reviewing files that changed from the base of the PR and between 731d54e and b59076f.

📒 Files selected for processing: 14
  • AGENTS.md
  • CHANGELOG.md
  • docs/src/content/docs/architecture.md
  • docs/src/content/docs/deployment.md
  • docs/src/content/docs/ingest-pipeline.md
  • internal/ingest/trace.go
  • internal/ingest/trace_test.go
  • internal/ingest/worker.go
  • internal/mq/embedded.go
  • internal/mq/external_consumer.go
  • internal/mq/mqtest/cases.go
  • internal/mq/mqtest/mqtest.go
  • internal/testutil/otlp.go
  • tests/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.md
  • docs/src/content/docs/architecture.md
  • docs/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.go
  • internal/mq/embedded.go
  • internal/mq/external_consumer.go
  • internal/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.go
  • internal/mq/embedded.go
  • internal/testutil/otlp.go
  • internal/mq/external_consumer.go
  • internal/mq/mqtest/cases.go
  • docs/src/content/docs/deployment.md
  • docs/src/content/docs/architecture.md
  • internal/ingest/worker.go
  • AGENTS.md
  • CHANGELOG.md
  • internal/ingest/trace.go
  • tests/integration/otel_test.go
  • docs/src/content/docs/ingest-pipeline.md
  • internal/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.go
  • internal/mq/embedded.go
  • internal/testutil/otlp.go
  • internal/mq/external_consumer.go
  • internal/mq/mqtest/cases.go
  • internal/ingest/worker.go
  • internal/ingest/trace.go
  • internal/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.md
  • docs/src/content/docs/architecture.md
  • AGENTS.md
  • CHANGELOG.md
  • docs/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!

Comment thread internal/ingest/worker.go
@github-project-automation github-project-automation Bot moved this from Backlog to In review in WaveHouse Task Board Oct 14, 2026
@taitelee
taitelee merged commit 5d20684 into main Oct 14, 2026
35 checks passed
@taitelee
taitelee deleted the claude/774-ingest-trace-continuity-6z0vca branch October 14, 2026 20:38
@github-project-automation github-project-automation Bot moved this from In review to Done in WaveHouse Task Board Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/docs Documentation, site/, README area/ingest Ingest pipeline (Bento, batching, DLQ) documentation Improvements or additions to documentation go Pull requests that update go code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

bug(observability): ingest traces end at the queue publish, and nothing tests trace continuity

1 participant