From f4bbfdbb958b049796277c80891746c12ca23803 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Wed, 11 Nov 2026 14:59:53 +0100 Subject: [PATCH 1/3] refactor(api): match the time ranges probe's ClickHouse errors by code --- .../__tests__/timeseriesEngine.test.ts | 33 ++++++++++++ .../api/src/controllers/timeseriesEngine.ts | 25 ++++++--- packages/api/src/mcp/__tests__/query.test.ts | 2 +- packages/api/src/mcp/tools/query/helpers.ts | 51 +------------------ packages/api/src/utils/clickhouseErrors.ts | 45 ++++++++++++++++ 5 files changed, 98 insertions(+), 58 deletions(-) create mode 100644 packages/api/src/utils/clickhouseErrors.ts diff --git a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts index f1f0d2b36e..499d2aebc1 100644 --- a/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts +++ b/packages/api/src/controllers/__tests__/timeseriesEngine.test.ts @@ -1,8 +1,41 @@ +import { ClickHouseError } from '@clickhouse/client-common'; +import { ClickHouseQueryError } from '@hyperdx/common-utils/dist/clickhouse'; + import { formatMatrixResponse, formatVectorResponse, + isNoTimeRangesTableError, } from '@/controllers/timeseriesEngine'; +describe('isNoTimeRangesTableError', () => { + // The shape ClickhouseClient throws: the server's error wrapped as `cause`. + const queryError = (type: string) => + Object.assign(new ClickHouseQueryError('query failed', 'DESCRIBE ...'), { + cause: new ClickHouseError({ message: 'failed', code: '0', type }), + }); + + it.each([ + ['UNKNOWN_FUNCTION', 'servers before 26.10'], + ['UNKNOWN_TABLE', '26.10 and later'], + ])('accepts %s (%s)', type => { + expect(isNoTimeRangesTableError(queryError(type))).toBe(true); + }); + + it('rejects any other ClickHouse error', () => { + expect(isNoTimeRangesTableError(queryError('TIMEOUT_EXCEEDED'))).toBe( + false, + ); + }); + + it('rejects an error without a ClickHouse type, whatever its message', () => { + expect( + isNoTimeRangesTableError( + new Error('Unknown table function timeSeriesTimeRanges'), + ), + ).toBe(false); + }); +}); + describe('formatMatrixResponse', () => { it('converts prometheusQueryRange rows into Prometheus matrix shape', () => { const rows = [ diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index 5c76bd8ef2..df09e33c36 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -15,6 +15,7 @@ import { } from '@hyperdx/common-utils/dist/types'; import { ClickhouseClient } from '@/clickhouse'; +import { getClickHouseErrorType } from '@/utils/clickhouseErrors'; import logger from '@/utils/logger'; export const PROMETHEUS_MAX_EXECUTION_SEC = 30; @@ -33,14 +34,21 @@ export type TimeSeriesTagsQueryArgs = { signal?: AbortSignal; }; -// What a table without a time ranges table answers (version 7 or earlier, or -// store_time_ranges off): servers before 26.10 have no timeSeriesTimeRanges(), -// newer ones reject the table. Any other error is unexpected. -const isNoTimeRangesTableError = (e: unknown): boolean => - e instanceof Error && - /Unknown table function timeSeriesTimeRanges|has no TimeRanges target table/.test( - e.message, - ); +/** + * What a table without a time ranges table answers (version 7 or earlier, or + * store_time_ranges off): servers before 26.10 have no timeSeriesTimeRanges() + * (UNKNOWN_FUNCTION), newer ones reject the table (UNKNOWN_TABLE). Any other + * error is unexpected. + * + * 26.10 also answers UNKNOWN_TABLE when the TimeSeries table itself is + * missing. That case already failed, and warned, on the Tags probe that + * timeSeriesTimeBounds runs first, so treating it as expected here hides + * nothing. + */ +export const isNoTimeRangesTableError = (e: unknown): boolean => { + const type = getClickHouseErrorType(e); + return type === 'UNKNOWN_FUNCTION' || type === 'UNKNOWN_TABLE'; +}; /** * Where the given TimeSeries table keeps each series' time range, if anywhere: @@ -60,6 +68,7 @@ async function timeSeriesTimeBounds({ database: string; table: string; }): Promise<'tags' | 'timeRanges' | undefined> { + // Tags first: a version 7 table returns there without a second query. for (const [innerTableType, bounds] of [ ['Tags', 'tags'], ['TimeRanges', 'timeRanges'], diff --git a/packages/api/src/mcp/__tests__/query.test.ts b/packages/api/src/mcp/__tests__/query.test.ts index 1fb8106a59..c45606b11a 100644 --- a/packages/api/src/mcp/__tests__/query.test.ts +++ b/packages/api/src/mcp/__tests__/query.test.ts @@ -17,7 +17,6 @@ import { assertSourceKindMatchesSelect, clickHouseErrorResult, errorHint, - getClickHouseErrorType, INCREASE_TOP_N_CAP, isServerError, mergeWhereIntoSelectItems, @@ -34,6 +33,7 @@ import { } from '@/mcp/tools/query/schemas'; import { defaultOrderBy, resolveOrderBy } from '@/mcp/tools/query/table'; import { getErrorCategory } from '@/mcp/utils/errors'; +import { getClickHouseErrorType } from '@/utils/clickhouseErrors'; describe('parseTimeRange', () => { it('should return default range (last 15 minutes) when no arguments provided', () => { diff --git a/packages/api/src/mcp/tools/query/helpers.ts b/packages/api/src/mcp/tools/query/helpers.ts index 98871ea467..282bf2f056 100644 --- a/packages/api/src/mcp/tools/query/helpers.ts +++ b/packages/api/src/mcp/tools/query/helpers.ts @@ -1,7 +1,4 @@ -import { - ClickHouseError, - type ClickHouseSettings, -} from '@clickhouse/client-common'; +import { type ClickHouseSettings } from '@clickhouse/client-common'; import { getHeatmapMode } from '@hyperdx/common-utils/dist/core/heatmap'; import { getMetadata } from '@hyperdx/common-utils/dist/core/metadata'; import { PromqlChartError } from '@hyperdx/common-utils/dist/core/promqlChart'; @@ -45,6 +42,7 @@ import { isConfigTile, } from '@/routers/external-api/v2/utils/dashboards'; import { isQueryTimeoutError } from '@/tasks/checkAlerts/errors'; +import { isClickHouseError } from '@/utils/clickhouseErrors'; import { trimToolResponse } from '@/utils/trimToolResponse'; import type { ExternalDashboardTileWithId } from '@/utils/zod'; import { externalDashboardTileSchemaWithId } from '@/utils/zod'; @@ -843,51 +841,6 @@ const SERVER_NODE_ERROR_CODES = new Set([ 'UND_ERR_CONNECT_TIMEOUT', ]); -/** - * Check whether an error is a ClickHouseError, using both `instanceof` - * and a constructor-name fallback. The fallback handles the case where - * multiple copies of `@clickhouse/client-common` are installed (e.g. - * the root workspace uses one version while `common-utils` bundles - * another). In that scenario, `instanceof` fails because the class - * identities are different even though the shapes are identical. - */ -function isClickHouseError( - err: unknown, -): err is ClickHouseError & { type: string } { - if (err instanceof ClickHouseError) return true; - if ( - err instanceof Error && - err.constructor?.name === 'ClickHouseError' && - 'type' in err && - typeof (err as Record).type === 'string' - ) { - return true; - } - return false; -} - -/** - * Extract the ClickHouse error `type` from an error, walking `.cause` to - * find the original `ClickHouseError` from `@clickhouse/client-common`. - * - * Uses `instanceof` with a constructor-name fallback to handle duplicate - * package installations across the monorepo (see `isClickHouseError`). - * - * @internal Exported for testing only. - */ -export function getClickHouseErrorType(e: unknown): string | undefined { - if (!(e instanceof Error)) return undefined; - // ClickHouseQueryError wraps the original ClickHouseError as .cause - if (isClickHouseError(e.cause)) { - return e.cause.type; - } - // Direct ClickHouseError (has .type on itself) - if (isClickHouseError(e)) { - return e.type; - } - return undefined; -} - /** * Check whether an error represents a system-level infrastructure failure * rather than a user query issue. Covers both ClickHouse server-side error diff --git a/packages/api/src/utils/clickhouseErrors.ts b/packages/api/src/utils/clickhouseErrors.ts new file mode 100644 index 0000000000..cc0b1c1f0e --- /dev/null +++ b/packages/api/src/utils/clickhouseErrors.ts @@ -0,0 +1,45 @@ +import { ClickHouseError } from '@clickhouse/client-common'; + +/** + * Check whether an error is a ClickHouseError, using both `instanceof` + * and a constructor-name fallback. The fallback handles the case where + * multiple copies of `@clickhouse/client-common` are installed (e.g. + * the root workspace uses one version while `common-utils` bundles + * another). In that scenario, `instanceof` fails because the class + * identities are different even though the shapes are identical. + */ +export function isClickHouseError( + err: unknown, +): err is ClickHouseError & { type: string } { + if (err instanceof ClickHouseError) return true; + if ( + err instanceof Error && + err.constructor?.name === 'ClickHouseError' && + 'type' in err && + typeof err.type === 'string' + ) { + return true; + } + return false; +} + +/** + * Extract the ClickHouse error `type` (e.g. `UNKNOWN_TABLE`) from an error, + * walking `.cause` to find the original `ClickHouseError` from + * `@clickhouse/client-common`. + * + * Uses `instanceof` with a constructor-name fallback to handle duplicate + * package installations across the monorepo (see `isClickHouseError`). + */ +export function getClickHouseErrorType(e: unknown): string | undefined { + if (!(e instanceof Error)) return undefined; + // ClickHouseQueryError wraps the original ClickHouseError as .cause + if (isClickHouseError(e.cause)) { + return e.cause.type; + } + // Direct ClickHouseError (has .type on itself) + if (isClickHouseError(e)) { + return e.type; + } + return undefined; +} From 4df8f14aa1c6e34962276deb4e37fdfd805ea9d3 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Wed, 11 Nov 2026 15:19:44 +0100 Subject: [PATCH 2/3] feat(api): serve PromQL label lookups from ClickHouse's Prometheus HTTP API --- .../promql-labels-via-clickhouse-http-api.md | 8 + agent_docs/observability.md | 7 +- .../__tests__/promqlBackend.test.ts | 79 ++++++- packages/api/src/controllers/promql.ts | 82 +++++-- .../api/src/controllers/timeseriesEngine.ts | 31 ++- .../api/__tests__/prometheus.int.test.ts | 200 ++++++++++++++++++ packages/api/src/routers/api/prometheus.ts | 52 +++-- .../src/__tests__/clickhouseVersion.test.ts | 23 ++ .../src/core/clickhouseVersion.ts | 13 ++ 9 files changed, 438 insertions(+), 57 deletions(-) create mode 100644 .changeset/promql-labels-via-clickhouse-http-api.md diff --git a/.changeset/promql-labels-via-clickhouse-http-api.md b/.changeset/promql-labels-via-clickhouse-http-api.md new file mode 100644 index 0000000000..7821da8ad2 --- /dev/null +++ b/.changeset/promql-labels-via-clickhouse-http-api.md @@ -0,0 +1,8 @@ +--- +'@hyperdx/api': minor +'@hyperdx/common-utils': minor +--- + +feat: serve PromQL label lookups from ClickHouse's Prometheus HTTP API + +On ClickHouse 26.9+ with the `prometheus_api_v1` handler, `/labels` and `/label/:name/values` for a ClickHouse-backed PromQL source now proxy to the handler instead of querying the TimeSeries tags table with SQL. Older servers, servers without the handler, and sources set to `table-function` keep the SQL path, so a source that runs queries over HTTP on 26.6-26.8 still gets label autocomplete. diff --git a/agent_docs/observability.md b/agent_docs/observability.md index d19c2e922d..1de5356b42 100644 --- a/agent_docs/observability.md +++ b/agent_docs/observability.md @@ -305,9 +305,10 @@ calls `/api/v1/prometheus/query{,_range}`, and `/labels` and (`QUERY_ATTRIBUTION_HEADER`), and the router parses it with `parseLogComment` into a `withQueryAttribution` scope layered over the auth middleware's `api` scope. The browser's surface and ids win; `trace` and `label` stay the -server's. Queries the route runs through a ClickHouse client (label lookups, -and the `prometheusQuery()`/`prometheusQueryRange()` fallback) pick the scope -up as usual. Requests proxied to ClickHouse's Prometheus HTTP API on 26.6+ are tagged +server's. Queries the route runs through a ClickHouse client (label lookups +before 26.9, and the `prometheusQuery()`/`prometheusQueryRange()` fallback) +pick the scope up as usual. Requests proxied to ClickHouse's Prometheus HTTP +API (queries on 26.6+, label lookups on 26.9+) are tagged by `clickhousePrometheusRequest`: `log_comment` as a URL setting, `query_id` as the `X-ClickHouse-Query-Id` header, because the handler rejects `query_id` as an unknown setting in the URL. diff --git a/packages/api/src/controllers/__tests__/promqlBackend.test.ts b/packages/api/src/controllers/__tests__/promqlBackend.test.ts index 99e8af0567..43dea0a04a 100644 --- a/packages/api/src/controllers/__tests__/promqlBackend.test.ts +++ b/packages/api/src/controllers/__tests__/promqlBackend.test.ts @@ -2,17 +2,24 @@ import type { PromqlSourceBackend } from '@hyperdx/common-utils/dist/types'; import { clearBackendProbeCache, + resolveClickhouseLabelBackend, resolveClickhouseQueryBackend, } from '@/controllers/promql'; -import { connectionSupportsPrometheusHttpApi } from '@/controllers/timeseriesEngine'; +import { connectionPrometheusHttpApiSupport } from '@/controllers/timeseriesEngine'; jest.mock('@/controllers/timeseriesEngine', () => ({ ...jest.requireActual('@/controllers/timeseriesEngine'), - connectionSupportsPrometheusHttpApi: jest.fn(), + connectionPrometheusHttpApiSupport: jest.fn(), })); const mockFetch = jest.mocked(global.fetch); -const mockVersionCheck = jest.mocked(connectionSupportsPrometheusHttpApi); +const mockVersionCheck = jest.mocked(connectionPrometheusHttpApiSupport); + +// What the version check answers for a server on each side of the 26.6 (query) +// and 26.9 (label lookup) cut-offs. +const CLICKHOUSE_26_10 = { query: true, labels: true }; +const CLICKHOUSE_26_8 = { query: true, labels: false }; +const CLICKHOUSE_26_5 = { query: false, labels: false }; const handlerAnswer = () => new Response(JSON.stringify({ status: 'success', data: 'up' })); @@ -46,7 +53,7 @@ describe('resolveClickhouseQueryBackend', () => { beforeEach(() => { clearBackendProbeCache(); mockVersionCheck.mockReset(); - mockVersionCheck.mockResolvedValue(true); + mockVersionCheck.mockResolvedValue(CLICKHOUSE_26_10); mockFetch.mockReset(); }); @@ -74,7 +81,7 @@ describe('resolveClickhouseQueryBackend', () => { }); it('keeps a server that is too old without probing the handler', async () => { - mockVersionCheck.mockResolvedValue(false); + mockVersionCheck.mockResolvedValue(CLICKHOUSE_26_5); await expect(resolveKind()).resolves.toBe('table-function'); await expect(resolveKind()).resolves.toBe('table-function'); @@ -196,3 +203,65 @@ describe('resolveClickhouseQueryBackend', () => { }); }); }); + +describe('resolveClickhouseLabelBackend', () => { + const resolveLabels = (promqlBackend?: PromqlSourceBackend) => + resolveClickhouseLabelBackend( + // @ts-expect-error mock connection carries only the fields the resolver reads + target(undefined, promqlBackend), + ); + + beforeEach(() => { + clearBackendProbeCache(); + mockVersionCheck.mockReset(); + mockFetch.mockReset(); + mockFetch.mockImplementation(async () => handlerAnswer()); + }); + + it.each([[undefined], ['auto' as const], ['clickhouse-http' as const]])( + 'uses the HTTP API on 26.9+ with the handler when the setting is %s', + async setting => { + mockVersionCheck.mockResolvedValue(CLICKHOUSE_26_10); + + await expect(resolveLabels(setting)).resolves.toBe('clickhouse-http'); + }, + ); + + // 26.6-26.8 answers queries over HTTP but not label lookups, so a source on + // the HTTP API must not lose its labels there. + it.each([[undefined], ['clickhouse-http' as const]])( + 'reads labels with SQL on 26.8 even though queries use the HTTP API (setting %s)', + async setting => { + mockVersionCheck.mockResolvedValue(CLICKHOUSE_26_8); + + await expect(resolveLabels(setting)).resolves.toBe('sql'); + await expect(resolveKind(undefined, setting)).resolves.toBe( + 'clickhouse-http', + ); + }, + ); + + it('reads labels with SQL when the server has no handler', async () => { + mockVersionCheck.mockResolvedValue(CLICKHOUSE_26_10); + mockFetch.mockImplementation(async () => noHandlerAnswer()); + + await expect(resolveLabels()).resolves.toBe('sql'); + }); + + it('reads labels with SQL for a table-function source, without a probe', async () => { + await expect(resolveLabels('table-function')).resolves.toBe('sql'); + + expect(mockVersionCheck).not.toHaveBeenCalled(); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('shares the kept probe answer with queries', async () => { + mockVersionCheck.mockResolvedValue(CLICKHOUSE_26_10); + + await resolveKind(); + await expect(resolveLabels()).resolves.toBe('clickhouse-http'); + + expect(mockVersionCheck).toHaveBeenCalledTimes(1); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/api/src/controllers/promql.ts b/packages/api/src/controllers/promql.ts index 3e687c0b20..2227194eff 100644 --- a/packages/api/src/controllers/promql.ts +++ b/packages/api/src/controllers/promql.ts @@ -18,9 +18,10 @@ import { ClickhouseClient } from '@/clickhouse'; import { getConnectionById } from '@/controllers/connection'; import { getSource } from '@/controllers/sources'; import { - connectionSupportsPrometheusHttpApi, + connectionPrometheusHttpApiSupport, PROMETHEUS_MAX_EXECUTION_SEC, PROMETHEUS_MAX_RESULT_ROWS, + type PrometheusHttpApiSupport, queryInstantViaTableFunction, queryLabelNames, queryLabelValues, @@ -486,11 +487,17 @@ export function clearBackendProbeCache(): void { /** The probe could not tell, so its outcome must not be kept. */ class InconclusiveProbeError extends Error {} +const NO_PROMETHEUS_HTTP_API: PrometheusHttpApiSupport = { + query: false, + labels: false, +}; + /** - * Asks a ClickHouse connection whether it answers PromQL over HTTP. Two - * conditions, cheapest first: the server is new enough (26.6+), and the - * prometheus_api_v1 handler is actually configured — a self-managed server - * needs an `` rule for that, which a stock install lacks. The + * Asks a ClickHouse connection which parts of the Prometheus HTTP API it + * serves. Two conditions, cheapest first: the server is new enough (26.6+ for + * queries, 26.9+ for label lookups), and the prometheus_api_v1 handler is + * actually configured — a self-managed server needs an `` rule + * for that, which a stock install lacks. The * handler answers every request, error or not, with Prometheus JSON * (`{status, ...}`); a server without it answers a plain-text 404. * `format_query` is used as the probe because it is the one endpoint that @@ -504,13 +511,13 @@ class InconclusiveProbeError extends Error {} async function probePrometheusHttpApi( client: ClickhouseClient, connection: { id: string; host: string; username: string; password?: string }, -): Promise { - const supported = await connectionSupportsPrometheusHttpApi({ +): Promise { + const supported = await connectionPrometheusHttpApiSupport({ client, connectionId: connection.id, }); if (supported === undefined) throw new InconclusiveProbeError(); - if (!supported) return false; + if (!supported.query) return NO_PROMETHEUS_HTTP_API; let status: number; let text: string; @@ -531,27 +538,27 @@ async function probePrometheusHttpApi( throw new InconclusiveProbeError(); } - if (typeof parseJSON(text)?.status === 'string') return true; - if (status === 404) return false; + if (typeof parseJSON(text)?.status === 'string') return supported; + if (status === 404) return NO_PROMETHEUS_HTTP_API; throw new InconclusiveProbeError(); } /** - * Whether a ClickHouse connection answers PromQL over HTTP, keeping a - * definitive answer for BACKEND_PROBE_TTL_MS so each query does not pay for a - * `SELECT version()` and a probe request first. Keyed on the host as well as - * the connection, so editing a connection's host drops its answer. Concurrent - * callers share one probe. An inconclusive probe is answered "no" for this - * request only. + * Which parts of the Prometheus HTTP API a ClickHouse connection serves, + * keeping a definitive answer for BACKEND_PROBE_TTL_MS so each request does + * not pay for a `SELECT version()` and a probe request first. Keyed on the + * host as well as the connection, so editing a connection's host drops its + * answer. Concurrent callers share one probe. An inconclusive probe is + * answered "none" for this request only. */ -async function clickhouseServesPrometheusHttpApi( +async function prometheusHttpApiSupport( client: ClickhouseClient, connection: { id: string; host: string; username: string; password?: string }, -): Promise { +): Promise { const key = `${connection.id}|${connection.host}`; const span = trace.getActiveSpan(); - const kept = backendProbeCache.get(key); + const kept = backendProbeCache.get(key); if (kept !== undefined) { span?.setAttribute('hyperdx.promql.backend_probe', 'cached'); return kept; @@ -567,7 +574,7 @@ async function clickhouseServesPrometheusHttpApi( return served; } catch { span?.setAttribute('hyperdx.promql.backend_probe', 'inconclusive'); - return false; + return NO_PROMETHEUS_HTTP_API; } } @@ -586,10 +593,11 @@ export async function connectionServesPrometheusHttpApi(connection: { isPrometheusEndpoint?: boolean; }): Promise { if (connection.isPrometheusEndpoint) return false; - return clickhouseServesPrometheusHttpApi( + const support = await prometheusHttpApiSupport( newPromqlClickhouseClient(connection), connection, ); + return support.query; } type ClickhouseQueryBackend = @@ -631,7 +639,7 @@ export async function resolveClickhouseQueryBackend({ let useHttpApi: boolean; if (promqlBackend === 'auto') { - useHttpApi = await clickhouseServesPrometheusHttpApi(client, connection); + useHttpApi = (await prometheusHttpApiSupport(client, connection)).query; } else { span?.setAttribute('hyperdx.promql.backend_probe', 'skipped'); useHttpApi = promqlBackend === 'clickhouse-http'; @@ -657,6 +665,36 @@ export async function resolveClickhouseQueryBackend({ }; } +/** + * How to answer a label lookup (`/labels`, `/label//values`) on a + * ClickHouse target: through the Prometheus HTTP API, or with SQL against the + * tags table. The handler serves label lookups only from 26.9, three releases + * after it serves queries, so a source on the HTTP API reads labels with SQL + * on 26.6-26.8 rather than losing them. That holds for a source set to + * `clickhouse-http` too, so it is probed like `auto`. A source set to + * `table-function` reads them with SQL, with no probe. + */ +export async function resolveClickhouseLabelBackend({ + connection, + promqlBackend = 'auto', +}: PromqlTarget): Promise<'clickhouse-http' | 'sql'> { + const span = trace.getActiveSpan(); + span?.setAttribute('hyperdx.promql.backend_setting', promqlBackend); + + let labelBackend: 'clickhouse-http' | 'sql' = 'sql'; + if (promqlBackend === 'table-function') { + span?.setAttribute('hyperdx.promql.backend_probe', 'skipped'); + } else { + const support = await prometheusHttpApiSupport( + newPromqlClickhouseClient(connection), + connection, + ); + if (support.labels) labelBackend = 'clickhouse-http'; + } + span?.setAttribute('hyperdx.promql.label_backend', labelBackend); + return labelBackend; +} + /** How to evaluate PromQL on a target. Probes ClickHouse targets for the HTTP API. */ export async function resolvePromqlQueryBackend( target: PromqlTarget, diff --git a/packages/api/src/controllers/timeseriesEngine.ts b/packages/api/src/controllers/timeseriesEngine.ts index df09e33c36..6acf667016 100644 --- a/packages/api/src/controllers/timeseriesEngine.ts +++ b/packages/api/src/controllers/timeseriesEngine.ts @@ -3,7 +3,10 @@ import { chSql, concatChSql, } from '@hyperdx/common-utils/dist/clickhouse'; -import { supportsPrometheusHttpApi } from '@hyperdx/common-utils/dist/core/clickhouseVersion'; +import { + supportsPrometheusHttpApi, + supportsPrometheusLabelsApi, +} from '@hyperdx/common-utils/dist/core/clickhouseVersion'; import { Metadata, MetadataCache, @@ -179,7 +182,7 @@ async function getSeriesFilterConditions({ // that would answer a different question. if (timeBounds == null) { throw new Error( - 'match[] requires a TimeSeries table that stores time ranges (store_time_ranges = 1, or store_min_time_and_max_time = 1 before version 8)', + 'match[] requires a TimeSeries table that stores time ranges (store_time_ranges = 1, or store_min_time_and_max_time = 1 before version 8), or the prometheus_api_v1 HTTP handler on ClickHouse 26.9+', ); } conditions.push( @@ -460,19 +463,29 @@ export async function queryInstantViaTableFunction({ } /** - * Whether the connection's ClickHouse is new enough for the Prometheus HTTP - * API, or `undefined` when its version could not be read. `SELECT version()` - * runs each call (throwaway cache); callers that keep the answer decide how - * long to. + * The parts of ClickHouse's Prometheus HTTP API a server can serve: queries + * (26.6+) and label lookups (26.9+). + */ +export type PrometheusHttpApiSupport = { query: boolean; labels: boolean }; + +/** + * Which parts of the Prometheus HTTP API the connection's ClickHouse is new + * enough for, or `undefined` when its version could not be read. + * `SELECT version()` runs each call (throwaway cache); callers that keep the + * answer decide how long to. */ -export async function connectionSupportsPrometheusHttpApi({ +export async function connectionPrometheusHttpApiSupport({ client, connectionId, }: { client: ClickhouseClient; connectionId: string; -}): Promise { +}): Promise { const metadata = new Metadata(client, new MetadataCache()); const version = await metadata.getServerVersion({ connectionId }); - return version == null ? undefined : supportsPrometheusHttpApi(version); + if (version == null) return undefined; + return { + query: supportsPrometheusHttpApi(version), + labels: supportsPrometheusLabelsApi(version), + }; } 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 b5ed7ddfab..5b5bdba683 100644 --- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts +++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts @@ -1823,6 +1823,8 @@ describe('prometheus router', () => { // controllers/__tests__/timeseriesEngine.int.test.ts. What only the route // can show is that query-string params reach it intact — bounds in unix // seconds where the controller takes milliseconds, and a stringified limit. + // The sources are set to table-function so this is the SQL path on any + // server version. describe('ClickHouse-backed', () => { const TABLE = 'prom_label_values_bounded_test'; const OLD_METRIC = 'label_values_old_metric'; @@ -1840,6 +1842,7 @@ describe('prometheus router', () => { const { agent, team } = await getLoggedInAgent(server); const source = await seedClickHouseSource(team._id, { tableName: TABLE, + promqlBackend: 'table-function', }); const res = await agent .get(`/v1/prometheus/label/${labelName}/values`) @@ -1898,6 +1901,7 @@ describe('prometheus router', () => { const { agent, team } = await getLoggedInAgent(server); const source = await seedClickHouseSource(team._id, { tableName: TABLE, + promqlBackend: 'table-function', }); const res = await agent @@ -1987,6 +1991,7 @@ describe('prometheus router', () => { const { agent, team } = await getLoggedInAgent(server); const source = await seedClickHouseSource(team._id, { tableName: TABLE, + promqlBackend: 'table-function', }); const res = await agent @@ -2076,6 +2081,7 @@ describe('prometheus router', () => { expect(mockFetch).not.toHaveBeenCalled(); }); + // Set to table-function, like the label values tests above. describe('ClickHouse-backed', () => { const TABLE = 'prom_labels_test'; const OLD_METRIC = 'labels_old_metric'; @@ -2091,6 +2097,7 @@ describe('prometheus router', () => { const { agent, team } = await getLoggedInAgent(server); const source = await seedClickHouseSource(team._id, { tableName: TABLE, + promqlBackend: 'table-function', }); const res = await agent .get('/v1/prometheus/labels') @@ -2189,6 +2196,7 @@ describe('prometheus router', () => { const { agent, team } = await getLoggedInAgent(server); const source = await seedClickHouseSource(team._id, { tableName: TABLE, + promqlBackend: 'table-function', }); const res = await agent @@ -2208,6 +2216,198 @@ describe('prometheus router', () => { }); }); + describe("label lookups through ClickHouse's prometheus_api_v1 handler", () => { + const ROUTES = [ + ['/v1/prometheus/labels', '/labels'], + ['/v1/prometheus/label/job/values', '/label/job/values'], + ] as const; + + const stubServerVersion = (version: [number, number, number, number]) => + jest + .spyOn(Metadata.prototype, 'getServerVersion') + .mockResolvedValue(version); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it.each(ROUTES)( + "proxies %s with the source's table, the request's params and the connection's credentials", + async (route, apiPath) => { + stubServerVersion([26, 10, 0, 0]); + const { agent, team } = await getLoggedInAgent(server); + const source = await seedClickHouseSource(team._id, { + databaseName: 'otel', + tableName: 'metrics_ts', + }); + const promResponse = { status: 'success', data: ['api'] }; + mockFetch + .mockResolvedValueOnce(handlerProbeResponse()) + .mockResolvedValueOnce(fakeUpstreamResponse(promResponse)); + + const res = await agent + .get(route) + .set( + QUERY_ATTRIBUTION_HEADER, + buildLogComment({ surface: 'dashboard', tile: 'tile-1' })!, + ) + .query( + `sourceId=${source._id.toString()}&start=1700000000&end=1700000060` + + `&limit=25&database=other_db&${matchQueryString(['up'])}`, + ) + .expect(200); + + expect(res.body).toEqual(promResponse); + expect(mockFetch).toHaveBeenCalledTimes(2); + const [calledUrl, init] = mockFetch.mock.calls[1]; + const url = new URL(String(calledUrl)); + expect(url.pathname).toBe( + `${CLICKHOUSE_PROMETHEUS_API_PREFIX}${apiPath}`, + ); + expect(url.searchParams.getAll('database')).toEqual(['otel']); + expect(url.searchParams.getAll('table')).toEqual(['metrics_ts']); + expect(url.searchParams.get('start')).toBe('1700000000'); + expect(url.searchParams.get('end')).toBe('1700000060'); + expect(url.searchParams.get('limit')).toBe('25'); + expect(url.searchParams.getAll('match[]')).toEqual(['up']); + expect(url.searchParams.has('sourceId')).toBe(false); + expect(JSON.parse(url.searchParams.get('log_comment')!)).toMatchObject({ + surface: 'dashboard', + tile: 'tile-1', + }); + expect(init?.headers).toMatchObject({ + 'X-ClickHouse-User': config.CLICKHOUSE_USER, + 'X-ClickHouse-Key': config.CLICKHOUSE_PASSWORD, + 'X-ClickHouse-Query-Id': expect.stringMatching(/^hdx-dashboard-/), + }); + }, + ); + + // 26.8's handler answers label lookups with a 400 ("not implemented"), so + // the route must never send them there. + it.each(ROUTES)( + 'reads %s with SQL on ClickHouse 26.8, whose handler serves only queries', + async route => { + stubServerVersion([26, 8, 36, 3]); + const { agent, team } = await getLoggedInAgent(server); + const source = await seedClickHouseSource(team._id, { + tableName: 'no_such_table_for_label_sql', + }); + mockFetch.mockResolvedValueOnce(handlerProbeResponse()); + + const res = await agent + .get(route) + .query({ sourceId: source._id.toString() }) + .expect(400); + + // Only the probe reached the handler; the SQL lookup failed on the + // missing table. + expect(mockFetch).toHaveBeenCalledTimes(1); + expect(res.body.error).toMatch(/no_such_table_for_label_sql/); + }, + ); + + // Needs ClickHouse 26.9+, whose handler serves label lookups. + describe('against the CI ClickHouse', () => { + const TABLE = 'prom_labels_http_api_live'; + const START = 1700000000; + + let agent: Awaited>['agent']; + let sourceId: string; + + beforeEach(async () => { + mockFetch.mockImplementation(realFetch); + const loggedIn = await getLoggedInAgent(server); + agent = loggedIn.agent; + const source = await seedClickHouseSource(loggedIn.team._id, { + tableName: TABLE, + }); + sourceId = source._id.toString(); + }); + + const lookup = async (route: string, query: string) => { + const res = await agent + .get(route) + .query(`sourceId=${sourceId}${query}`) + .expect(200); + const handlerPaths = mockFetch.mock.calls + .map(([url]) => new URL(String(url)).pathname) + .filter( + path => path !== `${CLICKHOUSE_PROMETHEUS_API_PREFIX}/format_query`, + ); + return { body: res.body, handlerPaths }; + }; + + beforeAll(async () => { + await seedTimeSeriesTagsTable({ + table: TABLE, + series: [ + { + metricName: 'live_up', + tags: { job: 'api' }, + startSec: START, + endSec: START + 60, + }, + { + metricName: 'live_up', + tags: { job: 'web', region: 'eu' }, + startSec: START, + endSec: START + 60, + }, + ], + withSamples: true, + }); + }); + + afterAll(async () => { + await dropTimeSeriesTable({ table: TABLE }); + }); + + it('answers /labels from the handler', async () => { + const { body, handlerPaths } = await lookup( + '/v1/prometheus/labels', + '', + ); + + expect(body).toEqual({ + status: 'success', + data: ['__name__', 'job', 'region'], + }); + expect(handlerPaths).toEqual([ + `${CLICKHOUSE_PROMETHEUS_API_PREFIX}/labels`, + ]); + }); + + it('answers /label//values narrowed by match[] and the window', async () => { + const route = '/v1/prometheus/label/job/values'; + const window = `&start=${START}&end=${START + 60}`; + + expect((await lookup(route, '')).body).toEqual({ + status: 'success', + data: ['api', 'web'], + }); + const narrowed = await lookup( + route, + `${window}&${matchQueryString(['live_up{region="eu"}'])}`, + ); + expect(narrowed.body).toEqual({ status: 'success', data: ['web'] }); + expect(narrowed.handlerPaths).toContain( + `${CLICKHOUSE_PROMETHEUS_API_PREFIX}/label/job/values`, + ); + expect( + (await lookup(route, `&start=${START + 3600}&end=${START + 7200}`)) + .body, + ).toEqual({ status: 'success', data: [] }); + }); + + it('forwards limit', async () => { + const { body } = await lookup('/v1/prometheus/labels', '&limit=1'); + + expect(body).toMatchObject({ status: 'success', data: ['__name__'] }); + }); + }); + }); + describe('GET /v1/prometheus/query_exemplars', () => { it('returns 400 when query parameter is missing', async () => { const { agent } = await getLoggedInAgent(server); diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts index b266bc8a31..918409e495 100644 --- a/packages/api/src/routers/api/prometheus.ts +++ b/packages/api/src/routers/api/prometheus.ts @@ -17,6 +17,7 @@ import { clickhousePrometheusRequest, newPromqlClickhouseClient, PROMETHEUS_MAX_RESOLUTION, + resolveClickhouseLabelBackend, resolveClickhouseQueryBackend, resolvePromqlTarget, } from '@/controllers/promql'; @@ -792,18 +793,19 @@ type ClickHouseLabelLookup = ( /** * Handles a Prometheus label lookup — `/labels` or `/label/:name/values` — from - * whichever backend the connection points at. + * whichever backend the connection points at. `apiPath` is the Prometheus API + * path below `/api/v1`, e.g. `/labels`. */ async function handleLabelLookup( req: express.Request, res: express.Response, { subject, - proxyPath, + apiPath, queryClickHouse, }: { subject: LabelLookupSubject; - proxyPath: string; + apiPath: string; queryClickHouse: ClickHouseLabelLookup; }, ) { @@ -832,24 +834,23 @@ async function handleLabelLookup( } const { connection, databaseName, tableName } = target; + const proxyParams = { + ...(start != null ? { start: String(start) } : {}), + ...(end != null ? { end: String(end) } : {}), + // Any non-negative integer. Pass 0 ("unlimited") on too: proxyToPrometheus + // sends it as an omitted key, which also clears a host-pinned `limit`. + ...(limit != null ? { limit: String(limit) } : {}), + // Restored under the name Prometheus expects + ...(match != null ? { 'match[]': match } : {}), + }; + // Proxy to Prometheus if endpoint is set if (connection.isPrometheusEndpoint) { backend = 'prometheus'; const status = await proxyToPrometheus( connection.host, - proxyPath, - { - ...(start != null ? { start: String(start) } : {}), - ...(end != null ? { end: String(end) } : {}), - // `limit` is 0 or absent when validation passes (see the schema - // above); 0 means "unlimited" to Prometheus and must still be - // forwarded (`v == null` is the merge loop's own absence check), - // otherwise a request explicitly asking for unlimited results - // silently loses to a host-pinned `limit`. - ...(limit != null ? { limit: String(limit) } : {}), - // Restored under the name Prometheus expects - ...(match != null ? { 'match[]': match } : {}), - }, + `/api/v1${apiPath}`, + proxyParams, res, ); recordProxyOutcome(status, subject, backend); @@ -857,6 +858,21 @@ async function handleLabelLookup( } backend = 'clickhouse'; + if ((await resolveClickhouseLabelBackend(target)) === 'clickhouse-http') { + const { upstream, headers } = clickhousePrometheusRequest(connection); + const status = await proxyToPrometheus( + upstream, + `${CLICKHOUSE_PROMETHEUS_API_PREFIX}${apiPath}`, + { ...proxyParams, database: databaseName, table: tableName }, + res, + headers, + CLICKHOUSE_HTTP_API_UNAVAILABLE_MESSAGE, + ); + recordProxyOutcome(status, subject, backend); + return; + } + + // Servers before 26.9, or without the handler, or a table-function source const client = newPromqlClickhouseClient(connection); const startMs = start != null ? Math.floor(start * 1000) : undefined; @@ -892,7 +908,7 @@ async function handleLabelLookup( router.get('/labels', (req, res) => handleLabelLookup(req, res, { subject: 'labels', - proxyPath: '/api/v1/labels', + apiPath: '/labels', queryClickHouse: queryLabelNames, }), ); @@ -909,7 +925,7 @@ router.get('/label/:name/values', (req, res) => { return handleLabelLookup(req, res, { subject: 'label_values', - proxyPath: `/api/v1/label/${labelName}/values`, + apiPath: `/label/${labelName}/values`, queryClickHouse: args => queryLabelValues({ ...args, labelName }), }); }); diff --git a/packages/common-utils/src/__tests__/clickhouseVersion.test.ts b/packages/common-utils/src/__tests__/clickhouseVersion.test.ts index 580bef9085..bac73aacbb 100644 --- a/packages/common-utils/src/__tests__/clickhouseVersion.test.ts +++ b/packages/common-utils/src/__tests__/clickhouseVersion.test.ts @@ -5,6 +5,7 @@ import { supportsDirectReadMap, supportsMergeTreeIndex, supportsPrometheusHttpApi, + supportsPrometheusLabelsApi, } from '@/core/clickhouseVersion'; type ClickHouseVersionTuple = readonly [number, number, number, number]; @@ -301,3 +302,25 @@ describe('supportsPrometheusHttpApi', () => { expect(supportsPrometheusHttpApi(undefined)).toBe(false); }); }); + +describe('supportsPrometheusLabelsApi', () => { + it.each([ + [[26, 9, 0, 0]], + [[26, 10, 3, 7]], + [[27, 0, 0, 0]], + ])('accepts %j', version => { + expect(supportsPrometheusLabelsApi(version)).toBe(true); + }); + + it.each([ + [[26, 8, 36, 3]], + [[26, 6, 0, 0]], + [[25, 12, 0, 0]], + ])('rejects %j', version => { + expect(supportsPrometheusLabelsApi(version)).toBe(false); + }); + + it('returns false when the version is unknown', () => { + expect(supportsPrometheusLabelsApi(undefined)).toBe(false); + }); +}); diff --git a/packages/common-utils/src/core/clickhouseVersion.ts b/packages/common-utils/src/core/clickhouseVersion.ts index e4d1278075..f26eb3216b 100644 --- a/packages/common-utils/src/core/clickhouseVersion.ts +++ b/packages/common-utils/src/core/clickhouseVersion.ts @@ -181,3 +181,16 @@ export function supportsPrometheusHttpApi( ): boolean { return isClickHouseVersionAtLeast(version, PROMETHEUS_HTTP_API_MIN); } + +/** + * First release whose Prometheus HTTP API serves `/api/v1/labels` and + * `/api/v1/label//values`. Older handlers answer both with a 400 + * ("not implemented"). + */ +const PROMETHEUS_LABELS_API_MIN: ClickHouseVersion = [26, 9, 0, 0]; + +export function supportsPrometheusLabelsApi( + version: ClickHouseVersion | undefined, +): boolean { + return isClickHouseVersionAtLeast(version, PROMETHEUS_LABELS_API_MIN); +} From d4468c3e015a3588c6d147704979ecbc0ba429a2 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Thu, 12 Nov 2026 12:32:01 +0100 Subject: [PATCH 3/3] fix(api): cap label values returned by ClickHouse's Prometheus handler --- .../api/__tests__/prometheus.int.test.ts | 30 +++++++++++++++++++ packages/api/src/routers/api/prometheus.ts | 14 ++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) 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 5b5bdba683..3c67422fa2 100644 --- a/packages/api/src/routers/api/__tests__/prometheus.int.test.ts +++ b/packages/api/src/routers/api/__tests__/prometheus.int.test.ts @@ -17,6 +17,7 @@ import { clickhousePrometheusUpstream, joinPrometheusUpstreamUrl, } from '@/controllers/promql'; +import { PROMETHEUS_MAX_RESULT_ROWS } from '@/controllers/timeseriesEngine'; import { DEFAULT_DATABASE, dropTimeSeriesTable, @@ -2283,6 +2284,35 @@ describe('prometheus router', () => { }, ); + // The handler returns all values in one row, which max_result_rows can't cap. + it.each([ + ['no limit', ''], + ['limit=0', '&limit=0'], + ['a limit over the cap', `&limit=${PROMETHEUS_MAX_RESULT_ROWS + 1}`], + ])( + 'caps the values the handler returns when the request has %s', + async (_case, limitQuery) => { + stubServerVersion([26, 10, 0, 0]); + const { agent, team } = await getLoggedInAgent(server); + const source = await seedClickHouseSource(team._id); + mockFetch + .mockResolvedValueOnce(handlerProbeResponse()) + .mockResolvedValueOnce( + fakeUpstreamResponse({ status: 'success', data: [] }), + ); + + await agent + .get('/v1/prometheus/label/job/values') + .query(`sourceId=${source._id.toString()}${limitQuery}`) + .expect(200); + + const url = new URL(String(mockFetch.mock.calls[1][0])); + expect(url.searchParams.getAll('limit')).toEqual([ + String(PROMETHEUS_MAX_RESULT_ROWS), + ]); + }, + ); + // 26.8's handler answers label lookups with a 400 ("not implemented"), so // the route must never send them there. it.each(ROUTES)( diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts index 918409e495..be27d84050 100644 --- a/packages/api/src/routers/api/prometheus.ts +++ b/packages/api/src/routers/api/prometheus.ts @@ -22,6 +22,7 @@ import { resolvePromqlTarget, } from '@/controllers/promql'; import { + PROMETHEUS_MAX_RESULT_ROWS, queryInstantViaTableFunction, queryLabelNames, queryLabelValues, @@ -860,10 +861,21 @@ async function handleLabelLookup( backend = 'clickhouse'; if ((await resolveClickhouseLabelBackend(target)) === 'clickhouse-http') { const { upstream, headers } = clickhousePrometheusRequest(connection); + // The handler returns every value in a single row, so the upstream's + // max_result_rows never applies. Cap the values themselves instead. + const cappedLimit = Math.min( + limit || PROMETHEUS_MAX_RESULT_ROWS, + PROMETHEUS_MAX_RESULT_ROWS, + ); const status = await proxyToPrometheus( upstream, `${CLICKHOUSE_PROMETHEUS_API_PREFIX}${apiPath}`, - { ...proxyParams, database: databaseName, table: tableName }, + { + ...proxyParams, + limit: String(cappedLimit), + database: databaseName, + table: tableName, + }, res, headers, CLICKHOUSE_HTTP_API_UNAVAILABLE_MESSAGE,