diff --git a/.changeset/series-limit-ranking-range.md b/.changeset/series-limit-ranking-range.md new file mode 100644 index 0000000000..385ae5f87c --- /dev/null +++ b/.changeset/series-limit-ranking-range.md @@ -0,0 +1,14 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +'@hyperdx/api': patch +--- + +feat(charts): choose the range the Series Limit ranks over + +A time chart with a Series Limit picks its top N series from the most recent +part of the time range, so a series with no recent events was dropped even when +it was the largest overall. Display Settings now has a "Rank series over" +option: "Most recent window" (the default, unchanged behaviour) or "Full range". +The option is also available as `seriesLimitRankingRange` in the external API +and MCP, and the Series Limit descriptions now say how the ranking works. diff --git a/packages/api/openapi.json b/packages/api/openapi.json index 0a7b87fb39..40e9007bd0 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -1845,9 +1845,19 @@ "seriesLimit": { "type": "integer", "minimum": 0, - "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series.\n", + "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series. The range the ranking covers is set by seriesLimitRankingRange.\n", "example": 5 }, + "seriesLimitRankingRange": { + "type": "string", + "enum": [ + "recent", + "full" + ], + "description": "Range the top-N ranking for seriesLimit covers when the chart is fetched in chunks. \"recent\" ranks over the newest chunk window only, so a series with no events in that window is dropped. \"full\" ranks over the whole time range, which is accurate but scans more data. Ignored without a positive seriesLimit and a groupBy.\n", + "default": "recent", + "example": "full" + }, "formulas": { "type": "array", "maxItems": 10, @@ -1922,9 +1932,19 @@ "seriesLimit": { "type": "integer", "minimum": 0, - "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series.", + "description": "Maximum number of series rendered (top-N by value). Omit to use the default render cap, set 0 for unlimited, or a positive N to keep the top N series. The range the ranking covers is set by seriesLimitRankingRange.", "example": 5 }, + "seriesLimitRankingRange": { + "type": "string", + "enum": [ + "recent", + "full" + ], + "description": "Range the top-N ranking for seriesLimit covers when the chart is fetched in chunks. \"recent\" ranks over the newest chunk window only, so a series with no events in that window is dropped. \"full\" ranks over the whole time range, which is accurate but scans more data. Ignored without a positive seriesLimit and a groupBy.\n", + "default": "recent", + "example": "full" + }, "formulas": { "type": "array", "maxItems": 10, diff --git a/packages/api/src/mcp/tools/dashboards/schemas.ts b/packages/api/src/mcp/tools/dashboards/schemas.ts index a944ff5fff..8773fde484 100644 --- a/packages/api/src/mcp/tools/dashboards/schemas.ts +++ b/packages/api/src/mcp/tools/dashboards/schemas.ts @@ -16,6 +16,7 @@ import { MetricsDataType, NumberTileColorConditionSchema, SearchConditionTrimmedLanguageSchema, + SeriesLimitRankingRangeSchema, StaticListDashboardFilterSchema, } from '@hyperdx/common-utils/dist/types'; import { z } from 'zod'; @@ -103,13 +104,24 @@ const sqlOnlyVariableMacroDescription = const timeChartSeriesLimitDescription = 'Maximum number of series to fetch (the "Series Limit" display setting). ' + - 'Keeps the top-N groups by aggregated value over the queried range and ' + - 'drops the rest. Requires `groupBy`; ignored on a chart without one. ' + - 'Three-state: omit to apply the default render cap, 0 for unlimited, or a ' + - 'positive N to keep the top N.'; + 'Keeps the top-N groups by aggregated value and drops the rest. The ' + + 'ranking covers the range set by `seriesLimitRankingRange`, which defaults ' + + 'to the most recent part of the queried range. Requires `groupBy`; ignored ' + + 'on a chart without one. Three-state: omit to apply the default render ' + + 'cap, 0 for unlimited, or a positive N to keep the top N.'; const seriesLimitSchema = z.number().int().nonnegative().optional(); +const seriesLimitRankingRangeSchema = + SeriesLimitRankingRangeSchema.optional().describe( + 'Range the top-N ranking for `seriesLimit` covers (the "Rank series ' + + 'over" display setting). "recent" (default) ranks over the newest ' + + 'chunk of the queried range only, so a group with no recent events is ' + + 'dropped even if it is the largest overall. "full" ranks over the ' + + 'whole queried range, which is accurate but scans more data. Ignored ' + + 'without a positive `seriesLimit` and a `groupBy`.', + ); + const numberTileColorDescription = 'Static color for the displayed number, as a palette token such as ' + '"chart-blue", "chart-green", or "chart-success" (see the enum for the ' + @@ -705,6 +717,7 @@ export const mcpLineTileSchema = mcpTileLayoutSchema.extend({ 'Scale the y-axis to the data range instead of starting at zero.', ), seriesLimit: seriesLimitSchema.describe(timeChartSeriesLimitDescription), + seriesLimitRankingRange: seriesLimitRankingRangeSchema, formulas: mcpTileFormulasSchema, showOperandSeries: mcpShowOperandSeriesSchema, }), @@ -726,6 +739,7 @@ export const mcpBarTileSchema = mcpTileLayoutSchema.extend({ .optional() .describe(tileLevelNumberFormatDescription), seriesLimit: seriesLimitSchema.describe(timeChartSeriesLimitDescription), + seriesLimitRankingRange: seriesLimitRankingRangeSchema, formulas: mcpTileFormulasSchema, showOperandSeries: mcpShowOperandSeriesSchema, }), diff --git a/packages/api/src/routers/external-api/v2/dashboards.ts b/packages/api/src/routers/external-api/v2/dashboards.ts index 51546ca632..6d67ff613d 100644 --- a/packages/api/src/routers/external-api/v2/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/dashboards.ts @@ -703,8 +703,21 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: > * Maximum number of series rendered (top-N by value). Omit to use * the default render cap, set 0 for unlimited, or a positive N to - * keep the top N series. + * keep the top N series. The range the ranking covers is set by + * seriesLimitRankingRange. * example: 5 + * seriesLimitRankingRange: + * type: string + * enum: [recent, full] + * description: > + * Range the top-N ranking for seriesLimit covers when the chart is + * fetched in chunks. "recent" ranks over the newest chunk window + * only, so a series with no events in that window is dropped. + * "full" ranks over the whole time range, which is accurate but + * scans more data. Ignored without a positive seriesLimit and a + * groupBy. + * default: recent + * example: full * formulas: * type: array * maxItems: 10 @@ -773,8 +786,21 @@ const EXTERNAL_DASHBOARD_PROJECTION = { * description: >- * Maximum number of series rendered (top-N by value). Omit to use * the default render cap, set 0 for unlimited, or a positive N to - * keep the top N series. + * keep the top N series. The range the ranking covers is set by + * seriesLimitRankingRange. * example: 5 + * seriesLimitRankingRange: + * type: string + * enum: [recent, full] + * description: > + * Range the top-N ranking for seriesLimit covers when the chart is + * fetched in chunks. "recent" ranks over the newest chunk window + * only, so a series with no events in that window is dropped. + * "full" ranks over the whole time range, which is accurate but + * scans more data. Ignored without a positive seriesLimit and a + * groupBy. + * default: recent + * example: full * formulas: * type: array * maxItems: 10 diff --git a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts index 3306ab728d..119b31d7bc 100644 --- a/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts +++ b/packages/api/src/routers/external-api/v2/utils/__tests__/dashboards.test.ts @@ -448,6 +448,131 @@ describe('convertToExternalDashboard orphan-ref heal', () => { }); }, ); + + // seriesLimitRankingRange defaults to 'recent' when absent. An explicit + // value must survive a GET->PUT round-trip; null maps to absent. + describe.each([DisplayType.Line, DisplayType.StackedBar])( + 'seriesLimitRankingRange serialization for %s tiles', + displayType => { + function readRankingRange( + seriesLimitRankingRange: 'recent' | 'full' | null | undefined, + ): unknown { + const doc = makeDoc({ + tiles: [ + makeTile({ + id: 'ranking-range-tile', + config: { + displayType, + source: new mongoose.Types.ObjectId().toString(), + name: 'Ranking range tile', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange, + }, + }), + ], + }); + const wire = JSON.parse( + JSON.stringify(convertToExternalDashboard(doc)), + ); + return wire.tiles[0].config.seriesLimitRankingRange; + } + + it('passes full through unchanged', () => { + expect(readRankingRange('full')).toBe('full'); + }); + + it('passes an explicit recent through unchanged', () => { + expect(readRankingRange('recent')).toBe('recent'); + }); + + it('emits it as absent when stored as null or not set', () => { + expect(readRankingRange(null)).toBeUndefined(); + expect(readRankingRange(undefined)).toBeUndefined(); + }); + + // The PUT side: what convertToInternalTileConfig stores for the value + // a client sends. Without this, a save that drops the field would pass. + function writeRankingRange( + seriesLimitRankingRange: 'recent' | 'full' | undefined, + ): unknown { + const shared = { + sourceId: new mongoose.Types.ObjectId().toString(), + select: [{ aggFn: 'count' as const, where: '' }], + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange, + }; + const internal = convertToInternalTileConfig({ + id: 'ranking-range-tile', + x: 0, + y: 0, + w: 12, + h: 4, + name: 'Ranking range tile', + config: + displayType === DisplayType.StackedBar + ? { ...shared, displayType: 'stacked_bar' as const } + : { ...shared, displayType: 'line' as const }, + }); + if (!isBuilderSavedChartConfig(internal.config)) { + throw new Error('Expected a builder config for a time chart tile'); + } + return internal.config.seriesLimitRankingRange; + } + + it('stores full when a client sends it', () => { + expect(writeRankingRange('full')).toBe('full'); + }); + + it('stores an explicit recent when a client sends it', () => { + expect(writeRankingRange('recent')).toBe('recent'); + }); + + it('stores nothing when a client omits it', () => { + expect(writeRankingRange(undefined)).toBeUndefined(); + }); + + it.each(['recent', 'full'] as const)( + 'keeps %s through GET then PUT', + value => { + const doc = makeDoc({ + tiles: [ + makeTile({ + id: 'ranking-range-tile', + config: { + displayType, + source: new mongoose.Types.ObjectId().toString(), + name: 'Ranking range tile', + select: [{ aggFn: 'count', valueExpression: '' }], + where: '', + groupBy: 'ServiceName', + seriesLimit: 5, + seriesLimitRankingRange: value, + }, + }), + ], + }); + const [externalTile] = convertToExternalDashboard(doc).tiles; + const externalConfig = externalTile.config; + if (!externalConfig) { + throw new Error('Expected the tile to have an external config'); + } + const internal = convertToInternalTileConfig({ + ...externalTile, + config: externalConfig, + }); + if (!isBuilderSavedChartConfig(internal.config)) { + throw new Error('Expected a builder config for a time chart tile'); + } + expect(internal.config.seriesLimitRankingRange).toBe(value); + expect(internal.config.seriesLimit).toBe(5); + }, + ); + }, + ); }); describe('convertToExternalDashboard stale aggregation params', () => { diff --git a/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts b/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts index f7356078f9..032dba6c2d 100644 --- a/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts +++ b/packages/api/src/routers/external-api/v2/utils/alertChartConfig.ts @@ -112,6 +112,9 @@ export const EVALUATION_INERT_CONFIG_KEYS: ReadonlySet = new Set([ // Evaluation-relevant only with a groupBy, which (on variants whose shape // lacks seriesLimit, i.e. number) is itself refused when present. 'seriesLimit', + // Only read by the chunked time-chart fetch in the app; the alert task + // does not chunk its queries. + 'seriesLimitRankingRange', // The alert task derives granularity from alert.interval // (tasks/checkAlerts/index.ts: `${windowSizeInMins} minute`) and the alert // detail chart from intervalToGranularity, so the stored value only affects diff --git a/packages/api/src/routers/external-api/v2/utils/dashboards.ts b/packages/api/src/routers/external-api/v2/utils/dashboards.ts index fa3a4eb311..a0f37b034c 100644 --- a/packages/api/src/routers/external-api/v2/utils/dashboards.ts +++ b/packages/api/src/routers/external-api/v2/utils/dashboards.ts @@ -339,6 +339,8 @@ export const convertToExternalTileChartConfig = ( // Three-state passthrough: 0 (unlimited) and positive N round-trip; // null/undefined map to absent (the default-cap state). seriesLimit: config.seriesLimit ?? undefined, + // null/undefined map to absent (the default, 'recent'). + seriesLimitRankingRange: config.seriesLimitRankingRange ?? undefined, ...externalFormulaFields, ...externalShowOperandSeriesField, }; @@ -360,6 +362,8 @@ export const convertToExternalTileChartConfig = ( // Three-state passthrough: 0 (unlimited) and positive N round-trip; // null/undefined map to absent (the default-cap state). seriesLimit: config.seriesLimit ?? undefined, + // null/undefined map to absent (the default, 'recent'). + seriesLimitRankingRange: config.seriesLimitRankingRange ?? undefined, ...externalFormulaFields, ...externalShowOperandSeriesField, }; @@ -775,6 +779,7 @@ export function convertToInternalTileConfig( fillNulls: externalConfig.fillNulls === false ? false : undefined, seriesReturnType: externalConfig.asRatio ? 'ratio' : undefined, seriesLimit: externalConfig.seriesLimit, + seriesLimitRankingRange: externalConfig.seriesLimitRankingRange, name, } satisfies BuilderSavedChartConfig; break; diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 78622d9c1a..37f482e930 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -24,6 +24,7 @@ import { QueryExpressionDashboardFilterSchema, scheduleStartAtSchema, SearchConditionLanguageSchema as whereLanguageSchema, + SeriesLimitRankingRangeSchema, StaticListDashboardFilterSchema, tagsSchema, validateAlertChannelSelection, @@ -354,6 +355,9 @@ const externalDashboardLineChartConfigSchema = // Three-state, matching the internal SharedChartSettingsSchema.seriesLimit: // omitted = default render cap, 0 = unlimited, positive N = top-N by peak. seriesLimit: z.number().int().nonnegative().optional(), + // Matches the internal SelectSQLStatementSchema.seriesLimitRankingRange: + // omitted = 'recent' (rank over the newest chunk window). + seriesLimitRankingRange: SeriesLimitRankingRangeSchema.optional(), }); const externalDashboardLineRawSqlChartConfigSchema = @@ -376,6 +380,9 @@ const externalDashboardBarChartConfigSchema = // Three-state, matching the internal SharedChartSettingsSchema.seriesLimit: // omitted = default render cap, 0 = unlimited, positive N = top-N by peak. seriesLimit: z.number().int().nonnegative().optional(), + // Matches the internal SelectSQLStatementSchema.seriesLimitRankingRange: + // omitted = 'recent' (rank over the newest chunk window). + seriesLimitRankingRange: SeriesLimitRankingRangeSchema.optional(), }); const externalDashboardBarRawSqlChartConfigSchema = diff --git a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx index 20f3ee0894..8a87a9d405 100644 --- a/packages/app/src/components/ChartDisplaySettingsDrawer.tsx +++ b/packages/app/src/components/ChartDisplaySettingsDrawer.tsx @@ -9,6 +9,7 @@ import { DisplayType, MAX_LEGEND_TEMPLATE_LENGTH, NumberFormat, + SeriesLimitRankingRange, } from '@hyperdx/common-utils/dist/types'; import { Alert, @@ -19,6 +20,7 @@ import { Drawer, Group, NumberInput, + SegmentedControl, Stack, Text, } from '@mantine/core'; @@ -58,6 +60,10 @@ export type ChartConfigDisplaySettings = Pick< // for the authoritative semantics. The editor clears to `null` (not // `undefined`) so the cleared state survives JSON round-tripping via the URL. seriesLimit?: number | null; + // Builder group-by time charts only: the range the top-N ranking covers when + // the chart is fetched in chunks. null/undefined = 'recent' (the newest + // chunk window). See SeriesLimitRankingRangeSchema. + seriesLimitRankingRange?: SeriesLimitRankingRange | null; // PromQL-only: Handlebars template over each series' Prometheus label set // that renders the legend/tooltip name. legendTemplate?: string; @@ -107,6 +113,7 @@ function applyDefaultSettings( // Coerce to null so `reset` clears the input; undefined leaves the // previously registered field value in place. seriesLimit: settings.seriesLimit ?? null, + seriesLimitRankingRange: settings.seriesLimitRankingRange ?? 'recent', legendTemplate: settings.legendTemplate ?? '', color: settings.color, colorRules: settings.colorRules @@ -148,6 +155,7 @@ export default function ChartDisplaySettingsDrawer({ }, [appliedDefaults, reset]); const fillNulls = useWatch({ control, name: 'fillNulls' }); + const seriesLimit = useWatch({ control, name: 'seriesLimit' }); const isFillNullsEnabled = shouldFillNullsWithZero(fillNulls); const handleClose = useCallback(() => { @@ -170,6 +178,10 @@ export default function ChartDisplaySettingsDrawer({ onChange( { ...rest, + // 'recent' is the default, so persist it as null to keep saved + // configs unchanged unless the user opts into the full range. + seriesLimitRankingRange: + formValues.seriesLimitRankingRange === 'full' ? 'full' : null, numberFormat: numberFormatExplicit ? formValues.numberFormat : undefined, @@ -201,6 +213,13 @@ export default function ChartDisplaySettingsDrawer({ // excluded — its series come from Prometheus, not this pipeline. const showSeriesLimit = isTimeChart && configType !== 'promql'; const isRawSqlTimeChart = showSeriesLimit && configType === 'sql'; + // The ranking range only matters where the __hdx_series_limit CTE runs: + // builder time charts with a positive Series Limit. + const showSeriesLimitRankingRange = + showSeriesLimit && + !isRawSqlTimeChart && + seriesLimit != null && + seriesLimit > 0; // Every PromQL display that surfaces a series name. A number tile shows one // value and a table gives each label its own column, so neither has a legend. @@ -304,7 +323,7 @@ export default function ChartDisplaySettingsDrawer({ description={ isRawSqlTimeChart ? `Maximum number of series rendered, keeping those with the largest values. Leave empty for the default (${MAX_RENDERED_TIME_CHART_SERIES}); set 0 for unlimited.` - : `Maximum number of series fetched for a group-by chart, keeping those with the largest values. Leave empty for the default (${MAX_RENDERED_TIME_CHART_SERIES}); set 0 for unlimited.` + : `Maximum number of series fetched for a group-by chart, keeping those with the largest values in the ranking range below. Leave empty for the default (${MAX_RENDERED_TIME_CHART_SERIES}); set 0 for unlimited.` } placeholder={`Default (${MAX_RENDERED_TIME_CHART_SERIES})`} min={0} @@ -318,6 +337,36 @@ export default function ChartDisplaySettingsDrawer({ /> )} + {showSeriesLimitRankingRange && ( + + + Rank series over + + + Most recent window ranks series by the newest part of the time + range only, so a series with no recent events is dropped. Full + range ranks over the whole time range, which is accurate but + slower on long ranges. + + ( + + )} + /> + + )} )} diff --git a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx index 46cad955c2..61d60e52e1 100644 --- a/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx +++ b/packages/app/src/components/DBEditTimeChartForm/EditTimeChartForm.tsx @@ -342,6 +342,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -357,6 +358,7 @@ export default function EditTimeChartForm({ 'groupByColumnsOnLeft', 'alternateRowBackground', 'seriesLimit', + 'seriesLimitRankingRange', 'color', 'colorRules', 'backgroundChart', @@ -388,6 +390,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -402,6 +405,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -883,6 +887,7 @@ export default function EditTimeChartForm({ groupByColumnsOnLeft, alternateRowBackground, seriesLimit, + seriesLimitRankingRange, color, colorRules, backgroundChart, @@ -906,6 +911,8 @@ export default function EditTimeChartForm({ // survives JSON round-tripping through the URL query state; otherwise // the dropped key lets RHF's `values` sync restore the stale value. setValue('seriesLimit', seriesLimit ?? null); + // Same null-not-undefined rule as seriesLimit above. + setValue('seriesLimitRankingRange', seriesLimitRankingRange ?? null); setValue('color', color); setValue('colorRules', colorRules); setValue('backgroundChart', backgroundChart); diff --git a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx index cc0a906752..8db9b2312d 100644 --- a/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx +++ b/packages/app/src/components/__tests__/ChartDisplaySettingsDrawer.test.tsx @@ -271,6 +271,134 @@ describe('ChartDisplaySettingsDrawer', () => { }); }); + describe('series limit ranking range setting', () => { + const builderProps = { ...baseProps, configType: 'builder' as const }; + + it('is hidden until a positive Series Limit is set', async () => { + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + expect( + screen.queryByTestId('series-limit-ranking-range'), + ).not.toBeInTheDocument(); + + await user.type( + screen.getByRole('textbox', { name: /series limit/i }), + '5', + ); + + expect( + screen.getByTestId('series-limit-ranking-range'), + ).toBeInTheDocument(); + }); + + it('is hidden when Series Limit is 0 (unlimited)', () => { + renderWithMantine( + , + ); + + expect( + screen.queryByTestId('series-limit-ranking-range'), + ).not.toBeInTheDocument(); + }); + + it('is hidden for raw SQL time charts, which rank client-side', () => { + renderWithMantine( + , + ); + + expect( + screen.queryByTestId('series-limit-ranking-range'), + ).not.toBeInTheDocument(); + }); + + it('defaults to the most recent window', () => { + renderWithMantine( + , + ); + + expect( + screen.getByRole('radio', { name: /most recent window/i }), + ).toBeChecked(); + expect( + screen.getByRole('radio', { name: /full range/i }), + ).not.toBeChecked(); + }); + + it('calls onChange with seriesLimitRankingRange full when selected', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + await user.click(screen.getByRole('radio', { name: /full range/i })); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0]).toMatchObject({ + seriesLimit: 5, + seriesLimitRankingRange: 'full', + }); + expect(onChange.mock.calls[0][1]).toBe(true); + }); + + // The default is stored as null so existing saved configs are unchanged + // and the cleared state survives the URL round-trip. + it('emits null when switched back to the most recent window', async () => { + const onChange = jest.fn(); + const user = userEvent.setup(); + + renderWithMantine( + , + ); + + expect(screen.getByRole('radio', { name: /full range/i })).toBeChecked(); + + await user.click( + screen.getByRole('radio', { name: /most recent window/i }), + ); + await user.click(screen.getByRole('button', { name: /apply/i })); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange.mock.calls[0][0].seriesLimitRankingRange).toBeNull(); + }); + }); + describe('categorical series limit setting (pie/bar)', () => { const builderProps = { ...baseProps, configType: 'builder' as const }; diff --git a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx index 0793af11e0..9a03ec5dbe 100644 --- a/packages/app/src/hooks/__tests__/useChartConfig.test.tsx +++ b/packages/app/src/hooks/__tests__/useChartConfig.test.tsx @@ -1566,6 +1566,157 @@ describe('useChartConfig', () => { } }); + it('ranks the series limit over the full range on each chunk when seriesLimitRankingRange is full', async () => { + const dateRange: [Date, Date] = [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ]; + const config = createMockChartConfig({ + dateRange, + granularity: '3 hour', + seriesLimit: 3, + seriesLimitRankingRange: 'full', + }); + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { enableQueryChunking: true }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + // Every chunk still shares one ranking range, so the union across + // chunks stays within the limit. It is the whole chart range, so a + // group that is only busy early in the range is kept. + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect(windowed.seriesLimitDateRange).toEqual(dateRange); + } + }); + + it('returns a series that is only active early in the range when seriesLimitRankingRange is full', async () => { + const dateRange: [Date, Date] = [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ]; + const config = createMockChartConfig({ + dateRange, + granularity: '3 hour', + seriesLimit: 1, + seriesLimitRankingRange: 'full', + }); + + // The ranking is resolved in SQL. With the full range pinned, the one + // kept group is the early-only one, so the newest chunks come back + // empty and only the oldest chunk returns rows for it. + const earlyOnlyRows = [ + { + 'count()': '10000', + SeverityText: 'early-only', + __hdx_time_bucket: '2025-10-01T03:00:00Z', + }, + { + 'count()': '9000', + SeverityText: 'early-only', + __hdx_time_bucket: '2025-10-01T06:00:00Z', + }, + ]; + mockClickhouseClient.queryChartConfig + .mockResolvedValueOnce(createMockQueryResponse([])) + .mockResolvedValueOnce(createMockQueryResponse([])) + .mockResolvedValueOnce(createMockQueryResponse(earlyOnlyRows)); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { enableQueryChunking: true }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + // The oldest chunk, the only one with rows, ranked over the full range. + expect(calls[2][0].config.seriesLimitDateRange).toEqual(dateRange); + expect(calls[2][0].config.seriesLimit).toBe(1); + + // The early-only series reaches the chart, and it is the only one. + const rows = result.current.data?.data ?? []; + expect(rows).toEqual(earlyOnlyRows); + const series = new Set(rows.map(r => r.SeverityText)); + expect([...series]).toEqual(['early-only']); + expect(series.size).toBeLessThanOrEqual(config.seriesLimit ?? 0); + }); + + it('pins the series-limit ranking to the newest window when seriesLimitRankingRange is recent', async () => { + const config = createMockChartConfig({ + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], + granularity: '3 hour', + seriesLimit: 3, + seriesLimitRankingRange: 'recent', + }); + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { enableQueryChunking: true }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const newestWindow: [Date, Date] = [ + new Date('2025-10-01T18:00:00.000Z'), + new Date('2025-10-02T00:00:00.000Z'), + ]; + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect(windowed.seriesLimitDateRange).toEqual(newestWindow); + } + }); + + it('does not set seriesLimitDateRange without a positive seriesLimit, even when seriesLimitRankingRange is full', async () => { + const config = createMockChartConfig({ + dateRange: [ + new Date('2025-10-01 00:00:00Z'), + new Date('2025-10-02 00:00:00Z'), + ], + granularity: '3 hour', + seriesLimitRankingRange: 'full', + }); + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => useQueriedChartConfig(config, { enableQueryChunking: true }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect('seriesLimitDateRange' in windowed).toBe(false); + } + }); + it('does not set seriesLimitDateRange when the query is not chunked', async () => { const config = createMockChartConfig({ dateRange: [ @@ -2117,6 +2268,37 @@ describe('useChartConfig', () => { }); }); + it('ranks the series limit over the full range with parallel queries when seriesLimitRankingRange is full', async () => { + const { config } = setupParallelQueries(); + const configWithLimit = { + ...config, + seriesLimit: 3, + seriesLimitRankingRange: 'full' as const, + }; + + mockClickhouseClient.queryChartConfig.mockResolvedValue( + createMockQueryResponse([]), + ); + + const { result } = renderHook( + () => + useQueriedChartConfig(configWithLimit, { + enableQueryChunking: true, + enableParallelQueries: true, + }), + { wrapper }, + ); + + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + await waitFor(() => expect(result.current.isFetching).toBe(false)); + + const calls = mockClickhouseClient.queryChartConfig.mock.calls; + expect(calls).toHaveLength(3); + for (const [{ config: windowed }] of calls) { + expect(windowed.seriesLimitDateRange).toEqual(config.dateRange); + } + }); + it('pins the series-limit ranking to the newest window with parallel queries', async () => { const { config } = setupParallelQueries(); const configWithLimit = { ...config, seriesLimit: 3 }; diff --git a/packages/app/src/hooks/useChartConfig.tsx b/packages/app/src/hooks/useChartConfig.tsx index ea96708564..3ea62a550f 100644 --- a/packages/app/src/hooks/useChartConfig.tsx +++ b/packages/app/src/hooks/useChartConfig.tsx @@ -192,11 +192,18 @@ async function* fetchDataInChunks({ // Every chunk must rank the __hdx_series_limit CTE over the same fixed // range, or each window keeps its own top-N and the union across chunks - // exceeds seriesLimit. The newest window is used (rather than the full - // chart range) to bound the ranking scan; the trade-off is that series - // are picked by recent activity, so groups with no events in the newest - // window are dropped from the chart. - const rankingDateRange = windows[0]?.dateRange; + // exceeds seriesLimit. By default the newest window is used (rather than + // the full chart range) to bound the ranking scan; the trade-off is that + // series are picked by recent activity, so groups with no events in the + // newest window are dropped from the chart. A tile can opt into ranking + // over the full chart range with `seriesLimitRankingRange: 'full'`. + const fullDateRange = config.dateRange; + const rankOverFullRange = + isBuilderChartConfig(config) && config.seriesLimitRankingRange === 'full'; + const rankingDateRange = + rankOverFullRange && fullDateRange != null + ? fullDateRange + : windows[0]?.dateRange; // Only a positive seriesLimit emits the __hdx_series_limit CTE (0 = unlimited, // null = default), so only then does the ranking need a pinned date range. const seriesLimit = diff --git a/packages/common-utils/src/__tests__/renderChartConfig.test.ts b/packages/common-utils/src/__tests__/renderChartConfig.test.ts index 2264838536..86359655d2 100644 --- a/packages/common-utils/src/__tests__/renderChartConfig.test.ts +++ b/packages/common-utils/src/__tests__/renderChartConfig.test.ts @@ -771,6 +771,74 @@ describe('renderChartConfig', () => { ); }); + it('ranks over the full chart range from every chunk when the caller pins it', async () => { + // seriesLimitRankingRange: 'full' pins the ranking to the whole chart + // range. A group that is only active in the oldest chunk is then inside + // the ranking scan of every chunk, the newest one included. + const chartRange: [Date, Date] = [ + new Date('2025-02-12T00:00:00Z'), + new Date('2025-02-13T00:00:00Z'), + ]; + const newestWindow: [Date, Date] = [ + new Date('2025-02-12T18:00:00Z'), + chartRange[1], + ]; + const oldestWindow: [Date, Date] = [ + chartRange[0], + new Date('2025-02-12T06:00:00Z'), + ]; + const render = async ( + dateRange: [Date, Date], + dateRangeEndInclusive: boolean, + seriesLimitDateRange: [Date, Date], + ) => + parameterizedQueryToSql( + await renderChartConfig( + { + ...baseLogsConfig, + seriesLimit: 1, + dateRange, + dateRangeEndInclusive, + seriesLimitDateRange, + }, + mockMetadata, + querySettings, + ), + ); + const cteOf = (sql: string) => { + const start = sql.indexOf('`__hdx_series_limit` AS ('); + const end = sql.indexOf(') SELECT '); + expect(start).toBeGreaterThanOrEqual(0); + expect(end).toBeGreaterThan(start); + return sql.slice(start, end); + }; + const outerOf = (sql: string) => sql.slice(sql.indexOf(') SELECT ')); + const chartStart = String(chartRange[0].getTime()); + const chartEnd = String(chartRange[1].getTime()); + const newestStart = String(newestWindow[0].getTime()); + + const newestFull = await render(newestWindow, true, chartRange); + const oldestFull = await render(oldestWindow, false, chartRange); + + // Both chunks rank over the whole chart range and keep the same top N. + expect(cteOf(newestFull)).toBe(cteOf(oldestFull)); + expect(cteOf(newestFull)).toContain(chartStart); + expect(cteOf(newestFull)).toContain(chartEnd); + // The ranking scan is not narrowed to the newest window. + expect(cteOf(newestFull)).not.toContain(newestStart); + // The outer query still reads only its own chunk. + expect(outerOf(newestFull)).toContain(newestStart); + expect(outerOf(oldestFull)).not.toContain(chartEnd); + // The limit applies to the ranking, so the union across chunks is N. + expect(cteOf(newestFull)).toMatch(/LIMIT 1\b/); + + // Contrast: pinned to the newest window, the ranking scan starts at + // that window and cannot see the oldest chunk's groups. + const newestRecent = await render(newestWindow, true, newestWindow); + expect(cteOf(newestRecent)).toContain(newestStart); + expect(cteOf(newestRecent)).not.toContain(chartStart); + }); + it('does not emit a series-limit CTE without a group-by', async () => { const sql = parameterizedQueryToSql( await renderChartConfig( diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 69c26e4699..e90092ae28 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -471,6 +471,18 @@ export const ChSqlSchema = z.object({ // When making changes here, consider if they need to be made to the external API // schema as well (packages/api/src/utils/zod.ts). +// Which range the `__hdx_series_limit` CTE ranks groups over when a time +// chart is fetched in chunks: +// - `recent` (default): the newest chunk window only. Cheap, but groups with +// no events in that window are dropped, however large they are earlier on. +// - `full`: the whole chart date range. Accurate, but every chunk scans the +// full range to rank. +// A query that is not chunked always ranks over its whole date range. +export const SeriesLimitRankingRangeSchema = z.enum(['recent', 'full']); +export type SeriesLimitRankingRange = z.infer< + typeof SeriesLimitRankingRangeSchema +>; + export const SelectSQLStatementSchema = z.object({ select: SelectListSchema, from: z.object({ @@ -490,6 +502,9 @@ export const SelectSQLStatementSchema = z.object({ // (0 = unlimited, null/undefined = default cap). On builder group-by time // charts a positive value additionally drives the __hdx_series_limit CTE. seriesLimit: z.number().int().nonnegative().nullish(), + // Builder group-by time charts only: the range the top-N ranking covers. + // Nullish = `recent`. See SeriesLimitRankingRangeSchema. + seriesLimitRankingRange: SeriesLimitRankingRangeSchema.nullish(), }); export type SQLInterval = z.infer; @@ -1883,9 +1898,10 @@ export type DateRange = { dateRangeStartInclusive?: boolean; // default true dateRangeEndInclusive?: boolean; // default true // Runtime-only, set by query chunking when dateRange is narrowed to a - // window: a fixed ranking range (the newest chunk window) used by the - // `__hdx_series_limit` CTE so every chunk ranks (and keeps) the same - // top-N series. Never persisted. + // window: a fixed ranking range used by the `__hdx_series_limit` CTE so + // every chunk ranks (and keeps) the same top-N series. It is the newest + // chunk window, or the full chart range when `seriesLimitRankingRange` is + // `full`. Never persisted. seriesLimitDateRange?: [Date, Date]; // Runtime-only, populated from the queried MetricSource's // `minAutoGranularity` (when set) by whichever caller resolves the source