diff --git a/.changeset/mcp-promql-sources.md b/.changeset/mcp-promql-sources.md new file mode 100644 index 0000000000..f90a0a187a --- /dev/null +++ b/.changeset/mcp-promql-sources.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/api': minor +--- + +feat: Describe PromQL sources in MCP and hide them when PromQL is disabled + +`clickstack_describe_source` now returns the label names and metric names of a PromQL source, from either a ClickHouse TimeSeries table or a Prometheus-compatible endpoint, instead of a column schema. `clickstack_list_sources` and `clickstack_describe_source` show which backend a PromQL source uses, and for TimeSeries tables explain how to read them with `clickstack_sql`. Prometheus endpoint sources no longer show their placeholder table. When `ENABLE_PROMQL` is off, MCP hides PromQL sources from listing, describing and deleting, and `clickstack_save_source` no longer accepts `kind: "promql"`. diff --git a/.changeset/prometheus-proxy-basic-auth.md b/.changeset/prometheus-proxy-basic-auth.md new file mode 100644 index 0000000000..b4ecfc59e0 --- /dev/null +++ b/.changeset/prometheus-proxy-basic-auth.md @@ -0,0 +1,7 @@ +--- +'@hyperdx/api': patch +--- + +fix: Authenticate to Prometheus connections whose host includes credentials + +A Prometheus connection host such as `http://user:pass@prometheus:9090` made every PromQL query fail with "Failed to reach Prometheus", because `fetch` rejects URLs that carry credentials. The credentials are now sent as a Basic auth header. diff --git a/packages/api/src/controllers/promql.ts b/packages/api/src/controllers/promql.ts new file mode 100644 index 0000000000..3f0c8eefff --- /dev/null +++ b/packages/api/src/controllers/promql.ts @@ -0,0 +1,206 @@ +import { z } from 'zod'; + +import { ClickhouseClient } from '@/clickhouse'; +import { getConnectionById } from '@/controllers/connection'; +import { + queryLabelNames, + queryLabelValues, +} from '@/controllers/timeseriesEngine'; + +type PromqlBackend = 'prometheus' | 'clickhouse'; + +const PROMETHEUS_FETCH_TIMEOUT_MS = 30_000; + +/** + * Join a Connection host with an absolute Prometheus API path. + * + * `new URL('/api/v1/query_range', 'http://host:8481/select/0/prometheus')` + * discards `/select/0/prometheus` because an absolute path replaces the base + * pathname. VictoriaMetrics cluster (and any Prometheus-compatible server + * mounted under a prefix) needs that prefix kept. Host userinfo, query, and + * hash are left untouched. + * + * `path` must be an absolute path (every call site passes a literal starting + * with `/`) -- this is not a general-purpose URL joiner. + * + * @see https://github.com/hyperdxio/hyperdx/issues/3046 + */ +export function joinPrometheusUpstreamUrl( + upstreamHost: string, + path: string, +): URL { + const url = new URL(upstreamHost); + // `new URL('prometheus:9090')` succeeds with an opaque path (`prometheus:` + // scheme). The pathname setter is a no-op there, so without this guard the + // helper would return the host unchanged, `fetch` would fail, and the proxy + // would 502 / increment query_errors for a user misconfiguration. Same check + // as clickhouseProxy.ts. + if (url.protocol !== 'http:' && url.protocol !== 'https:') { + throw new TypeError('Connection host must be http(s)'); + } + // Strip ALL trailing slashes, not just one -- a host saved with a doubled + // trailing slash (e.g. `http://prom:9090//`) would otherwise leave a `//` + // in the joined path, which most servers treat as a distinct (404) path. + const basePath = url.pathname.replace(/\/+$/, ''); + url.pathname = `${basePath}${path}`; + return url; +} + +/** + * Removes `user:pass@` from `url` and returns it as a Basic auth header. fetch + * rejects a URL that carries credentials, so they cannot stay in the host. + */ +export function takeBasicAuthHeader(url: URL): Record { + if (!url.username && !url.password) return {}; + const credentials = `${decodeURIComponent(url.username)}:${decodeURIComponent(url.password)}`; + url.username = ''; + url.password = ''; + return { + Authorization: `Basic ${Buffer.from(credentials).toString('base64')}`, + }; +} + +const prometheusBodySchema = z.object({ + status: z.string(), + data: z.unknown(), + error: z.string().optional(), +}); + +const labelListSchema = z.array(z.string()); + +/** + * GETs a Prometheus API path from an external Prometheus-compatible endpoint + * and returns the response's `data`, checked against `dataSchema`. Errors name + * only the origin and path: the host may carry credentials in its userinfo or + * query string. + */ +async function fetchPrometheusData( + host: string, + path: string, + params: Record, + dataSchema: T, + signal?: AbortSignal, +): Promise> { + const url = joinPrometheusUpstreamUrl(host, path); + for (const [key, value] of Object.entries(params)) { + url.searchParams.set(key, value); + } + const redactedTarget = `${url.origin}${url.pathname}`; + const timeout = AbortSignal.timeout(PROMETHEUS_FETCH_TIMEOUT_MS); + + let resp: Response; + try { + resp = await fetch(url, { + headers: takeBasicAuthHeader(url), + signal: signal ? AbortSignal.any([signal, timeout]) : timeout, + }); + } catch (err) { + throw new Error( + err instanceof Error && err.name === 'TimeoutError' + ? `Prometheus request to ${redactedTarget} timed out` + : `Failed to reach Prometheus at ${redactedTarget}`, + ); + } + + const body = prometheusBodySchema.safeParse( + await resp.json().catch(() => undefined), + ); + if (!resp.ok || !body.success || body.data.status !== 'success') { + throw new Error( + (body.success && body.data.error) || + `Prometheus at ${redactedTarget} returned HTTP ${resp.status}`, + ); + } + const data = dataSchema.safeParse(body.data.data); + if (!data.success) { + throw new Error( + `Prometheus at ${redactedTarget} returned an unexpected response`, + ); + } + return data.data; +} + +export type PromqlLabelCatalog = { + backend: PromqlBackend; + 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. + */ +export async function getPromqlLabelCatalog({ + teamId, + source, + startMs, + endMs, + limit, + signal, +}: { + teamId: string; + source: { + connection: { toString(): string }; + from: { databaseName: string; tableName: string }; + }; + startMs: number; + endMs: number; + limit: number; + signal?: AbortSignal; +}): Promise { + const connection = await getConnectionById( + teamId, + source.connection.toString(), + true, + ); + if (!connection) { + throw new Error('Connection not found'); + } + + if (connection.isPrometheusEndpoint) { + const params = { + start: String(startMs / 1000), + end: String(endMs / 1000), + limit: String(limit + 1), + }; + const [labelNames, metricNames] = await Promise.all([ + fetchPrometheusData( + connection.host, + '/api/v1/labels', + params, + labelListSchema, + signal, + ), + fetchPrometheusData( + connection.host, + '/api/v1/label/__name__/values', + params, + labelListSchema, + signal, + ), + ]); + return { backend: 'prometheus', labelNames, metricNames }; + } + + const args = { + client: new ClickhouseClient({ + host: connection.host, + username: connection.username, + password: connection.password, + requestTimeout: PROMETHEUS_FETCH_TIMEOUT_MS, + }), + connectionId: connection.id, + databaseName: source.from.databaseName, + tableName: source.from.tableName, + startMs, + endMs, + limit: limit + 1, + signal, + }; + const [labelNames, metricNames] = await Promise.all([ + queryLabelNames(args), + 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 7ed0286d41..64fc8c4b77 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -28,6 +28,8 @@ export type TimeSeriesTagsQueryArgs = { endMs?: number; limit?: number; match?: string[]; + /** Cancels the DISTINCT query; the tags-table column lookup is not cancellable. */ + signal?: AbortSignal; }; /** @@ -177,6 +179,7 @@ async function queryDistinctTagsValues({ value, conditions, limit, + signal, }: { client: ClickhouseClient; databaseName: string; @@ -184,6 +187,7 @@ async function queryDistinctTagsValues({ value: ChSql; conditions: ChSql[]; limit?: number; + signal?: AbortSignal; }): Promise { const where = conditions.length ? chSql`WHERE ${concatChSql(' AND ', ...conditions)}` @@ -203,6 +207,7 @@ async function queryDistinctTagsValues({ query: query.sql, query_params: query.params, format: 'JSON', + abort_signal: signal, clickhouse_settings: { allow_experimental_time_series_table: 1, max_execution_time: PROMETHEUS_MAX_EXECUTION_SEC, diff --git a/packages/api/src/mcp/__tests__/describeSourceTimeout.test.ts b/packages/api/src/mcp/__tests__/describeSourceTimeout.test.ts index edd4fe7b44..75d4d99a62 100644 --- a/packages/api/src/mcp/__tests__/describeSourceTimeout.test.ts +++ b/packages/api/src/mcp/__tests__/describeSourceTimeout.test.ts @@ -13,6 +13,9 @@ jest.mock('@/clickhouse', () => ({ ClickhouseClient: jest.fn() })); jest.mock('@hyperdx/common-utils/dist/core/metadata', () => ({ getMetadata: () => mockMetadata, })); +jest.mock('@/controllers/promql', () => ({ + getPromqlLabelCatalog: jest.fn(), +})); jest.mock('@/mcp/tools/sources/metricNames', () => ({ sampleMetricNamesWithLookback: jest.fn(), })); @@ -23,6 +26,7 @@ jest.mock('@/utils/trimToolResponse', () => ({ import { SourceKind } from '@hyperdx/common-utils/dist/types'; import { getConnectionById } from '@/controllers/connection'; +import { getPromqlLabelCatalog } from '@/controllers/promql'; import { getSource } from '@/controllers/sources'; import { DESCRIBE_BACKSTOP_MS, @@ -58,6 +62,13 @@ const metricSource = { metricTables: { gauge: 'otel_metrics_gauge', sum: 'otel_metrics_sum' }, }; +const promqlSource = { + ...traceSource, + kind: SourceKind.Promql, + timestampValueExpression: 'timestamp', + from: { databaseName: 'default', tableName: 'metrics_ts' }, +}; + const never = () => new Promise(() => {}); const rejectOnAbort = (signal: AbortSignal) => @@ -70,7 +81,7 @@ function getHandler() { null; registerDescribeSource({ server: {}, - context: { teamId: 'team-1', userId: 'user-1' }, + context: { teamId: 'team-1', userId: 'user-1', promqlEnabled: true }, registerTool: (_name: string, _config: unknown, h: typeof handler) => { handler = h; }, @@ -353,4 +364,40 @@ describe('clickstack_describe_source deadline', () => { expect(source.skippedStages).toEqual(['metricNames']); }); }); + + describe('PromQL sources', () => { + beforeEach(() => { + jest + .mocked(getSource) + // @ts-expect-error mock source only has the fields describe_source reads + .mockResolvedValue(promqlSource); + }); + + it('aborts the label lookup at the deadline', async () => { + jest + .mocked(getPromqlLabelCatalog) + .mockImplementation(({ signal }) => rejectOnAbort(signal!)); + + const result = await runWithClock(DESCRIBE_TIMEOUT_MS); + + expect(result.isError).toBeFalsy(); + const output = parse(result); + expect(output.partial).toBe(true); + expect(output.source.config).toMatchObject({ kind: SourceKind.Promql }); + }); + + it('returns the source config at the backstop when the lookup hangs', async () => { + jest.mocked(getPromqlLabelCatalog).mockImplementation(never); + + const result = await runWithClock( + DESCRIBE_TIMEOUT_MS + DESCRIBE_BACKSTOP_MS, + ); + + expect(result.isError).toBeFalsy(); + const output = parse(result); + expect(output.partial).toBe(true); + expect(output.error).toContain('timed out'); + expect(output.source.config).toMatchObject({ kind: SourceKind.Promql }); + }); + }); }); diff --git a/packages/api/src/mcp/__tests__/promqlSources.int.test.ts b/packages/api/src/mcp/__tests__/promqlSources.int.test.ts new file mode 100644 index 0000000000..6ec22dd529 --- /dev/null +++ b/packages/api/src/mcp/__tests__/promqlSources.int.test.ts @@ -0,0 +1,276 @@ +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { Client } from '@modelcontextprotocol/sdk/client/index.js'; + +import * as config from '@/config'; +import { + DEFAULT_DATABASE, + dropTimeSeriesTable, + getLoggedInAgent, + getServer, + seedTimeSeriesTagsTable, +} from '@/fixtures'; +import Connection from '@/models/connection'; +import { Source } from '@/models/source'; + +import { callTool, createTestClient, getFirstText } from './mcpTestUtils'; + +const mockFetch = jest.mocked(global.fetch); + +const TABLE = 'mcp_promql_sources'; + +const prometheusResponse = (data: unknown) => + new Response(JSON.stringify({ status: 'success', data })); + +describe('MCP PromQL sources', () => { + const server = getServer(); + let team: any; + let user: any; + let promqlSource: any; + let client: Client; + + const connect = async (promqlEnabled: boolean) => { + client = await createTestClient({ + teamId: team._id.toString(), + userId: user._id.toString(), + promqlEnabled, + }); + }; + + const createPrometheusSource = async () => { + const connection = await Connection.create({ + team: team._id, + name: 'Prometheus', + host: 'http://user:secret@prom.example.com', + username: '', + password: '', + isPrometheusEndpoint: true, + }); + return Source.create({ + kind: SourceKind.Promql, + team: team._id, + connection: connection._id, + name: 'Prometheus metrics', + from: { databaseName: 'prometheus', tableName: 'prometheus' }, + timestampValueExpression: 'timestamp', + }); + }; + + beforeAll(async () => { + await server.start(); + const nowSec = Math.floor(Date.now() / 1000); + await seedTimeSeriesTagsTable({ + table: TABLE, + series: [ + { + metricName: 'http_requests_total', + tags: { job: 'checkout', instance: 'checkout-0' }, + startSec: nowSec - 600, + endSec: nowSec - 60, + }, + { + metricName: 'up', + tags: { job: 'cart' }, + startSec: nowSec - 600, + endSec: nowSec - 60, + }, + ], + withSamples: true, + }); + }); + + beforeEach(async () => { + ({ team, user } = await getLoggedInAgent(server)); + const connection = await Connection.create({ + team: team._id, + name: 'Default', + host: config.CLICKHOUSE_HOST, + username: config.CLICKHOUSE_USER, + password: config.CLICKHOUSE_PASSWORD, + }); + promqlSource = await Source.create({ + kind: SourceKind.Promql, + team: team._id, + connection: connection._id, + name: 'TimeSeries metrics', + from: { databaseName: DEFAULT_DATABASE, tableName: TABLE }, + timestampValueExpression: 'timestamp', + }); + }); + + afterEach(async () => { + await client?.close(); + await server.clearDBs(); + }); + + afterAll(async () => { + await dropTimeSeriesTable({ table: TABLE }); + await server.stop(); + }); + + describe('with PromQL enabled', () => { + beforeEach(() => connect(true)); + + it('lists a TimeSeries source with its table, with usage guidance', async () => { + const result = await callTool(client, 'clickstack_list_sources'); + + const output = JSON.parse(getFirstText(result)); + expect(output.sources).toHaveLength(1); + expect(output.sources[0]).toMatchObject({ + id: promqlSource._id.toString(), + kind: SourceKind.Promql, + backend: 'clickhouse', + table: `${DEFAULT_DATABASE}.${TABLE}`, + }); + expect(output.promqlUsage).toContain('timeSeriesData'); + }); + + it('lists a Prometheus endpoint source without its placeholder table', async () => { + const source = await createPrometheusSource(); + + const result = await callTool(client, 'clickstack_list_sources'); + + const output = JSON.parse(getFirstText(result)); + const listed = output.sources.find( + (s: any) => s.id === source._id.toString(), + ); + expect(listed.backend).toBe('prometheus'); + expect(listed.table).toBeUndefined(); + expect(listed.database).toBeUndefined(); + }); + + it('reads a TimeSeries table through clickstack_sql as the hint describes', async () => { + const table = `${DEFAULT_DATABASE}.${TABLE}`; + const result = await callTool(client, 'clickstack_sql', { + connectionId: promqlSource.connection.toString(), + sql: + `SELECT t.metric_name AS metric, count() AS samples ` + + `FROM timeSeriesData(${table}) AS d ` + + `INNER JOIN timeSeriesTags(${table}) AS t ON d.id = t.id ` + + `GROUP BY metric ORDER BY metric`, + }); + + expect(result.isError).toBeFalsy(); + const text = getFirstText(result); + expect(text).toContain('http_requests_total'); + expect(text).toContain('up'); + }); + + it('describes a TimeSeries table by its label names and metric names', async () => { + const result = await callTool(client, 'clickstack_describe_source', { + sourceId: promqlSource._id.toString(), + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.source).toMatchObject({ + id: promqlSource._id.toString(), + kind: SourceKind.Promql, + backend: 'clickhouse', + table: `${DEFAULT_DATABASE}.${TABLE}`, + }); + expect(output.labelNames).toEqual(['__name__', 'instance', 'job']); + expect(output.metricNames).toEqual(['http_requests_total', 'up']); + expect(output.columns).toBeUndefined(); + expect(output.nextSteps.sql).toContain( + `timeSeriesData(${DEFAULT_DATABASE}.${TABLE})`, + ); + }); + + it('describes a Prometheus endpoint source through its label API', async () => { + const source = await createPrometheusSource(); + mockFetch.mockImplementation(async input => + prometheusResponse( + String(input).includes('/label/__name__/values') + ? ['up'] + : ['__name__', 'job'], + ), + ); + + const result = await callTool(client, 'clickstack_describe_source', { + sourceId: source._id.toString(), + }); + + const output = JSON.parse(getFirstText(result)); + expect(output.source.backend).toBe('prometheus'); + expect(output.source.table).toBeUndefined(); + expect(output.nextSteps.sql).toBeUndefined(); + expect(output.labelNames).toEqual(['__name__', 'job']); + expect(output.metricNames).toEqual(['up']); + const urls = mockFetch.mock.calls.map( + ([input]) => new URL(String(input)), + ); + expect(urls.map(u => u.pathname).sort()).toEqual([ + '/api/v1/label/__name__/values', + '/api/v1/labels', + ]); + expect(urls[0].searchParams.get('start')).toBeTruthy(); + // fetch rejects a URL with credentials, so they travel as a header. + for (const [input, init] of mockFetch.mock.calls) { + expect(new URL(String(input)).username).toBe(''); + expect(init?.headers).toEqual({ + Authorization: `Basic ${Buffer.from('user:secret').toString('base64')}`, + }); + } + }); + + it('still returns the source config when the label lookup fails', async () => { + const source = await createPrometheusSource(); + mockFetch.mockRejectedValue(new Error('fetch failed')); + + const result = await callTool(client, 'clickstack_describe_source', { + sourceId: source._id.toString(), + }); + + expect(result.isError).toBeFalsy(); + const output = JSON.parse(getFirstText(result)); + expect(output.source.config).toMatchObject({ kind: SourceKind.Promql }); + expect(output.partial).toBe(true); + expect(output.error).toContain('http://prom.example.com/api/v1/'); + expect(output.error).not.toContain('secret'); + }); + }); + + describe('with PromQL disabled', () => { + beforeEach(() => connect(false)); + + it('hides PromQL sources from clickstack_list_sources', async () => { + const result = await callTool(client, 'clickstack_list_sources'); + + const output = JSON.parse(getFirstText(result)); + expect(output.sources).toEqual([]); + expect(output.promqlUsage).toBeUndefined(); + }); + + it('treats a PromQL source as not found in clickstack_describe_source', async () => { + const result = await callTool(client, 'clickstack_describe_source', { + sourceId: promqlSource._id.toString(), + }); + + expect(result.isError).toBe(true); + expect(getFirstText(result)).toContain('not found'); + }); + + it('treats a PromQL source as not found in clickstack_delete_source', async () => { + const result = await callTool(client, 'clickstack_delete_source', { + id: promqlSource._id.toString(), + }); + + expect(result.isError).toBe(true); + expect(await Source.findById(promqlSource._id)).not.toBeNull(); + }); + + it('rejects kind promql in clickstack_save_source', async () => { + const result = await callTool(client, 'clickstack_save_source', { + kind: 'promql', + name: 'New PromQL', + connection: promqlSource.connection.toString(), + databaseName: DEFAULT_DATABASE, + tableName: TABLE, + timestampValueExpression: 'timestamp', + }); + + expect(result.isError).toBe(true); + expect(await Source.countDocuments({ name: 'New PromQL' })).toBe(0); + }); + }); +}); diff --git a/packages/api/src/mcp/prompts/dashboards/index.ts b/packages/api/src/mcp/prompts/dashboards/index.ts index a589f443e3..08d0469181 100644 --- a/packages/api/src/mcp/prompts/dashboards/index.ts +++ b/packages/api/src/mcp/prompts/dashboards/index.ts @@ -3,6 +3,7 @@ import { z } from 'zod'; import { getConnectionsByTeam } from '@/controllers/connection'; import { getSources } from '@/controllers/sources'; import type { PromptDefinition } from '@/mcp/tools/types'; +import { isHiddenSource } from '@/mcp/utils/promql'; import logger from '@/utils/logger'; import { @@ -44,10 +45,11 @@ const dashboardPrompts: PromptDefinition = (server, context) => { let logSourceId: string; try { - const [sources, connections] = await Promise.all([ + const [allSources, connections] = await Promise.all([ getSources(teamId), getConnectionsByTeam(teamId), ]); + const sources = allSources.filter(s => !isHiddenSource(context, s)); sourceSummary = buildSourceSummary( sources.map(s => ({ diff --git a/packages/api/src/mcp/tools/sources/deleteSource.ts b/packages/api/src/mcp/tools/sources/deleteSource.ts index be9850735a..c3c064301f 100644 --- a/packages/api/src/mcp/tools/sources/deleteSource.ts +++ b/packages/api/src/mcp/tools/sources/deleteSource.ts @@ -1,9 +1,10 @@ import { z } from 'zod'; -import { deleteSource } from '@/controllers/sources'; +import { deleteSource, getSource } from '@/controllers/sources'; import { IDEMPOTENT_DESTRUCTIVE_TOOL } from '@/mcp/tools/annotations'; import type { ToolRegistrar } from '@/mcp/tools/types'; import { mcpUserError } from '@/mcp/utils/errors'; +import { isHiddenSource } from '@/mcp/utils/promql'; import { objectIdSchema } from '@/utils/zod'; export function registerDeleteSource({ @@ -29,6 +30,11 @@ export function registerDeleteSource({ }), }, async ({ id: sourceId }) => { + const source = await getSource(teamId, sourceId); + if (source && isHiddenSource(context, source)) { + return mcpUserError('Source not found'); + } + // Team-scoped; returns null when no such source exists for this team, so // its return value (not a separate pre-check) is the success signal. const deleted = await deleteSource(teamId, sourceId); diff --git a/packages/api/src/mcp/tools/sources/describePromqlSource.ts b/packages/api/src/mcp/tools/sources/describePromqlSource.ts new file mode 100644 index 0000000000..44c8fdd2fc --- /dev/null +++ b/packages/api/src/mcp/tools/sources/describePromqlSource.ts @@ -0,0 +1,107 @@ +import { quoteIdentifierIfNeeded } from '@hyperdx/common-utils/dist/core/metadata'; + +import { getPromqlLabelCatalog } from '@/controllers/promql'; +import type { ToolResult } from '@/mcp/tools/types'; +import { PROMQL_QUERY_USAGE, timeSeriesSqlUsage } from '@/mcp/utils/promql'; +import logger from '@/utils/logger'; + +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 + * instead of a column schema, from either a ClickHouse TimeSeries table or a + * Prometheus-compatible endpoint. + */ +export async function describePromqlSource({ + teamId, + source, + meta, + signal, + progress, +}: { + teamId: string; + source: Parameters[0]['source']; + meta: Record; + signal: AbortSignal; + /** Same contract as describe_source's: snapshot answers a stuck lookup. */ + progress: { snapshot?: () => ToolResult; partial?: boolean }; +}): Promise { + const partialResult = (error: string) => + toResult({ source: meta, partial: true, error }); + + // The lookup may outlive the tool's deadline (the tags-table column check + // ignores the signal), so the config is answerable from the start. + progress.snapshot = () => + partialResult('Listing labels timed out before it finished.'); + + const endMs = Date.now(); + try { + const catalog = await getPromqlLabelCatalog({ + teamId, + source, + startMs: endMs - LABEL_LOOKBACK_MS, + endMs, + limit: Math.max(MAX_LABEL_NAMES, MAX_METRIC_NAMES), + signal, + }); + const truncated = + catalog.labelNames.length > MAX_LABEL_NAMES || + catalog.metricNames.length > MAX_METRIC_NAMES; + const table = + catalog.backend === 'clickhouse' + ? `${quoteIdentifierIfNeeded(source.from.databaseName)}.${quoteIdentifierIfNeeded(source.from.tableName)}` + : undefined; + return toResult( + { + source: { + ...meta, + 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.`, + } + : {}), + }, + table && timeSeriesSqlUsage(table), + ); + } catch (e) { + logger.warn( + { teamId, error: e }, + 'Failed to list labels for a PromQL source', + ); + progress.partial = true; + return partialResult( + `Could not list labels: ${e instanceof Error ? e.message : String(e)}`, + ); + } +} + +function toResult( + output: Record, + sqlUsage?: string, +): ToolResult { + return { + content: [ + { + type: 'text' as const, + text: JSON.stringify( + { + ...output, + nextSteps: { + query: PROMQL_QUERY_USAGE, + ...(sqlUsage ? { sql: sqlUsage } : {}), + }, + }, + null, + 2, + ), + }, + ], + }; +} diff --git a/packages/api/src/mcp/tools/sources/describeSource.ts b/packages/api/src/mcp/tools/sources/describeSource.ts index ff52479143..aeda0f3bd8 100644 --- a/packages/api/src/mcp/tools/sources/describeSource.ts +++ b/packages/api/src/mcp/tools/sources/describeSource.ts @@ -13,8 +13,9 @@ import { ClickhouseClient } from '@/clickhouse'; import { getConnectionById } from '@/controllers/connection'; import { getSource } from '@/controllers/sources'; import { READ_ONLY_TOOL } from '@/mcp/tools/annotations'; -import type { ToolRegistrar, ToolResult } from '@/mcp/tools/types'; +import type { McpContext, ToolRegistrar, ToolResult } from '@/mcp/tools/types'; import { mcpServerError, mcpUserError } from '@/mcp/utils/errors'; +import { isHiddenSource } from '@/mcp/utils/promql'; import { MCP_QUERY_MAX_EXECUTION_SEC, MCP_TIMEOUT_GRACE_MS, @@ -25,6 +26,7 @@ import { import logger from '@/utils/logger'; import { trimToolResponse } from '@/utils/trimToolResponse'; +import { describePromqlSource } from './describePromqlSource'; import { DISCOVERABLE_METRIC_KINDS, QUERYABLE_METRIC_KINDS, @@ -87,13 +89,14 @@ type DescribeProgress = { * `signal` aborts, and the result is flagged partial. */ async function describeSourceSchema( - teamId: string, + context: McpContext, sourceId: string, signal: AbortSignal, progress: DescribeProgress, ): Promise { + const { teamId } = context; const source = await getSource(teamId, sourceId); - if (!source) { + if (!source || isHiddenSource(context, source)) { return mcpUserError( `Source "${sourceId}" not found. Call clickstack_list_sources to see available source IDs.`, ); @@ -114,6 +117,10 @@ async function describeSourceSchema( meta.section = source.section; } + if (source.kind === SourceKind.Promql) { + return describePromqlSource({ teamId, source, meta, signal, progress }); + } + if ( 'eventAttributesExpression' in source && source.eventAttributesExpression @@ -617,8 +624,7 @@ export function registerDescribeSource({ let outcome: TimeoutOutcome; try { outcome = await runWithTimeout( - signal => - describeSourceSchema(teamId.toString(), sourceId, signal, progress), + signal => describeSourceSchema(context, sourceId, signal, progress), { timeoutMs: DESCRIBE_TIMEOUT_MS, graceMs: DESCRIBE_BACKSTOP_MS }, ); } catch (e) { diff --git a/packages/api/src/mcp/tools/sources/listSources.ts b/packages/api/src/mcp/tools/sources/listSources.ts index ce65fa822d..9ec04b200d 100644 --- a/packages/api/src/mcp/tools/sources/listSources.ts +++ b/packages/api/src/mcp/tools/sources/listSources.ts @@ -19,6 +19,7 @@ import { import { getSources } from '@/controllers/sources'; import { READ_ONLY_TOOL } from '@/mcp/tools/annotations'; import type { ToolRegistrar } from '@/mcp/tools/types'; +import { isHiddenSource, PROMQL_SOURCE_USAGE } from '@/mcp/utils/promql'; import { runWithTimeout } from '@/mcp/utils/timeout'; import logger from '@/utils/logger'; @@ -237,7 +238,14 @@ export function registerListSources({ metricTables: Record; }> = []; - const sourceSummaries = sources.map(s => { + const prometheusEndpointIds = new Set( + connections + .filter(c => c.isPrometheusEndpoint) + .map(c => c._id.toString()), + ); + + const visibleSources = sources.filter(s => !isHiddenSource(context, s)); + const sourceSummaries = visibleSources.map(s => { const meta: Record = { id: s._id.toString(), name: s.name, @@ -250,10 +258,20 @@ export function registerListSources({ // be. An unqualified name only resolves in the connection's default // database. Metric sources query the tables in metricTables, even // when from.tableName is set, so they get the database instead. - if (s.kind !== SourceKind.Metric && s.from.tableName) { - meta.table = `${quoteIdentifierIfNeeded(s.from.databaseName)}.${quoteIdentifierIfNeeded(s.from.tableName)}`; - } else { - meta.database = quoteIdentifierIfNeeded(s.from.databaseName); + // A PromQL source on a Prometheus endpoint has no ClickHouse table: + // its database and table are placeholders, so it gets neither. + const onPrometheusEndpoint = + s.kind === SourceKind.Promql && + prometheusEndpointIds.has(s.connection.toString()); + if (s.kind === SourceKind.Promql) { + meta.backend = onPrometheusEndpoint ? 'prometheus' : 'clickhouse'; + } + if (!onPrometheusEndpoint) { + if (s.kind !== SourceKind.Metric && s.from.tableName) { + meta.table = `${quoteIdentifierIfNeeded(s.from.databaseName)}.${quoteIdentifierIfNeeded(s.from.tableName)}`; + } else { + meta.database = quoteIdentifierIfNeeded(s.from.databaseName); + } } if (s.section) { @@ -342,6 +360,9 @@ export function registerListSources({ '(cpu/memory gauges).', } : {}), + ...(visibleSources.some(s => s.kind === SourceKind.Promql) + ? { promqlUsage: PROMQL_SOURCE_USAGE } + : {}), nextStep: 'Call clickstack_describe_source with a sourceId above to get the full column schema, ' + 'attribute keys, and sampled low-cardinality values before writing queries. ' + diff --git a/packages/api/src/mcp/tools/sources/saveSource.ts b/packages/api/src/mcp/tools/sources/saveSource.ts index f796c32ce3..ed90ed4ef0 100644 --- a/packages/api/src/mcp/tools/sources/saveSource.ts +++ b/packages/api/src/mcp/tools/sources/saveSource.ts @@ -11,9 +11,10 @@ import { mcpUserError, validateObjectId, } from '@/mcp/utils/errors'; +import { isPromqlEnabled } from '@/mcp/utils/promql'; import { isDuplicateKeyError } from '@/utils/errors'; -import { buildSourceInput, mcpSaveSourceSchema } from './schemas'; +import { buildMcpSaveSourceSchema, buildSourceInput } from './schemas'; export function registerSaveSource({ context, @@ -40,7 +41,7 @@ export function registerSaveSource({ 'spanNameExpression, spanKindExpression; session needs traceSourceId; ' + 'metric needs metricTables and resourceAttributesExpression. Get ' + 'connection and source IDs from clickstack_list_sources.', - inputSchema: mcpSaveSourceSchema, + inputSchema: buildMcpSaveSourceSchema(isPromqlEnabled(context)), }, async input => { const sourceId = input.id; diff --git a/packages/api/src/mcp/tools/sources/schemas.ts b/packages/api/src/mcp/tools/sources/schemas.ts index b3be96b1b6..f2cc515173 100644 --- a/packages/api/src/mcp/tools/sources/schemas.ts +++ b/packages/api/src/mcp/tools/sources/schemas.ts @@ -19,6 +19,12 @@ const _assertSourceKindsMatchEnum: readonly (typeof SOURCE_KINDS)[number][] = [ ]; void _assertSourceKindsMatchEnum; +const NON_PROMQL_SOURCE_KINDS = ['log', 'trace', 'session', 'metric'] as const; + +const KIND_DESCRIPTION = + 'Type of telemetry the table holds. Determines which other fields are ' + + 'required; see the tool description.'; + // Advanced nested config, modeled loosely (SourceSchemaNoId does the strict // validation) so it round-trips through describe_source -> save_source. // Granularity fields use the internal " " form (e.g. "5 minute"). @@ -150,7 +156,7 @@ const metricTablesSchema = z 'metric sources.', ); -export const mcpSaveSourceSchema = z.object({ +const mcpSaveSourceSchema = z.object({ // ── Update selector ── id: z .string() @@ -160,12 +166,7 @@ export const mcpSaveSourceSchema = z.object({ ), // ── Always required (base) ── - kind: z - .enum(SOURCE_KINDS) - .describe( - 'Type of telemetry the table holds. Determines which other fields are ' + - 'required; see the tool description.', - ), + kind: z.enum(SOURCE_KINDS).describe(KIND_DESCRIPTION), name: z.string().min(1).describe('Display name shown in source selectors.'), connection: z .string() @@ -451,6 +452,15 @@ export const mcpSaveSourceSchema = z.object({ export type McpSaveSourceInput = z.infer; +/** The save_source input schema, without the promql kind when PromQL is off. */ +export function buildMcpSaveSourceSchema(promqlEnabled: boolean) { + return promqlEnabled + ? mcpSaveSourceSchema + : mcpSaveSourceSchema.extend({ + kind: z.enum(NON_PROMQL_SOURCE_KINDS).describe(KIND_DESCRIPTION), + }); +} + /** * Assemble the flat MCP input into the SourceSchemaNoId shape: nest from.*, * drop the update selector `id`, and strip undefined keys so they don't collide diff --git a/packages/api/src/mcp/tools/types.ts b/packages/api/src/mcp/tools/types.ts index 920c6b94a6..7aa68a11e3 100644 --- a/packages/api/src/mcp/tools/types.ts +++ b/packages/api/src/mcp/tools/types.ts @@ -15,6 +15,11 @@ export type McpContext = { * Identity of the calling MCP client application, parsed from User-Agent. */ mcpClient?: McpClientInfo; + /** + * Whether PromQL sources, tiles and filters are exposed. Defaults to the + * deployment's ENABLE_PROMQL; read it through `isPromqlEnabled`. + */ + promqlEnabled?: boolean; }; /** diff --git a/packages/api/src/mcp/utils/promql.ts b/packages/api/src/mcp/utils/promql.ts new file mode 100644 index 0000000000..d771e05b42 --- /dev/null +++ b/packages/api/src/mcp/utils/promql.ts @@ -0,0 +1,34 @@ +import { SourceKind } from '@hyperdx/common-utils/dist/types'; + +import * as config from '@/config'; +import type { McpContext } from '@/mcp/tools/types'; + +export function isPromqlEnabled(context: McpContext): boolean { + return context.promqlEnabled ?? config.IS_PROMQL_ENABLED; +} + +/** Whether MCP hides `source` from the agent: PromQL sources while PromQL is off. */ +export function isHiddenSource( + context: McpContext, + source: { kind: SourceKind }, +): boolean { + return source.kind === SourceKind.Promql && !isPromqlEnabled(context); +} + +export const PROMQL_QUERY_USAGE = + 'PromQL sources hold Prometheus metrics and are queried with PromQL. The ' + + 'builder query tools (clickstack_timeseries, clickstack_table, ' + + 'clickstack_search) do not read them.'; + +export function timeSeriesSqlUsage(table: string): string { + return ( + `clickstack_sql can read ${table} through the timeSeriesTags(${table}) ` + + `(id, metric_name, tags) and timeSeriesData(${table}) (id, timestamp, ` + + 'value) table functions, joined on id.' + ); +} + +export const PROMQL_SOURCE_USAGE = + `${PROMQL_QUERY_USAGE} A PromQL source with backend "clickhouse" is a ` + + `ClickHouse TimeSeries table: ${timeSeriesSqlUsage('')} ` + + 'Call clickstack_describe_source to list label names and metric names.'; diff --git a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts index f0e9bb826f..836b9bdbb8 100644 --- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts +++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts @@ -7,6 +7,7 @@ import { SourceKind } from '@hyperdx/common-utils/dist/types'; import { Types } from 'mongoose'; import * as config from '@/config'; +import { joinPrometheusUpstreamUrl } from '@/controllers/promql'; import { DEFAULT_DATABASE, dropTimeSeriesTable, @@ -20,7 +21,6 @@ import { Source } from '@/models/source'; import { CLICKHOUSE_PROMETHEUS_API_PREFIX, clickhousePrometheusUpstream, - joinPrometheusUpstreamUrl, PROMETHEUS_MAX_EXEMPLAR_WINDOW_SEC, } from '@/routers/api/prometheus'; @@ -517,10 +517,9 @@ describe('prometheus router', () => { }); // Every other userinfo test asserts what's redacted for *display* (the - // error bodies below). The actual outgoing `fetch` target must still - // carry real credentials, or authenticated Prometheus connections would - // silently stop authenticating. - it('still sends real credentials to the actual upstream fetch call', async () => { + // error bodies below). The outgoing request must still carry the real + // credentials, as a header: fetch rejects a URL that includes them. + it('sends host credentials to the upstream as a Basic auth header', async () => { const { agent, team } = await getLoggedInAgent(server); const source = await seedPrometheusSource( team._id, @@ -545,7 +544,11 @@ describe('prometheus router', () => { }) .expect(200); - expect(mockFetch.mock.calls[0][0] as string).toContain('user:pw@'); + const [target, init] = mockFetch.mock.calls[0]; + expect(new URL(String(target)).username).toBe(''); + expect(init?.headers).toEqual({ + Authorization: `Basic ${Buffer.from('user:pw').toString('base64')}`, + }); }); it('leaves a host-pinned param untouched when the request never references it', async () => { diff --git a/packages/api/src/routers/api/__tests__/prometheus.test.ts b/packages/api/src/routers/api/__tests__/prometheus.test.ts index 4fc42e1510..b6a788ebeb 100644 --- a/packages/api/src/routers/api/__tests__/prometheus.test.ts +++ b/packages/api/src/routers/api/__tests__/prometheus.test.ts @@ -8,9 +8,9 @@ jest.mock('@/utils/instrumentation', () => { }; }); +import { joinPrometheusUpstreamUrl } from '@/controllers/promql'; import { isClientDisconnect, - joinPrometheusUpstreamUrl, parseDuration, parseTimestamp, recordProxyOutcome, diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts index 95cdf6e5bd..65faa1c461 100644 --- a/packages/api/src/routers/api/prometheus.ts +++ b/packages/api/src/routers/api/prometheus.ts @@ -18,6 +18,10 @@ import z from 'zod'; import { ClickhouseClient } from '@/clickhouse'; import { getConnectionById } from '@/controllers/connection'; +import { + joinPrometheusUpstreamUrl, + takeBasicAuthHeader, +} from '@/controllers/promql'; import { getSource } from '@/controllers/sources'; import { connectionSupportsPrometheusHttpApi, @@ -217,41 +221,6 @@ export function isClientDisconnect(err: unknown): boolean { ); } -/** - * Join a Connection host with an absolute Prometheus API path. - * - * `new URL('/api/v1/query_range', 'http://host:8481/select/0/prometheus')` - * discards `/select/0/prometheus` because an absolute path replaces the base - * pathname. VictoriaMetrics cluster (and any Prometheus-compatible server - * mounted under a prefix) needs that prefix kept. Host userinfo, query, and - * hash are left untouched. - * - * `path` must be an absolute path (every call site passes a literal starting - * with `/`) -- this is not a general-purpose URL joiner. - * - * @see https://github.com/hyperdxio/hyperdx/issues/3046 - */ -export function joinPrometheusUpstreamUrl( - upstreamHost: string, - path: string, -): URL { - const url = new URL(upstreamHost); - // `new URL('prometheus:9090')` succeeds with an opaque path (`prometheus:` - // scheme). The pathname setter is a no-op there, so without this guard the - // helper would return the host unchanged, `fetch` would fail, and the proxy - // would 502 / increment query_errors for a user misconfiguration. Same check - // as clickhouseProxy.ts. - if (url.protocol !== 'http:' && url.protocol !== 'https:') { - throw new TypeError('Connection host must be http(s)'); - } - // Strip ALL trailing slashes, not just one -- a host saved with a doubled - // trailing slash (e.g. `http://prom:9090//`) would otherwise leave a `//` - // in the joined path, which most servers treat as a distinct (404) path. - const basePath = url.pathname.replace(/\/+$/, ''); - url.pathname = `${basePath}${path}`; - return url; -} - // Only real Prometheus API params are ever caller-settable in // proxyToPrometheus's query merge below. `params` there is built upstream by // spreading the *entire* `req.query`/`req.body` with no allowlist (see @@ -380,7 +349,10 @@ async function clickhouseServesPrometheusHttpApi( ); url.searchParams.set('query', 'up'); const resp = await fetch(url, { - headers: clickhouseAuthHeaders(connection), + headers: { + ...takeBasicAuthHeader(url), + ...clickhouseAuthHeaders(connection), + }, signal: AbortSignal.timeout(PROBE_TIMEOUT_MS), }); return typeof JSON.parse(await resp.text())?.status === 'string'; @@ -458,6 +430,9 @@ async function proxyToPrometheus( url.searchParams.set(k, v); } } + // fetch rejects a URL that carries credentials, so `user:pw@` in the host + // becomes a Basic auth header instead. + const authHeader = takeBasicAuthHeader(url); const target = url.toString(); // A connection host may carry basic-auth credentials (`http://user:pw@host`) @@ -467,14 +442,13 @@ async function proxyToPrometheus( // the browser. `url.origin` never includes userinfo (WHATWG URL spec), and // dropping the query/hash entirely -- rather than trying to redact only the // secret-shaped parts of it -- means there's no query key to ever miss. - // `target` itself is unaffected and still carries everything needed to - // authenticate. + // `target` still carries the query string, and `authHeader` the userinfo. const redactedTarget = `${url.origin}${url.pathname}`; let upstreamResp: Response; try { upstreamResp = await fetch(target, { - headers, + headers: { ...authHeader, ...headers }, signal: AbortSignal.timeout(PROMETHEUS_PROXY_TIMEOUT_MS), }); } catch (err) {