Skip to content

feat(observability): time HTTP requests and query-path ClickHouse calls - #866

Merged
taitelee merged 6 commits into
mainfrom
claude/metrics-http-query-tier-fh85bw
Oct 20, 2026
Merged

taitelee merged 6 commits into
mainfrom
claude/metrics-http-query-tier-fh85bw

Conversation

@taitelee

Copy link
Copy Markdown
Member

Requested by Taite · project thread

Summary

The first tier of #94. Before this, the only request metrics came from otelhttp. Those carry no route, so a slow or failing endpoint could not be told apart from the rest. Nothing timed the statements the query paths send to ClickHouse.

  • wavehouse_http_request_duration_seconds{route, method, status_class} times every request on server.port except SSE streams. The middleware lives in newProbeRouter, so both the API router and the ops-only router get it. It sits outside jsonRecoverer, so a panic counts as a 5xx. route is the chi pattern (/v1/pipes/{name}), never the path. It is the mount's own pattern (/v1/*, /v1/ops/*) for a request a mount answers short of a route, and unmatched for one that reached no mount. method is net/http's or other.
  • wavehouse_clickhouse_query_duration_seconds{operation, result} times each statement sent by POST /v1/query (query), a pipe (pipe, pipe_write) or POST /v1/ops/query (ops_query). result is ok, canceled (the client went away), or the response's error code without its clickhouse. prefix, through the same chFailureOf the response uses. It is recorded inside the singleflight closure, so a cache hit records nothing and coalesced requests record once.
  • A histogram's _count gives both the rate and the error rate, so there is no separate _errors_total. Neither metric carries a tenant or table label.
  • The docs are in deployment.md § Request latency and ClickHouse queries and in architecture.md. The CHANGELOG and AGENTS.md are updated too.

This touches router.go (one line), which #855 also edits. It should merge after #855.

Test plan

  • internal/api/metrics_test.go covers:
    • Route-pattern labeling: pattern not path, /v1/*, /v1/ops/*, unmatched. Panic as 5xx, /v1/stream skipped.
    • The ops router's wiring.
    • Every row of the ClickHouse error matrix, across query, pipe and pipe_write.
    • The proxy's ok, rejected and response-cap paths, plus a refused request recording nothing.
    • Client gone vs. the time-cap backstop.
    • Nil no-op.
  • go test -race ./internal/api/ ./internal/app/ and golangci-lint on ./internal/api/...
  • GitHub CI

Related Issues

Part of #94, part of #777. The worker's INSERT timing, the next tier, waits on #770.


Generated by Claude Code

taitelee and others added 5 commits October 20, 2026 01:28
wavehouse_http_request_duration_seconds{route, method, status_class}
times every request but the SSE streams, by route pattern, on every
listener. wavehouse_clickhouse_query_duration_seconds{operation, result}
times each statement /v1/query, the pipes and /v1/ops/query send, with
result ok or the response's error code. First tier of #94.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YVNGPfArHqjc3Xt6N9App
…ailable

Also correct where a tenant refusal's route label lands and that only
server.port is timed, per pre-push review.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YVNGPfArHqjc3Xt6N9App
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YVNGPfArHqjc3Xt6N9App
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YVNGPfArHqjc3Xt6N9App
…he unmatched sentence

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019YVNGPfArHqjc3Xt6N9App
@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 977c4020-b5a1-4841-812f-8e7ff2bef0e2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 688d35e7-0423-4364-8781-65159fadf212
📥 Commits

Reviewing files that changed from the base of the PR and between 4ef3ae1 and 7628776.

📒 Files selected for processing: 10
  • AGENTS.md
  • CHANGELOG.md
  • docs/src/content/docs/architecture.md
  • docs/src/content/docs/deployment.md
  • internal/api/metrics.go
  • internal/api/metrics_test.go
  • internal/api/pipes.go
  • internal/api/query.go
  • internal/api/router.go
  • internal/api/structured_query.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.

📜 Recent review details
🧰 Additional context used
📚 Code guidelines: 4
.github/prompts/docs-review.md — auto-discovered
AGENTS.md — auto-discovered
.github/copilot-instructions.md — auto-discovered
CONTRIBUTING.md — auto-discovered
📓 Path-based instructions: 9
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...

📄 CodeRabbit inference engine (.github/prompts/docs-review.md)

Files:

  • docs/src/content/docs/deployment.md
  • docs/src/content/docs/architecture.md
Source excerpt: **Bearer-token-only CORS posture (security)** — Bearer JWT on every request, no cookies/sessions; `corsMiddleware` deliberately **never** emits `Access-Control-Allow-Credentials` (not needed, and `*` + credentials is a spec...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • internal/api/router.go
Source excerpt: **Schema-driven ingest** — `POST /v1/ingest?table={table}` takes flat JSON, validated against the discovered schema (unknown fields rejected, types/nullability enforced).

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • internal/api/router.go
  • internal/api/structured_query.go
  • internal/api/query.go
  • internal/api/pipes.go
  • internal/api/metrics.go
  • internal/api/metrics_test.go
Source excerpt: **Table-driven tests**: Use `tests := []struct{ name string; ...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • internal/api/metrics_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
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
Source excerpt: **WH001 applies to every tracked Markdown file, with no carve-out** — `AGENTS.md`, `CHANGELOG.md`, `.github/` CI docs and `.claude/` agent prompts included.

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • docs/src/content/docs/deployment.md
  • docs/src/content/docs/architecture.md
  • AGENTS.md
  • CHANGELOG.md
Source excerpt: **Go 1.26**, strict formatting (`gofumpt`, enforced by CI) Source excerpt: **Structured logging** with `log/slog` (JSON handler), through the default logger: call `slog.InfoContext(ctx, …)` and its siblings (the context carr...

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • internal/api/router.go
  • internal/api/structured_query.go
  • internal/api/query.go
  • internal/api/pipes.go
  • internal/api/metrics.go
  • internal/api/metrics_test.go
Source excerpt: **Comment the *why*, not the *what*.**

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • internal/api/router.go
  • docs/src/content/docs/deployment.md
  • docs/src/content/docs/architecture.md
  • internal/api/structured_query.go
  • AGENTS.md
  • CHANGELOG.md
  • internal/api/query.go
  • internal/api/pipes.go
  • internal/api/metrics.go
  • internal/api/metrics_test.go
🧠 Learnings: 2
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse

Timestamp: 2026-10-20T06:38:30.379Z
Learning: Source excerpt:
# AGENTS.md — AI Agent Instructions for WaveHouse

## Documentation Sync

| 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), `pkg/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 / event format | `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md` (CH schema) |
| Change deployment / Docker | ...
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).

Applied to files:

  • internal/api/metrics_test.go
🪛 LanguageTool
AGENTS.md

[typographical] ~31-~31: Consider using an em dash in dialogues and enumerations.
Context: - api/ — Chi HTTP router, JWT/JWKS ...

(DASH_RULE)

CHANGELOG.md

[typographical] ~13-~13: Consider using an em dash in dialogues and enumerations.
Context: - **HTTP request latency and query-path C...

(DASH_RULE)

[style] ~13-~13: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...hat the query paths sent to ClickHouse. wavehouse_http_request_duration_seconds{route, method, status_class} times every request on server.port but the SSE streams, under the route's pattern (/v1/pipes/{name}), never the path; wavehouse_clickhouse_query_duration_seconds{operation, result} times each statement POST /v1/query, a pipe (pipe, pipe_write) or POST /v1/ops/query sends, with result ok, canceled (the client went away) or the response's error code without its clickhouse. prefix, so a histogram's _count is both the rate and the error rate. A cache hit records no query, and reque...

(TOO_LONG_SENTENCE)

🔇 Additional comments: 11
internal/api/metrics.go: 1

1-175: LGTM!

internal/api/router.go: 1

168-168: LGTM!

internal/api/metrics_test.go: 1

1-255: LGTM!

internal/api/pipes.go: 1

47-53: LGTM!

Also applies to: 196-196, 227-227, 239-247

internal/api/query.go: 1

73-75: LGTM!

Also applies to: 130-130, 269-274, 287-294, 311-319

internal/api/structured_query.go: 1

51-53: LGTM!

Also applies to: 73-73, 260-260

AGENTS.md: 1

31-31: LGTM!

CHANGELOG.md: 1

13-14: LGTM!

docs/src/content/docs/architecture.md: 1

90-90: LGTM!

docs/src/content/docs/deployment.md: 2

845-861: LGTM!

838-843: 🎯 Functional Correctness

The finding is refuted. The configuration section does not enumerate exact label names. It permits labels from a closed set, statuses, and error classes. route, method, and operation are closed sets; status_class is a status class; and result contains fixed statuses and error classes. These labels follow the documented rule, so no configuration change is required.

📝 Summary

Summary by CodeRabbit

  • New Features

    • Added HTTP request-latency metrics, including route, method, and status classifications.
    • Added ClickHouse query-duration metrics covering successful, canceled, and failed operations.
    • Added visibility into query outcomes across standard, streaming, proxy, and deferred operations.
  • Documentation

    • Documented metric labels, exclusions, latency buckets, query-result classifications, and PromQL examples for monitoring errors, unavailable queries, and p99 latency.

The API now records HTTP request and ClickHouse query durations as OpenTelemetry histograms. The metrics include route, method, status, operation, and result labels. Documentation describes the classifications, exclusions, and PromQL queries.

Changes: API metrics

Layer / File(s) Summary
Metric instruments and HTTP collection
internal/api/metrics.go, internal/api/router.go, internal/api/metrics_test.go
Defines HTTP and ClickHouse duration histograms and result classifications. Adds middleware to record routed HTTP request durations, excluding /v1/stream. Tests cover route and method labels, status classes, unmatched routes, stream exclusion, and router instrumentation.
Query handler recording and observability documentation
internal/api/pipes.go, internal/api/query.go, internal/api/structured_query.go, internal/api/metrics_test.go, AGENTS.md, CHANGELOG.md, docs/src/content/docs/architecture.md, docs/src/content/docs/deployment.md
Initializes query metrics in pipe, query, and structured-query handlers. Records query durations and outcomes, including errors and response-size limits. Tests cover operation results, proxy outcomes, nil-safe behavior, and cancellation classification. Documentation describes the metric labels, exclusions, result categories, and PromQL examples.

Priority: ⬇️ Low

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

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant QueryHandler
  participant ClickHouse
  participant queryMetrics
  participant OpenTelemetry
  QueryHandler->>ClickHouse: execute query
  ClickHouse-->>QueryHandler: return response or error
  QueryHandler->>queryMetrics: observe duration and outcome
  queryMetrics->>OpenTelemetry: record histogram
Loading

Suggested reviewers: ericandrechek

Merge Risk: ⚪ Minimal · up to 76287

The latency metrics appear ready to merge after normal checks; no material issue was identified.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main changes: HTTP request timing and ClickHouse query-path timing for observability.
Description check Passed The description directly explains the new HTTP and ClickHouse metrics, their labels, exclusions, behavior, documentation, and test coverage.
Docstring Coverage Passed Docstring coverage is 85.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 6 files. (4 skipped: 4 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 85.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 20 functions across 6 files. (4 skipped: 4 unsupported.)

✨ Finishing Touches
📝 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/observability Metrics, logs, traces, health, profiling area/api HTTP handlers, routing, middleware area/docs Documentation, site/, README labels Oct 20, 2026
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

📚 Docs preview

✅ Live: https://pr-866-wavehouse-docs.wave-rf.workers.dev (follows this PR)
This commit (24fe1b7): https://a1ffdfe2-wavehouse-docs.wave-rf.workers.dev (exactly this commit; open it if the link above looks stale)

  • Commit: 24fe1b7 Merge remote-tracking branch 'origin/main' into claude/metrics-http-query-tier-fh85bw
  • Author: @taitelee
  • Committed: 2026-10-20 09:09 EDT
  • Uploaded: 2026-10-20 09:52 EDT (run)
Earlier commits (1)
Commit Preview (that commit only) Uploaded
7628776 https://8b8810d4-wavehouse-docs.wave-rf.workers.dev 2026-10-19 21:44 EDT

@github-code-quality

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

Copy link
Copy Markdown
Contributor

Code coverage overview

Languages: Go

Go

main 6179c67 claude/metrics-http-... 24fe1b7 +/-
93% 93% 0%
View coverage of the 8 most impacted files.
File main 6179c67 claude/metrics-http-... 24fe1b7 +/-
internal/mq/external.go 86% 85% -1%
internal/mq/embedded.go 90% 90% 0%
internal/ingest/worker.go 97% 97% 0%
internal/api/st...ctured_query.go 98% 98% 0%
internal/api/router.go 99% 99% 0%
internal/api/pipes.go 99% 99% 0%
internal/api/query.go 89% 90% +1%
internal/api/metrics.go 0% 100% +100%

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


Updated October 20, 2026 13:54 UTC

Copy link
Copy Markdown
Member Author

@coderabbitai review


Generated by Claude Code

@coderabbitai

coderabbitai Bot commented Oct 20, 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[bot]
coderabbitai Bot previously approved these changes Oct 20, 2026
…uery-tier-fh85bw

# Conflicts:
#	internal/api/router.go
@taitelee
taitelee marked this pull request as ready for review October 20, 2026 13:50
@taitelee
taitelee requested review from a team and EricAndrechek October 20, 2026 13:50
@taitelee
taitelee merged commit 9703e86 into main Oct 20, 2026
@taitelee
taitelee deleted the claude/metrics-http-query-tier-fh85bw branch October 20, 2026 14:13
@github-project-automation github-project-automation Bot moved this from Backlog to Done in WaveHouse Task Board Oct 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api HTTP handlers, routing, middleware area/docs Documentation, site/, README area/observability Metrics, logs, traces, health, profiling documentation Improvements or additions to documentation go Pull requests that update go code

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

1 participant