From 341a5433cceee7187815805f7de0577048d83eb5 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Tue, 3 Nov 2026 14:09:59 +0100 Subject: [PATCH 1/3] refactor: Share PromQL chart planning and rows between app and MCP The app and MCP each had their own copy of PromQL query planning and result shaping. Move it to common-utils core/promqlChart.ts and use it from both, so MCP returns exactly what the dashboard draws. MCP table tiles now return a column per label, capped at 10,000 rows, and histogram heatmaps return rows keyed by le. Heatmap auto steps are sized for 245 buckets, as in the app. The app behaves as before. --- .changeset/mcp-promql-shared-chart.md | 5 + packages/api/src/controllers/promql.ts | 17 +- .../tools/query/__tests__/promqlTile.test.ts | 58 +- packages/api/src/mcp/tools/query/helpers.ts | 4 + .../api/src/mcp/tools/query/promqlTile.ts | 214 ++------ packages/app/src/ChartUtils.tsx | 48 +- packages/app/src/HDXMultiSeriesTableChart.tsx | 4 +- .../DBHeatmapChart/heatmapQueries.ts | 14 +- .../app/src/hooks/useOffsetPaginatedQuery.tsx | 2 +- .../utils/__tests__/promqlChartQuery.test.ts | 291 +--------- packages/app/src/utils/promqlChartQuery.ts | 381 ++----------- .../src/core/__tests__/promqlChart.test.ts | 512 ++++++++++++++++++ packages/common-utils/src/core/promql.ts | 2 +- packages/common-utils/src/core/promqlChart.ts | 425 +++++++++++++++ packages/common-utils/src/core/utils.ts | 28 + 15 files changed, 1129 insertions(+), 876 deletions(-) create mode 100644 .changeset/mcp-promql-shared-chart.md create mode 100644 packages/common-utils/src/core/__tests__/promqlChart.test.ts create mode 100644 packages/common-utils/src/core/promqlChart.ts diff --git a/.changeset/mcp-promql-shared-chart.md b/.changeset/mcp-promql-shared-chart.md new file mode 100644 index 0000000000..12a3d67c73 --- /dev/null +++ b/.changeset/mcp-promql-shared-chart.md @@ -0,0 +1,5 @@ +--- +'@hyperdx/api': patch +--- + +fix: MCP `query_tile` returns PromQL table and histogram heatmap tiles in the rows the dashboard draws diff --git a/packages/api/src/controllers/promql.ts b/packages/api/src/controllers/promql.ts index 24492d53a2..96894609db 100644 --- a/packages/api/src/controllers/promql.ts +++ b/packages/api/src/controllers/promql.ts @@ -3,6 +3,7 @@ import { buildQueryId, } from '@hyperdx/common-utils/dist/clickhouse'; import { getCurrentQueryAttribution } from '@hyperdx/common-utils/dist/clickhouse/node'; +import type { PromqlQuery } from '@hyperdx/common-utils/dist/core/promqlChart'; import { SourceKind, type SQLInterval } from '@hyperdx/common-utils/dist/types'; import { z } from 'zod'; @@ -520,16 +521,6 @@ export async function resolvePromqlQueryBackend( return resolveClickhouseQueryBackend(target); } -export type PromqlQuery = - | { - kind: 'range'; - expr: string; - startSec: number; - endSec: number; - stepSec: number; - } - | { kind: 'instant'; expr: string; timeSec: number }; - const sampleSchema = z.tuple([z.number(), z.string()]); const metricSchema = z.record(z.string()); @@ -559,7 +550,11 @@ function promqlQueryParams(query: PromqlQuery): Record { end: String(query.endSec), step: String(query.stepSec), } - : { query: query.expr, time: String(query.timeSec) }; + : { + query: query.expr, + time: String(query.timeSec), + ...(query.limit != null ? { limit: String(query.limit) } : {}), + }; } async function evaluatePromqlQuery( diff --git a/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts b/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts index 971857e2b0..1badd86105 100644 --- a/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts +++ b/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts @@ -57,6 +57,30 @@ describe('buildPromqlTileQueries', () => { ]); }); + it("caps an instant table query's series at the table's row cap", () => { + const [{ query }] = buildPromqlTileQueries({ + config: { + displayType: DisplayType.Table, + promqlExpression: [{ expression: 'up', queryType: 'instant' }], + }, + dateRange, + }); + + expect(query).toMatchObject({ kind: 'instant', limit: 10_000 }); + }); + + it("sizes a heatmap's auto step for the heatmap's finer buckets", () => { + const [{ query }] = buildPromqlTileQueries({ + config: { + displayType: DisplayType.Heatmap, + promqlExpression: [{ expression: 'up' }], + }, + dateRange, + }); + + expect(query).toMatchObject({ kind: 'range', stepSec: 15 }); + }); + it('runs an instant expression on a time chart as a range query', () => { const [{ query }] = buildPromqlTileQueries({ config: { @@ -237,7 +261,7 @@ describe('toPromqlTileResult', () => { expect(result.data).toEqual([{ series_name: 'up', value: 3 }]); }); - it('leaves out the time bucket for an instant table', () => { + it('gives an instant table a column per label and no time column', () => { const result = toPromqlTileResult( { displayType: DisplayType.Table, @@ -256,9 +280,35 @@ describe('toPromqlTileResult', () => { ); expect(result.data).toEqual([ - { series_name: '{instance="checkout-0"}', value: 1 }, - { series_name: '{instance="checkout-1"}', value: 0 }, + { instance: 'checkout-0', job: 'checkout', Value: 1 }, + { instance: 'checkout-1', job: 'checkout', Value: 0 }, + ]); + expect(result.meta.map(m => m.name)).toEqual(['instance', 'job', 'Value']); + expect(result.seriesLabels).toBeUndefined(); + }); + + it('keys a histogram heatmap by bucket', () => { + const result = toPromqlTileResult( + { + displayType: DisplayType.Heatmap, + promqlExpression: 'sum by (le) (rate(x_bucket[5m]))', + heatmap: { mode: 'histogram' }, + }, + [ + { + expression: 'sum by (le) (rate(x_bucket[5m]))', + isBucketed: true, + result: [ + { metric: { le: '0.5' }, values: [[t0, 2]] }, + { metric: { le: '+Inf' }, values: [[t0, 3]] }, + ], + }, + ], + ); + + expect(result.data).toEqual([ + { __hdx_time_bucket: '2026-11-02T10:00:00.000Z', le: '0.5', value: 2 }, + { __hdx_time_bucket: '2026-11-02T10:00:00.000Z', le: '+Inf', value: 3 }, ]); - expect(result.meta.map(m => m.name)).toEqual(['series_name', 'value']); }); }); diff --git a/packages/api/src/mcp/tools/query/helpers.ts b/packages/api/src/mcp/tools/query/helpers.ts index 695e13f69f..98871ea467 100644 --- a/packages/api/src/mcp/tools/query/helpers.ts +++ b/packages/api/src/mcp/tools/query/helpers.ts @@ -4,6 +4,7 @@ import { } 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'; import { convertToCategoricalChartConfig, getFirstTimestampValueExpression, @@ -727,6 +728,9 @@ export async function runConfigTile( ? mcpUserError(e.message) : mcpServerError(e.message); } + if (e instanceof PromqlChartError) { + return mcpUserError(e.message); + } // The table-function backend throws ClickHouse errors as they come. return clickHouseErrorResult(e); } diff --git a/packages/api/src/mcp/tools/query/promqlTile.ts b/packages/api/src/mcp/tools/query/promqlTile.ts index 432111eb0f..0e12be5ad2 100644 --- a/packages/api/src/mcp/tools/query/promqlTile.ts +++ b/packages/api/src/mcp/tools/query/promqlTile.ts @@ -1,29 +1,24 @@ import { - displayTypeSupportsInstantQuery, displayTypeSupportsReducer, getQueriedPromqlSeries, - isRangeQuery, - promqlSeriesQueryType, - promqlStep, - reducePromqlSamples, } from '@hyperdx/common-utils/dist/core/promql'; -import { renderPromqlSeriesNames } from '@hyperdx/common-utils/dist/core/seriesNameTemplate'; import { - convertDateRangeToGranularityString, - convertGranularityToSeconds, - getAlignedDateRange, -} from '@hyperdx/common-utils/dist/core/utils'; -import { substitutePromqlChartConfigTemplates } from '@hyperdx/common-utils/dist/macros'; + getPromqlChartQueries, + getPromqlChartRange, + PromqlExpressionSamples, + PromqlQuery, + reducePromqlChartRows, + toPromqlChartResult, + toPromqlSamples, +} from '@hyperdx/common-utils/dist/core/promqlChart'; +import { convertGranularityToSeconds } from '@hyperdx/common-utils/dist/core/utils'; import type { ChartVariable, PromqlSavedChartConfig, PromqlSeries, - SQLInterval, } from '@hyperdx/common-utils/dist/types'; import { - PromqlQuery, - PromqlQueryData, PromQLQueryError, resolvePromqlQueryBackend, resolvePromqlTarget, @@ -38,15 +33,13 @@ type PromqlTileConfig = Pick< | 'granularity' | 'alignDateRangeToGranularity' | 'legendTemplate' + | 'heatmap' >; -/** The bucket count the app's time charts size an `auto` granularity for. */ -const AUTO_GRANULARITY_MAX_BUCKETS = 80; - /** * The queries a PromQL tile sends, one per expression its display type plots. - * Granularity, alignment and query type follow the app's chart converters, so - * the tile returns the same samples it shows on the dashboard. + * Granularity, alignment and query type are the dashboard's, so the tile + * returns the same samples it shows there. */ export function buildPromqlTileQueries({ config, @@ -59,175 +52,58 @@ export function buildPromqlTileQueries({ minGranularitySeconds?: number; variables?: ChartVariable[]; }): (PromqlSeries & { query: PromqlQuery })[] { - const granularity: SQLInterval = - !config.granularity || config.granularity === 'auto' - ? convertDateRangeToGranularityString( - dateRange, - AUTO_GRANULARITY_MAX_BUCKETS, - minGranularitySeconds, - ) - : config.granularity; - - const allowsInstant = displayTypeSupportsInstantQuery(config); - - const alignedRange = - (!allowsInstant || isRangeQuery(config)) && - config.alignDateRangeToGranularity !== false - ? getAlignedDateRange(dateRange, granularity) - : dateRange; - - const substituted = substitutePromqlChartConfigTemplates({ + const range = getPromqlChartRange({ ...config, - granularity, - dateRange: alignedRange, + dateRange, minGranularitySeconds, - variables, }); - - const [startSec, endSec] = alignedRange.map(date => date.getTime() / 1000); - - // promqlStep answers a Prometheus duration such as "60s". - const stepSec = Number.parseFloat( - promqlStep(granularity, alignedRange, minGranularitySeconds), + return getPromqlChartQueries( + { + ...config, + granularity: range.granularity, + minGranularitySeconds, + variables, + }, + range.dateRange, ); - - return getQueriedPromqlSeries(substituted).map(series => ({ - ...series, - query: - allowsInstant && promqlSeriesQueryType(series) === 'instant' - ? { kind: 'instant', expr: series.expression, timeSec: endSec } - : { - kind: 'range', - expr: series.expression, - startSec, - endSec, - stepSec, - }, - })); -} - -type SampledSeries = { - metric: Record; - values: [number, number][]; -}; - -/** One expression's result, with its samples parsed. */ -type PromqlExpressionSamples = PromqlSeries & { - /** Whether the result is bucketed by time. */ - isBucketed: boolean; - result: SampledSeries[]; -}; - -function toSamples( - data: PromqlQueryData, -): Pick { - switch (data.resultType) { - case 'matrix': - return { - isBucketed: true, - result: data.result.map(({ metric, values }) => ({ - metric, - values: values.map(([ts, value]) => [ts, Number.parseFloat(value)]), - })), - }; - case 'vector': - return { - isBucketed: false, - result: data.result.map(({ metric, value }) => ({ - metric, - values: [[value[0], Number.parseFloat(value[1])]], - })), - }; - case 'scalar': - return { - isBucketed: false, - result: [ - { - metric: {}, - values: [[data.result[0], Number.parseFloat(data.result[1])]], - }, - ], - }; - case 'string': - throw new PromQLQueryError( - 'A PromQL chart needs numeric samples; this expression returned a string.', - 'user', - ); - } } /** - * A PromQL tile's result as chart rows, named like the dashboard names its - * series. Number, pie and bar tiles reduce each series to one value. Each - * series' labels are listed once in `seriesLabels` rather than on every row. + * A PromQL tile's result in the rows the dashboard draws. Number, pie and bar + * tiles reduce each series to one value. Each series' labels are listed once + * in `seriesLabels` rather than on every row. */ export function toPromqlTileResult( config: PromqlTileConfig, expressions: PromqlExpressionSamples[], ) { - const names = renderPromqlSeriesNames( + const { promqlSeriesLabels, ...rows } = toPromqlChartResult( + config, expressions, - config.legendTemplate?.trim() || undefined, ); - const reduces = displayTypeSupportsReducer(config); - const reducer = getQueriedPromqlSeries(config)[0]?.reducer; - - const isBucketed = !reduces && expressions.some(e => e.isBucketed); - - const data: Record[] = []; - - const seriesLabels = new Map>(); - - // Names come back grouped by expression in series order, so they line up - // with the series once both are flattened. - const seriesNames = names.flat(); - expressions - .flatMap(({ result }) => result) - .forEach(({ metric, values }, i) => { - const series_name = seriesNames.at(i) ?? ''; - if (reduces) { - const value = reducePromqlSamples( - values.map(([, sample]) => sample), - reducer, - ); - // A series with no usable sample reads as no value, not zero. - if (value == null) return; - seriesLabels.set(series_name, metric); - data.push({ series_name, value }); - return; - } - seriesLabels.set(series_name, metric); - for (const [ts, value] of values) { - data.push( - isBucketed - ? { - __hdx_time_bucket: new Date(ts * 1000).toISOString(), - series_name, - value, - } - : { series_name, value }, - ); - } - }); - + if (!displayTypeSupportsReducer(config)) { + return { ...rows, seriesLabels: promqlSeriesLabels }; + } + const reduced = reducePromqlChartRows( + rows, + getQueriedPromqlSeries(config)[0]?.reducer, + ); + const labelsByName = new Map(Object.entries(promqlSeriesLabels ?? {})); return { - data, - meta: [ - ...(isBucketed - ? [{ name: '__hdx_time_bucket', type: 'DateTime64(3)' }] - : []), - { name: 'series_name', type: 'String' }, - { name: 'value', type: 'Float64' }, - ], - rows: data.length, - seriesLabels: Object.fromEntries(seriesLabels), + ...reduced, + seriesLabels: Object.fromEntries( + reduced.data.flatMap(({ series_name }) => { + const labels = labelsByName.get(String(series_name)); + return labels ? [[series_name, labels]] : []; + }), + ), }; } /** * Runs a PromQL tile and returns its rows. Throws PromQLQueryError for a - * missing source or a rejected or failed query, and ClickHouse errors from the - * table-function backend. + * missing source or a rejected or failed query, PromqlChartError for a result + * the tile can't draw, and ClickHouse errors from the table-function backend. */ export async function runPromqlTile({ teamId, @@ -267,7 +143,7 @@ export async function runPromqlTile({ const expressions = await Promise.all( queries.map(async ({ query, ...series }) => ({ ...series, - ...toSamples(await runPromqlQuery(backend, query, signal)), + ...toPromqlSamples(await runPromqlQuery(backend, query, signal)), })), ); return toPromqlTileResult(config, expressions); diff --git a/packages/app/src/ChartUtils.tsx b/packages/app/src/ChartUtils.tsx index 166e19f98d..fbbd1c4faf 100644 --- a/packages/app/src/ChartUtils.tsx +++ b/packages/app/src/ChartUtils.tsx @@ -12,18 +12,18 @@ import { import { DEFAULT_PROMQL_REDUCER, getQueriedPromqlSeries, - isRangeQuery, } from '@hyperdx/common-utils/dist/core/promql'; +import { getPromqlChartRange } from '@hyperdx/common-utils/dist/core/promqlChart'; import { isMetricChartConfig } from '@hyperdx/common-utils/dist/core/renderChartConfig'; import { SERIES_KEY_JOINER } from '@hyperdx/common-utils/dist/core/seriesNameTemplate'; import { - convertDateRangeToGranularityString, convertGranularityToSeconds, convertToCategoricalChartConfig, convertToNumberChartConfig, convertToTableChartConfig, getAlignedDateRange, getPercentileAggFnLevel, + getTimeChartGranularity, Granularity, hasPositiveSeriesLimit, TIME_SERIES_DISPLAY_TYPE_BY_NAME, @@ -106,21 +106,6 @@ export const DEFAULT_CHART_CONFIG: Omit< alignDateRangeToGranularity: true, }; -export function getTimeChartGranularity( - granularity: T | 'auto' | undefined, - dateRange: [Date, Date], - minGranularitySeconds?: number, - maxBuckets = 80, -) { - return granularity === 'auto' || granularity == null - ? convertDateRangeToGranularityString( - dateRange, - maxBuckets, - minGranularitySeconds, - ) - : granularity; -} - function getTimeChartDateRange( dateRange: [Date, Date], alignDateRangeToGranularity: boolean | undefined, @@ -133,31 +118,6 @@ function getTimeChartDateRange( export const MAX_TIME_CHART_SERIES = DEFAULT_SERIES_LIMIT; -/** - * A PromQL config's resolved granularity, and its date range aligned to that - * granularity's buckets when it runs a range query, so its samples land on the - * same boundaries as the timeseries charts' and stay put across refreshes. - */ -function getAlignedRangeAndGranularity( - config: PromqlChartConfig & DateRange, -): Pick { - const granularity = getTimeChartGranularity( - config.granularity, - config.dateRange, - config.minGranularitySeconds, - ); - return { - granularity, - dateRange: isRangeQuery(config) - ? getTimeChartDateRange( - config.dateRange, - config.alignDateRangeToGranularity, - granularity, - ) - : config.dateRange, - }; -} - /** * Converts the given config into one that is suitable for a tile that shows * 1 value per series (number, pie, and bar tiles). The reducer defaults to @@ -168,7 +128,7 @@ export function convertToReducedPromqlChartConfig( ): PromqlChartConfig & DateRange { return { ...config, - ...getAlignedRangeAndGranularity(config), + ...getPromqlChartRange(config), promqlExpression: getQueriedPromqlSeries(config).map(series => ({ ...series, reducer: series.reducer ?? DEFAULT_PROMQL_REDUCER, @@ -200,7 +160,7 @@ export function convertToPromqlSparklineChartConfig( export function convertToPromqlTableChartConfig( config: PromqlChartConfig & DateRange, ): PromqlChartConfig & DateRange { - return { ...config, ...getAlignedRangeAndGranularity(config) }; + return { ...config, ...getPromqlChartRange(config) }; } export function convertToTimeChartConfig( diff --git a/packages/app/src/HDXMultiSeriesTableChart.tsx b/packages/app/src/HDXMultiSeriesTableChart.tsx index c7463e69c6..5df9362e6c 100644 --- a/packages/app/src/HDXMultiSeriesTableChart.tsx +++ b/packages/app/src/HDXMultiSeriesTableChart.tsx @@ -1,6 +1,7 @@ import { useCallback, useMemo, useState } from 'react'; import Link from 'next/link'; import cx from 'classnames'; +import { MAX_TABLE_ROWS } from '@hyperdx/common-utils/dist/core/utils'; import { ChartPaletteToken, ColorCondition, @@ -48,9 +49,6 @@ import focusStyles from '@styles/focus.module.scss'; export type TableVariant = 'default' | 'muted'; -// Arbitrary limit to prevent OOM crashes for very large result sets. Most result sets should be paginated anyway. -export const MAX_TABLE_ROWS = 10_000; - export const Table = ({ data, groupColumnName, diff --git a/packages/app/src/components/DBHeatmapChart/heatmapQueries.ts b/packages/app/src/components/DBHeatmapChart/heatmapQueries.ts index 948df6920f..c0604e8ada 100644 --- a/packages/app/src/components/DBHeatmapChart/heatmapQueries.ts +++ b/packages/app/src/components/DBHeatmapChart/heatmapQueries.ts @@ -1,5 +1,9 @@ import { getHeatmapMode } from '@hyperdx/common-utils/dist/core/heatmap'; -import { getAlignedDateRange } from '@hyperdx/common-utils/dist/core/utils'; +import { + getAlignedDateRange, + getTimeChartGranularity, + HEATMAP_AUTO_GRANULARITY_BUCKETS, +} from '@hyperdx/common-utils/dist/core/utils'; import { isPromqlChartConfig } from '@hyperdx/common-utils/dist/guards'; import { BuilderChartConfigWithDateRange, @@ -9,11 +13,7 @@ import { SQLInterval, } from '@hyperdx/common-utils/dist/types'; -import { - convertToTimeChartConfig, - getTimeChartGranularity, - isAggregateFunction, -} from '@/ChartUtils'; +import { convertToTimeChartConfig, isAggregateFunction } from '@/ChartUtils'; import { heatmapLowQuantile } from './heatmapBounds'; import type { HeatmapScaleType } from './heatmapGrid'; @@ -73,8 +73,6 @@ export type HeatmapQuery = config: HeatmapSeriesChartConfig; }; -const HEATMAP_AUTO_GRANULARITY_BUCKETS = 245; - /** * `minGranularitySeconds` floors only auto granularity: an explicit * granularity is the user's choice and is kept as-is. diff --git a/packages/app/src/hooks/useOffsetPaginatedQuery.tsx b/packages/app/src/hooks/useOffsetPaginatedQuery.tsx index e8b5b573f6..9492d435d0 100644 --- a/packages/app/src/hooks/useOffsetPaginatedQuery.tsx +++ b/packages/app/src/hooks/useOffsetPaginatedQuery.tsx @@ -17,6 +17,7 @@ import { renderChartConfig } from '@hyperdx/common-utils/dist/core/renderChartCo import { isFirstOrderByAscending, isTimestampExpressionInFirstOrderBy, + MAX_TABLE_ROWS, } from '@hyperdx/common-utils/dist/core/utils'; import { isBuilderChartConfig, @@ -36,7 +37,6 @@ import { import api from '@/api'; import { getClickhouseClient } from '@/clickhouse'; -import { MAX_TABLE_ROWS } from '@/HDXMultiSeriesTableChart'; import { useMetadataWithSettings } from '@/hooks/useMetadata'; import { useMVOptimizationExplanation } from '@/hooks/useMVOptimizationExplanation'; import { useQueryAttribution } from '@/queryAttribution'; diff --git a/packages/app/src/utils/__tests__/promqlChartQuery.test.ts b/packages/app/src/utils/__tests__/promqlChartQuery.test.ts index 6a1d0c9acb..8556506b15 100644 --- a/packages/app/src/utils/__tests__/promqlChartQuery.test.ts +++ b/packages/app/src/utils/__tests__/promqlChartQuery.test.ts @@ -1,301 +1,12 @@ import { DisplayType } from '@hyperdx/common-utils/dist/types'; import { prometheusApi } from '@/api'; -import { - queryPromqlChartConfig, - toHistogramRows, - toTableRows, -} from '@/utils/promqlChartQuery'; - -// The real module is a Mantine/TanStack table component; only the row cap is -// needed here. -jest.mock('@/HDXMultiSeriesTableChart', () => ({ - MAX_TABLE_ROWS: 10_000, -})); +import { queryPromqlChartConfig } from '@/utils/promqlChartQuery'; jest.mock('@/api', () => ({ prometheusApi: { query: jest.fn(), queryRange: jest.fn() }, })); -const instantResult = ( - metric: Record, - ts: number, - value: number, -) => ({ - expression: 'up', - isBucketed: false, - result: [{ metric, values: [[ts, value]] as [number, number][] }], -}); - -describe('toTableRows', () => { - it('projects one row per series for an instant query, with no time column', () => { - const result = toTableRows([ - { - expression: 'up', - isBucketed: false, - result: [ - { - metric: { __name__: 'up', service: 'accounting' }, - values: [[1700000000, 1]], - }, - { - metric: { __name__: 'up', service: 'billing' }, - values: [[1700000000, 0]], - }, - ], - }, - ]); - - expect(result.data).toEqual([ - { __name__: 'up', service: 'accounting', Value: 1 }, - { __name__: 'up', service: 'billing', Value: 0 }, - ]); - expect(result.meta).toEqual([ - { name: '__name__', type: 'String' }, - { name: 'service', type: 'String' }, - { name: 'Value', type: 'Float64' }, - ]); - expect(result.rows).toBe(2); - }); - - it('projects one row per sample for a range query, with a time column first', () => { - const result = toTableRows([ - { - expression: 'up', - isBucketed: true, - result: [ - { - metric: { service: 'accounting' }, - values: [ - [1700000000, 1], - [1700000060, 2], - ], - }, - ], - }, - ]); - - expect(result.data).toEqual([ - { - Timestamp: '2023-11-14T22:13:20.000Z', - service: 'accounting', - Value: 1, - }, - { - Timestamp: '2023-11-14T22:14:20.000Z', - service: 'accounting', - Value: 2, - }, - ]); - expect(Object.keys(result.data[0])).toEqual([ - 'Timestamp', - 'service', - 'Value', - ]); - expect(result.meta?.[0]).toEqual({ - name: 'Timestamp', - type: 'DateTime64(3)', - }); - }); - - it('blanks a label the series does not carry, and orders __name__ first', () => { - const result = toTableRows([ - { - expression: 'up', - isBucketed: false, - result: [ - { metric: { pod: 'a', __name__: 'up' }, values: [[1, 1]] }, - { metric: { service: 'billing' }, values: [[1, 2]] }, - ], - }, - ]); - - expect(result.data).toEqual([ - { __name__: 'up', pod: 'a', service: '', Value: 1 }, - { __name__: '', pod: '', service: 'billing', Value: 2 }, - ]); - }); - - it('renames the reserved columns when a label already uses them', () => { - const result = toTableRows([ - { - expression: 'up', - isBucketed: true, - result: [ - { - metric: { Timestamp: 'noon', Value: 'high' }, - values: [[1700000000, 7]], - }, - ], - }, - ]); - - expect(result.data).toEqual([ - { - Timestamp_1: '2023-11-14T22:13:20.000Z', - Timestamp: 'noon', - Value: 'high', - Value_1: 7, - }, - ]); - expect(result.meta).toEqual([ - { name: 'Timestamp_1', type: 'DateTime64(3)' }, - { name: 'Timestamp', type: 'String' }, - { name: 'Value', type: 'String' }, - { name: 'Value_1', type: 'Float64' }, - ]); - }); - - it('caps the rows it returns', () => { - const result = toTableRows( - [ - { - expression: 'up', - isBucketed: true, - result: [ - { - metric: { service: 'accounting' }, - values: Array.from( - { length: 10 }, - (_, i) => [1700000000 + i, i] as [number, number], - ), - }, - ], - }, - ], - 3, - ); - - expect(result.data).toHaveLength(3); - expect(result.rows).toBe(3); - }); - - it('unions the labels across expressions', () => { - const result = toTableRows([ - instantResult({ service: 'accounting' }, 1, 1), - instantResult({ pod: 'a' }, 1, 2), - ]); - - expect(result.data).toEqual([ - { pod: '', service: 'accounting', Value: 1 }, - { pod: 'a', service: '', Value: 2 }, - ]); - }); - - it('returns only a value column when there are no expressions', () => { - expect(toTableRows([])).toEqual({ - data: [], - meta: [{ name: 'Value', type: 'Float64' }], - rows: 0, - isComplete: true, - }); - }); - - it('returns only a value column for an instant query that matched nothing', () => { - expect( - toTableRows([{ expression: 'up', isBucketed: false, result: [] }]), - ).toEqual({ - data: [], - meta: [{ name: 'Value', type: 'Float64' }], - rows: 0, - isComplete: true, - }); - }); - - // The columns come from the series, not the samples, so an empty range - // result still describes the table it would have filled. - it('keeps the columns of a range series that has no samples', () => { - const result = toTableRows([ - { - expression: 'up', - isBucketed: true, - result: [{ metric: { service: 'web' }, values: [] }], - }, - ]); - - expect(result.data).toEqual([]); - expect(result.rows).toBe(0); - expect(result.meta).toEqual([ - { name: 'Timestamp', type: 'DateTime64(3)' }, - { name: 'service', type: 'String' }, - { name: 'Value', type: 'Float64' }, - ]); - }); - - it('handles label keys that collide with Object.prototype members', () => { - const result = toTableRows([ - instantResult(JSON.parse('{"constructor":"a","__proto__":"b"}'), 1, 1), - instantResult({ pod: 'x' }, 1, 2), - ]); - - expect(result.meta?.map(({ name }) => name)).toEqual([ - '__proto__', - 'constructor', - 'pod', - 'Value', - ]); - expect(Object.entries(result.data[0])).toEqual([ - ['__proto__', 'b'], - ['constructor', 'a'], - ['pod', ''], - ['Value', 1], - ]); - expect(Object.entries(result.data[1])).toEqual([ - ['__proto__', ''], - ['constructor', ''], - ['pod', 'x'], - ['Value', 2], - ]); - }); -}); - -describe('toHistogramRows', () => { - const bucket = (metric: Record, value: number) => ({ - metric, - values: [[1700000000, value]] as [number, number][], - }); - const rangeResult = (...result: ReturnType[]) => ({ - expression: 'sum by (le) (rate(x_bucket[5m]))', - isBucketed: true, - result, - }); - - it('keys each sample by its le label', () => { - const result = toHistogramRows([ - rangeResult(bucket({ le: '0.5' }, 2), bucket({ le: '+Inf' }, 3)), - ]); - expect(result.data).toEqual([ - { __hdx_time_bucket: '2023-11-14T22:13:20.000Z', le: '0.5', value: 2 }, - { __hdx_time_bucket: '2023-11-14T22:13:20.000Z', le: '+Inf', value: 3 }, - ]); - expect(result.meta?.map(({ name }) => name)).toEqual([ - '__hdx_time_bucket', - 'le', - 'value', - ]); - }); - - it.each>([{}, { le: '' }, { le: 'abc' }])( - 'rejects a series without a numeric le: %p', - metric => { - expect(() => toHistogramRows([rangeResult(bucket(metric, 1))])).toThrow( - /need series with an "le" label/, - ); - }, - ); - - it('rejects buckets that were not aggregated by le', () => { - expect(() => - toHistogramRows([ - rangeResult( - bucket({ le: '0.5', instance: 'a' }, 1), - bucket({ le: '0.5', instance: 'b' }, 1), - ), - ]), - ).toThrow('Several series share le="0.5"'); - }); -}); - describe('queryPromqlChartConfig', () => { const dateRange: [Date, Date] = [ new Date('2024-01-01T00:00:00Z'), diff --git a/packages/app/src/utils/promqlChartQuery.ts b/packages/app/src/utils/promqlChartQuery.ts index 183d0d926a..8468ce79bd 100644 --- a/packages/app/src/utils/promqlChartQuery.ts +++ b/packages/app/src/utils/promqlChartQuery.ts @@ -1,310 +1,53 @@ import type { QueryAttribution } from '@hyperdx/common-utils/dist/clickhouse'; import { - displayTypeSupportsInstantQuery, - getQueriedPromqlSeries, - parsePromqlLe, - promqlSeriesQueryType, - promqlStep, - reducePromqlSamples, -} from '@hyperdx/common-utils/dist/core/promql'; -import { renderPromqlSeriesNames } from '@hyperdx/common-utils/dist/core/seriesNameTemplate'; -import { substitutePromqlChartConfigTemplates } from '@hyperdx/common-utils/dist/macros'; + getPromqlChartQueries, + PromqlExpressionSamples, + PromqlQuery, + reducePromqlChartRows, + toPromqlChartResult, + toPromqlSamples, +} from '@hyperdx/common-utils/dist/core/promqlChart'; import { DateRange, - DisplayType, - PrometheusMatrixResult, PromqlChartConfig, PromqlReducer, PromqlSeries, } from '@hyperdx/common-utils/dist/types'; -import { prometheusApi, PrometheusInstantQueryResponse } from '@/api'; -import { MAX_TABLE_ROWS } from '@/HDXMultiSeriesTableChart'; +import { prometheusApi } from '@/api'; import { ChartQueryResult } from '@/types'; -/** One series of a result: its labels, and its `[unix seconds, value]` samples. */ -type SampledSeries = { - metric: Record; - values: [number, number][]; -}; - -/** One expression's query result, normalized across the two endpoints. */ -type ExpressionResult = PromqlSeries & { - /** Whether the result is bucketed by time. */ - isBucketed: boolean; - result: SampledSeries[]; -}; - -/** An instant query's response, normalized across response types (scalar, vector, matrix). */ -function normalizeInstantResult(response: PrometheusInstantQueryResponse): { - isBucketed: boolean; - result: SampledSeries[]; -} { - if (response.status !== 'success' || !response.data) { - throw new Error(response.error ?? 'PromQL query failed'); - } - - const { data } = response; - switch (data.resultType) { - case 'vector': - return { - isBucketed: false, - result: data.result.map(({ metric, value }) => ({ - metric, - values: [[value[0], parseFloat(value[1])]], - })), - }; - case 'scalar': - return { - isBucketed: false, - result: [ - { - metric: {}, - values: [[data.result[0], parseFloat(data.result[1])]], - }, - ], - }; - case 'matrix': - return { isBucketed: true, result: toSampledSeries(data.result) }; - default: - throw new Error( - 'A PromQL chart needs numeric samples; this expression returned a string.', - ); - } -} - -/** A matrix result's series, with their samples parsed. */ -const toSampledSeries = (result: PrometheusMatrixResult[]): SampledSeries[] => - result.map(({ metric, values }) => ({ - metric, - values: values.map(([ts, value]) => [ts, parseFloat(value)]), - })); - -/** Evaluate one expression at `evalTime`, at the instant endpoint. */ -async function fetchInstantExpression( +/** Evaluate one expression's query at the instant or range endpoint. */ +async function fetchExpression( config: PromqlChartConfig, - series: PromqlSeries, - evalTime: Date, + { query, ...series }: PromqlSeries & { query: PromqlQuery }, signal: AbortSignal, attribution: QueryAttribution, -): Promise { - const response = await prometheusApi.query({ - query: series.expression, - time: evalTime.getTime() / 1000, - sourceId: config.source, - // A vector is one sample per series, so capping series caps a table's - // rows exactly. Range queries go uncapped: series can be sparse, so no - // series count bounds their samples without dropping rows that would fit. - limit: - config.displayType === DisplayType.Table ? MAX_TABLE_ROWS : undefined, - signal, - attribution, - }); - - return { - ...series, - ...normalizeInstantResult(response), - }; -} - -/** Evaluate one expression over the whole window, at the range endpoint. */ -async function fetchRangeExpression( - config: PromqlChartConfig & Pick, - series: PromqlSeries, - dateRange: [Date, Date], - signal: AbortSignal, - attribution: QueryAttribution, -): Promise { - const [startDate, endDate] = dateRange; - const response = await prometheusApi.queryRange({ - query: series.expression, - start: startDate.getTime() / 1000, - end: endDate.getTime() / 1000, - step: promqlStep( - config.granularity, - dateRange, - config.minGranularitySeconds, - ), - sourceId: config.source, - signal, - attribution, - }); +): Promise { + const response = + query.kind === 'instant' + ? await prometheusApi.query({ + query: query.expr, + time: query.timeSec, + sourceId: config.source, + limit: query.limit, + signal, + attribution, + }) + : await prometheusApi.queryRange({ + query: query.expr, + start: query.startSec, + end: query.endSec, + step: `${query.stepSec}s`, + sourceId: config.source, + signal, + attribution, + }); if (response.status !== 'success' || !response.data) { throw new Error(response.error ?? 'PromQL query failed'); } - - // The range endpoint only ever answers with a matrix. - return { - ...series, - isBucketed: true, - result: toSampledSeries(response.data.result), - }; -} - -/** - * Rows for the whole chart, shaped like a ClickHouse response. - * - * Rows carry a `__hdx_time_bucket` if the result of any expression spans time, - * otherwise no `__hdx_time_bucket` is included. Samples keep the timestamps - * Prometheus gave them. - */ -function toChartRows( - expressions: ExpressionResult[], - legendTemplate?: string, -): ChartQueryResult { - const seriesNames = renderPromqlSeriesNames(expressions, legendTemplate); - const isBucketed = expressions.some(result => result.isBucketed); - - const data: Record[] = []; - const promqlSeriesLabels: Record> = {}; - expressions.forEach(({ result: series }, expressionIndex) => { - series.forEach(({ values, metric }, seriesIndex) => { - const seriesName = seriesNames[expressionIndex][seriesIndex]; - promqlSeriesLabels[seriesName] = metric; - for (const [ts, value] of values) { - data.push( - isBucketed - ? { - __hdx_time_bucket: new Date(ts * 1000).toISOString(), - value, - series_name: seriesName, - } - : { value, series_name: seriesName }, - ); - } - }); - }); - - return { - data, - meta: [ - ...(isBucketed - ? [{ name: '__hdx_time_bucket', type: 'DateTime64(3)' }] - : []), - { name: 'value', type: 'Float64' }, - { name: 'series_name', type: 'String' }, - ], - rows: data.length, - isComplete: true, - promqlSeriesLabels, - }; -} - -/** `base`, or `base_N` with the lowest N that doesn't collide with a label column. */ -function uniqueColumnName(base: string, taken: Set): string { - if (!taken.has(base)) return base; - let n = 1; - while (taken.has(`${base}_${n}`)) n++; - return `${base}_${n}`; -} - -/** Every label any series carries, `__name__` first and the rest alphabetical. */ -function labelColumns(expressions: ExpressionResult[]): string[] { - const keys = new Set(); - for (const { result } of expressions) { - for (const { metric } of result) { - for (const key of Object.keys(metric)) keys.add(key); - } - } - const hasName = keys.delete('__name__'); - return [...(hasName ? ['__name__'] : []), ...[...keys].sort()]; -} - -/** Every sample of every series, lazily, so a capped caller stops reading early. */ -function* samples(expressions: ExpressionResult[]) { - for (const { result } of expressions) { - for (const { metric, values } of result) { - for (const [ts, value] of values) yield { metric, ts, value }; - } - } -} - -/** - * Rows for a table tile: one per sample, with a column per Prometheus label, - * including __name__, plus Timestamp and Value column. Every row carries the - * same keys in the same order. - */ -export function toTableRows( - expressions: ExpressionResult[], - maxRows: number = MAX_TABLE_ROWS, -): ChartQueryResult { - const labels = labelColumns(expressions); - const taken = new Set(labels); - const timeColumn = expressions.some(({ isBucketed }) => isBucketed) - ? uniqueColumnName('Timestamp', taken) - : undefined; - if (timeColumn) taken.add(timeColumn); - const valueColumn = uniqueColumnName('Value', taken); - - // Cap the total number of rows before the rows end up in the react-query cache. - const data: Record[] = []; - for (const { metric, ts, value } of samples(expressions)) { - if (data.length >= maxRows) break; - const row: [string, string | number][] = []; - if (timeColumn) row.push([timeColumn, new Date(ts * 1000).toISOString()]); - for (const label of labels) { - row.push([label, Object.hasOwn(metric, label) ? metric[label] : '']); - } - row.push([valueColumn, value]); - data.push(Object.fromEntries(row)); - } - - return { - data, - meta: [ - ...(timeColumn ? [{ name: timeColumn, type: 'DateTime64(3)' }] : []), - ...labels.map(name => ({ name, type: 'String' })), - { name: valueColumn, type: 'Float64' }, - ], - rows: data.length, - isComplete: true, - }; -} - -/** - * Rows for a histogram heatmap: one per sample, grouped by the series' `le` - * bucket. Every series must be a distinct bucket (le). - */ -export function toHistogramRows( - expressions: ExpressionResult[], -): ChartQueryResult { - const seenLe = new Set(); - const data: Record[] = []; - for (const { result } of expressions) { - for (const { metric, values } of result) { - const le = metric.le; - if (Number.isNaN(parsePromqlLe(le))) { - throw new Error( - 'Histogram heatmaps need series with an "le" label, e.g. sum by (le) (rate(x_bucket[$__rate_interval])).', - ); - } - if (seenLe.has(le)) { - throw new Error( - `Several series share le="${le}". Aggregate them by bucket with sum by (le) (...).`, - ); - } - seenLe.add(le); - for (const [ts, value] of values) { - data.push({ - __hdx_time_bucket: new Date(ts * 1000).toISOString(), - le, - value, - }); - } - } - } - - return { - data, - meta: [ - { name: '__hdx_time_bucket', type: 'DateTime64(3)' }, - { name: 'le', type: 'String' }, - { name: 'value', type: 'Float64' }, - ], - rows: data.length, - isComplete: true, - }; + return { ...series, ...toPromqlSamples(response.data) }; } /** @@ -318,42 +61,12 @@ export async function queryPromqlChartConfig( // Required so a new caller can't forget it and leave its queries untagged. attribution: QueryAttribution, ): Promise { - // Expand dashboard variables and macros before sending to Prometheus. - const substituted = substitutePromqlChartConfigTemplates({ - ...config, - dateRange, - }); - const results = await Promise.all( - getQueriedPromqlSeries(substituted).map(series => - displayTypeSupportsInstantQuery(substituted) && - promqlSeriesQueryType(series) === 'instant' - ? fetchInstantExpression( - substituted, - series, - dateRange[1], - signal, - attribution, - ) - : fetchRangeExpression( - substituted, - series, - dateRange, - signal, - attribution, - ), + getPromqlChartQueries(config, dateRange).map(query => + fetchExpression(config, query, signal, attribution), ), ); - - // Rows keyed by the series' `le` bucket. Done here so that - // alias and legend templates don't interfere with the le label. - if (substituted.heatmap?.mode === 'histogram') { - return toHistogramRows(results); - } - - return substituted.displayType === DisplayType.Table - ? toTableRows(results) - : toChartRows(results, substituted.legendTemplate?.trim() || undefined); + return { ...toPromqlChartResult(config, results), isComplete: true }; } /** @@ -364,30 +77,8 @@ export function reduceBucketRows( result: ChartQueryResult, reducer: PromqlReducer | undefined, ): ChartQueryResult { - const samplesBySeries = new Map(); - for (const row of result.data) { - const name = String(row.series_name ?? ''); - const samples = samplesBySeries.get(name); - if (samples) { - samples.push(Number(row.value)); - } else { - samplesBySeries.set(name, [Number(row.value)]); - } - } - - const data: Record[] = []; - for (const [seriesName, samples] of samplesBySeries) { - const value = reducePromqlSamples(samples, reducer); - if (value != null) data.push({ series_name: seriesName, value }); - } - return { - data, - meta: [ - { name: 'series_name', type: 'String' }, - { name: 'value', type: 'Float64' }, - ], - rows: data.length, + ...reducePromqlChartRows(result, reducer), isComplete: result.isComplete, }; } diff --git a/packages/common-utils/src/core/__tests__/promqlChart.test.ts b/packages/common-utils/src/core/__tests__/promqlChart.test.ts new file mode 100644 index 0000000000..67b0b669db --- /dev/null +++ b/packages/common-utils/src/core/__tests__/promqlChart.test.ts @@ -0,0 +1,512 @@ +import { + getPromqlChartQueries, + getPromqlChartRange, + PromqlChartError, + reducePromqlChartRows, + toPromqlChartResult, + toPromqlHistogramRows, + toPromqlSamples, + toPromqlTableRows, +} from '@/core/promqlChart'; +import { MAX_TABLE_ROWS } from '@/core/utils'; +import { DisplayType, PromqlReducer } from '@/types'; + +const dateRange: [Date, Date] = [ + new Date('2024-01-01T00:00:30Z'), + new Date('2024-01-01T01:00:30Z'), +]; + +const instantResult = ( + metric: Record, + ts: number, + value: number, +) => ({ + expression: 'up', + isBucketed: false, + result: [{ metric, values: [[ts, value]] as [number, number][] }], +}); + +describe('getPromqlChartRange', () => { + it('sizes an auto granularity for a time chart and aligns the range to it', () => { + expect( + getPromqlChartRange({ + displayType: DisplayType.Line, + promqlExpression: [{ expression: 'up' }], + granularity: 'auto', + dateRange, + }), + ).toEqual({ + granularity: '1 minute', + dateRange: [ + new Date('2024-01-01T00:00:00Z'), + new Date('2024-01-01T01:01:00Z'), + ], + }); + }); + + it('sizes an auto granularity for a heatmap for finer buckets', () => { + expect( + getPromqlChartRange({ + displayType: DisplayType.Heatmap, + promqlExpression: [{ expression: 'up' }], + dateRange, + }).granularity, + ).toBe('15 second'); + }); + + it("floors an auto granularity at the source's minimum", () => { + expect( + getPromqlChartRange({ + displayType: DisplayType.Line, + promqlExpression: [{ expression: 'up' }], + dateRange, + minGranularitySeconds: 300, + }).granularity, + ).toBe('5 minute'); + }); + + it('leaves the range of a tile that only runs instant queries unaligned', () => { + expect( + getPromqlChartRange({ + displayType: DisplayType.Number, + promqlExpression: [{ expression: 'up', queryType: 'instant' }], + dateRange, + }).dateRange, + ).toEqual(dateRange); + }); + + it('aligns a time chart even when its expressions ask for instant queries', () => { + expect( + getPromqlChartRange({ + displayType: DisplayType.Line, + promqlExpression: [{ expression: 'up', queryType: 'instant' }], + granularity: '1 minute', + dateRange, + }).dateRange[0], + ).toEqual(new Date('2024-01-01T00:00:00Z')); + }); + + it('leaves the range unaligned when alignment is off', () => { + expect( + getPromqlChartRange({ + displayType: DisplayType.Line, + promqlExpression: [{ expression: 'up' }], + granularity: '5 minute', + alignDateRangeToGranularity: false, + dateRange, + }), + ).toEqual({ granularity: '5 minute', dateRange }); + }); +}); + +describe('getPromqlChartQueries', () => { + const [startSec, endSec] = dateRange.map(d => d.getTime() / 1000); + + it('runs a time chart expression as a range query on the granularity step', () => { + expect( + getPromqlChartQueries( + { + displayType: DisplayType.Line, + granularity: '5 minute', + promqlExpression: [ + { expression: 'up', queryType: 'instant' }, + { expression: 'down' }, + ], + }, + dateRange, + ).map(({ query }) => query), + ).toEqual([ + { kind: 'range', expr: 'up', startSec, endSec, stepSec: 300 }, + { kind: 'range', expr: 'down', startSec, endSec, stepSec: 300 }, + ]); + }); + + it('caps the series of an instant table query, and nothing else', () => { + const instant = (displayType: DisplayType) => + getPromqlChartQueries( + { + displayType, + promqlExpression: [{ expression: 'up', queryType: 'instant' }], + }, + dateRange, + )[0].query; + + expect(instant(DisplayType.Table)).toEqual({ + kind: 'instant', + expr: 'up', + timeSec: endSec, + limit: MAX_TABLE_ROWS, + }); + expect(instant(DisplayType.Number)).toEqual({ + kind: 'instant', + expr: 'up', + timeSec: endSec, + limit: undefined, + }); + }); + + it('expands variables and macros', () => { + expect( + getPromqlChartQueries( + { + displayType: DisplayType.Line, + granularity: '1 minute', + promqlExpression: [ + { expression: 'rate(up{job="$job"}[$__interval])' }, + ], + variables: [{ name: 'job', values: ['api'] }], + }, + dateRange, + )[0].query.expr, + ).toBe('rate(up{job="api"}[60s])'); + }); +}); + +describe('toPromqlSamples', () => { + it('reads a scalar as one unlabelled series', () => { + expect( + toPromqlSamples({ resultType: 'scalar', result: [1700000000, '2.5'] }), + ).toEqual({ + isBucketed: false, + result: [{ metric: {}, values: [[1700000000, 2.5]] }], + }); + }); + + it('rejects a string result', () => { + expect(() => + toPromqlSamples({ resultType: 'string', result: [1700000000, 'a'] }), + ).toThrow(PromqlChartError); + }); +}); + +describe('toPromqlChartResult', () => { + it('names time chart series by the legend template', () => { + const result = toPromqlChartResult( + { displayType: DisplayType.Line, legendTemplate: '{{service}}' }, + [ + { + expression: 'up', + isBucketed: true, + result: [{ metric: { service: 'api' }, values: [[1700000000, 1]] }], + }, + ], + ); + expect(result.data).toEqual([ + { + __hdx_time_bucket: '2023-11-14T22:13:20.000Z', + value: 1, + series_name: 'api', + }, + ]); + expect(result.promqlSeriesLabels).toEqual({ api: { service: 'api' } }); + }); + + it('gives a table tile a column per label', () => { + expect( + toPromqlChartResult({ displayType: DisplayType.Table }, [ + instantResult({ service: 'api' }, 1, 1), + ]).data, + ).toEqual([{ service: 'api', Value: 1 }]); + }); + + it('keys a histogram heatmap by bucket', () => { + expect( + toPromqlChartResult( + { displayType: DisplayType.Heatmap, heatmap: { mode: 'histogram' } }, + [ + { + expression: 'sum by (le) (x_bucket)', + isBucketed: true, + result: [{ metric: { le: '+Inf' }, values: [[1700000000, 4]] }], + }, + ], + ).data, + ).toEqual([ + { __hdx_time_bucket: '2023-11-14T22:13:20.000Z', le: '+Inf', value: 4 }, + ]); + }); +}); + +describe('reducePromqlChartRows', () => { + it('reduces each series, dropping series with no usable sample', () => { + expect( + reducePromqlChartRows( + { + data: [ + { series_name: 'a', value: 1 }, + { series_name: 'a', value: 3 }, + { series_name: 'b', value: NaN }, + ], + }, + PromqlReducer.Max, + ).data, + ).toEqual([{ series_name: 'a', value: 3 }]); + }); +}); + +describe('toPromqlTableRows', () => { + it('projects one row per series for an instant query, with no time column', () => { + const result = toPromqlTableRows([ + { + expression: 'up', + isBucketed: false, + result: [ + { + metric: { __name__: 'up', service: 'accounting' }, + values: [[1700000000, 1]], + }, + { + metric: { __name__: 'up', service: 'billing' }, + values: [[1700000000, 0]], + }, + ], + }, + ]); + + expect(result.data).toEqual([ + { __name__: 'up', service: 'accounting', Value: 1 }, + { __name__: 'up', service: 'billing', Value: 0 }, + ]); + expect(result.meta).toEqual([ + { name: '__name__', type: 'String' }, + { name: 'service', type: 'String' }, + { name: 'Value', type: 'Float64' }, + ]); + expect(result.rows).toBe(2); + }); + + it('projects one row per sample for a range query, with a time column first', () => { + const result = toPromqlTableRows([ + { + expression: 'up', + isBucketed: true, + result: [ + { + metric: { service: 'accounting' }, + values: [ + [1700000000, 1], + [1700000060, 2], + ], + }, + ], + }, + ]); + + expect(result.data).toEqual([ + { + Timestamp: '2023-11-14T22:13:20.000Z', + service: 'accounting', + Value: 1, + }, + { + Timestamp: '2023-11-14T22:14:20.000Z', + service: 'accounting', + Value: 2, + }, + ]); + expect(Object.keys(result.data[0])).toEqual([ + 'Timestamp', + 'service', + 'Value', + ]); + expect(result.meta[0]).toEqual({ + name: 'Timestamp', + type: 'DateTime64(3)', + }); + }); + + it('blanks a label the series does not carry, and orders __name__ first', () => { + const result = toPromqlTableRows([ + { + expression: 'up', + isBucketed: false, + result: [ + { metric: { pod: 'a', __name__: 'up' }, values: [[1, 1]] }, + { metric: { service: 'billing' }, values: [[1, 2]] }, + ], + }, + ]); + + expect(result.data).toEqual([ + { __name__: 'up', pod: 'a', service: '', Value: 1 }, + { __name__: '', pod: '', service: 'billing', Value: 2 }, + ]); + }); + + it('renames the reserved columns when a label already uses them', () => { + const result = toPromqlTableRows([ + { + expression: 'up', + isBucketed: true, + result: [ + { + metric: { Timestamp: 'noon', Value: 'high' }, + values: [[1700000000, 7]], + }, + ], + }, + ]); + + expect(result.data).toEqual([ + { + Timestamp_1: '2023-11-14T22:13:20.000Z', + Timestamp: 'noon', + Value: 'high', + Value_1: 7, + }, + ]); + expect(result.meta).toEqual([ + { name: 'Timestamp_1', type: 'DateTime64(3)' }, + { name: 'Timestamp', type: 'String' }, + { name: 'Value', type: 'String' }, + { name: 'Value_1', type: 'Float64' }, + ]); + }); + + it('caps the rows it returns', () => { + const result = toPromqlTableRows( + [ + { + expression: 'up', + isBucketed: true, + result: [ + { + metric: { service: 'accounting' }, + values: Array.from( + { length: 10 }, + (_, i) => [1700000000 + i, i] as [number, number], + ), + }, + ], + }, + ], + 3, + ); + + expect(result.data).toHaveLength(3); + expect(result.rows).toBe(3); + }); + + it('unions the labels across expressions', () => { + const result = toPromqlTableRows([ + instantResult({ service: 'accounting' }, 1, 1), + instantResult({ pod: 'a' }, 1, 2), + ]); + + expect(result.data).toEqual([ + { pod: '', service: 'accounting', Value: 1 }, + { pod: 'a', service: '', Value: 2 }, + ]); + }); + + it('returns only a value column when there are no expressions', () => { + expect(toPromqlTableRows([])).toEqual({ + data: [], + meta: [{ name: 'Value', type: 'Float64' }], + rows: 0, + }); + }); + + it('returns only a value column for an instant query that matched nothing', () => { + expect( + toPromqlTableRows([{ expression: 'up', isBucketed: false, result: [] }]), + ).toEqual({ + data: [], + meta: [{ name: 'Value', type: 'Float64' }], + rows: 0, + }); + }); + + // The columns come from the series, not the samples, so an empty range + // result still describes the table it would have filled. + it('keeps the columns of a range series that has no samples', () => { + const result = toPromqlTableRows([ + { + expression: 'up', + isBucketed: true, + result: [{ metric: { service: 'web' }, values: [] }], + }, + ]); + + expect(result.data).toEqual([]); + expect(result.rows).toBe(0); + expect(result.meta).toEqual([ + { name: 'Timestamp', type: 'DateTime64(3)' }, + { name: 'service', type: 'String' }, + { name: 'Value', type: 'Float64' }, + ]); + }); + + it('handles label keys that collide with Object.prototype members', () => { + const result = toPromqlTableRows([ + instantResult(JSON.parse('{"constructor":"a","__proto__":"b"}'), 1, 1), + instantResult({ pod: 'x' }, 1, 2), + ]); + + expect(result.meta.map(({ name }) => name)).toEqual([ + '__proto__', + 'constructor', + 'pod', + 'Value', + ]); + expect(Object.entries(result.data[0])).toEqual([ + ['__proto__', 'b'], + ['constructor', 'a'], + ['pod', ''], + ['Value', 1], + ]); + expect(Object.entries(result.data[1])).toEqual([ + ['__proto__', ''], + ['constructor', ''], + ['pod', 'x'], + ['Value', 2], + ]); + }); +}); + +describe('toPromqlHistogramRows', () => { + const bucket = (metric: Record, value: number) => ({ + metric, + values: [[1700000000, value]] as [number, number][], + }); + const rangeResult = (...result: ReturnType[]) => ({ + expression: 'sum by (le) (rate(x_bucket[5m]))', + isBucketed: true, + result, + }); + + it('keys each sample by its le label', () => { + const result = toPromqlHistogramRows([ + rangeResult(bucket({ le: '0.5' }, 2), bucket({ le: '+Inf' }, 3)), + ]); + expect(result.data).toEqual([ + { __hdx_time_bucket: '2023-11-14T22:13:20.000Z', le: '0.5', value: 2 }, + { __hdx_time_bucket: '2023-11-14T22:13:20.000Z', le: '+Inf', value: 3 }, + ]); + expect(result.meta.map(({ name }) => name)).toEqual([ + '__hdx_time_bucket', + 'le', + 'value', + ]); + }); + + it.each>([{}, { le: '' }, { le: 'abc' }])( + 'rejects a series without a numeric le: %p', + metric => { + expect(() => + toPromqlHistogramRows([rangeResult(bucket(metric, 1))]), + ).toThrow(/need series with an "le" label/); + }, + ); + + it('rejects buckets that were not aggregated by le', () => { + expect(() => + toPromqlHistogramRows([ + rangeResult( + bucket({ le: '0.5', instance: 'a' }, 1), + bucket({ le: '0.5', instance: 'b' }, 1), + ), + ]), + ).toThrow('Several series share le="0.5"'); + }); +}); diff --git a/packages/common-utils/src/core/promql.ts b/packages/common-utils/src/core/promql.ts index 7f070801b5..122aa2338a 100644 --- a/packages/common-utils/src/core/promql.ts +++ b/packages/common-utils/src/core/promql.ts @@ -107,7 +107,7 @@ export function reducePromqlSamples( * A HyperDX granularity ("5 minute") as a Prometheus step, in seconds. A * `minGranularitySeconds` floor only applies when the granularity is `auto`. */ -const promqlStepSeconds = ( +export const promqlStepSeconds = ( granularity: string | undefined, dateRange?: [Date, Date], minGranularitySeconds?: number, diff --git a/packages/common-utils/src/core/promqlChart.ts b/packages/common-utils/src/core/promqlChart.ts new file mode 100644 index 0000000000..16efdbe9db --- /dev/null +++ b/packages/common-utils/src/core/promqlChart.ts @@ -0,0 +1,425 @@ +import { + displayTypeSupportsInstantQuery, + getQueriedPromqlSeries, + isRangeQuery, + parsePromqlLe, + promqlSeriesQueryType, + promqlStepSeconds, + reducePromqlSamples, +} from '@/core/promql'; +import { renderPromqlSeriesNames } from '@/core/seriesNameTemplate'; +import { + getAlignedDateRange, + getTimeChartGranularity, + HEATMAP_AUTO_GRANULARITY_BUCKETS, + MAX_TABLE_ROWS, +} from '@/core/utils'; +import { substitutePromqlChartConfigTemplates } from '@/macros'; +import { + ChartVariable, + DisplayType, + PrometheusMatrixResult, + PrometheusVectorResult, + PromqlConditionFilter, + PromqlExpressionList, + PromqlReducer, + PromqlSeries, +} from '@/types'; + +/** A query result a PromQL chart can't draw, such as a string or an unaggregated histogram. */ +export class PromqlChartError extends Error { + constructor(message: string) { + super(message); + this.name = 'PromqlChartError'; + } +} + +/** + * A PromQL chart's resolved granularity, and its date range aligned to that + * granularity's buckets when it runs a range query, so its samples land on the + * same boundaries as the timeseries charts' and stay put across refreshes. + * Time charts and heatmaps always run range queries. + */ +export function getPromqlChartRange(config: { + promqlExpression?: PromqlExpressionList; + displayType?: DisplayType; + granularity?: string; + alignDateRangeToGranularity?: boolean; + dateRange: [Date, Date]; + minGranularitySeconds?: number; +}): { granularity: string; dateRange: [Date, Date] } { + const granularity = getTimeChartGranularity( + config.granularity, + config.dateRange, + config.minGranularitySeconds, + config.displayType === DisplayType.Heatmap + ? HEATMAP_AUTO_GRANULARITY_BUCKETS + : undefined, + ); + const runsRangeQuery = + !displayTypeSupportsInstantQuery(config) || isRangeQuery(config); + return { + granularity, + dateRange: + runsRangeQuery && config.alignDateRangeToGranularity !== false + ? getAlignedDateRange(config.dateRange, granularity) + : config.dateRange, + }; +} + +export type PromqlQuery = + | { + kind: 'range'; + expr: string; + startSec: number; + endSec: number; + stepSec: number; + } + | { + kind: 'instant'; + expr: string; + timeSec: number; + /** The most series to return. */ + limit?: number; + }; + +/** + * The queries a PromQL chart sends, one per expression it plots, with its + * variables and macros expanded. Expects a resolved granularity and the date + * range from getPromqlChartRange. + */ +export function getPromqlChartQueries( + config: { + promqlExpression: PromqlExpressionList; + displayType?: DisplayType; + granularity?: string; + variables?: ChartVariable[]; + filters?: PromqlConditionFilter[]; + minGranularitySeconds?: number; + }, + dateRange: [Date, Date], +): (PromqlSeries & { query: PromqlQuery })[] { + const substituted = substitutePromqlChartConfigTemplates({ + ...config, + dateRange, + }); + const [startSec, endSec] = dateRange.map(date => date.getTime() / 1000); + const stepSec = promqlStepSeconds( + substituted.granularity, + dateRange, + substituted.minGranularitySeconds, + ); + + return getQueriedPromqlSeries(substituted).map(series => ({ + ...series, + query: + displayTypeSupportsInstantQuery(substituted) && + promqlSeriesQueryType(series) === 'instant' + ? { + kind: 'instant', + expr: series.expression, + timeSec: endSec, + // A vector is one sample per series, so capping series caps a + // table's rows exactly. Range queries go uncapped: series can be + // sparse, so no series count bounds their samples without dropping + // rows that would fit. + limit: + substituted.displayType === DisplayType.Table + ? MAX_TABLE_ROWS + : undefined, + } + : { + kind: 'range', + expr: series.expression, + startSec, + endSec, + stepSec, + }, + })); +} + +/** One series of a result: its labels, and its `[unix seconds, value]` samples. */ +type SampledSeries = { + metric: Record; + values: [number, number][]; +}; + +/** One expression's query result, normalized across the two endpoints. */ +export type PromqlExpressionSamples = PromqlSeries & { + /** Whether the result is bucketed by time. */ + isBucketed: boolean; + result: SampledSeries[]; +}; + +/** The `data` of a Prometheus query or query_range response. */ +export type PromqlResultData = + | { resultType: 'matrix'; result: PrometheusMatrixResult[] } + | { resultType: 'vector'; result: PrometheusVectorResult[] } + | { resultType: 'scalar'; result: [number, string] } + | { resultType: 'string'; result: unknown }; + +/** A query result's series, with their samples parsed. */ +export function toPromqlSamples( + data: PromqlResultData, +): Pick { + switch (data.resultType) { + case 'matrix': + return { + isBucketed: true, + result: data.result.map(({ metric, values }) => ({ + metric, + values: values.map(([ts, value]) => [ts, parseFloat(value)]), + })), + }; + case 'vector': + return { + isBucketed: false, + result: data.result.map(({ metric, value }) => ({ + metric, + values: [[value[0], parseFloat(value[1])]], + })), + }; + case 'scalar': + return { + isBucketed: false, + result: [ + { + metric: {}, + values: [[data.result[0], parseFloat(data.result[1])]], + }, + ], + }; + default: + throw new PromqlChartError( + 'A PromQL chart needs numeric samples; this expression returned a string.', + ); + } +} + +/** Chart rows, shaped like a ClickHouse response. */ +export type PromqlChartRows = { + data: Record[]; + meta: { name: string; type: string }[]; + rows: number; + /** Time series only: each series' labels, by series name. */ + promqlSeriesLabels?: Record>; +}; + +/** + * Rows for the whole chart, named like the chart names its series. + * + * Rows carry a `__hdx_time_bucket` if the result of any expression spans time, + * otherwise no `__hdx_time_bucket` is included. Samples keep the timestamps + * Prometheus gave them. + */ +export function toPromqlChartRows( + expressions: PromqlExpressionSamples[], + legendTemplate?: string, +): PromqlChartRows { + const seriesNames = renderPromqlSeriesNames(expressions, legendTemplate); + const isBucketed = expressions.some(result => result.isBucketed); + + const data: Record[] = []; + const promqlSeriesLabels: Record> = {}; + expressions.forEach(({ result: series }, expressionIndex) => { + series.forEach(({ values, metric }, seriesIndex) => { + const seriesName = seriesNames[expressionIndex][seriesIndex]; + promqlSeriesLabels[seriesName] = metric; + for (const [ts, value] of values) { + data.push( + isBucketed + ? { + __hdx_time_bucket: new Date(ts * 1000).toISOString(), + value, + series_name: seriesName, + } + : { value, series_name: seriesName }, + ); + } + }); + }); + + return { + data, + meta: [ + ...(isBucketed + ? [{ name: '__hdx_time_bucket', type: 'DateTime64(3)' }] + : []), + { name: 'value', type: 'Float64' }, + { name: 'series_name', type: 'String' }, + ], + rows: data.length, + promqlSeriesLabels, + }; +} + +/** `base`, or `base_N` with the lowest N that doesn't collide with a label column. */ +function uniqueColumnName(base: string, taken: Set): string { + if (!taken.has(base)) return base; + let n = 1; + while (taken.has(`${base}_${n}`)) n++; + return `${base}_${n}`; +} + +/** Every label any series carries, `__name__` first and the rest alphabetical. */ +function labelColumns(expressions: PromqlExpressionSamples[]): string[] { + const keys = new Set(); + for (const { result } of expressions) { + for (const { metric } of result) { + for (const key of Object.keys(metric)) keys.add(key); + } + } + const hasName = keys.delete('__name__'); + return [...(hasName ? ['__name__'] : []), ...[...keys].sort()]; +} + +/** Every sample of every series, lazily, so a capped caller stops reading early. */ +function* samples(expressions: PromqlExpressionSamples[]) { + for (const { result } of expressions) { + for (const { metric, values } of result) { + for (const [ts, value] of values) yield { metric, ts, value }; + } + } +} + +/** + * Rows for a table tile: one per sample, with a column per Prometheus label, + * including __name__, plus Timestamp and Value column. Every row carries the + * same keys in the same order. + */ +export function toPromqlTableRows( + expressions: PromqlExpressionSamples[], + maxRows: number = MAX_TABLE_ROWS, +): PromqlChartRows { + const labels = labelColumns(expressions); + const taken = new Set(labels); + const timeColumn = expressions.some(({ isBucketed }) => isBucketed) + ? uniqueColumnName('Timestamp', taken) + : undefined; + if (timeColumn) taken.add(timeColumn); + const valueColumn = uniqueColumnName('Value', taken); + + // Cap the rows here, before the caller holds every sample. + const data: Record[] = []; + for (const { metric, ts, value } of samples(expressions)) { + if (data.length >= maxRows) break; + const row: [string, string | number][] = []; + if (timeColumn) row.push([timeColumn, new Date(ts * 1000).toISOString()]); + for (const label of labels) { + row.push([label, Object.hasOwn(metric, label) ? metric[label] : '']); + } + row.push([valueColumn, value]); + data.push(Object.fromEntries(row)); + } + + return { + data, + meta: [ + ...(timeColumn ? [{ name: timeColumn, type: 'DateTime64(3)' }] : []), + ...labels.map(name => ({ name, type: 'String' })), + { name: valueColumn, type: 'Float64' }, + ], + rows: data.length, + }; +} + +/** + * Rows for a histogram heatmap: one per sample, grouped by the series' `le` + * bucket. Every series must be a distinct bucket (le). + */ +export function toPromqlHistogramRows( + expressions: PromqlExpressionSamples[], +): PromqlChartRows { + const seenLe = new Set(); + const data: Record[] = []; + for (const { result } of expressions) { + for (const { metric, values } of result) { + const le = metric.le; + if (Number.isNaN(parsePromqlLe(le))) { + throw new PromqlChartError( + 'Histogram heatmaps need series with an "le" label, e.g. sum by (le) (rate(x_bucket[$__rate_interval])).', + ); + } + if (seenLe.has(le)) { + throw new PromqlChartError( + `Several series share le="${le}". Aggregate them by bucket with sum by (le) (...).`, + ); + } + seenLe.add(le); + for (const [ts, value] of values) { + data.push({ + __hdx_time_bucket: new Date(ts * 1000).toISOString(), + le, + value, + }); + } + } + } + + return { + data, + meta: [ + { name: '__hdx_time_bucket', type: 'DateTime64(3)' }, + { name: 'le', type: 'String' }, + { name: 'value', type: 'Float64' }, + ], + rows: data.length, + }; +} + +/** A PromQL chart's rows, in the shape its display type draws. */ +export function toPromqlChartResult( + config: { + displayType?: DisplayType; + legendTemplate?: string; + heatmap?: { mode?: string }; + }, + expressions: PromqlExpressionSamples[], +): PromqlChartRows { + // Rows keyed by the series' `le` bucket, so that alias and legend templates + // don't interfere with the le label. + if (config.heatmap?.mode === 'histogram') { + return toPromqlHistogramRows(expressions); + } + return config.displayType === DisplayType.Table + ? toPromqlTableRows(expressions) + : toPromqlChartRows( + expressions, + config.legendTemplate?.trim() || undefined, + ); +} + +/** + * Aggregate samples for each series according to the specified reducer. A series + * that reduces to nothing is dropped, reading as no value rather than a zero. + */ +export function reducePromqlChartRows( + result: Pick, + reducer: PromqlReducer | undefined, +): PromqlChartRows { + const samplesBySeries = new Map(); + for (const row of result.data) { + const name = String(row.series_name ?? ''); + const seriesSamples = samplesBySeries.get(name); + if (seriesSamples) { + seriesSamples.push(Number(row.value)); + } else { + samplesBySeries.set(name, [Number(row.value)]); + } + } + + const data: Record[] = []; + for (const [seriesName, seriesSamples] of samplesBySeries) { + const value = reducePromqlSamples(seriesSamples, reducer); + if (value != null) data.push({ series_name: seriesName, value }); + } + + return { + data, + meta: [ + { name: 'series_name', type: 'String' }, + { name: 'value', type: 'Float64' }, + ], + rows: data.length, + }; +} diff --git a/packages/common-utils/src/core/utils.ts b/packages/common-utils/src/core/utils.ts index d7231ff27e..7c93078458 100644 --- a/packages/common-utils/src/core/utils.ts +++ b/packages/common-utils/src/core/utils.ts @@ -55,6 +55,18 @@ import { SkipIndexMetadata, TableMetadata } from './metadata'; /** The default maximum number of buckets setting when determining a bucket duration for 'auto' granularity */ export const DEFAULT_AUTO_GRANULARITY_MAX_BUCKETS = 60; +/** The bucket count a time chart sizes an 'auto' granularity for. */ +export const TIME_CHART_AUTO_GRANULARITY_BUCKETS = 80; + +/** The bucket count a heatmap sizes an 'auto' granularity for. */ +export const HEATMAP_AUTO_GRANULARITY_BUCKETS = 245; + +/** + * The most rows a table tile shows. Arbitrary limit to prevent OOM crashes for + * very large result sets. Most result sets should be paginated anyway. + */ +export const MAX_TABLE_ROWS = 10_000; + /** Code-point limit for row-lookup string hashes (matches ClickHouse leftUTF8). */ export const LEFT_UTF8_HASH_CODE_POINTS = 1000; @@ -615,6 +627,22 @@ export function hashCode(str: string) { return hash; } +/** A chart's granularity, with 'auto' (or none) resolved for its date range. */ +export function getTimeChartGranularity( + granularity: T | 'auto' | undefined, + dateRange: [Date, Date], + minGranularitySeconds?: number, + maxBuckets = TIME_CHART_AUTO_GRANULARITY_BUCKETS, +) { + return granularity === 'auto' || granularity == null + ? convertDateRangeToGranularityString( + dateRange, + maxBuckets, + minGranularitySeconds, + ) + : granularity; +} + export function convertDateRangeToGranularityString( dateRange: [Date, Date], maxNumBuckets: number = DEFAULT_AUTO_GRANULARITY_MAX_BUCKETS, From 431475c876974724888ab863ea52ff05a2f0b930 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Tue, 3 Nov 2026 16:50:12 +0100 Subject: [PATCH 2/3] fix(common-utils): avoid object-injection lint warnings in promqlChart Replace variable-key object accesses with .at()/Map lookups so common-utils stays within its --max-warnings 80 budget. --- packages/common-utils/src/core/promqlChart.ts | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/common-utils/src/core/promqlChart.ts b/packages/common-utils/src/core/promqlChart.ts index 16efdbe9db..6c6829e8cf 100644 --- a/packages/common-utils/src/core/promqlChart.ts +++ b/packages/common-utils/src/core/promqlChart.ts @@ -220,11 +220,17 @@ export function toPromqlChartRows( const isBucketed = expressions.some(result => result.isBucketed); const data: Record[] = []; - const promqlSeriesLabels: Record> = {}; + const promqlSeriesLabels = new Map>(); expressions.forEach(({ result: series }, expressionIndex) => { series.forEach(({ values, metric }, seriesIndex) => { - const seriesName = seriesNames[expressionIndex][seriesIndex]; - promqlSeriesLabels[seriesName] = metric; + const seriesName = seriesNames.at(expressionIndex)?.at(seriesIndex); + if (seriesName == null) { + // Unreachable: renderPromqlSeriesNames returns one name per series. + throw new Error( + `No name rendered for series ${expressionIndex}:${seriesIndex}`, + ); + } + promqlSeriesLabels.set(seriesName, metric); for (const [ts, value] of values) { data.push( isBucketed @@ -249,7 +255,7 @@ export function toPromqlChartRows( { name: 'series_name', type: 'String' }, ], rows: data.length, - promqlSeriesLabels, + promqlSeriesLabels: Object.fromEntries(promqlSeriesLabels), }; } @@ -277,7 +283,8 @@ function labelColumns(expressions: PromqlExpressionSamples[]): string[] { function* samples(expressions: PromqlExpressionSamples[]) { for (const { result } of expressions) { for (const { metric, values } of result) { - for (const [ts, value] of values) yield { metric, ts, value }; + const labelValues = new Map(Object.entries(metric)); + for (const [ts, value] of values) yield { labelValues, ts, value }; } } } @@ -301,12 +308,12 @@ export function toPromqlTableRows( // Cap the rows here, before the caller holds every sample. const data: Record[] = []; - for (const { metric, ts, value } of samples(expressions)) { + for (const { labelValues, ts, value } of samples(expressions)) { if (data.length >= maxRows) break; const row: [string, string | number][] = []; if (timeColumn) row.push([timeColumn, new Date(ts * 1000).toISOString()]); for (const label of labels) { - row.push([label, Object.hasOwn(metric, label) ? metric[label] : '']); + row.push([label, labelValues.get(label) ?? '']); } row.push([valueColumn, value]); data.push(Object.fromEntries(row)); From 22e3141dace0dca24861cac53c7faf3fe0d33a50 Mon Sep 17 00:00:00 2001 From: Karl Power Date: Wed, 4 Nov 2026 11:09:56 +0100 Subject: [PATCH 3/3] fix(api): Skip undefined values when trimming MCP tool responses trimObject measured every field with JSON.stringify(value).length, which throws for undefined values because JSON.stringify(undefined) returns undefined. PromQL table and histogram tiles started returning seriesLabels: undefined, so any instant table result over 50k characters crashed with "Cannot read properties of undefined (reading 'length')". Drop undefined fields in trimObject, as JSON.stringify does for the whole object, and omit seriesLabels from tile results that have none. --- .../mcp/tools/query/__tests__/promqlTile.test.ts | 4 +++- packages/api/src/mcp/tools/query/promqlTile.ts | 6 +++++- .../src/utils/__tests__/trimToolResponse.test.ts | 14 ++++++++++++++ packages/api/src/utils/trimToolResponse.ts | 6 +++++- 4 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts b/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts index 1badd86105..dcc916cfcb 100644 --- a/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts +++ b/packages/api/src/mcp/tools/query/__tests__/promqlTile.test.ts @@ -284,7 +284,8 @@ describe('toPromqlTileResult', () => { { instance: 'checkout-1', job: 'checkout', Value: 0 }, ]); expect(result.meta.map(m => m.name)).toEqual(['instance', 'job', 'Value']); - expect(result.seriesLabels).toBeUndefined(); + // An undefined key would break trimToolResponse on large results. + expect(result).not.toHaveProperty('seriesLabels'); }); it('keys a histogram heatmap by bucket', () => { @@ -310,5 +311,6 @@ describe('toPromqlTileResult', () => { { __hdx_time_bucket: '2026-11-02T10:00:00.000Z', le: '0.5', value: 2 }, { __hdx_time_bucket: '2026-11-02T10:00:00.000Z', le: '+Inf', value: 3 }, ]); + expect(result).not.toHaveProperty('seriesLabels'); }); }); diff --git a/packages/api/src/mcp/tools/query/promqlTile.ts b/packages/api/src/mcp/tools/query/promqlTile.ts index 0e12be5ad2..e332c068e2 100644 --- a/packages/api/src/mcp/tools/query/promqlTile.ts +++ b/packages/api/src/mcp/tools/query/promqlTile.ts @@ -82,7 +82,11 @@ export function toPromqlTileResult( expressions, ); if (!displayTypeSupportsReducer(config)) { - return { ...rows, seriesLabels: promqlSeriesLabels }; + // Table and histogram rows carry no per-series labels; leave the key out. + return { + ...rows, + ...(promqlSeriesLabels ? { seriesLabels: promqlSeriesLabels } : {}), + }; } const reduced = reducePromqlChartRows( rows, diff --git a/packages/api/src/utils/__tests__/trimToolResponse.test.ts b/packages/api/src/utils/__tests__/trimToolResponse.test.ts index 0b4f67fd00..688c954af7 100644 --- a/packages/api/src/utils/__tests__/trimToolResponse.test.ts +++ b/packages/api/src/utils/__tests__/trimToolResponse.test.ts @@ -102,6 +102,20 @@ describe('trimToolResponse', () => { expect(isTrimmed).toBe(false); expect(result).toEqual(obj); }); + + it('should drop undefined values instead of throwing', () => { + const obj = { + data: Array.from({ length: 100 }, (_, i) => ({ id: i })), + rows: 100, + seriesLabels: undefined, + }; + + const { data: result, isTrimmed } = trimToolResponse(obj, 200); + expect(isTrimmed).toBe(true); + expect(result).not.toHaveProperty('seriesLabels'); + expect(result.rows).toBe(100); + expect(result.data.length).toBeLessThan(100); + }); }); describe('getAIMetadata structure', () => { diff --git a/packages/api/src/utils/trimToolResponse.ts b/packages/api/src/utils/trimToolResponse.ts index 8288c8bc97..548b10b767 100644 --- a/packages/api/src/utils/trimToolResponse.ts +++ b/packages/api/src/utils/trimToolResponse.ts @@ -293,7 +293,11 @@ function trimObject( path: string, stats: FallbackStats, ): any { - const entries = Object.entries(obj); + // JSON.stringify drops undefined fields and returns undefined for them, which + // would break the size math below, so drop them here too. + const entries = Object.entries(obj).filter( + ([, value]) => value !== undefined, + ); if (entries.length === 0) return obj; // Values within an equal share stay whole; the large ones split what is left,