From c248209bdd2c9195b692e429c68cbfe0c0011e43 Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Mon, 2 Nov 2026 16:56:16 -0500 Subject: [PATCH 1/2] feat: add dependent adhoc filters --- .changeset/adhoc-filter-suggestions-where.md | 13 ++ packages/app/AGENTS.md | 2 +- packages/app/src/DBDashboardPage.tsx | 1 + packages/app/src/DashboardFilters.tsx | 20 +-- .../src/__tests__/DashboardFilters.test.ts | 7 +- .../AdhocConditionEditor.tsx | 13 +- .../AdhocDashboardFilter.tsx | 126 ++++++++++++--- .../__tests__/AdhocDashboardFilter.test.tsx | 102 ++++++++++++ .../__tests__/useAdhocFilterOptions.test.tsx | 44 +++++- .../useAdhocFilterOptions.ts | 50 +++++- .../src/components/DashboardFilterLabel.tsx | 14 ++ .../AdhocFilterEditForm.tsx | 33 +++- .../DashboardFilterEditForm.tsx | 12 +- .../__tests__/filterFormState.test.ts | 51 ++++++ .../DashboardFiltersModal/filterFormState.ts | 19 ++- .../FilterPill/FilterAutocomplete.tsx | 53 ++++++- .../FilterPill/FilterConditionEditor.tsx | 53 +++++-- .../__tests__/FilterConditionEditor.test.tsx | 134 ++++++++++++++-- packages/app/src/hooks/useMetadata.tsx | 7 +- .../features/dashboard-adhoc-filters.spec.ts | 78 ++++++++- .../tests/e2e/page-objects/DashboardPage.ts | 9 +- .../src/__tests__/filters.test.ts | 24 +++ .../src/__tests__/metadata.int.test.ts | 120 +++++++++++++- .../src/__tests__/metadata.test.ts | 148 +++++++++++++++++- .../common-utils/src/__tests__/types.test.ts | 30 ++++ packages/common-utils/src/core/metadata.ts | 55 ++++++- .../src/core/renderChartConfig.ts | 3 +- packages/common-utils/src/filters.ts | 5 +- packages/common-utils/src/types.ts | 6 + 29 files changed, 1126 insertions(+), 106 deletions(-) create mode 100644 .changeset/adhoc-filter-suggestions-where.md diff --git a/.changeset/adhoc-filter-suggestions-where.md b/.changeset/adhoc-filter-suggestions-where.md new file mode 100644 index 0000000000..402c283793 --- /dev/null +++ b/.changeset/adhoc-filter-suggestions-where.md @@ -0,0 +1,13 @@ +--- +'@hyperdx/common-utils': minor +'@hyperdx/app': minor +--- + +feat: narrow ad hoc dashboard filter suggestions with a condition + +An ad hoc filter on SQL sources can now carry a SQL or Lucene "Keys and values +filter", like a queried filter's dropdown values filter, that limits the rows +its keys and values are suggested from. It can reference the dashboard's other +variables. While it references a required filter with nothing selected, the ad +hoc filter can't add or edit conditions and names the filter it is waiting on. +Failed suggestions now show the database's error message. diff --git a/packages/app/AGENTS.md b/packages/app/AGENTS.md index 643577dfae..af3b7c8411 100644 --- a/packages/app/AGENTS.md +++ b/packages/app/AGENTS.md @@ -1,6 +1,6 @@ -# This is NOT the Next.js you know +## This is NOT the Next.js you know This version has breaking changes — APIs, conventions, and file structure may all differ from your training data. Read the relevant guide in `node_modules/next/dist/docs/` (resolved from this file's directory; in monorepos the `next` package may not be visible from the repo root) before writing any code. Heed deprecation notices. diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index 2a691b1703..356edb332d 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -3321,6 +3321,7 @@ function DBDashboardPage({ onSetAdhocConditions={setAdhocConditions} dateRange={searchedTimeRange} variables={variables} + unsatisfiedRequiredFilters={unsatisfiedRequiredFilters} /> )} {/* Selection indicator */} diff --git a/packages/app/src/DashboardFilters.tsx b/packages/app/src/DashboardFilters.tsx index cf5f926230..5c15cdf9a5 100644 --- a/packages/app/src/DashboardFilters.tsx +++ b/packages/app/src/DashboardFilters.tsx @@ -25,6 +25,7 @@ import { DashboardFilterLabel, FilterCaution, FilterEffect, + getPendingVariablesTooltip, } from './components/DashboardFilterLabel'; import { FilterLinkToggle } from './components/FilterLinkToggle'; import { VirtualMultiSelect } from './components/VirtualMultiSelect/VirtualMultiSelect'; @@ -46,20 +47,6 @@ interface DashboardFilterSelectProps { pendingVariables?: string[]; } -/** - * Explain a dropdown query that may list nothing until one of the variables it - * depends on is selected. Only the bare SQL reference form gets here — the - * macros and Lucene both have an empty state that lists every value. - */ -export const getPendingVariablesTooltip = ( - pendingVariables: string[], -): string => { - const names = pendingVariables.map(name => `$${name}`).join(', '); - return `Filter depends on ${names}, which ${ - pendingVariables.length === 1 ? 'has' : 'have' - } no selected value.`; -}; - /** * Explain a required filter that has nothing selected, naming how far the block * it imposes reaches. @@ -184,6 +171,8 @@ interface DashboardFilterProps { * as the gate for variable-specific copy. */ variables?: ChartVariable[]; + /** Required filters with nothing selected, which can block ad hoc filters. */ + unsatisfiedRequiredFilters?: DashboardFilter[]; } const DashboardFilters = ({ @@ -194,6 +183,7 @@ const DashboardFilters = ({ adhocConditionsByFilterId, onSetAdhocConditions, variables, + unsatisfiedRequiredFilters, }: DashboardFilterProps) => { // "Link" mode (opt-in, off by default): each dropdown's values are narrowed by // the others' selections. Off by default because contingent value lookups @@ -235,6 +225,8 @@ const DashboardFilters = ({ } dateRange={dateRange} effect={getFilterEffect(filter)} + variables={variables} + unsatisfiedRequiredFilters={unsatisfiedRequiredFilters} /> ); } diff --git a/packages/app/src/__tests__/DashboardFilters.test.ts b/packages/app/src/__tests__/DashboardFilters.test.ts index 5c5df455ab..6113b57271 100644 --- a/packages/app/src/__tests__/DashboardFilters.test.ts +++ b/packages/app/src/__tests__/DashboardFilters.test.ts @@ -1,10 +1,7 @@ import { DashboardFilter } from '@hyperdx/common-utils/dist/types'; -import { - getFilterEffect, - getPendingVariablesTooltip, - getRequiredFilterTooltip, -} from '@/DashboardFilters'; +import { getPendingVariablesTooltip } from '@/components/DashboardFilterLabel'; +import { getFilterEffect, getRequiredFilterTooltip } from '@/DashboardFilters'; const baseFilter: DashboardFilter = { id: 'filter1', diff --git a/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx b/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx index 51135a5549..cdb68e95bb 100644 --- a/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx +++ b/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx @@ -1,4 +1,5 @@ import { useState } from 'react'; +import { ResolvedFilterValuesQuery } from '@hyperdx/common-utils/dist/filters'; import { AdhocDashboardFilter, AdhocFilterCondition, @@ -15,6 +16,8 @@ import { useAdhocFilterOptions } from './useAdhocFilterOptions'; type AdhocConditionEditorProps = { filter: AdhocDashboardFilter; dateRange: [Date, Date]; + /** The filter's `where`, variables expanded, narrowing the suggestions. */ + suggestionsWhere?: ResolvedFilterValuesQuery; /** The condition being edited; unset when adding one. */ initial?: AdhocFilterCondition; onSubmit: (condition: AdhocFilterCondition) => void; @@ -27,11 +30,17 @@ type AdhocConditionEditorProps = { export function AdhocConditionEditor({ filter, dateRange, + suggestionsWhere, initial, onSubmit, }: AdhocConditionEditorProps) { const [valueKey, setValueKey] = useState(initial?.key ?? ''); - const { keys, values } = useAdhocFilterOptions(filter, valueKey, dateRange); + const { keys, values } = useAdhocFilterOptions( + filter, + valueKey, + dateRange, + suggestionsWhere, + ); const isPromql = filter.sourceType === 'promql'; return ( @@ -41,9 +50,11 @@ export function AdhocConditionEditor({ keyOptions={keys.data} isLoadingKeys={keys.isLoading} isKeysError={keys.isError} + keysErrorMessage={keys.error?.message} valueOptions={values.data} isLoadingValues={values.isLoading} isValuesError={values.isError} + valuesErrorMessage={values.error?.message} onKeyChange={setValueKey} keyLabel={isPromql ? 'Label' : 'Key'} keyPlaceholder={ diff --git a/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx b/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx index 2bc0eaae61..93ac1a511b 100644 --- a/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx +++ b/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx @@ -1,11 +1,19 @@ -import { useState } from 'react'; -import { isAdhocConditionSupported } from '@hyperdx/common-utils/dist/filters'; +import { useMemo, useState } from 'react'; +import { getBlockingRequiredFilters } from '@hyperdx/common-utils/dist/dashboardFilterValues'; +import { + getPendingFilterValuesVariables, + isAdhocConditionSupported, + resolveFilterValuesWhere, +} from '@hyperdx/common-utils/dist/filters'; import { ADHOC_FILTER_MAX_CONDITIONS, AdhocDashboardFilter as AdhocDashboardFilterType, AdhocFilterCondition, AdhocFilterConditionSchema, + ChartVariable, + DashboardFilter, } from '@hyperdx/common-utils/dist/types'; +import { getVariableReferences } from '@hyperdx/common-utils/dist/variables'; import { ActionIcon, CloseButton, @@ -19,7 +27,9 @@ import { IconPlus } from '@tabler/icons-react'; import { DashboardFilterLabel, + FilterCaution, FilterEffect, + getPendingVariablesTooltip, } from '@/components/DashboardFilterLabel'; import { FilterPill, getFilterOperatorDisplay } from '@/components/FilterPill'; @@ -35,6 +45,10 @@ type AdhocDashboardFilterProps = { onChange: (conditions: AdhocFilterCondition[]) => void; dateRange: [Date, Date]; effect: FilterEffect; + /** The dashboard's variables, which the filter's `where` may reference. */ + variables?: ChartVariable[]; + /** Required filters with nothing selected. */ + unsatisfiedRequiredFilters?: DashboardFilter[]; }; /** A dashboard filter bar control holding one ad hoc filter's conditions. */ @@ -44,11 +58,47 @@ export function AdhocDashboardFilter({ onChange, dateRange, effect, + variables, + unsatisfiedRequiredFilters, }: AdhocDashboardFilterProps) { const [isAdding, setIsAdding] = useState(false); const isAtLimit = conditions.length >= ADHOC_FILTER_MAX_CONDITIONS; + // The `where` narrowing the suggestions, and what it is still waiting on. + // Blocked the way a tile is: by the required filters it references, or by + // one that blocks every tile. + const { suggestionsWhere, pendingVariables, blockingFilterNames } = + useMemo(() => { + const where = filter.sourceType === 'sql' ? filter.where : undefined; + const query = { where, whereLanguage: filter.whereLanguage }; + return { + suggestionsWhere: resolveFilterValuesWhere(query, variables), + pendingVariables: getPendingFilterValuesVariables(query, variables), + blockingFilterNames: getBlockingRequiredFilters( + unsatisfiedRequiredFilters ?? [], + { + referencedVariableNames: getVariableReferences(where ?? '').map( + reference => reference.name, + ), + consumesBroadcastFilters: false, + }, + ).map(blocking => blocking.name), + }; + }, [ + filter.sourceType, + filter.where, + filter.whereLanguage, + variables, + unsatisfiedRequiredFilters, + ]); + const dependsOn = blockingFilterNames.length + ? `This filter depends on: ${blockingFilterNames.join(', ')}` + : undefined; + const disabledReason = dependsOn ?? suggestionsWhere.error; + const isDisabled = !!disabledReason; + const toggleAdding = () => { + if (isDisabled) return; if (isAtLimit) { notifyAdhocFilterFull(filter); return; @@ -71,7 +121,27 @@ export function AdhocDashboardFilter({ return ( - + + {dependsOn && ( + + )} + {!isDisabled && pendingVariables.length > 0 && ( + + )} + {suggestionsWhere.error && ( + + )} + remove(index)} - renderPopover={close => ( - { - if (!isStorable(next)) return; - replace(index, next); - close(); - }} - /> - )} + // Editing would look up suggestions; removing still works. + renderPopover={ + isDisabled + ? undefined + : close => ( + { + if (!isStorable(next)) return; + replace(index, next); + close(); + }} + /> + ) + } data-testid="adhoc-condition-pill" /> ); @@ -121,8 +197,8 @@ export function AdhocDashboardFilter({ setIsAdding(true)} - style={{ cursor: 'pointer', flex: 1 }} + onClick={isDisabled ? undefined : () => setIsAdding(true)} + style={{ cursor: isDisabled ? 'default' : 'pointer', flex: 1 }} > {filter.name} @@ -137,7 +213,7 @@ export function AdhocDashboardFilter({ /> )} @@ -167,6 +248,7 @@ export function AdhocDashboardFilter({ { if (!isStorable(condition)) return; onChange([...conditions, condition]); diff --git a/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx b/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx index 00f3f346c5..748872a049 100644 --- a/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx +++ b/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx @@ -1,7 +1,9 @@ +import { ComponentProps } from 'react'; import { ADHOC_FILTER_MAX_CONDITIONS, AdhocDashboardFilter as AdhocDashboardFilterType, AdhocFilterCondition, + DashboardFilter, } from '@hyperdx/common-utils/dist/types'; import { notifications } from '@mantine/notifications'; import { screen } from '@testing-library/react'; @@ -43,6 +45,7 @@ const addButton = () => screen.getByTestId('adhoc-filter-add-Conditions'); const renderFilter = ( conditions: AdhocFilterCondition[], definition: AdhocDashboardFilterType = filter, + props: Partial> = {}, ) => { const onChange = jest.fn(); renderWithMantine( @@ -52,6 +55,7 @@ const renderFilter = ( onChange={onChange} dateRange={dateRange} effect={{ hasEffect: true, tooltip: '' }} + {...props} />, ); return onChange; @@ -165,4 +169,102 @@ describe('AdhocDashboardFilter', () => { expect(onChange).toHaveBeenCalledWith([]); }); + + describe('narrowed by a condition', () => { + const narrowed: AdhocDashboardFilterType = { + ...filter, + where: 'Namespace IN ($namespace)', + whereLanguage: 'sql', + }; + const namespaceFilter: DashboardFilter = { + id: 'ns', + type: 'STATIC_LIST', + name: 'Namespace', + options: ['ns-a'], + isBroadcastEnabled: false, + isVariableEnabled: true, + variableName: 'namespace', + minSelections: 1, + }; + const namespace = (values: string[]) => [{ name: 'namespace', values }]; + + it('waits for a required filter the condition references', async () => { + const onChange = renderFilter([existing], narrowed, { + variables: namespace([]), + unsatisfiedRequiredFilters: [namespaceFilter], + }); + + expect(addButton()).toHaveAttribute('aria-disabled', 'true'); + await userEvent.hover(addButton()); + expect( + await screen.findByText('This filter depends on: Namespace'), + ).toBeInTheDocument(); + expect( + screen.getByTestId('dashboard-filter-blocked-Conditions'), + ).toBeInTheDocument(); + + await userEvent.click(addButton()); + await userEvent.click(screen.getByTestId('adhoc-condition-pill')); + expect(screen.queryByTestId(`${editorId}-key`)).not.toBeInTheDocument(); + + // Removing looks nothing up, so it still works. + await userEvent.click( + screen.getByRole('button', { name: 'Remove filter' }), + ); + expect(onChange).toHaveBeenCalledWith([]); + }); + + it('suggests from the rows the condition matches once it resolves', async () => { + renderFilter([], narrowed, { variables: namespace(['ns-a']) }); + + await userEvent.click(addButton()); + + expect(await screen.findByTestId(`${editorId}-key`)).toBeInTheDocument(); + expect(jest.mocked(useAdhocFilterOptions)).toHaveBeenLastCalledWith( + narrowed, + expect.any(String), + dateRange, + { where: "Namespace IN ('ns-a')", whereLanguage: 'sql' }, + ); + }); + + it('flags an optional variable the condition needs a value for', () => { + renderFilter([], narrowed, { variables: namespace([]) }); + + expect( + screen.getByTestId('dashboard-filter-pending-variable-Conditions'), + ).toBeInTheDocument(); + expect(addButton()).not.toHaveAttribute('aria-disabled', 'true'); + }); + + it("stops suggesting when the condition's variables can't be expanded", async () => { + renderFilter( + [], + { ...narrowed, where: '$__filter(Namespace, $nope)' }, + { variables: namespace(['ns-a']) }, + ); + + expect( + screen.getByTestId('dashboard-filter-error-Conditions'), + ).toBeInTheDocument(); + expect(addButton()).toHaveAttribute('aria-disabled', 'true'); + await userEvent.click(addButton()); + expect(screen.queryByTestId(`${editorId}-key`)).not.toBeInTheDocument(); + }); + + it('ignores a condition on a PromQL filter', async () => { + renderFilter( + [], + { ...narrowed, sourceType: 'promql' }, + { + variables: namespace([]), + unsatisfiedRequiredFilters: [namespaceFilter], + }, + ); + + await userEvent.click(addButton()); + + expect(await screen.findByTestId(`${editorId}-key`)).toBeInTheDocument(); + }); + }); }); diff --git a/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx b/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx index 8f31610946..e0880af24f 100644 --- a/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx +++ b/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx @@ -1,5 +1,6 @@ import { ReactNode } from 'react'; import { ColumnMeta, JSDataType } from '@hyperdx/common-utils/dist/clickhouse'; +import { ResolvedFilterValuesQuery } from '@hyperdx/common-utils/dist/filters'; import { AdhocDashboardFilter, SourceKind, @@ -202,13 +203,15 @@ describe('useAdhocFilterOptions', () => { const renderOptions = ( adhocFilter: AdhocDashboardFilter, key = 'SeverityText', + suggestionsWhere?: ResolvedFilterValuesQuery, ) => { const queryClient = new QueryClient(); const wrapper = ({ children }: { children: ReactNode }) => ( {children} ); return renderHook( - () => useAdhocFilterOptions(adhocFilter, key, DATE_RANGE), + () => + useAdhocFilterOptions(adhocFilter, key, DATE_RANGE, suggestionsWhere), { wrapper }, ); }; @@ -247,6 +250,45 @@ describe('useAdhocFilterOptions', () => { expect(chartConfigs).toEqual([expect.objectContaining({ source: 'logs' })]); }); + it('reads keys and values only from rows matching its condition', () => { + mockSources = [ + { ...source('logs', SourceKind.Log), implicitColumnExpression: 'Body' }, + ]; + mockAllFields([[column('SeverityText', 'LowCardinality(String)')]]); + const suggestionsWhere = { + where: 'Namespace:$namespace', + whereLanguage: 'lucene' as const, + }; + renderOptions(filter('sql', ['logs']), 'SeverityText', suggestionsWhere); + + expect(jest.mocked(useMultipleAllFields).mock.calls.at(-1)![1]).toEqual( + expect.objectContaining({ + conditions: [ + expect.objectContaining({ + ...suggestionsWhere, + implicitColumnExpression: 'Body', + }), + ], + }), + ); + expect(lastSqlLookup()[0].chartConfigs).toEqual([ + expect.objectContaining(suggestionsWhere), + ]); + }); + + it('reads every row when its condition is blank', () => { + mockSources = [source('logs', SourceKind.Log)]; + mockAllFields([[column('SeverityText', 'LowCardinality(String)')]]); + renderOptions(filter('sql', ['logs']), 'SeverityText', { + where: ' ', + whereLanguage: 'sql', + }); + + expect( + jest.mocked(useMultipleAllFields).mock.calls.at(-1)![1]?.conditions, + ).toBeUndefined(); + }); + it('quotes a map column name that is not a bare identifier', () => { mockSources = [source('logs', SourceKind.Log)]; mockAllFields([[column('log-attributes', 'Map(String, String)')]]); diff --git a/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts b/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts index 55c2af2db3..aca36927e2 100644 --- a/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts +++ b/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts @@ -11,6 +11,7 @@ import { Field, tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { getAdhocFilterSourceKinds, isAdhocComparableDataType, + ResolvedFilterValuesQuery, } from '@hyperdx/common-utils/dist/filters'; import { AdhocDashboardFilter, @@ -143,6 +144,8 @@ type AdhocFilterSuggestions = { data: string[]; isLoading: boolean; isError: boolean; + /** Set when every source's lookup failed. */ + error?: Error | null; }; type AdhocFilterKeys = AdhocFilterSuggestions & { @@ -154,15 +157,32 @@ type AdhocFilterKeys = AdhocFilterSuggestions & { function useSqlSourceKeys( sources: TSource[], dateRange: [Date, Date], + suggestionsWhere: ResolvedFilterValuesQuery | undefined, ): AdhocFilterKeys { const tableConnections = useMemo(() => sources.map(tcFromSource), [sources]); + const where = suggestionsWhere?.where.trim() + ? suggestionsWhere.where + : undefined; + const whereLanguage = suggestionsWhere?.whereLanguage; + const conditions = useMemo( + () => + where + ? sources.map(source => ({ + ...pickSourceConfigFields(source), + where, + whereLanguage, + })) + : undefined, + [sources, where, whereLanguage], + ); const { fieldsByTableConnection, columnsByTableConnection, isFieldMetadataDisabled, isLoading, isError, - } = useMultipleAllFields(tableConnections, { dateRange }); + error, + } = useMultipleAllFields(tableConnections, { dateRange, conditions }); return useMemo(() => { const keysBySourceId = new Map(); @@ -184,6 +204,7 @@ function useSqlSourceKeys( columnNames, isLoading, isError, + error, }; }, [ sources, @@ -192,6 +213,7 @@ function useSqlSourceKeys( isFieldMetadataDisabled, isLoading, isError, + error, ]); } @@ -226,6 +248,7 @@ function usePromqlSourceKeys( columnNames: NO_COLUMNS, isLoading: results.some(result => result.isLoading), isError: results.some(result => result.isError), + error: results.find(result => result.error)?.error, }; }, [sources], @@ -259,7 +282,10 @@ function useSqlSourceValues( key: string, dateRange: [Date, Date], columnNames: Set, + suggestionsWhere: ResolvedFilterValuesQuery | undefined, ): AdhocFilterSuggestions { + const where = suggestionsWhere?.where ?? ''; + const whereLanguage = suggestionsWhere?.whereLanguage ?? 'sql'; const chartConfigs = useMemo( () => sources.map(source => ({ @@ -267,10 +293,10 @@ function useSqlSourceValues( dateRange, source: source.id, select: '', - where: '', - whereLanguage: 'sql' as const, + where, + whereLanguage, })), - [sources, dateRange], + [sources, dateRange, where, whereLanguage], ); // Quoted like the search sidebar does, so a column named e.g. // `log-attributes` isn't read as a subtraction. @@ -278,14 +304,14 @@ function useSqlSourceValues( () => (key ? [toQuotedClickHouseKeyExpression(key, columnNames)] : []), [key, columnNames], ); - const { data, isFetching, isError } = useMultipleGetKeyValues( + const { data, isFetching, isError, error } = useMultipleGetKeyValues( { chartConfigs, keys, limit: SINGLE_KEY_VALUES_LIMIT }, // The previous key's values aren't suggestions for this one. { placeholderData: undefined, retry: false, enabled: sources.length > 0 }, ); const values = useMemo(() => getValueSuggestions(data), [data]); - return { data: values, isLoading: !!key && isFetching, isError }; + return { data: values, isLoading: !!key && isFetching, isError, error }; } const combineValues = ( @@ -294,6 +320,7 @@ const combineValues = ( data: unionSorted(results.map(result => result.data)), isLoading: results.some(result => result.isFetching), isError: results.some(result => result.isError), + error: results.find(result => result.error)?.error, }); function usePromqlSourceValues( @@ -332,16 +359,22 @@ function usePromqlSourceValues( * Suggestions for an ad hoc filter's condition editor, deduplicated across its * sources. Keys are the columns and map / JSON keys of its SQL sources, in the * search sidebar's key format, or the label names of its PromQL sources. - * Values are those observed for `key`. + * Values are those observed for `key`. For SQL sources, `suggestionsWhere` (the + * filter's `where` with variables expanded) narrows the rows both are read from. */ export function useAdhocFilterOptions( filter: AdhocDashboardFilter, key: string, dateRange: [Date, Date], + suggestionsWhere?: ResolvedFilterValuesQuery, ): { keys: AdhocFilterSuggestions; values: AdhocFilterSuggestions } { const sources = useAdhocFilterSources(filter); const isPromql = filter.sourceType === 'promql'; - const sqlKeys = useSqlSourceKeys(isPromql ? NO_SOURCES : sources, dateRange); + const sqlKeys = useSqlSourceKeys( + isPromql ? NO_SOURCES : sources, + dateRange, + suggestionsWhere, + ); const promqlKeys = usePromqlSourceKeys( isPromql ? sources : NO_SOURCES, dateRange, @@ -360,6 +393,7 @@ export function useAdhocFilterOptions( lookupKey, dateRange, keys.columnNames, + suggestionsWhere, ); const promqlValues = usePromqlSourceValues( isPromql ? sourcesWithKey : NO_SOURCES, diff --git a/packages/app/src/components/DashboardFilterLabel.tsx b/packages/app/src/components/DashboardFilterLabel.tsx index 37f8927fbc..893e706970 100644 --- a/packages/app/src/components/DashboardFilterLabel.tsx +++ b/packages/app/src/components/DashboardFilterLabel.tsx @@ -5,6 +5,20 @@ import { IconAlertTriangle, IconHelp } from '@tabler/icons-react'; /** What a filter does with its value, from `getFilterEffect`. */ export type FilterEffect = { hasEffect: boolean; tooltip: string }; +/** + * Explain a dropdown query that may list nothing until one of the variables it + * depends on is selected. Only the bare SQL reference form gets here — the + * macros and Lucene both have an empty state that lists every value. + */ +export const getPendingVariablesTooltip = ( + pendingVariables: string[], +): string => { + const names = pendingVariables.map(name => `$${name}`).join(', '); + return `Filter depends on ${names}, which ${ + pendingVariables.length === 1 ? 'has' : 'have' + } no selected value.`; +}; + /** One of the caution icons a filter's label row can carry, with its tooltip. */ export const FilterCaution = ({ label, diff --git a/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx b/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx index 83cffdf038..e09cbb3213 100644 --- a/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx +++ b/packages/app/src/components/DashboardFiltersModal/AdhocFilterEditForm.tsx @@ -1,4 +1,6 @@ +import { useMemo } from 'react'; import { Controller, UseFormSetValue, useWatch } from 'react-hook-form'; +import { tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; import { getAdhocFilterSourceKinds } from '@hyperdx/common-utils/dist/filters'; import { AdhocFilterSourceType, @@ -8,10 +10,12 @@ import { import { Box, Divider, SegmentedControl } from '@mantine/core'; import { CheckBoxControlled } from '@/components/InputControlled'; +import SearchWhereInput from '@/components/SearchInput/SearchWhereInput'; import { SourceMultiSelectControlled } from '@/components/SourceMultiSelect'; import { IS_PROMQL_ENABLED } from '@/config'; import { useSources } from '@/source'; +import { TOOLTIP_PORTAL_TARGET } from './constants'; import { CustomInputWrapper } from './CustomInputWrapper'; import { FilterFormControl, FilterFormValues } from './filterFormState'; import { VariableNameInput } from './VariableNameInput'; @@ -35,11 +39,18 @@ export const AdhocFilterEditForm = ({ otherFilters, }: AdhocFilterEditFormProps) => { const { data: allSources } = useSources(); - const [sourceType, isBroadcastEnabled] = useWatch({ + const [sourceType, isBroadcastEnabled, sourceIds] = useWatch({ control, - name: ['sourceType', 'isBroadcastEnabled'], + name: ['sourceType', 'isBroadcastEnabled', 'sources'], }); const allowedSourceKinds = getAdhocFilterSourceKinds(sourceType); + const tableConnections = useMemo( + () => + (allSources ?? []) + .filter(source => sourceIds?.includes(source.id)) + .map(tcFromSource), + [allSources, sourceIds], + ); // Kept visible for a saved PromQL filter so its source type can be changed const showSourceType = sourceType === 'promql' || @@ -107,6 +118,24 @@ export const AdhocFilterEditForm = ({ }} /> + {sourceType === 'sql' && ( + + + + )} ) : formFilterType === 'ADHOC' ? ( - + + + ) : formFilterType === 'PROMETHEUS_LABEL' ? ( { it('defaults a new filter to SQL ad hoc sources', () => { expect(toFormValues()).toMatchObject({ sourceType: 'sql', sources: [] }); }); + + it("seeds the shared where fields from a stored ad hoc filter's condition", () => { + const values = toFormValues({ + id: 'a', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs'], + where: 'Namespace:$namespace', + whereLanguage: 'lucene', + isVariableEnabled: true, + }); + + expect(values).toMatchObject({ + where: 'Namespace:$namespace', + whereLanguage: 'lucene', + }); + }); }); describe('toSavedFilter', () => { @@ -243,6 +261,39 @@ describe('toSavedFilter', () => { }); }); + it('keeps the trimmed condition narrowing a SQL ad hoc filter', () => { + const saved = toSavedFilter( + formValues({ + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs'], + where: ' Namespace IN ($namespace) ', + whereLanguage: 'sql', + }), + ); + + expect(saved).toMatchObject({ + where: 'Namespace IN ($namespace)', + whereLanguage: 'sql', + }); + }); + + it('drops the condition from a PromQL ad hoc filter, which has no input for it', () => { + const saved = toSavedFilter( + formValues({ + type: 'ADHOC', + name: 'Conditions', + sourceType: 'promql', + sources: ['prom'], + where: 'Namespace IN ($namespace)', + }), + ); + + expect(saved).not.toHaveProperty('where'); + expect(saved).not.toHaveProperty('whereLanguage'); + }); + it('drops the ad hoc broadcast scope when broadcast is off', () => { const saved = toSavedFilter( formValues({ diff --git a/packages/app/src/components/DashboardFiltersModal/filterFormState.ts b/packages/app/src/components/DashboardFiltersModal/filterFormState.ts index 538e317bc5..7e450282eb 100644 --- a/packages/app/src/components/DashboardFiltersModal/filterFormState.ts +++ b/packages/app/src/components/DashboardFiltersModal/filterFormState.ts @@ -81,8 +81,12 @@ export const toFormValues = ( expression: queried?.expression ?? '', source: queried?.source ?? promqlLabel?.source ?? presetSourceId ?? '', sourceMetricType: queried?.sourceMetricType, - where: queried?.where ?? '', - whereLanguage: queried?.whereLanguage ?? getStoredLanguage() ?? 'sql', + where: queried?.where ?? adhoc?.where ?? '', + whereLanguage: + queried?.whereLanguage ?? + adhoc?.whereLanguage ?? + getStoredLanguage() ?? + 'sql', appliesToSourceIds: queried?.appliesToSourceIds ?? adhoc?.appliesToSourceIds ?? [], @@ -137,6 +141,11 @@ export const toSavedFilter = (values: FilterFormValues): DashboardFilter => { } const appliesTo = values.appliesToSourceIds?.filter(id => !!id?.length); + const trimmedWhere = values.where?.trim() ?? ''; + const whereFields = { + where: trimmedWhere || undefined, + whereLanguage: trimmedWhere ? (values.whereLanguage ?? 'sql') : undefined, + }; // Selection limits don't apply to conditions, so they aren't stored. if (values.type === 'ADHOC') { @@ -147,6 +156,8 @@ export const toSavedFilter = (values: FilterFormValues): DashboardFilter => { name: values.name, sourceType: values.sourceType, sources: values.sources, + // Only SQL suggestions can be narrowed; the input is hidden otherwise + ...(values.sourceType === 'sql' ? whereFields : {}), isBroadcastEnabled, // Hidden, and so unvalidated, while broadcast is off appliesToSourceIds: @@ -156,13 +167,11 @@ export const toSavedFilter = (values: FilterFormValues): DashboardFilter => { }); } - const trimmedWhere = values.where?.trim() ?? ''; const isVariableEnabled = isFilterVariableEnabled(values); return DashboardFilterSchema.parse({ ...rest, - where: trimmedWhere || undefined, - whereLanguage: trimmedWhere ? (values.whereLanguage ?? 'sql') : undefined, + ...whereFields, appliesToSourceIds: appliesTo?.length ? appliesTo : undefined, isBroadcastEnabled: isFilterBroadcastEnabled(values), isVariableEnabled, diff --git a/packages/app/src/components/FilterPill/FilterAutocomplete.tsx b/packages/app/src/components/FilterPill/FilterAutocomplete.tsx index 4a68d8aeb0..7a1ee5a427 100644 --- a/packages/app/src/components/FilterPill/FilterAutocomplete.tsx +++ b/packages/app/src/components/FilterPill/FilterAutocomplete.tsx @@ -1,5 +1,10 @@ import { Ref } from 'react'; -import { Autocomplete, AutocompleteProps, Loader } from '@mantine/core'; +import { + Autocomplete, + AutocompleteProps, + Loader, + OptionsFilter, +} from '@mantine/core'; /** * While the user is keyboard-navigating suggestions, Enter must pick the @@ -12,6 +17,46 @@ const isSuggestionHighlighted = (input: HTMLInputElement) => { return !!list?.querySelector('[data-combobox-selected]'); }; +/** + * 0 when `label` starts with `query`, 1 when one of its parts does, 2 when it + * only contains it, and Infinity when it doesn't. + */ +const matchRank = (label: string, query: string) => { + let rank = Infinity; + for ( + let i = label.indexOf(query); + i !== -1; + i = label.indexOf(query, i + 1) + ) { + if (i === 0) return 0; + // A part starts after a separator, e.g. the map key in `Map['key']`. + rank = Math.min(rank, /[a-z0-9]/.test(label[i - 1]) ? 2 : 1); + } + return rank; +}; + +/** + * Case-insensitive matches, best rank first and shorter first within a rank, + * so "Serv" lists `ServiceName` above every `LogAttributes['agentToServer…']`. + * Ranked before the limit is applied, so the best match can't be cut off. + */ +const rankedFilter: OptionsFilter = ({ options, search, limit }) => { + const query = search.trim().toLowerCase(); + if (!query) return options.slice(0, limit); + return options + .flatMap(option => { + if ('group' in option) return []; + const rank = matchRank(option.label.toLowerCase(), query); + return rank === Infinity ? [] : [{ option, rank }]; + }) + .sort( + (a, b) => + a.rank - b.rank || a.option.label.length - b.option.label.length, + ) + .slice(0, limit) + .map(({ option }) => option); +}; + export type FilterAutocompleteProps = { value: string; onChange: (value: string) => void; @@ -26,7 +71,10 @@ export type FilterAutocompleteProps = { 'aria-label'?: string; 'data-testid'?: string; ref?: Ref; -} & Pick; +} & Pick< + AutocompleteProps, + 'w' | 'size' | 'autoFocus' | 'mb' | 'error' | 'onBlur' +>; /** Suggestion input for filter keys and values that also accepts free text. */ export function FilterAutocomplete({ @@ -45,6 +93,7 @@ export function FilterAutocomplete({ {...props} size={size} data={options} + filter={rankedFilter} value={value} onChange={onChange} onOptionSubmit={onOptionSubmit} diff --git a/packages/app/src/components/FilterPill/FilterConditionEditor.tsx b/packages/app/src/components/FilterPill/FilterConditionEditor.tsx index fbee6097c6..6d3751a525 100644 --- a/packages/app/src/components/FilterPill/FilterConditionEditor.tsx +++ b/packages/app/src/components/FilterPill/FilterConditionEditor.tsx @@ -27,11 +27,19 @@ export type FilterConditionEditorProps = { keyOptions: string[]; isLoadingKeys?: boolean; isKeysError?: boolean; + /** Why the key suggestions failed, shown after the generic error. */ + keysErrorMessage?: string; /** Suggestions for the key last reported by `onKeyChange`. */ valueOptions: string[]; isLoadingValues?: boolean; isValuesError?: boolean; - /** The trimmed key, once typing pauses, so the caller can fetch its values. */ + /** Why the value suggestions failed, shown after the generic error. */ + valuesErrorMessage?: string; + /** + * The key to fetch values for: a suggested key once typing pauses, or any key + * once committed (Enter, picking it, or leaving the input). Empty while + * another key is being typed. + */ onKeyChange?: (key: string) => void; keyLabel?: string; keyPlaceholder?: string; @@ -42,6 +50,12 @@ export type FilterConditionEditorProps = { const KEY_DEBOUNCE_MS = 300; const SUGGESTIONS_ERROR = "Couldn't load suggestions"; +// Database errors can run to several lines; the first says what went wrong. +const suggestionsError = (message: string | undefined) => { + const firstLine = message?.trim().split('\n')[0]; + return firstLine ? `${SUGGESTIONS_ERROR}: ${firstLine}` : SUGGESTIONS_ERROR; +}; + /** Pick a key, an operator, and a value. Free text is accepted for both inputs. */ export function FilterConditionEditor({ operators, @@ -49,9 +63,11 @@ export function FilterConditionEditor({ keyOptions, isLoadingKeys, isKeysError, + keysErrorMessage, valueOptions, isLoadingValues, isValuesError, + valuesErrorMessage, onKeyChange, keyLabel = 'Key', keyPlaceholder = 'Select a key', @@ -67,14 +83,20 @@ export function FilterConditionEditor({ const trimmedKey = key.trim(); const [debouncedKey] = useDebouncedValue(trimmedKey, KEY_DEBOUNCE_MS); + const [committedKey, setCommittedKey] = useState(trimmedKey); + const commitKey = (next: string) => setCommittedKey(next.trim()); + // A partial key names no column, so looking it up would only fail. + const lookupKey = + trimmedKey === committedKey || + (trimmedKey === debouncedKey && keyOptions.includes(trimmedKey)) + ? trimmedKey + : ''; useEffect(() => { - onKeyChange?.(debouncedKey); - }, [debouncedKey, onKeyChange]); + onKeyChange?.(lookupKey); + }, [lookupKey, onKeyChange]); - // Values shown while the key is still settling belong to the previous key. - const isValueLoading = - !!trimmedKey && (!!isLoadingValues || debouncedKey !== trimmedKey); - const isValuesErrorShown = !!trimmedKey && !isValueLoading && !!isValuesError; + const isValueLoading = !!lookupKey && !!isLoadingValues; + const isValuesErrorShown = !!lookupKey && !isValueLoading && !!isValuesError; const selectedOperator = operators.find(op => op.value === operator); const canSubmit = !!trimmedKey && operator != null; @@ -90,11 +112,20 @@ export function FilterConditionEditor({ placeholder={keyPlaceholder} options={keyOptions} isLoading={isLoadingKeys} - error={!isLoadingKeys && isKeysError ? SUGGESTIONS_ERROR : undefined} + error={ + !isLoadingKeys && isKeysError + ? suggestionsError(keysErrorMessage) + : undefined + } value={key} onChange={setKey} + onOptionSubmit={commitKey} + onBlur={() => commitKey(key)} // Enter moves on to the value instead of applying an empty-value filter. - onSubmit={() => valueInputRef.current?.focus()} + onSubmit={() => { + commitKey(key); + valueInputRef.current?.focus(); + }} data-testid={`${testId}-key`} /> @@ -112,7 +143,9 @@ export function FilterConditionEditor({ placeholder={selectedOperator?.valuePlaceholder ?? 'Select a value'} options={valueOptions} isLoading={isValueLoading} - error={isValuesErrorShown ? SUGGESTIONS_ERROR : undefined} + error={ + isValuesErrorShown ? suggestionsError(valuesErrorMessage) : undefined + } value={value} onChange={setValue} onSubmit={submit} diff --git a/packages/app/src/components/FilterPill/__tests__/FilterConditionEditor.test.tsx b/packages/app/src/components/FilterPill/__tests__/FilterConditionEditor.test.tsx index ce82a1e90b..19e9740bbf 100644 --- a/packages/app/src/components/FilterPill/__tests__/FilterConditionEditor.test.tsx +++ b/packages/app/src/components/FilterPill/__tests__/FilterConditionEditor.test.tsx @@ -1,4 +1,4 @@ -import { act, screen } from '@testing-library/react'; +import { act, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { @@ -43,14 +43,14 @@ describe('FilterConditionEditor', () => { ).toBeInTheDocument(); }); - it('shows a loading indicator on the value input once a key is entered', async () => { - renderEditor({ isLoadingValues: true }); + it("shows a loading indicator on the value input while a key's values load", async () => { + renderEditor({ isLoadingValues: true, keyOptions: ['ServiceName'] }); expect( screen.queryByTestId('filter-condition-editor-value-loading'), ).toBeNull(); await userEvent.type(keyInput(), 'ServiceName'); expect( - screen.getByTestId('filter-condition-editor-value-loading'), + await screen.findByTestId('filter-condition-editor-value-loading'), ).toBeInTheDocument(); }); @@ -59,8 +59,21 @@ describe('FilterConditionEditor', () => { expect(screen.getByText("Couldn't load suggestions")).toBeInTheDocument(); }); + it('says why keys failed to load, using the first line of the error', () => { + renderEditor({ + isKeysError: true, + keysErrorMessage: + "Primary key (Namespace) is not used and setting 'force_primary_key' is set\nStack trace: ...", + }); + expect( + screen.getByText( + "Couldn't load suggestions: Primary key (Namespace) is not used and setting 'force_primary_key' is set", + ), + ).toBeInTheDocument(); + }); + it('shows a value error only once the key settles', async () => { - renderEditor({ isValuesError: true }); + renderEditor({ isValuesError: true, keyOptions: ['ServiceName'] }); expect(screen.queryByText("Couldn't load suggestions")).toBeNull(); await userEvent.type(keyInput(), 'ServiceName'); expect( @@ -80,6 +93,52 @@ describe('FilterConditionEditor', () => { }); }); + describe('ranking key suggestions', () => { + /** The key input's suggestions, in order, once `typed` is entered. */ + const typedOptions = async (typed: string) => { + await userEvent.type(keyInput(), typed); + const listbox = document.getElementById( + keyInput().getAttribute('aria-controls') ?? '', + ); + const options = await within(listbox!).findAllByRole('option', { + hidden: true, + }); + return options.map(option => option.textContent); + }; + + it('lists keys starting with the text first, then keys with a part starting with it', async () => { + renderEditor({ + keyOptions: [ + "LogAttributes['agentToServer.status']", + "ResourceAttributes['service.name']", + 'ServiceName', + ], + }); + + expect(await typedOptions('Serv')).toEqual([ + 'ServiceName', + "ResourceAttributes['service.name']", + "LogAttributes['agentToServer.status']", + ]); + }); + + it('ranks every match before cutting the list down', async () => { + renderEditor({ + keyOptions: [ + ...Array.from( + { length: 250 }, + (_, i) => `LogAttributes['agentToServer.${i}']`, + ), + 'ServiceName', + ], + }); + + const options = await typedOptions('Serv'); + expect(options[0]).toBe('ServiceName'); + expect(options).toHaveLength(200); + }); + }); + it('moves focus to the value input on Enter in the key input', async () => { const onSubmit = renderEditor(); await userEvent.type(keyInput(), 'ServiceName{enter}'); @@ -147,20 +206,61 @@ describe('FilterConditionEditor', () => { expect(applyButton()).toBeDisabled(); }); - it('reports the key only once it stops changing', async () => { - jest.useFakeTimers(); - try { + describe('reporting the key to look up values for', () => { + beforeEach(() => jest.useFakeTimers()); + afterEach(() => jest.useRealTimers()); + + const setup = ( + props: Partial> = {}, + ) => { const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); const onKeyChange = jest.fn(); - renderEditor({ onKeyChange }); - await user.type(keyInput(), 'Svc'); - expect(onKeyChange).not.toHaveBeenCalledWith('Sv'); - act(() => { - jest.advanceTimersByTime(300); + renderEditor({ keyOptions: ['ServiceName'], onKeyChange, ...props }); + const pause = () => + act(() => { + jest.advanceTimersByTime(300); + }); + return { user, onKeyChange, pause }; + }; + + it('reports a suggested key once typing pauses, never a partial one', async () => { + const { user, onKeyChange, pause } = setup(); + + await user.type(keyInput(), 'Service'); + pause(); + expect(onKeyChange).not.toHaveBeenCalledWith('Service'); + + await user.type(keyInput(), 'Name'); + pause(); + expect(onKeyChange).toHaveBeenLastCalledWith('ServiceName'); + }); + + it('reports a key typed by hand once Enter commits it', async () => { + const { user, onKeyChange, pause } = setup(); + + await user.type(keyInput(), 'Custom'); + pause(); + expect(onKeyChange).not.toHaveBeenCalledWith('Custom'); + + await user.keyboard('{Enter}'); + expect(onKeyChange).toHaveBeenLastCalledWith('Custom'); + }); + + it('reports a key typed by hand once focus leaves the key input', async () => { + const { user, onKeyChange } = setup(); + + await user.type(keyInput(), 'Custom'); + await user.click(valueInput()); + + expect(onKeyChange).toHaveBeenLastCalledWith('Custom'); + }); + + it('reports the key being edited right away', () => { + const { onKeyChange } = setup({ + initial: { key: 'Custom', operator: '=', value: '' }, }); - expect(onKeyChange).toHaveBeenLastCalledWith('Svc'); - } finally { - jest.useRealTimers(); - } + + expect(onKeyChange).toHaveBeenLastCalledWith('Custom'); + }); }); }); diff --git a/packages/app/src/hooks/useMetadata.tsx b/packages/app/src/hooks/useMetadata.tsx index 7ca8d72364..7f5dec5961 100644 --- a/packages/app/src/hooks/useMetadata.tsx +++ b/packages/app/src/hooks/useMetadata.tsx @@ -8,6 +8,7 @@ import { } from '@hyperdx/common-utils/dist/clickhouse'; import { Field, + FieldsCondition, MetricNames, parseKeyPath, TableConnection, @@ -248,6 +249,7 @@ type MultipleAllFieldsOptions = { // would offer fields that exist in one table but not another. intersect?: boolean; enabled?: boolean; + conditions?: (FieldsCondition | undefined)[]; }; /** @@ -270,6 +272,7 @@ export function useMultipleAllFields( timestampValueExpression, intersect, enabled = true, + conditions, }: MultipleAllFieldsOptions = {}, ) { const metadata = useMetadataWithSettings(); @@ -286,10 +289,11 @@ export function useMultipleAllFields( dateRange ? [dateRange[0].getTime(), dateRange[1].getTime()] : undefined, timestampValueExpression, isFieldMetadataDisabled, + conditions, ], queryFn: () => Promise.all( - tableConnections.map(async (tc): Promise => { + tableConnections.map(async (tc, i): Promise => { let columns: ColumnMeta[] = []; try { columns = await metadata.getColumns({ @@ -304,6 +308,7 @@ export function useMultipleAllFields( dateRange, timestampValueExpression: timestampValueExpression ?? tc.timestampValueExpression, + condition: conditions?.[i], }); return { fields, columns }; } catch (error) { diff --git a/packages/app/tests/e2e/features/dashboard-adhoc-filters.spec.ts b/packages/app/tests/e2e/features/dashboard-adhoc-filters.spec.ts index 5ecf45c4f9..c2412e95ef 100644 --- a/packages/app/tests/e2e/features/dashboard-adhoc-filters.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-adhoc-filters.spec.ts @@ -66,7 +66,10 @@ test.describe( await expect(dashboardPage.variableNameInput).toBeVisible(); await expect(form.getByTestId('source-selector')).toHaveCount(0); - await expect(form.locator('div.cm-editor')).toHaveCount(0); + await expect(form.getByText('Filter expression')).toHaveCount(0); + await expect( + form.getByText('Keys and values filter', { exact: true }), + ).toBeVisible(); await expect( form.getByTestId('filter-variable-enabled-checkbox'), ).toHaveCount(0); @@ -220,6 +223,10 @@ test.describe( await expect( form.getByText(DEFAULT_LOGS_SOURCE_NAME, { exact: true }), ).toHaveCount(0); + // Only SQL suggestions can be narrowed by a condition. + await expect( + form.getByText('Keys and values filter', { exact: true }), + ).toHaveCount(0); }); await test.step('Only PromQL sources are offered', async () => { @@ -437,6 +444,75 @@ test.describe( }); }); + /** Open the editor, pick `key`, and wait for `value` to be suggested. */ + const openValueSuggestions = async ( + filterName: string, + key: string, + value: string, + ) => { + await dashboardPage.openAddAdhocCondition(filterName); + await dashboardPage.getAdhocConditionInput(filterName, 'key').fill(key); + await dashboardPage.getAdhocConditionInput(filterName, 'value').click(); + await expect( + dashboardPage.getAdhocConditionOption(filterName, value), + ).toBeVisible({ timeout: 30000 }); + }; + + test('suggests only from the rows its keys and values filter matches', async () => { + const filterName = 'Accounting'; + + await createDashboardWithAdhocFilter( + filterName, + [DEFAULT_LOGS_SOURCE_NAME], + { where: { value: "ServiceName = 'accounting'", language: 'sql' } }, + ); + + await openValueSuggestions(filterName, 'ServiceName', 'accounting'); + expect(await dashboardPage.getOpenFilterDropdownOptions()).toEqual([ + 'accounting', + ]); + }); + + test('waits for a required filter its keys and values filter references', async ({ + page, + }) => { + const filterName = 'Conditions'; + const addButton = page.getByTestId(`adhoc-filter-add-${filterName}`); + + await test.step('Narrow an ad hoc filter by a required filter', async () => { + await dashboardPage.createNewDashboard(); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addStaticListFilterToDashboard( + 'Service', + ['accounting', 'ad'], + { variableName: 'svc', required: true }, + ); + await dashboardPage.addAdhocFilterToDashboard( + filterName, + [DEFAULT_LOGS_SOURCE_NAME], + { where: { value: 'ServiceName IN ($svc)', language: 'sql' } }, + ); + await dashboardPage.closeFiltersModal(); + }); + + await test.step('Adding a condition waits for a service', async () => { + await expect(addButton).toHaveAttribute('aria-disabled', 'true'); + await addButton.hover(); + await expect( + page.getByText('This filter depends on: Service'), + ).toBeVisible(); + }); + + await test.step('A selected service unblocks it and narrows its suggestions', async () => { + await dashboardPage.toggleFilterValue('Service', 'ad'); + await expect(addButton).not.toHaveAttribute('aria-disabled', 'true'); + await openValueSuggestions(filterName, 'ServiceName', 'ad'); + expect(await dashboardPage.getOpenFilterDropdownOptions()).toEqual([ + 'ad', + ]); + }); + }); + test('picks conditions from the labels of a PromQL source', async () => { const filterName = 'Labels'; diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index c4a0035379..fcc829a790 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -1058,8 +1058,9 @@ export class DashboardPage { } /** - * Fill the "Dropdown values filter" (the filter's `where`) in the open filter - * edit form, replacing whatever is there. + * Fill the "Dropdown values filter" (the filter's `where`, an ad hoc filter's + * "Keys and values filter") in the open filter edit form, replacing whatever + * is there. * * The language is always set explicitly: the form seeds it from the * `hdx-search-where-language` localStorage key, which other specs write, so @@ -1599,6 +1600,7 @@ export class DashboardPage { options?: { variableName?: string; sourceType?: 'ClickHouse' | 'Prometheus'; + where?: FilterWhereOptions; }, ) { await this.addFiltersButton.click(); @@ -1612,6 +1614,9 @@ export class DashboardPage { this.getAdhocFilterSourcesInput(), sourceNames, ); + if (options?.where) { + await this.fillFilterDropdownValuesWhere(options.where); + } if (options?.variableName !== undefined) { await this.variableNameInput.fill(options.variableName); } diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 5cfe4683cd..2df1a18c17 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -796,6 +796,30 @@ describe('filters', () => { ]); }); + it("flags a malformed where clause on an ad hoc filter's suggestions", () => { + expect( + validateDashboardFilterQueries([ + { + id: 'conds', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs'], + isVariableEnabled: true, + where: 'ServiceName = =', + whereLanguage: 'sql', + }, + ]), + ).toEqual([ + { + filterId: 'conds', + filterName: 'Conditions', + language: 'sql', + where: 'ServiceName = =', + }, + ]); + }); + it('only reports the invalid filters in a mixed list', () => { const issues = validateDashboardFilterQueries([ filter({ id: 'a', where: 'ServiceName:*', whereLanguage: 'lucene' }), diff --git a/packages/common-utils/src/__tests__/metadata.int.test.ts b/packages/common-utils/src/__tests__/metadata.int.test.ts index e27a2ccc0e..55853a7421 100644 --- a/packages/common-utils/src/__tests__/metadata.int.test.ts +++ b/packages/common-utils/src/__tests__/metadata.int.test.ts @@ -3,7 +3,7 @@ import { ClickHouseClient } from '@clickhouse/client'; import { ClickhouseClient as HdxClickhouseClient } from '@/clickhouse/node'; import { supportsMergeTreeTextIndex } from '@/core/clickhouseVersion'; -import { Metadata, MetadataCache } from '@/core/metadata'; +import { Field, Metadata, MetadataCache } from '@/core/metadata'; import { parseKvItemsCastExpression, parseKvItemsExpression, @@ -1453,6 +1453,124 @@ describe('Metadata Integration Tests', () => { }); }); + // Shaped like LogHouse: a two-column timestamp, a date partition, a sort key + // led by the namespace, and settings that reject scans which skip either. + describe('getAllFields with a condition', () => { + const tableName = 'test_all_fields_condition'; + const tableConnection = { + databaseName: 'default', + tableName, + connectionId: 'test_connection', + timestampValueExpression: 'EventTime, EventDate', + }; + const nsACondition = { + where: "Namespace = 'ns-a'", + whereLanguage: 'sql' as const, + }; + const lastTwoHours = (): [Date, Date] => [ + new Date(Date.now() - 2 * 60 * 60 * 1000), + new Date(), + ]; + const mapKeys = (fields: Field[]) => + fields + .filter(({ path }) => path.length === 2 && path[0] === 'LogAttributes') + .map(({ path }) => path[1]); + + beforeAll(async () => { + await client.command({ + query: `DROP TABLE IF EXISTS default.${tableName}`, + }); + await client.command({ + query: `CREATE TABLE default.${tableName} ( + EventDate Date, + EventTime DateTime, + Namespace LowCardinality(String), + ServiceName LowCardinality(String), + LogAttributes Map(String, String) + ) + ENGINE = MergeTree() + PARTITION BY EventDate + ORDER BY (Namespace, ServiceName) + `, + }); + + const eventTime = new Date(Date.now() - 60 * 60 * 1000) + .toISOString() + .slice(0, 19) + .replace('T', ' '); + const row = { EventDate: eventTime.slice(0, 10), EventTime: eventTime }; + await client.insert({ + table: `default.${tableName}`, + values: [ + { + ...row, + Namespace: 'ns-a', + ServiceName: 'api', + LogAttributes: { 'a.key': '1' }, + }, + { + ...row, + Namespace: 'ns-b', + ServiceName: 'api', + LogAttributes: { 'b.key': '2' }, + }, + ], + format: 'JSONEachRow', + }); + }); + + afterAll(async () => { + await client.command({ + query: `DROP TABLE IF EXISTS default.${tableName}`, + }); + }); + + it('reads Map keys only from rows matching a SQL condition', async () => { + const metadata = new Metadata(hdxClient, new MetadataCache()); + + const fields = await metadata.getAllFields({ + ...tableConnection, + dateRange: lastTwoHours(), + condition: nsACondition, + }); + + expect(mapKeys(fields)).toEqual(['a.key']); + }); + + it('reads Map keys only from rows matching a Lucene condition', async () => { + const metadata = new Metadata(hdxClient, new MetadataCache()); + + const fields = await metadata.getAllFields({ + ...tableConnection, + dateRange: lastTwoHours(), + condition: { where: 'Namespace:"ns-b"', whereLanguage: 'lucene' }, + }); + + expect(mapKeys(fields)).toEqual(['b.key']); + }); + + it('lets a condition on the sort key pass force_primary_key, which rejects the unscoped scan', async () => { + const metadata = new Metadata(hdxClient, new MetadataCache(), { + force_primary_key: 1, + force_index_by_date: 1, + }); + + await expect( + metadata.getAllFields({ + ...tableConnection, + dateRange: lastTwoHours(), + }), + ).rejects.toThrow(/INDEX_NOT_USED|force_primary_key/); + + const fields = await metadata.getAllFields({ + ...tableConnection, + dateRange: lastTwoHours(), + condition: nsACondition, + }); + expect(mapKeys(fields)).toEqual(['a.key']); + }); + }); + describe.each([ { path: 'key', diff --git a/packages/common-utils/src/__tests__/metadata.test.ts b/packages/common-utils/src/__tests__/metadata.test.ts index dab2349b0b..ffdf8e791a 100644 --- a/packages/common-utils/src/__tests__/metadata.test.ts +++ b/packages/common-utils/src/__tests__/metadata.test.ts @@ -9,7 +9,10 @@ import { quoteResultColumnNameIfNeeded, } from '@/core/metadata'; import * as renderChartConfigModule from '@/core/renderChartConfig'; -import { timeFilterExpr } from '@/core/renderChartConfig'; +import { + renderWhereExpressionStr, + timeFilterExpr, +} from '@/core/renderChartConfig'; import { isBuilderChartConfig } from '@/guards'; import { TextIndexInfoLookup } from '@/queryParser'; import { BuilderChartConfigWithDateRange, SourceKind, TSource } from '@/types'; @@ -29,6 +32,7 @@ jest.mock('../core/renderChartConfig', () => ({ renderChartConfig: jest .fn() .mockResolvedValue({ sql: 'SELECT 1', params: {} }), + renderWhereExpressionStr: jest.fn().mockResolvedValue('__CONDITION__'), timeFilterExpr: jest .fn() .mockResolvedValue({ sql: '__TIME_FILTER__', params: {} }), @@ -2876,6 +2880,72 @@ describe('Metadata', () => { expect(scanCall.query).toContain('__TIME_FILTER__'); }); + describe('with a condition', () => { + const conditionSql = "ServiceName = 'api'"; + const buildScopedMetadata = () => { + const md = buildMetadata(); + jest.spyOn(md, 'getMapColumnTextIndexes').mockResolvedValue( + new Map([ + [ + 'LogAttributes', + { + key: { + indexName: 'idx_log_attr_keys', + mapColumn: 'LogAttributes', + }, + }, + ], + ]), + ); + jest.spyOn(md, 'getColumn').mockResolvedValue({ + name: 'LogAttributes', + type: 'Map(String, String)', + } as ColumnMeta); + return md; + }; + + it('scans only matching rows, skipping the text index and the rollup', async () => { + const md = buildScopedMetadata(); + (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ + json: () => Promise.resolve({ data: [{ keysArr: ['scoped.key'] }] }), + }); + + const keys = await md.getMapKeys({ ...baseArgs, conditionSql }); + + expect(keys).toEqual(['scoped.key']); + expect(mockClickhouseClient.query).toHaveBeenCalledTimes(1); + const { query, query_params } = ( + mockClickhouseClient.query as jest.Mock + ).mock.calls[0][0]; + expect(query).not.toContain('mergeTreeTextIndex'); + expect(Object.values(query_params)).not.toContain( + 'otel_logs_key_rollup_15m', + ); + expect(query).toContain(`__TIME_FILTER__ AND (${conditionSql})`); + }); + + it('caches its keys apart from the unscoped keys', async () => { + const md = buildScopedMetadata(); + (mockClickhouseClient.query as jest.Mock) + .mockResolvedValueOnce({ + json: () => Promise.resolve({ data: [{ key: 'indexed.key' }] }), + }) + .mockResolvedValueOnce({ + json: () => + Promise.resolve({ data: [{ keysArr: ['scoped.key'] }] }), + }); + + const unscoped = await md.getMapKeys({ ...baseArgs }); + const scoped = await md.getMapKeys({ ...baseArgs, conditionSql }); + const scopedRepeat = await md.getMapKeys({ ...baseArgs, conditionSql }); + + expect(unscoped).toEqual(['indexed.key']); + expect(scoped).toEqual(['scoped.key']); + expect(scopedRepeat).toEqual(['scoped.key']); + expect(mockClickhouseClient.query).toHaveBeenCalledTimes(2); + }); + }); + it('falls back to an unbounded scan (with a warning) when timestampValueExpression is missing', async () => { const md = buildMetadata(); const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); @@ -3243,6 +3313,82 @@ describe('Metadata', () => { ); }); + describe('with a condition', () => { + const mapColumn = (name: string): ColumnMeta => ({ + name, + type: 'Map(LowCardinality(String), String)', + default_type: '', + default_expression: '', + comment: '', + codec_expression: '', + ttl_expression: '', + }); + const tableConnection = { + databaseName: 'otel', + tableName: 'otel_logs', + connectionId: 'conn-1', + }; + + const buildMetadata = () => { + const md = new Metadata(mockClickhouseClient, new MetadataCache()); + (mockClickhouseClient.query as jest.Mock).mockResolvedValue({ + json: () => + Promise.resolve({ + data: [ + mapColumn('LogAttributes'), + mapColumn('ResourceAttributes'), + ], + }), + }); + return { + md, + getMapKeysSpy: jest.spyOn(md, 'getMapKeys').mockResolvedValue([]), + }; + }; + + it('renders it once and scans every Map column with it', async () => { + const { md, getMapKeysSpy } = buildMetadata(); + + await md.getAllFields({ + ...tableConnection, + condition: { + where: 'api', + whereLanguage: 'lucene', + implicitColumnExpression: 'Body', + }, + }); + + expect(renderWhereExpressionStr).toHaveBeenCalledTimes(1); + expect(renderWhereExpressionStr).toHaveBeenCalledWith( + expect.objectContaining({ + condition: 'api', + language: 'lucene', + implicitColumnExpression: 'Body', + from: { databaseName: 'otel', tableName: 'otel_logs' }, + connectionId: 'conn-1', + }), + ); + expect(getMapKeysSpy).toHaveBeenCalledTimes(2); + for (const [args] of getMapKeysSpy.mock.calls) { + expect(args.conditionSql).toBe('__CONDITION__'); + } + }); + + it('scans every row when it is blank', async () => { + const { md, getMapKeysSpy } = buildMetadata(); + + await md.getAllFields({ + ...tableConnection, + condition: { where: ' ' }, + }); + + expect(renderWhereExpressionStr).not.toHaveBeenCalled(); + expect(getMapKeysSpy).toHaveBeenCalledWith( + expect.objectContaining({ conditionSql: undefined }), + ); + }); + }); + it('should extract LowCardinality(String) type for Map sub-fields when value type is LowCardinality', async () => { // Simulate: Map(LowCardinality(String), LowCardinality(String)) // This is the "working" case — sub-fields get type LowCardinality(String) diff --git a/packages/common-utils/src/__tests__/types.test.ts b/packages/common-utils/src/__tests__/types.test.ts index 3c0786dda5..166f004708 100644 --- a/packages/common-utils/src/__tests__/types.test.ts +++ b/packages/common-utils/src/__tests__/types.test.ts @@ -712,6 +712,36 @@ describe('DashboardFilterSchema query-expression variant', () => { }); }); +describe('DashboardFilterSchema ad hoc variant', () => { + const adhocFilter = { + id: 'f1', + type: 'ADHOC' as const, + name: 'Conditions', + sourceType: 'sql' as const, + sources: ['source-1'], + isVariableEnabled: true as const, + }; + + it('keeps the condition that narrows its suggestions', () => { + const filter = { + ...adhocFilter, + where: 'Namespace IN ($namespace)', + whereLanguage: 'sql' as const, + }; + expect(DashboardFilterSchema.parse(filter)).toEqual(filter); + }); + + it('rejects a PromQL condition language', () => { + expect( + DashboardFilterSchema.safeParse({ + ...adhocFilter, + where: 'up', + whereLanguage: 'promql', + }).success, + ).toBe(false); + }); +}); + // Preset dashboards are broadcast-only, so the preset schema extends the // query-expression variant rather than the union — a static filter is // unrepresentable rather than rejected by a refinement. diff --git a/packages/common-utils/src/core/metadata.ts b/packages/common-utils/src/core/metadata.ts index 7471296951..a9e4c7a3f4 100644 --- a/packages/common-utils/src/core/metadata.ts +++ b/packages/common-utils/src/core/metadata.ts @@ -17,7 +17,11 @@ import { streamToAsyncIterator, tableExpr, } from '@/clickhouse'; -import { renderChartConfig, timeFilterExpr } from '@/core/renderChartConfig'; +import { + renderChartConfig, + renderWhereExpressionStr, + timeFilterExpr, +} from '@/core/renderChartConfig'; import { FilterState, filterStateToPredicate, @@ -371,6 +375,8 @@ const FIELD_METADATA_LOOKBACK_MS = 24 * 60 * 60 * 1000; // restart. #3475 export const COLUMNS_CACHE_TTL_MS = 5 * 60 * 1000; +const CONDITION_KEYS_CACHE_TTL_MS = 5 * 60 * 1000; + // Hour-aligned so a range-keyed cache rotates hourly instead of every call. export function defaultFieldMetadataDateRange(): [Date, Date] { const now = new Date(); @@ -1031,6 +1037,7 @@ export class Metadata { metadataMVs, dateRange: callerDateRange, timestampValueExpression, + conditionSql, signal, }: { databaseName: string; @@ -1042,6 +1049,7 @@ export class Metadata { metadataMVs?: MetadataMaterializedViews; dateRange?: [Date, Date]; timestampValueExpression?: string; + conditionSql?: string; signal?: AbortSignal; }) { inlineNonNegativeInt(maxKeys, 'maxKeys'); @@ -1064,11 +1072,14 @@ export class Metadata { alignedDateRange && timestampValueExpression ? `${alignedDateRange[0].getTime()}.${alignedDateRange[1].getTime()}.${timestampValueExpression}` : ''; - const cacheKey = metricName + const unscopedCacheKey = metricName ? `${connectionId}.${databaseName}.${tableName}.${column}.${metricName}.${dateRangeCacheSuffix}.${maxKeys}.keys` : alignedDateRange ? `${connectionId}.${databaseName}.${tableName}.${column}.${alignedCacheSuffix}.${maxKeys}.keys` : `${connectionId}.${databaseName}.${tableName}.${column}.${dateRangeCacheSuffix}.${maxKeys}.keys`; + const cacheKey = conditionSql + ? `${unscopedCacheKey}.${objectHash(conditionSql)}.condition` + : unscopedCacheKey; const cachedKeys = this.cache.get(cacheKey); if (cachedKeys != null) { @@ -1097,6 +1108,7 @@ export class Metadata { let rowPolicyDenied = false; // Without timestampValueExpression, partsFilter can't bound this either. #3037 if ( + !conditionSql && textIndexInfo?.key?.indexName && canQueryMergeTreeTextIndex && timestampValueExpression @@ -1144,6 +1156,7 @@ export class Metadata { ); } } else if ( + !conditionSql && textIndexInfo?.kv?.indexName && canQueryMergeTreeTextIndex && timestampValueExpression @@ -1193,7 +1206,7 @@ export class Metadata { // Rollup path: query the key rollup table filtered by ColumnIdentifier and date range. // Rollups are separate tables the source's row policy doesn't cover, so // after a policy denial only the source-table scan below is safe. - if (metadataMVs && alignedDateRange && !rowPolicyDenied) { + if (!conditionSql && metadataMVs && alignedDateRange && !rowPolicyDenied) { // Own cache key: the shipped OTel rollups only index NativeColumn, so // Map columns come back empty here and must fall through to the bounded // scan below. Caching [] under cacheKey would block that fallback. @@ -1303,6 +1316,7 @@ export class Metadata { }), ] : []), + ...(conditionSql ? [chSql`(${{ UNSAFE_RAW_SQL: conditionSql }})`] : []), ]; const where = whereConditions.length > 0 @@ -1352,7 +1366,7 @@ export class Metadata { `; } - return this.cache.getOrFetch(cacheKey, async () => { + const fetchKeys = async () => { const keys = await this.clickhouseClient .query<'JSON'>({ query: sql.sql, @@ -1382,6 +1396,11 @@ export class Metadata { return output.filter(r => r); }); return keys; + }; + // A condition changes with variable selections; expire its entries so + // they don't pile up. + return this.cache.getOrFetch(cacheKey, fetchKeys, { + ttlMs: conditionSql ? CONDITION_KEYS_CACHE_TTL_MS : undefined, }); } @@ -1875,9 +1894,11 @@ export class Metadata { metadataMVs, dateRange, timestampValueExpression, + condition, }: TableConnection & { dateRange?: [Date, Date]; timestampValueExpression?: string; + condition?: FieldsCondition; }) { const fields: Field[] = []; const columns = await this.getColumns({ @@ -1899,6 +1920,21 @@ export class Metadata { const mapColumns = filterColumnMetaByType(columns, [JSDataType.Map, JSDataType.JSON]) ?? []; + const conditionSql = + condition?.where.trim() && mapColumns.length > 0 + ? await renderWhereExpressionStr({ + condition: condition.where, + language: condition.whereLanguage ?? 'sql', + metadata: this, + from: { databaseName, tableName }, + implicitColumnExpression: condition.implicitColumnExpression, + bodyExpression: condition.bodyExpression, + useTextIndexForImplicitColumn: + condition.useTextIndexForImplicitColumn, + connectionId, + }) + : undefined; + await Promise.all( mapColumns.map(async column => { if (convertCHDataTypeToJSType(column.type) === JSDataType.JSON) { @@ -1931,6 +1967,7 @@ export class Metadata { metadataMVs, dateRange, timestampValueExpression, + conditionSql, }); const match = column.type.match(/Map\(.+,\s*(.+)\)/); @@ -3315,6 +3352,16 @@ export type TableConnection = { timestampValueExpression?: string; }; +/** A SQL or Lucene condition on the rows a table's Map keys are read from. */ +export type FieldsCondition = Pick< + BuilderChartConfig, + | 'where' + | 'whereLanguage' + | 'implicitColumnExpression' + | 'bodyExpression' + | 'useTextIndexForImplicitColumn' +>; + export type TableConnectionChoice = | { tableConnection?: never; diff --git a/packages/common-utils/src/core/renderChartConfig.ts b/packages/common-utils/src/core/renderChartConfig.ts index f0c5af8fbd..78a35da6b7 100644 --- a/packages/common-utils/src/core/renderChartConfig.ts +++ b/packages/common-utils/src/core/renderChartConfig.ts @@ -1260,7 +1260,8 @@ function renderFrom({ ); } -async function renderWhereExpressionStr({ +/** Render a SQL or Lucene condition as the SQL a chart query on `from` would use. */ +export async function renderWhereExpressionStr({ condition, language, metadata, diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 78e3d09c66..71f1d9d4a4 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -778,7 +778,8 @@ export type DashboardFilterQueryIssue = { /** * Validate the `where` clause of each dashboard filter *definition* (the query - * that scopes which values populate the filter's dropdown). + * that scopes which values populate the filter's dropdown, or which keys and + * values an ad hoc filter suggests). * * Useful at import time, where no values query is actually run: a filter whose * `where` clause is malformed would otherwise only surface as a failed query @@ -803,7 +804,7 @@ export function validateDashboardFilterQueries( for (const filter of filters) { // Only ClickHouse-queried filters carry a values query to validate. - if (!isQueryExpressionFilter(filter)) continue; + if (!isQueryExpressionFilter(filter) && !isAdhocFilter(filter)) continue; const where = filter.where ?? ''; if (!where.trim()) continue; const language = filter.whereLanguage ?? 'sql'; diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index aa17728361..f8f9ffe5d1 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -2423,6 +2423,12 @@ export const AdhocDashboardFilterSchema = dashboardFilterBaseSchema sourceType: AdhocFilterSourceTypeSchema, /** IDs of the sources whose keys and values the filter offers, all of `sourceType`. */ sources: z.array(z.string().min(1)).min(1), + /** + * Narrows the rows keys and values are suggested from, as a queried filter's + * `where` narrows its dropdown. SQL sources only, and never broadcast. + */ + where: QueryExpressionDashboardFilterSchema.shape.where, + whereLanguage: QueryExpressionDashboardFilterSchema.shape.whereLanguage, /** * Sources the conditions are broadcast to, all of `sourceType`. Undefined / * empty means the filter's `sources`. From ecd4464e11707942a12d3ae129923d1cdd94cbfa Mon Sep 17 00:00:00 2001 From: Aaron Knudtson <87577305+knudtty@users.noreply.github.com> Date: Fri, 6 Nov 2026 13:41:28 -0500 Subject: [PATCH 2/2] pr suggestions --- .../AdhocDashboardFilter.tsx | 23 +++++++------- .../__tests__/AdhocDashboardFilter.test.tsx | 30 +++++++++++++++++++ .../src/__tests__/filters.test.ts | 17 +++++++++++ packages/common-utils/src/filters.ts | 4 ++- 4 files changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx b/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx index 93ac1a511b..8686035ab9 100644 --- a/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx +++ b/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx @@ -1,6 +1,6 @@ import { useMemo, useState } from 'react'; -import { getBlockingRequiredFilters } from '@hyperdx/common-utils/dist/dashboardFilterValues'; import { + getDashboardVariableFilters, getPendingFilterValuesVariables, isAdhocConditionSupported, resolveFilterValuesWhere, @@ -65,24 +65,23 @@ export function AdhocDashboardFilter({ const isAtLimit = conditions.length >= ADHOC_FILTER_MAX_CONDITIONS; // The `where` narrowing the suggestions, and what it is still waiting on. - // Blocked the way a tile is: by the required filters it references, or by - // one that blocks every tile. + // Blocked only by the required filters it references: a global requirement + // blocks tiles, not the filter bar, same as queried dropdown filters. const { suggestionsWhere, pendingVariables, blockingFilterNames } = useMemo(() => { const where = filter.sourceType === 'sql' ? filter.where : undefined; const query = { where, whereLanguage: filter.whereLanguage }; + const referencedNames = new Set( + getVariableReferences(where ?? '').map(reference => reference.name), + ); return { suggestionsWhere: resolveFilterValuesWhere(query, variables), pendingVariables: getPendingFilterValuesVariables(query, variables), - blockingFilterNames: getBlockingRequiredFilters( - unsatisfiedRequiredFilters ?? [], - { - referencedVariableNames: getVariableReferences(where ?? '').map( - reference => reference.name, - ), - consumesBroadcastFilters: false, - }, - ).map(blocking => blocking.name), + blockingFilterNames: getDashboardVariableFilters( + unsatisfiedRequiredFilters, + ) + .filter(({ name }) => referencedNames.has(name)) + .map(({ filter: blocking }) => blocking.name), }; }, [ filter.sourceType, diff --git a/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx b/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx index 748872a049..14e3f710f5 100644 --- a/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx +++ b/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx @@ -252,6 +252,36 @@ describe('AdhocDashboardFilter', () => { expect(screen.queryByTestId(`${editorId}-key`)).not.toBeInTheDocument(); }); + it.each([ + ['no condition', filter], + ['a condition that does not reference it', narrowed], + ['a PromQL source', { ...narrowed, sourceType: 'promql' as const }], + ])( + 'is not blocked by an unreferenced global required filter with %s', + async (_label, definition) => { + renderFilter([], definition, { + variables: namespace(['ns-a']), + unsatisfiedRequiredFilters: [ + { + ...namespaceFilter, + id: 'env', + name: 'Env', + variableName: 'env', + isGlobalRequirement: true, + }, + ], + }); + + expect( + screen.queryByTestId('dashboard-filter-blocked-Conditions'), + ).not.toBeInTheDocument(); + await userEvent.click(addButton()); + expect( + await screen.findByTestId(`${editorId}-key`), + ).toBeInTheDocument(); + }, + ); + it('ignores a condition on a PromQL filter', async () => { renderFilter( [], diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index 2df1a18c17..0f26d14929 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -820,6 +820,23 @@ describe('filters', () => { ]); }); + it('skips the unused where clause of a PromQL ad hoc filter', () => { + expect( + validateDashboardFilterQueries([ + { + id: 'conds', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'promql', + sources: ['metrics'], + isVariableEnabled: true, + where: 'ServiceName = =', + whereLanguage: 'sql', + }, + ]), + ).toEqual([]); + }); + it('only reports the invalid filters in a mixed list', () => { const issues = validateDashboardFilterQueries([ filter({ id: 'a', where: 'ServiceName:*', whereLanguage: 'lucene' }), diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 71f1d9d4a4..3412e16d8c 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -804,7 +804,9 @@ export function validateDashboardFilterQueries( for (const filter of filters) { // Only ClickHouse-queried filters carry a values query to validate. - if (!isQueryExpressionFilter(filter) && !isAdhocFilter(filter)) continue; + // A PromQL ad hoc filter may store a `where`, but nothing reads it. + const isSqlAdhoc = isAdhocFilter(filter) && filter.sourceType === 'sql'; + if (!isQueryExpressionFilter(filter) && !isSqlAdhoc) continue; const where = filter.where ?? ''; if (!where.trim()) continue; const language = filter.whereLanguage ?? 'sql';