diff --git a/.changeset/promql-catalog-tags-table.md b/.changeset/promql-catalog-tags-table.md new file mode 100644 index 0000000000..fc3787e156 --- /dev/null +++ b/.changeset/promql-catalog-tags-table.md @@ -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. diff --git a/packages/api/src/controllers/promql.ts b/packages/api/src/controllers/promql.ts index 59ed8a33e7..88028a0a0b 100644 --- a/packages/api/src/controllers/promql.ts +++ b/packages/api/src/controllers/promql.ts @@ -256,22 +256,28 @@ async function fetchPrometheusData( 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, }: { @@ -279,10 +285,10 @@ export async function getPromqlLabelCatalog({ source: { connection: { toString(): string }; from: { databaseName: string; tableName: string }; - promqlBackend?: PromqlSourceBackend; }; startMs: number; endMs: number; + metricName?: string; limit: number; signal?: AbortSignal; }): Promise { @@ -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) => - 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 = {}) => 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, @@ -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 }; diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index 6acf667016..35afdb0f83 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -312,12 +312,20 @@ export async function queryLabelValues({ */ export async function queryLabelNames({ limit, + metricName, ...args -}: TimeSeriesTagsQueryArgs): Promise { +}: TimeSeriesTagsQueryArgs & { + /** Only this metric's series; the tags table is sorted by metric name. */ + metricName?: string; +}): Promise { // 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 }); } diff --git a/packages/api/src/mcp/__tests__/promqlSources.int.test.ts b/packages/api/src/mcp/__tests__/promqlSources.int.test.ts index b18de6eba7..119cc9c114 100644 --- a/packages/api/src/mcp/__tests__/promqlSources.int.test.ts +++ b/packages/api/src/mcp/__tests__/promqlSources.int.test.ts @@ -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'; @@ -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, }); @@ -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(), }); @@ -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 => @@ -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')); @@ -304,19 +371,10 @@ 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(); @@ -324,63 +382,13 @@ describe('MCP PromQL sources', () => { 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 () => { diff --git a/packages/api/src/mcp/prompts/dashboards/promqlContent.ts b/packages/api/src/mcp/prompts/dashboards/promqlContent.ts index 88d4cb813b..c48030dc2e 100644 --- a/packages/api/src/mcp/prompts/dashboards/promqlContent.ts +++ b/packages/api/src/mcp/prompts/dashboards/promqlContent.ts @@ -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. @@ -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": "", "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": "", "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).`; diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index f9e40efe58..528b8f4fbc 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -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, ' + diff --git a/packages/api/src/mcp/tools/query/promql.ts b/packages/api/src/mcp/tools/query/promql.ts index ac07150155..8246eed560 100644 --- a/packages/api/src/mcp/tools/query/promql.ts +++ b/packages/api/src/mcp/tools/query/promql.ts @@ -17,7 +17,7 @@ import { endTimeSchema, startTimeSchema } from './schemas'; const promqlSchema = z.object({ sourceId: objectIdSchema.describe( 'ID of a source whose kind is "promql". Call clickstack_list_sources to ' + - 'find it, and clickstack_describe_source for its metric and label names.', + "find it, and clickstack_describe_source for its metric names (with metricName, a metric's label names).", ), expression: z .string() @@ -63,7 +63,7 @@ export function registerPromql({ context, registerTool }: ToolRegistrar) { 'about Prometheus metrics; the builder tools (clickstack_timeseries, ' + 'clickstack_table, clickstack_search) do not read PromQL sources.\n\n' + 'Discovery: clickstack_list_sources lists sources with kind "promql"; ' + - 'clickstack_describe_source lists their metric names and label names.\n\n' + + "clickstack_describe_source lists their metric names, and with metricName that metric's label names.\n\n" + 'A range query (the default) returns rows { __hdx_time_bucket, ' + 'series_name, value } plus seriesLabels, which maps each series_name to ' + 'its labels once. An instant query returns one row per series with a ' + diff --git a/packages/api/src/mcp/tools/sources/describePromqlSource.ts b/packages/api/src/mcp/tools/sources/describePromqlSource.ts index 44c8fdd2fc..bbf6be628d 100644 --- a/packages/api/src/mcp/tools/sources/describePromqlSource.ts +++ b/packages/api/src/mcp/tools/sources/describePromqlSource.ts @@ -5,23 +5,28 @@ import type { ToolResult } from '@/mcp/tools/types'; import { PROMQL_QUERY_USAGE, timeSeriesSqlUsage } from '@/mcp/utils/promql'; import logger from '@/utils/logger'; +// A Prometheus endpoint lists names seen in this window; a ClickHouse table +// lists every name it holds. const LABEL_LOOKBACK_MS = 24 * 60 * 60 * 1000; const MAX_LABEL_NAMES = 100; const MAX_METRIC_NAMES = 200; /** - * describe_source for a PromQL source: its label names and metric names + * describe_source for a PromQL source: its metric names and label names * instead of a column schema, from either a ClickHouse TimeSeries table or a - * Prometheus-compatible endpoint. + * Prometheus-compatible endpoint. With `metricName`, the label names are that + * metric's. */ export async function describePromqlSource({ teamId, source, + metricName, meta, signal, progress, }: { teamId: string; + metricName?: string; source: Parameters[0]['source']; meta: Record; signal: AbortSignal; @@ -43,12 +48,33 @@ export async function describePromqlSource({ source, startMs: endMs - LABEL_LOOKBACK_MS, endMs, + metricName, limit: Math.max(MAX_LABEL_NAMES, MAX_METRIC_NAMES), signal, }); + const { labelNames, metricNames } = catalog; const truncated = - catalog.labelNames.length > MAX_LABEL_NAMES || - catalog.metricNames.length > MAX_METRIC_NAMES; + (labelNames?.length ?? 0) > MAX_LABEL_NAMES || + metricNames.length > MAX_METRIC_NAMES; + const notes = [ + ...(truncated + ? [ + `Lists are capped at ${MAX_LABEL_NAMES} label names and ${MAX_METRIC_NAMES} metric names.`, + ] + : []), + ...(catalog.backend === 'clickhouse' + ? [ + 'Names come from every series the table holds, including series with no recent samples.', + ] + : []), + ...(labelNames == null + ? ['Pass metricName to list the label names of one metric.'] + : []), + // A metric's label names always include __name__. + ...(metricName != null && labelNames?.length === 0 + ? [`No series has the metric name ${JSON.stringify(metricName)}.`] + : []), + ]; const table = catalog.backend === 'clickhouse' ? `${quoteIdentifierIfNeeded(source.from.databaseName)}.${quoteIdentifierIfNeeded(source.from.tableName)}` @@ -60,13 +86,11 @@ export async function describePromqlSource({ backend: catalog.backend, ...(table ? { table } : {}), }, - labelNames: catalog.labelNames.slice(0, MAX_LABEL_NAMES), - metricNames: catalog.metricNames.slice(0, MAX_METRIC_NAMES), - ...(truncated - ? { - note: `Lists are capped at ${MAX_LABEL_NAMES} label names and ${MAX_METRIC_NAMES} metric names.`, - } + ...(labelNames + ? { labelNames: labelNames.slice(0, MAX_LABEL_NAMES) } : {}), + metricNames: metricNames.slice(0, MAX_METRIC_NAMES), + ...(notes.length > 0 ? { note: notes.join(' ') } : {}), }, table && timeSeriesSqlUsage(table), ); diff --git a/packages/api/src/mcp/tools/sources/describeSource.ts b/packages/api/src/mcp/tools/sources/describeSource.ts index 881a75ed80..abc8cb283c 100644 --- a/packages/api/src/mcp/tools/sources/describeSource.ts +++ b/packages/api/src/mcp/tools/sources/describeSource.ts @@ -91,6 +91,7 @@ type DescribeProgress = { async function describeSourceSchema( context: McpContext, sourceId: string, + metricName: string | undefined, signal: AbortSignal, progress: DescribeProgress, ): Promise { @@ -118,7 +119,14 @@ async function describeSourceSchema( } if (source.kind === SourceKind.Promql) { - return describePromqlSource({ teamId, source, meta, signal, progress }); + return describePromqlSource({ + teamId, + source, + metricName, + meta, + signal, + progress, + }); } if ( @@ -593,9 +601,17 @@ export function registerDescribeSource({ .describe( 'The source ID to describe. Get this from clickstack_list_sources.', ), + metricName: z + .string() + .min(1) + .optional() + .describe( + 'PromQL sources only: list the label names of this metric. ' + + 'Without it, a PromQL source on a ClickHouse table lists metric names only.', + ), }), }, - async ({ sourceId }) => { + async ({ sourceId, metricName }) => { const progress: DescribeProgress = {}; const span = trace.getActiveSpan(); @@ -629,7 +645,14 @@ export function registerDescribeSource({ let outcome: TimeoutOutcome; try { outcome = await runWithTimeout( - signal => describeSourceSchema(context, sourceId, signal, progress), + signal => + describeSourceSchema( + context, + sourceId, + metricName, + signal, + progress, + ), { timeoutMs: DESCRIBE_TIMEOUT_MS, graceMs: DESCRIBE_BACKSTOP_MS }, ); } catch (e) { diff --git a/packages/api/src/mcp/utils/promql.ts b/packages/api/src/mcp/utils/promql.ts index 29b3626baa..f1ed273145 100644 --- a/packages/api/src/mcp/utils/promql.ts +++ b/packages/api/src/mcp/utils/promql.ts @@ -29,6 +29,7 @@ export function timeSeriesSqlUsage(table: string): string { } export const PROMQL_SOURCE_USAGE = - `${PROMQL_QUERY_USAGE} Call clickstack_describe_source to list label names ` + - 'and metric names. A PromQL source with backend "clickhouse" is a ' + + `${PROMQL_QUERY_USAGE} Call clickstack_describe_source to list metric names, ` + + "and with metricName to list that metric's label names. A PromQL source " + + 'with backend "clickhouse" is a ' + `ClickHouse TimeSeries table; to inspect its raw tags and samples, ${timeSeriesSqlUsage('')}`;