Skip to content

feat: Add a PromQL backend setting to ClickHouse PromQL sources - #3601

Open
karl-power wants to merge 5 commits into
mainfrom
claude/hdx-5419-promql-source-backend
Open

karl-power wants to merge 5 commits into
mainfrom
claude/hdx-5419-promql-source-backend

Conversation

@karl-power

Copy link
Copy Markdown
Contributor

Summary

Stop probing ClickHouse on every PromQL query: a PromQL source can now pin how ClickHouse evaluates it, and the probe answer is kept for 5 minutes when it is not pinned.

  • New promqlBackend source setting: auto (the default, unchanged behaviour), clickhouse-http (Prometheus HTTP API) or table-function (prometheusQuery). A pinned source skips the version check and the probe.
  • auto keeps the probe answer per connection for 5 minutes. Inconclusive probes (a timeout, a proxy's 502) are not kept.
  • A source pinned to clickhouse-http on a server without the prometheus_api_v1 handler fails with a clear error instead of a bare 404.
  • New "PromQL backend" select in the PromQL source form, hidden for Prometheus-compatible endpoints.
  • promqlBackend is accepted by the MCP save-source tool and the v2 sources API (docs and openapi.json updated).
  • New GET /connections/:id/prometheus-http-api reports whether a connection serves the HTTP API. Onboarding uses it to pin the PromQL source it creates to clickhouse-http, and leaves it on Auto when the answer is no.
  • Label and label-value lookups still read the table directly. HDX-5420 moves them onto this setting.
  • In auto, a kept "yes" can be stale for up to 5 minutes if the handler is removed. The error then points to Auto.
  • About 380 lines are source and the rest is tests and the regenerated openapi.json. Each commit stands alone, so it reads best commit by commit.

Screenshots or video

Screenshot 2026-11-10 at 13 27 33 Screenshot 2026-11-10 at 13 35 43

How to reproduce

On main:

  1. Open Team settings → Sources → "PromQL". There is no setting for how it is evaluated.
  2. Add a dashboard tile on that source with db_client_connection_count and press Enter to run it.
  3. In docker exec -it hdx-dev-<slot>-ch-server-1 clickhouse-client run SYSTEM FLUSH LOGS and then SELECT count() FROM system.query_log WHERE query LIKE 'SELECT version()%' AND query_id LIKE 'hdx-api-%' AND event_time > now() - INTERVAL 1 MINUTE. Re-run the tile with a different time range a few times and repeat. The count grows with every query.

On this branch:

  1. Repeat the steps above with the setting left on Auto. The count rises by at most 1 per 5 minutes however often you re-run the tile.
  2. Set "PromQL backend" to "Prometheus HTTP API", save and re-run the tile. No new hdx-api- version checks appear, and the query log shows WITH prometheus_query_step_% queries.
  3. Set it to "Table functions" and re-run. The query log shows prometheusQuery queries instead.
  4. Point a source at a ClickHouse server without the prometheus_api_v1 handler, pin it to "Prometheus HTTP API" and run a tile. The chart shows "This ClickHouse connection does not serve the Prometheus HTTP API…".

How to test on Vercel preview

N/A. PromQL queries go through the API, which the Vercel preview (local mode) does not have.

References

@karl-power
karl-power requested a review from a team as a code owner November 10, 2026 12:40
@vercel

vercel Bot commented Nov 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
hyperdx-oss Ready Ready Preview Nov 10, 2026 12:40pm UTC
hyperdx-storybook Ready Ready Preview Nov 10, 2026 12:40pm UTC

Request Review

@changeset-bot

changeset-bot Bot commented Nov 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: f768d99

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@hyperdx/api Patch
@hyperdx/app Patch
@hyperdx/common-utils Patch
@hyperdx/otel-collector Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@karl-power karl-power changed the title Claude/hdx 5419 promql source backend feat: Add a PromQL backend setting to ClickHouse PromQL sources Nov 10, 2026
@github-actions github-actions Bot added the review/tier-4 Critical — deep review + domain expert sign-off label Nov 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔴 Tier 4 — Critical

Touches authentication, tenancy data models, the public API or shipped database config — or substantially changes the query rendering engine, background tasks, the OTel pipeline, image build, or release CI.

Why this tier:

  • Critical-path files (1) — tenancy, public API, or shipped database config:
    • packages/api/src/routers/external-api/v2/sources.ts
  • Cross-layer change: touches frontend (packages/app) + backend (packages/api) + shared utils (packages/common-utils)

Review process: Deep review from a domain expert. Synchronous walkthrough may be required.
SLA: Schedule synchronous review within 2 business days.

Stats
  • Production files changed: 16
  • Production lines changed: 381 (+ 969 in test files, excluded from tier calculation)
  • Critical-path lines changed: 5
  • Branch: claude/hdx-5419-promql-source-backend
  • Author: karl-power

To override this classification, remove the review/tier-4 label and apply a different review/tier-* label. Manual overrides are preserved on subsequent pushes.

@github-actions

Copy link
Copy Markdown
Contributor

Duplicate code analysis

No new duplicate code introduced by this PR.


jscpd compared this branch against origin/main
across app, api and common-utils. Only new clones are listed; the codebase
currently holds 110 in total (1.20%).
Run yarn dupes locally to reproduce, or yarn dupes:all for the full picture.

@github-actions

Copy link
Copy Markdown
Contributor

PR Review

✅ No issues found.

@github-actions

Copy link
Copy Markdown
Contributor

Deep Review

✅ No critical issues found.

The PromQL-backend setting, the 5-minute probe cache, the new authenticated endpoint, and the onboarding auto-pin were verified end to end. Auth and team-scoping on GET /connections/:id/prometheus-http-api are correct (the decrypted connection is used only for the probe and never serialized — the response is strictly {supported}), the boolean | undefined return-type change has a single caller that handles undefined, the promqlBackend field round-trips through the v2 read path (toExternalSource parses with SourceSchema, which now carries the key), inconclusive probes are not cached, and pinned backends skip the probe. Test coverage for the cache (hit, inconclusive-not-kept, TTL expiry, per-host keying, concurrent sharing, pinned-skips-probe) and the error paths is thorough.

🟡 P2 — recommended

  • packages/api/src/controllers/promql.ts:529 — the probe reads the response with unbounded await resp.text(), unlike the query path in the same file which caps the body via readBoundedBody(resp, PROMETHEUS_MAX_RESPONSE_BYTES); a member-configured host returning a very large body within the 5s window could spike API-process memory.
    • Fix: read the probe response through readBoundedBody with a small byte cap instead of resp.text().
🔵 P3 nitpicks (2)
  • packages/api/src/routers/api/connections.ts:125 — the new GET /connections/:id/prometheus-http-api probe is reachable only from the session-cookie UI router; an agent choosing a backend via the MCP save_source tool or the v2 API cannot read the same signal and must fall back to auto.
    • Fix: expose the probe to agents via an MCP tool or a v2 endpoint backed by the already-exported connectionServesPrometheusHttpApi.
    • agent-native
  • packages/api/src/controllers/promql.ts:551 — backendProbeCache evicts only lazily on read, so editing a connection's host switches to a new id|host key and leaves the old entry resident until (never) re-read; growth is bounded by rare admin edits and each entry is tiny, so this is a minor unreclaimed-memory note, not a leak under traffic.
    • Fix: optionally key the cache on connection id alone, or prune prior-host entries when the host changes.
    • performance

Reviewers (6): correctness, security, performance, reliability/adversarial (orchestrator-verified), agent-native, learnings-researcher.

Testing gaps:

  • No test exercises the probe against an oversized response body (the P2 above is unguarded and untested).
  • Cross-team cache isolation is not tested explicitly; it is sound because the key embeds the globally unique connection id.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond

@github-actions

Copy link
Copy Markdown
Contributor

E2E Test Results

✅ All tests passed • 464 passed • 1 skipped • 1026s

Status Count
✅ Passed 464
❌ Failed 0
⚠️ Flaky 2
⏭️ Skipped 1

Tests ran across 4 shards in parallel.

View full report →

This branch was successfully deployed

2 active deployments
Preview – hyperdx-oss — f768d998 Deployed Nov 10, 2026 by vercel[bot]
Preview – hyperdx-storybook — f768d998 Deployed Nov 10, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review/tier-4 Critical — deep review + domain expert sign-off

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant