fix(mcp): list PromQL catalog names from the tags table, without a time window by bcanvural · Pull Request #3669 · ClickHouse/ClickStack

bcanvural · 2026-11-14T07:33:00Z

Summary

Fixes #3648 (parts 1 and 2).

clickstack_describe_source on a PromQL source listed label and metric names for the last 24 hours. On a ClickHouse TimeSeries table, that window costs a pass over every series id, whatever the window's length: the time ranges GROUP BY id on the SQL path, and a join with every series on ClickHouse's Prometheus HTTP API. On a table with about 217M series every call failed at an 8 GiB memory limit, so agents had no way to find metric names.

This PR:

reads metric names from the tags table alone (SELECT DISTINCT metric_name FROM timeSeriesTags(...)), with no time window, for every ClickHouse PromQL source, whichever backend evaluates its queries. The result notes that the names include series with no recent samples;
adds an optional metricName argument: on a ClickHouse table it lists that metric's label names with metric_name = …, which uses the tags table's sort key. Without it, a ClickHouse source returns metric names only, with a note on how to get label names;
leaves a Prometheus endpoint source as it was (window and /labels), and passes metricName to it as match[]={__name__="…"}.

The label routes (/labels, /label/:name/values) are unchanged, including the HTTP API path from #3628.

ClickHouse #127095 (open) rewrites the handler's label endpoints to read the tags table directly. Once it ships, the catalog could go back to the HTTP API, still without a window: with start/end and no match[], it joins the time ranges table as well.

Tests

promqlSources.int.test.ts: metric names come from the whole table (a series two days old is listed); metricName lists one metric's label names; a Prometheus endpoint gets metricName as match[]; an auto source on a server whose handler serves label lookups still reads names with SQL. The three tests that checked catalog lookups through the handler are replaced by that one.
PromQL int suites (promqlSources, timeseriesEngine, prometheus, promqlQueryTool): 208/208 on ClickHouse 26.10.3.7.
The repro test from PromQL describe_source can't list metric or label names on a large TimeSeries table: both catalog paths read every series id in the table #3648 with 2M series: a describe_source call takes 11–23 ms (was 656 ms on the SQL path and 3.2 s through the handler). The metric-name query uses 4.3 MiB (was 316 MiB and 811 MiB).
eslint, prettier and the ratchet are clean. tsc reports one error in checkAlerts/template.ts, which is already on main.

How to reproduce

Starting point: yarn dev with ClickHouse 26.10, and a PromQL source on a TimeSeries table with many series (or the repro test in #3648).

On main:

Call clickstack_describe_source on the source: on a large table it fails with a memory limit error (partial: true, no names).

On this branch:

The same call returns the metric names.
With metricName, it returns that metric's label names.

References

Issue: PromQL describe_source can't list metric or label names on a large TimeSeries table: both catalog paths read every series id in the table #3648
Related: feat(api): serve PromQL label lookups from ClickHouse's Prometheus HTTP API #3628 / feat(mcp): read PromQL label catalog from ClickHouse's Prometheus HTTP API #3630 (catalog through the HTTP API), PromQL label lookups with match[] read the whole TimeSeries tags table #3602 / perf(api): read only the matched metrics' tags rows in PromQL label lookups #3603 (match[] pins metric_name), ClickHouse #127095

…me window describe_source listed a PromQL source's names for the last 24 hours. On a ClickHouse TimeSeries table the window costs a pass over every series id (the time ranges GROUP BY on the SQL path, a join on the HTTP API), which runs out of memory on large tables. Read metric names from the tags table alone, and a metric's label names with the new metricName argument.

changeset-bot · 2026-11-14T07:33:04Z

🦋 Changeset detected

Latest commit: b34bee0

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

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/api	Patch
@hyperdx/app	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

vercel · 2026-11-14T07:33:06Z

@bcanvural is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2026-11-14T07:34:33Z

+  'client' | 'databaseName' | 'tableName' | 'limit' | 'signal'
+> & { metricName?: string }): Promise<string[]> {
+  const metric = chSql`${{ Identifier: 'metric_name' }}`;
+  if (metricName == null)


🔵 minor — queryTimeSeriesNames copies two queries that queryLabelValues and queryLabelNames in this file already run

Without metricName, this runs the same SELECT DISTINCT metric_name … ORDER BY val LIMIT n as queryLabelValues({ labelName: '__name__' }) with no startMs/endMs/match, because getSeriesFilterConditions returns [] when none of them is given (line 164). The arrayJoin(arrayConcat(['__name__'], mapKeys(tags))) expression is also copied word for word from queryLabelNames (line 319). Call queryLabelValues without a window to get the metric names. For the per-metric label names, let queryLabelNames take extra conditions, or pull the expression into a shared constant, so the label-name SQL has one source of truth.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Fixed in be348cb: the catalog calls queryLabelValues({ labelName: '__name__' }) without a window, and queryLabelNames takes an optional metricName. The new function is gone.

github-actions · 2026-11-14T07:34:33Z

    from: { databaseName: string; tableName: string };
-    promqlBackend?: PromqlSourceBackend;
  };
  startMs: number;


🔵 minor — Callers must still pass startMs/endMs to getPromqlLabelCatalog, but ClickHouse sources now ignore them

After this change only the isPrometheusEndpoint branch reads the window. Make startMs/endMs optional, or have the Prometheus caller compute them, so the signature no longer implies that ClickHouse sources are filtered by time. describePromqlSource.ts still computes LABEL_LOOKBACK_MS and passes it for both backends.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Not changing: the doc comment on getPromqlLabelCatalog says only a Prometheus endpoint answers for the window, and its only caller makes one call for both backends.

github-actions · 2026-11-14T07:34:35Z

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.

🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.

Anything about surrounding code, or asking you to widen the change (hoist a helper,
dedupe with another file, fix other call sites): never fix here, whatever the
severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

1 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 1 minor

1 posted as inline comment(s) on the changed lines.

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

github-actions · 2026-11-14T07:40:05Z

Deep Review

✅ No critical issues found. This is a well-scoped, well-tested fix. The highest-risk dimensions came back clean: the user-supplied metricName reaches ClickHouse as a bound {String} query param (never interpolated) and Prometheus as a percent-encoded, JSON.stringify-escaped match[] value, so no injection is introduced; the window-removal is the correct cure for the OOM and is memory-bounded by metric-name cardinality; and every prior review thread is addressed in the current diff.

The one point worth tracking is a scalability caveat, not a defect: the fix's core promise — that SELECT DISTINCT metric_name FROM timeSeriesTags(...) ORDER BY val LIMIT 201 short-circuits instead of scanning all series — depends on ClickHouse's optimize_distinct_in_order recognizing the tags-table sort key through the timeSeriesTags() table function. It stays memory-bounded and max_execution_time-capped regardless, but confirming the plan short-circuits via EXPLAIN on a large table would validate the headline improvement. The behavioral correctness of window removal is itself regression-protected: promqlSources.int.test.ts seeds a 2–3 day-old stale_metric series and asserts it appears, which would fail if a time window were reintroduced.

🔵 P3 nitpicks (2)

packages/api/src/mcp/tools/sources/describeSource.ts:94 — metricName is accepted by the tool schema for every source kind but is only threaded into the PromQL path, so passing it to a logs/traces/metrics source is silently ignored with no feedback to the calling agent.
- Fix: Note in the metricName argument description that it is ignored for non-PromQL sources so an agent can tell the argument had no effect.
packages/api/src/mcp/tools/sources/describePromqlSource.ts:72 — on a Prometheus endpoint the No series has the metric name "…" note fires whenever the windowed /labels lookup returns empty, which also happens for a metric that merely has no samples in the lookback window, so the note can overstate absence.
- Fix: Qualify the note for the Prometheus path to reflect the lookback window, or scope the definitive wording to the ClickHouse backend where it is accurate.

Reviewers (6): correctness/orchestrator analysis, security, performance, previous-comments, api-contract, maintainability.

Testing gaps:

No assertion that the ClickHouse metric-names path emits no time-window predicate / timeSeriesTimeRanges() scan; the scalability win (plan short-circuit on a large table) is validated only by correctness-level integration tests on a 3-series fixture, not by a query-shape or scale check.
metricName values containing special characters (quotes, braces, backslashes) are exercised in neither the ClickHouse nor the Prometheus path, though the parameterization/encoding for them was verified by inspection.

_{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}

Without a window, queryLabelValues('__name__') already reads only the tags table, so call it, and give queryLabelNames an optional metricName instead of a second copy of its query. Say when no series has the metricName.

bcanvural · 2026-11-14T07:44:46Z

be348cb reuses the existing label lookups (thread on timeseriesEngine.ts), adds a note when no series has the metricName (an existing metric always returns __name__), and checks that match[] reaches only /labels.

Not changing:

labelNames missing without metricName on a ClickHouse source: on purpose, listing every series' label names means reading the whole tags table. The note says how to get them.
metricName on a non-PromQL source: the argument description says PromQL only, and other sources answer as before.
The tool's top-level description: it already sends PromQL sources here through PROMQL_SOURCE_USAGE, which now mentions metricName.

github-actions · 2026-11-14T07:45:47Z

+        ? ['Pass metricName to list the label names of one metric.']
+        : []),
+      // A metric's label names always include __name__.
+      ...(labelNames?.length === 0


🔵 minor — Empty Prometheus label list without metricName produces note "No series has the metric name undefined."

The labelNames?.length === 0 check only means "unknown metric" when a metricName was passed. A Prometheus endpoint always returns labelNames, so one with no series in the 24h window, called without metricName, gets []. JSON.stringify(undefined) then renders the note as No series has the metric name undefined. Fix: guard the note with metricName != null && labelNames?.length === 0.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Fixed in c07061b: the note needs a metricName, and a test covers an endpoint with no labels in the window.

github-actions · 2026-11-14T08:01:14Z

+        ? ['Pass metricName to list the label names of one metric.']
+        : []),
+      // A metric's label names always include __name__.
+      ...(metricName != null && labelNames?.length === 0


🔵 minor — On a Prometheus endpoint, the 'No series has the metric name' note is wrong for metrics outside the 24h window

The endpoint path still filters /labels by [now-24h, now]. A metric whose last samples are older than that returns [], and the note then claims the metric does not exist. When catalog.backend === 'prometheus', word the note as 'in the last 24 hours'.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Answered in #3669 (comment).

github-actions · 2026-11-14T08:01:14Z

+    const notes = [
+      ...(truncated
+        ? [
+            `Lists are capped at ${MAX_LABEL_NAMES} label names and ${MAX_METRIC_NAMES} metric names.`,


🔵 minor — The truncation note mentions a label-name cap even when no label names are returned

On a ClickHouse source called without metricName, labelNames is undefined, so only the metric list can be cut. The note still says 'capped at 100 label names and 200 metric names'. Name only the lists that were actually truncated.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Answered in #3669 (comment).

… names The PromQL prompts and tool descriptions still said describe_source lists label names, which a ClickHouse source now does only with metricName.

bcanvural · 2026-11-14T08:09:23Z

b34bee0 updates the PromQL prompts and tool descriptions that said describe_source lists label names (the 🟠 in the latest summary): they now say to pass metricName.

On the two 🔵, not changing:

The unknown-metric note on a Prometheus endpoint: right, that path looks at the last 24 hours, so a metric with no samples since then gets the note too. Past two rounds I'm leaving it; happy to reword it if you'd like.
The truncation note naming both caps: it states the limits, and a list that wasn't cut is shorter than its cap.

github-actions · 2026-11-14T08:10:45Z

+        : []),
+      // A metric's label names always include __name__.
+      ...(metricName != null && labelNames?.length === 0
+        ? [`No series has the metric name ${JSON.stringify(metricName)}.`]


🔵 minor — On a Prometheus endpoint, the 'No series has the metric name' note is wrong for metrics older than 24 hours

On a Prometheus endpoint, /labels?match[]=... only covers [endMs - LABEL_LOOKBACK_MS, endMs] (controllers/promql.ts). An empty result there means the metric had no samples in the last 24 hours, not that the metric doesn't exist. Change the note to say "in the last 24 hours" when catalog.backend === 'prometheus'. The unwindowed ClickHouse path can keep the current wording.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

Answered in #3669 (comment).

bcanvural requested a review from a team as a code owner November 14, 2026 07:33

github-actions Bot added the external label Nov 14, 2026

github-actions Bot reviewed Nov 14, 2026

View reviewed changes

refactor(api): reuse the label lookups for the PromQL catalog

be348cb

Without a window, queryLabelValues('__name__') already reads only the tags table, so call it, and give queryLabelNames an optional metricName instead of a second copy of its query. Say when no series has the metricName.

github-actions Bot reviewed Nov 14, 2026

View reviewed changes

fix(mcp): add the unknown-metric note only when metricName is given

c07061b

github-actions Bot reviewed Nov 14, 2026

View reviewed changes

docs(mcp): tell agents to pass metricName for a PromQL metric's label…

b34bee0

… names The PromQL prompts and tool descriptions still said describe_source lists label names, which a ClickHouse source now does only with metricName.

github-actions Bot reviewed Nov 14, 2026

View reviewed changes

Conversation

bcanvural commented Nov 14, 2026

Summary

How to reproduce

References

Uh oh!

changeset-bot Bot commented Nov 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

vercel Bot commented Nov 14, 2026

Uh oh!

github-actions Bot Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Nov 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review

Uh oh!

github-actions Bot commented Nov 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deep Review

Uh oh!

bcanvural commented Nov 14, 2026

Uh oh!

github-actions Bot Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural commented Nov 14, 2026

Uh oh!

github-actions Bot Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

bcanvural Nov 14, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

changeset-bot Bot commented Nov 14, 2026 •

edited

Loading

github-actions Bot commented Nov 14, 2026 •

edited

Loading

github-actions Bot commented Nov 14, 2026 •

edited

Loading