Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/promql-catalog-tags-table.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@hyperdx/api': patch
---

fix(mcp): `clickstack_describe_source` on a PromQL source over a ClickHouse TimeSeries table reads metric names from the tags table without a time window, so it no longer runs out of memory on large tables. Label names are listed per metric with the new `metricName` argument.
82 changes: 33 additions & 49 deletions packages/api/src/controllers/promql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,33 +256,39 @@ async function fetchPrometheusData<T extends z.ZodTypeAny>(

export type PromqlLabelCatalog = {
backend: PromqlBackend;
labelNames: string[];
/** Without `metricName`, only a Prometheus endpoint lists label names. */
labelNames?: string[];
metricNames: string[];
};

/**
* Lists the label names and metric names a PromQL source holds in
* [startMs, endMs], at most `limit` of each. Returns `limit + 1` entries when
* there are more, so callers can tell the list was cut. A ClickHouse source
* reads them through the Prometheus HTTP API where resolveClickhouseLabelBackend
* allows it, and with SQL otherwise.
* Lists the metric names a PromQL source holds, and label names: of
* `metricName` when given, else of every series on a Prometheus endpoint. At
* most `limit` of each; returns `limit + 1` entries when there are more, so
* callers can tell the list was cut.
*
* A Prometheus endpoint answers for [startMs, endMs]. A ClickHouse TimeSeries
* table is read with SQL on its tags table, without the window, whatever
* backend evaluates its queries: the window costs a pass over every series
* id, on the HTTP API too, which large tables can't afford (#3648).
*/
export async function getPromqlLabelCatalog({
teamId,
source,
startMs,
endMs,
metricName,
limit,
signal,
}: {
teamId: string;
source: {
connection: { toString(): string };
from: { databaseName: string; tableName: string };
promqlBackend?: PromqlSourceBackend;
};
startMs: number;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

endMs: number;
metricName?: string;
limit: number;
signal?: AbortSignal;
}): Promise<PromqlLabelCatalog> {
Expand All @@ -295,53 +301,32 @@ export async function getPromqlLabelCatalog({
throw new Error('Connection not found');
}

const params = {
start: String(startMs / 1000),
end: String(endMs / 1000),
limit: String(limit + 1),
};
const fetchCatalog = (fetchLabels: (path: string) => Promise<string[]>) =>
Promise.all([
fetchLabels('/labels'),
fetchLabels('/label/__name__/values'),
]);

if (connection.isPrometheusEndpoint) {
const [labelNames, metricNames] = await fetchCatalog(path =>
const params = {
start: String(startMs / 1000),
end: String(endMs / 1000),
limit: String(limit + 1),
};
const fetchLabels = (path: string, extra: Record<string, string> = {}) =>
fetchPrometheusData(
connection.host,
`/api/v1${path}`,
params,
{ ...params, ...extra },
labelListSchema,
signal,
);
const [labelNames, metricNames] = await Promise.all([
fetchLabels(
'/labels',
metricName != null
? { 'match[]': `{__name__=${JSON.stringify(metricName)}}` }
: {},
),
);
fetchLabels('/label/__name__/values'),
]);
return { backend: 'prometheus', labelNames, metricNames };
}

const { databaseName, tableName } = source.from;
const labelBackend = await resolveClickhouseLabelBackend({
connection,
databaseName,
tableName,
promqlBackend: source.promqlBackend,
});
if (labelBackend === 'clickhouse-http') {
const [labelNames, metricNames] = await fetchCatalog(path => {
const { upstream, headers } = clickhousePrometheusRequest(connection);
return fetchPrometheusData(
upstream,
`${CLICKHOUSE_PROMETHEUS_API_PREFIX}${path}`,
{ ...params, database: databaseName, table: tableName },
labelListSchema,
signal,
headers,
CLICKHOUSE_HTTP_API_UNAVAILABLE_MESSAGE,
);
});
return { backend: 'clickhouse', labelNames, metricNames };
}

const args = {
client: new ClickhouseClient({
host: connection.host,
Expand All @@ -350,15 +335,14 @@ export async function getPromqlLabelCatalog({
requestTimeout: PROMETHEUS_FETCH_TIMEOUT_MS,
}),
connectionId: connection.id,
databaseName,
tableName,
startMs,
endMs,
databaseName: source.from.databaseName,
tableName: source.from.tableName,
limit: limit + 1,
signal,
};
// No startMs/endMs: without them neither lookup reads the time ranges.
const [labelNames, metricNames] = await Promise.all([
queryLabelNames(args),
metricName != null ? queryLabelNames({ ...args, metricName }) : undefined,
queryLabelValues({ ...args, labelName: '__name__' }),
]);
return { backend: 'clickhouse', labelNames, metricNames };
Expand Down
10 changes: 9 additions & 1 deletion packages/api/src/controllers/timeseriesEngine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,20 @@ export async function queryLabelValues({
*/
export async function queryLabelNames({
limit,
metricName,
...args
}: TimeSeriesTagsQueryArgs): Promise<string[]> {
}: TimeSeriesTagsQueryArgs & {
/** Only this metric's series; the tags table is sorted by metric name. */
metricName?: string;
}): Promise<string[]> {
// The engine keeps `__name__` in `metric_name`, not `tags`, so it is folded
// back in explicitly.
const value = chSql`arrayJoin(arrayConcat([${{ String: '__name__' }}], mapKeys(${{ Identifier: 'tags' }})))`;
const conditions = await getSeriesFilterConditions(args);
if (metricName != null)
conditions.push(
chSql`${{ Identifier: 'metric_name' }} = ${{ String: metricName }}`,
);

return queryDistinctTagsValues({ ...args, value, conditions, limit });
}
Expand Down
148 changes: 78 additions & 70 deletions packages/api/src/mcp/__tests__/promqlSources.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import { Source } from '@/models/source';
import { callTool, createTestClient, getFirstText } from './mcpTestUtils';

const mockFetch = jest.mocked(global.fetch);
const { realFetch } = globalThis as unknown as { realFetch: typeof fetch };

const TABLE = 'mcp_promql_sources';

Expand Down Expand Up @@ -80,6 +79,13 @@ describe('MCP PromQL sources', () => {
startSec: nowSec - 600,
endSec: nowSec - 60,
},
// Two days old: outside the old 24-hour catalog window.
{
metricName: 'stale_metric',
tags: { job: 'batch', region: 'eu' },
startSec: nowSec - 3 * 86400,
endSec: nowSec - 2 * 86400,
},
],
withSamples: true,
});
Expand Down Expand Up @@ -167,7 +173,7 @@ describe('MCP PromQL sources', () => {
expect(text).toContain('up');
});

it('describes a TimeSeries table by its label names and metric names', async () => {
it('describes a TimeSeries table by its metric names, with no time window', async () => {
const result = await callTool(client, 'clickstack_describe_source', {
sourceId: promqlSource._id.toString(),
});
Expand All @@ -180,14 +186,62 @@ describe('MCP PromQL sources', () => {
backend: 'clickhouse',
table: `${DEFAULT_DATABASE}.${TABLE}`,
});
expect(output.labelNames).toEqual(['__name__', 'instance', 'job']);
expect(output.metricNames).toEqual(['http_requests_total', 'up']);
expect(output.labelNames).toBeUndefined();
expect(output.metricNames).toEqual([
'http_requests_total',
'stale_metric',
'up',
]);
expect(output.note).toContain('metricName');
expect(output.columns).toBeUndefined();
expect(output.nextSteps.sql).toContain(
`timeSeriesData(${DEFAULT_DATABASE}.${TABLE})`,
);
});

it("lists one metric's label names with metricName", async () => {
const result = await callTool(client, 'clickstack_describe_source', {
sourceId: promqlSource._id.toString(),
metricName: 'stale_metric',
});

const output = JSON.parse(getFirstText(result));
expect(output.labelNames).toEqual(['__name__', 'job', 'region']);
expect(output.metricNames).toContain('stale_metric');
});

it('says so when no series has the metricName', async () => {
const result = await callTool(client, 'clickstack_describe_source', {
sourceId: promqlSource._id.toString(),
metricName: 'no_such_metric',
});

const output = JSON.parse(getFirstText(result));
expect(output.labelNames).toEqual([]);
expect(output.note).toContain('"no_such_metric"');
});

it('passes metricName to a Prometheus endpoint as a match[] selector', async () => {
const source = await createPrometheusSource();
mockFetch.mockImplementation(async () => prometheusResponse(['up']));

await callTool(client, 'clickstack_describe_source', {
sourceId: source._id.toString(),
metricName: 'http.server.duration',
});

const urls = mockFetch.mock.calls.map(
([input]) => new URL(String(input)),
);
const byPath = (path: string) => urls.find(u => u.pathname === path);
expect(byPath('/api/v1/labels')?.searchParams.get('match[]')).toBe(
'{__name__="http.server.duration"}',
);
expect(
byPath('/api/v1/label/__name__/values')?.searchParams.has('match[]'),
).toBe(false);
});

it('describes a Prometheus endpoint source through its label API', async () => {
const source = await createPrometheusSource();
mockFetch.mockImplementation(async input =>
Expand Down Expand Up @@ -225,6 +279,19 @@ describe('MCP PromQL sources', () => {
}
});

it('adds no metric-name note when an endpoint has no labels in the window', async () => {
const source = await createPrometheusSource();
mockFetch.mockImplementation(async () => prometheusResponse([]));

const result = await callTool(client, 'clickstack_describe_source', {
sourceId: source._id.toString(),
});

const output = JSON.parse(getFirstText(result));
expect(output.labelNames).toEqual([]);
expect(output.note).toBeUndefined();
});

it('still returns the source config when the label lookup fails', async () => {
const source = await createPrometheusSource();
mockFetch.mockRejectedValue(new Error('fetch failed'));
Expand Down Expand Up @@ -304,83 +371,24 @@ describe('MCP PromQL sources', () => {
versionSpy = undefined;
});

it("sends the label lookups with the source's table and the connection's credentials", async () => {
// The handler's label lookups join every series as well (#3648).
it('reads names with SQL even where the handler serves label lookups', async () => {
stubServerVersion([26, 10, 0, 0]);
mockFetch.mockImplementation(async input => {
const { pathname } = new URL(String(input));
if (pathname.endsWith('/format_query')) {
return prometheusResponse('up');
}
return prometheusResponse(
pathname.endsWith('/label/__name__/values')
? ['up']
: ['__name__', 'job'],
);
});
mockFetch.mockImplementation(async () => prometheusResponse('up'));

const output = await describeAutoSource();

expect(output.source).toMatchObject({
backend: 'clickhouse',
table: `${DEFAULT_DATABASE}.${TABLE}`,
});
expect(output.labelNames).toEqual(['__name__', 'job']);
expect(output.metricNames).toEqual(['up']);
const urls = labelLookupUrls();
expect(urls.map(u => u.pathname).sort()).toEqual([
`${CLICKHOUSE_PROMETHEUS_API_PREFIX}/label/__name__/values`,
`${CLICKHOUSE_PROMETHEUS_API_PREFIX}/labels`,
expect(output.metricNames).toEqual([
'http_requests_total',
'stale_metric',
'up',
]);
for (const url of urls) {
expect(url.searchParams.getAll('database')).toEqual([
DEFAULT_DATABASE,
]);
expect(url.searchParams.getAll('table')).toEqual([TABLE]);
expect(url.searchParams.get('start')).toBeTruthy();
expect(url.searchParams.get('end')).toBeTruthy();
expect(url.searchParams.get('limit')).toBe('201');
}
for (const [input, init] of mockFetch.mock.calls) {
if (new URL(String(input)).pathname.endsWith('/format_query')) {
continue;
}
expect(init?.headers).toMatchObject({
'X-ClickHouse-User': config.CLICKHOUSE_USER,
'X-ClickHouse-Key': config.CLICKHOUSE_PASSWORD,
'X-ClickHouse-Query-Id': expect.stringMatching(/^hdx-mcp-/),
});
}
});

// 26.8's handler answers label lookups with a 400 ("not implemented").
it('reads labels with SQL on ClickHouse 26.8, whose handler serves only queries', async () => {
stubServerVersion([26, 8, 36, 3]);
mockFetch.mockImplementation(async () => prometheusResponse('up'));

const output = await describeAutoSource();

expect(output.labelNames).toEqual(['__name__', 'instance', 'job']);
expect(output.metricNames).toEqual(['http_requests_total', 'up']);
expect(labelLookupUrls()).toEqual([]);
});

// Needs ClickHouse 26.9+, whose handler serves label lookups.
it('lists labels from the CI ClickHouse handler', async () => {
mockFetch.mockImplementation(realFetch);

const output = await describeAutoSource();

expect(output.labelNames).toEqual(['__name__', 'instance', 'job']);
expect(output.metricNames).toEqual(['http_requests_total', 'up']);
expect(
labelLookupUrls()
.map(u => u.pathname)
.sort(),
).toEqual([
`${CLICKHOUSE_PROMETHEUS_API_PREFIX}/label/__name__/values`,
`${CLICKHOUSE_PROMETHEUS_API_PREFIX}/labels`,
]);
});
});

it('keeps promqlBackend through a describe -> rename -> save round trip', async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/api/src/mcp/prompts/dashboards/promqlContent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ A source with kind "promql" holds Prometheus metrics, from a Prometheus-compatib

Discovery workflow:
1. clickstack_list_sources: find the source with kind "promql" and its backend ("prometheus" or "clickhouse").
2. clickstack_describe_source(sourceId): lists its metric names and label names.
2. clickstack_describe_source(sourceId): lists its metric names. clickstack_describe_source(sourceId, metricName): lists that metric's label names.
3. clickstack_promql(sourceId, expression, startTime, endTime): run the expression. The default queryType "range" returns time series rows; queryType "instant" evaluates once at endTime and returns one row per series with a column per label (use it for "what is the value right now" and top-N questions).
4. To keep a chart, save it as a PromQL tile (see "PROMQL TILES" in the create_dashboard prompt) and validate it with clickstack_query_tile. A tile with the same expression and granularity returns the same rows clickstack_promql did.

Expand Down Expand Up @@ -48,6 +48,6 @@ Sources with kind "promql" hold Prometheus metrics. Chart them with PROMQL tiles

- displayType works as for builder tiles. line, stacked_bar and stacked_line plot every expression; number, table, pie and bar query only the first and can set queryType "instant" to evaluate once at the end of the range.
- legendTemplate names each series from its labels, e.g. "{{job}} / {{instance}}".
- Filter with a PROMETHEUS_LABEL dashboard filter: { "type": "PROMETHEUS_LABEL", "name": "Job", "sourceId": "<promql source id>", "label": "job", "variableName": "job" }. Tiles reference it in a matcher: {job=~"$job"} (multi-select) or {job="$job"} with maxSelections: 1. clickstack_describe_source lists the label names.
- Filter with a PROMETHEUS_LABEL dashboard filter: { "type": "PROMETHEUS_LABEL", "name": "Job", "sourceId": "<promql source id>", "label": "job", "variableName": "job" }. Tiles reference it in a matcher: {job=~"$job"} (multi-select) or {job="$job"} with maxSelections: 1. clickstack_describe_source with metricName lists a metric's label names.
- Draft each expression with clickstack_promql first, then validate the saved tile with clickstack_query_tile. PromQL tiles cannot have alerts.
- Fetch the query_guide prompt for the PromQL syntax notes (rate(), histogram_quantile, the $__rate_interval macro).`;
2 changes: 1 addition & 1 deletion packages/api/src/mcp/tools/dashboards/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,7 @@ const mcpPromqlLabelFilterSchema = z
),
label: PromqlLabelDashboardFilterSchema.shape.label.describe(
'Label whose values fill the dropdown, e.g. "job". ' +
'clickstack_describe_source lists the label names of a promql source.',
'clickstack_describe_source with metricName lists the label names of a metric in a promql source.',
),
match: PromqlLabelDashboardFilterSchema.shape.match.describe(
'Optional series selector narrowing which series the values come from, ' +
Expand Down
Loading
Loading