From a627f8c4459dd1ae7465ad5e35eeb17e45bbc059 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Fri, 9 Oct 2026 11:48:38 -0400 Subject: [PATCH] feat: Render adhoc dashboard filter inputs --- .changeset/adhoc-filter-inputs.md | 6 + packages/app/src/DBDashboardPage.tsx | 13 +- packages/app/src/DashboardFilters.tsx | 78 ++-- .../src/__tests__/DashboardFilters.test.ts | 17 + .../AdhocConditionEditor.tsx | 56 +++ .../AdhocDashboardFilter.tsx | 185 ++++++++++ .../__tests__/AdhocDashboardFilter.test.tsx | 151 ++++++++ .../__tests__/useAdhocFilterOptions.test.tsx | 225 ++++++++++++ .../useAdhocFilterOptions.ts | 345 ++++++++++++++++++ .../components/DBSearchPageFilters/hooks.ts | 8 +- .../src/components/DashboardFilterLabel.tsx | 61 ++++ .../DashboardFiltersList.tsx | 9 - .../FilterPill/FilterAutocomplete.tsx | 2 +- .../FilterConditionEditor.stories.tsx | 13 + .../FilterPill/FilterConditionEditor.tsx | 8 + .../FilterPill/FilterPill.stories.tsx | 7 +- .../__tests__/FilterConditionEditor.test.tsx | 14 + .../components/FilterPill/filterOperators.ts | 9 + .../__tests__/useDashboardFilters.test.tsx | 153 ++++++++ .../usePresetDashboardFilters.test.tsx | 2 + .../app/src/hooks/useDashboardFilters.tsx | 75 +++- packages/app/src/hooks/useMetadata.tsx | 6 + .../src/hooks/usePromqlLabelFilterValues.tsx | 20 +- packages/app/src/hooks/usePromqlMetadata.ts | 40 +- .../features/dashboard-adhoc-filters.spec.ts | 214 ++++++++++- .../tests/e2e/page-objects/DashboardPage.ts | 88 ++++- packages/app/tests/e2e/utils/filters-param.ts | 12 +- .../__tests__/dashboardFilterValues.test.ts | 57 ++- .../src/__tests__/filters.test.ts | 23 +- .../src/clickhouse/__tests__/index.test.ts | 23 ++ packages/common-utils/src/clickhouse/index.ts | 15 + .../common-utils/src/dashboardFilterValues.ts | 39 +- packages/common-utils/src/filters.ts | 16 +- 33 files changed, 1852 insertions(+), 138 deletions(-) create mode 100644 .changeset/adhoc-filter-inputs.md create mode 100644 packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx create mode 100644 packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx create mode 100644 packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx create mode 100644 packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx create mode 100644 packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts create mode 100644 packages/app/src/components/DashboardFilterLabel.tsx diff --git a/.changeset/adhoc-filter-inputs.md b/.changeset/adhoc-filter-inputs.md new file mode 100644 index 0000000000..48162bfae0 --- /dev/null +++ b/.changeset/adhoc-filter-inputs.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/common-utils': patch +'@hyperdx/app': patch +--- + +feat: Render adhoc dashboard filter inputs diff --git a/packages/app/src/DBDashboardPage.tsx b/packages/app/src/DBDashboardPage.tsx index b7e102324b..cc6c0c873b 100644 --- a/packages/app/src/DBDashboardPage.tsx +++ b/packages/app/src/DBDashboardPage.tsx @@ -35,7 +35,6 @@ import { isTimeSeriesDisplayType, } from '@hyperdx/common-utils/dist/core/utils'; import { getBlockingRequiredFilterNames } from '@hyperdx/common-utils/dist/dashboardFilterValues'; -import { isAdhocFilter } from '@hyperdx/common-utils/dist/filters'; import { displayTypeRequiresSource, isBuilderChartConfig, @@ -1979,14 +1978,12 @@ function DBDashboardPage({ const [showFiltersModal, setShowFiltersModal] = useState(false); - // Ad hoc filters aren't rendered or applied yet - const filters = useMemo( - () => (dashboard?.filters ?? []).filter(filter => !isAdhocFilter(filter)), - [dashboard?.filters], - ); + const filters = useMemo(() => dashboard?.filters ?? [], [dashboard?.filters]); const { selectionByFilterId, setFilterValue, + adhocConditionsByFilterId, + setAdhocConditions, setFilterValueEntries, filterValueEntries, ignoredFilterExpressions, @@ -3351,6 +3348,8 @@ function DBDashboardPage({ filters={filters} selectionByFilterId={selectionByFilterId} onSetFilterValue={setFilterValue} + adhocConditionsByFilterId={adhocConditionsByFilterId} + onSetAdhocConditions={setAdhocConditions} dateRange={searchedTimeRange} variables={variables} /> @@ -3524,7 +3523,7 @@ function DBDashboardPage({ setShowFiltersModal(false)} - filters={dashboard?.filters ?? []} + filters={filters} onSaveFilter={handleSaveFilter} onRemoveFilter={handleRemoveFilter} isLoading={isSavingDashboard || isFetchingDashboard} diff --git a/packages/app/src/DashboardFilters.tsx b/packages/app/src/DashboardFilters.tsx index 9915d1154c..cf5f926230 100644 --- a/packages/app/src/DashboardFilters.tsx +++ b/packages/app/src/DashboardFilters.tsx @@ -4,6 +4,7 @@ import { getFilterBroadcastTarget, getFilterVariableName, getPendingFilterValuesVariables, + isAdhocFilter, isFilterGlobalRequirement, isFilterRequired, isFilterVariableEnabled, @@ -11,13 +12,20 @@ import { isStaticListFilter, } from '@hyperdx/common-utils/dist/filters'; import { + AdhocFilterCondition, ChartVariable, DashboardFilter, } from '@hyperdx/common-utils/dist/types'; import { isFilterSingleSelect } from '@hyperdx/common-utils/dist/variables'; -import { Group, Stack, Text, Tooltip } from '@mantine/core'; -import { IconAlertTriangle, IconHelp, IconRefresh } from '@tabler/icons-react'; +import { Group, Stack, Text } from '@mantine/core'; +import { IconRefresh } from '@tabler/icons-react'; +import { AdhocDashboardFilter } from './components/AdhocDashboardFilter/AdhocDashboardFilter'; +import { + DashboardFilterLabel, + FilterCaution, + FilterEffect, +} from './components/DashboardFilterLabel'; import { FilterLinkToggle } from './components/FilterLinkToggle'; import { VirtualMultiSelect } from './components/VirtualMultiSelect/VirtualMultiSelect'; import { useDashboardFilterValues } from './hooks/useDashboardFilterValues'; @@ -65,9 +73,7 @@ export const getRequiredFilterTooltip = (filter: DashboardFilter): string => * Describe what a filter does with the value you pick: broadcast it as a * condition, expose it as a variable, both, or neither. */ -export const getFilterEffect = ( - filter: DashboardFilter, -): { hasEffect: boolean; tooltip: string } => { +export const getFilterEffect = (filter: DashboardFilter): FilterEffect => { const parts: string[] = []; const broadcast = getFilterBroadcastTarget(filter); @@ -99,25 +105,6 @@ export const getFilterEffect = ( return { hasEffect: true, tooltip: parts.join(', ') }; }; -/** One of the caution icons a filter's label row can carry, with its tooltip. */ -const FilterCaution = ({ - label, - testId, - variant = 'warning', -}: { - label: string; - testId: string; - variant?: 'warning' | 'danger'; -}) => ( - - - -); - const DashboardFilterSelect = ({ filter, onChange, @@ -134,25 +121,7 @@ const DashboardFilterSelect = ({ return ( - - - {filter.name} - - - {effect.hasEffect ? ( - - ) : ( - - )} - + {isMissingRequiredValue && ( )} - +
; onSetFilterValue: (filterId: string, values: string[]) => void; + adhocConditionsByFilterId?: ReadonlyMap; + onSetAdhocConditions?: ( + filterId: string, + conditions: AdhocFilterCondition[], + ) => void; dateRange: [Date, Date]; /** * The dashboard's variables and their current selections. Defined only when @@ -217,6 +191,8 @@ const DashboardFilters = ({ dateRange, selectionByFilterId, onSetFilterValue, + adhocConditionsByFilterId, + onSetAdhocConditions, variables, }: DashboardFilterProps) => { // "Link" mode (opt-in, off by default): each dropdown's values are narrowed by @@ -248,6 +224,20 @@ const DashboardFilters = ({ return ( {filters.map(filter => { + if (isAdhocFilter(filter)) { + return ( + + onSetAdhocConditions?.(filter.id, conditions) + } + dateRange={dateRange} + effect={getFilterEffect(filter)} + /> + ); + } const queriedFilterValues = filterValuesById?.get(filter.id); const included = selectionByFilterId.get(filter.id)?.included; const selectedValues = included diff --git a/packages/app/src/__tests__/DashboardFilters.test.ts b/packages/app/src/__tests__/DashboardFilters.test.ts index d063d63a9c..5c5df455ab 100644 --- a/packages/app/src/__tests__/DashboardFilters.test.ts +++ b/packages/app/src/__tests__/DashboardFilters.test.ts @@ -117,6 +117,23 @@ describe('getFilterEffect', () => { }).tooltip, ).toEqual('Available as variable ($svc)'); }); + + it('counts the sources an ad hoc filter broadcasts to', () => { + expect( + getFilterEffect({ + id: 'adhoc1', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs', 'traces'], + isVariableEnabled: true, + variableName: 'conds', + }), + ).toEqual({ + hasEffect: true, + tooltip: 'Filters 2 sources, available as variable ($conds)', + }); + }); }); describe('getPendingVariablesTooltip', () => { diff --git a/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx b/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx new file mode 100644 index 0000000000..51135a5549 --- /dev/null +++ b/packages/app/src/components/AdhocDashboardFilter/AdhocConditionEditor.tsx @@ -0,0 +1,56 @@ +import { useState } from 'react'; +import { + AdhocDashboardFilter, + AdhocFilterCondition, +} from '@hyperdx/common-utils/dist/types'; + +import { + FilterConditionEditor, + FilterOperator, + getFilterOperators, +} from '@/components/FilterPill'; + +import { useAdhocFilterOptions } from './useAdhocFilterOptions'; + +type AdhocConditionEditorProps = { + filter: AdhocDashboardFilter; + dateRange: [Date, Date]; + /** The condition being edited; unset when adding one. */ + initial?: AdhocFilterCondition; + onSubmit: (condition: AdhocFilterCondition) => void; +}; + +/** + * Pick a condition from the filter's keys and values. Mounted only while its + * popover is open, so keys load on open and values once a key is picked. + */ +export function AdhocConditionEditor({ + filter, + dateRange, + initial, + onSubmit, +}: AdhocConditionEditorProps) { + const [valueKey, setValueKey] = useState(initial?.key ?? ''); + const { keys, values } = useAdhocFilterOptions(filter, valueKey, dateRange); + const isPromql = filter.sourceType === 'promql'; + + return ( + + operators={getFilterOperators(filter.sourceType)} + initial={initial} + keyOptions={keys.data} + isLoadingKeys={keys.isLoading} + isKeysError={keys.isError} + valueOptions={values.data} + isLoadingValues={values.isLoading} + isValuesError={values.isError} + onKeyChange={setValueKey} + keyLabel={isPromql ? 'Label' : 'Key'} + keyPlaceholder={ + isPromql ? 'Select a label' : 'Select a column or map key' + } + onSubmit={onSubmit} + data-testid={`adhoc-condition-editor-${filter.name}`} + /> + ); +} diff --git a/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx b/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx new file mode 100644 index 0000000000..a6c2bae9ba --- /dev/null +++ b/packages/app/src/components/AdhocDashboardFilter/AdhocDashboardFilter.tsx @@ -0,0 +1,185 @@ +import { useState } from 'react'; +import { + ADHOC_FILTER_MAX_CONDITIONS, + AdhocDashboardFilter as AdhocDashboardFilterType, + AdhocFilterCondition, + AdhocFilterConditionSchema, +} from '@hyperdx/common-utils/dist/types'; +import { + ActionIcon, + CloseButton, + Group, + Popover, + Stack, + Text, + Tooltip, +} from '@mantine/core'; +import { notifications } from '@mantine/notifications'; +import { IconPlus } from '@tabler/icons-react'; + +import { + DashboardFilterLabel, + FilterEffect, +} from '@/components/DashboardFilterLabel'; +import { FilterPill, getFilterOperatorDisplay } from '@/components/FilterPill'; + +import { AdhocConditionEditor } from './AdhocConditionEditor'; + +type AdhocDashboardFilterProps = { + filter: AdhocDashboardFilterType; + conditions: AdhocFilterCondition[]; + onChange: (conditions: AdhocFilterCondition[]) => void; + dateRange: [Date, Date]; + effect: FilterEffect; +}; + +/** A dashboard filter bar control holding one ad hoc filter's conditions. */ +export function AdhocDashboardFilter({ + filter, + conditions, + onChange, + dateRange, + effect, +}: AdhocDashboardFilterProps) { + const [isAdding, setIsAdding] = useState(false); + const isAtLimit = conditions.length >= ADHOC_FILTER_MAX_CONDITIONS; + + const toggleAdding = () => { + if (isAtLimit) { + notifications.show({ + id: `adhoc-filter-full-${filter.id}`, + color: 'yellow', + title: `${filter.name} is full`, + message: `A filter holds up to ${ADHOC_FILTER_MAX_CONDITIONS} conditions. Remove one to add another.`, + }); + return; + } + setIsAdding(!isAdding); + }; + // The URL parser drops an invalid entry whole, so one bad condition would + // lose the filter's others on reload. Keep the editor open to fix it instead. + const isStorable = (condition: AdhocFilterCondition) => { + if (AdhocFilterConditionSchema.safeParse(condition).success) return true; + const maxLength = AdhocFilterConditionSchema.shape.value.maxLength; + notifications.show({ + id: `adhoc-filter-too-long-${filter.id}`, + color: 'yellow', + title: 'Condition is too long', + message: `Keys and values can be up to ${maxLength?.toLocaleString()} characters.`, + }); + return false; + }; + const replace = (index: number, condition: AdhocFilterCondition) => + onChange( + conditions.map((existing, i) => (i === index ? condition : existing)), + ); + const remove = (index: number) => + onChange(conditions.filter((_existing, i) => i !== index)); + + return ( + + + + {conditions.map((condition, index) => { + const operator = getFilterOperatorDisplay( + filter.sourceType, + condition.operator, + ); + return ( + remove(index)} + renderPopover={close => ( + { + if (!isStorable(next)) return; + replace(index, next); + close(); + }} + /> + )} + data-testid="adhoc-condition-pill" + /> + ); + })} + {conditions.length === 0 && ( + setIsAdding(true)} + style={{ cursor: 'pointer', flex: 1 }} + > + {filter.name} + + )} + + {conditions.length > 0 && ( + onChange([])} + data-testid={`adhoc-filter-clear-${filter.name}`} + /> + )} + + + + + + + + + + { + if (!isStorable(condition)) return; + onChange([...conditions, condition]); + setIsAdding(false); + }} + /> + + + + + + ); +} diff --git a/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx b/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx new file mode 100644 index 0000000000..bae4b1d5f0 --- /dev/null +++ b/packages/app/src/components/AdhocDashboardFilter/__tests__/AdhocDashboardFilter.test.tsx @@ -0,0 +1,151 @@ +import { + ADHOC_FILTER_MAX_CONDITIONS, + AdhocDashboardFilter as AdhocDashboardFilterType, + AdhocFilterCondition, +} from '@hyperdx/common-utils/dist/types'; +import { notifications } from '@mantine/notifications'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import { AdhocDashboardFilter } from '@/components/AdhocDashboardFilter/AdhocDashboardFilter'; +import { useAdhocFilterOptions } from '@/components/AdhocDashboardFilter/useAdhocFilterOptions'; + +jest.mock('@mantine/notifications', () => ({ + notifications: { show: jest.fn() }, +})); + +jest.mock('@/components/AdhocDashboardFilter/useAdhocFilterOptions', () => ({ + useAdhocFilterOptions: jest.fn(), +})); + +const filter: AdhocDashboardFilterType = { + id: 'adhoc1', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs'], + isVariableEnabled: true, + variableName: 'conds', +}; + +const dateRange: [Date, Date] = [new Date(0), new Date(1000)]; + +const existing: AdhocFilterCondition = { + key: 'ServiceName', + operator: '!=', + value: 'api', +}; + +const editorId = 'adhoc-condition-editor-Conditions'; + +const addButton = () => screen.getByTestId('adhoc-filter-add-Conditions'); + +const renderFilter = (conditions: AdhocFilterCondition[]) => { + const onChange = jest.fn(); + renderWithMantine( + , + ); + return onChange; +}; + +describe('AdhocDashboardFilter', () => { + beforeEach(() => { + jest.mocked(useAdhocFilterOptions).mockReturnValue({ + keys: { + data: ['ServiceName', 'SeverityText'], + isLoading: false, + isError: false, + }, + values: { data: ['api', 'web'], isLoading: false, isError: false }, + }); + }); + + it('renders a pill per condition', () => { + renderFilter([existing, { key: 'Level', operator: '=~', value: '^err' }]); + + const pills = screen.getAllByTestId('adhoc-condition-pill'); + expect(pills[0]).toHaveTextContent('ServiceName != api'); + expect(pills[1]).toHaveTextContent('Level regex ^err'); + }); + + it('removes a condition', async () => { + const onChange = renderFilter([ + existing, + { key: 'Level', operator: '=', value: 'error' }, + ]); + + await userEvent.click( + screen.getAllByRole('button', { name: 'Remove filter' })[0], + ); + + expect(onChange).toHaveBeenCalledWith([ + { key: 'Level', operator: '=', value: 'error' }, + ]); + }); + + it('stops adding conditions at the limit', async () => { + renderFilter( + Array.from({ length: ADHOC_FILTER_MAX_CONDITIONS }, (_, i) => ({ + ...existing, + value: `${i}`, + })), + ); + + expect(addButton()).toHaveAttribute('aria-disabled', 'true'); + await userEvent.click(addButton()); + expect(screen.queryByTestId(`${editorId}-key`)).not.toBeInTheDocument(); + expect(notifications.show).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Conditions is full' }), + ); + }); + + it('edits a condition from its pill', async () => { + const onChange = renderFilter([ + existing, + { key: 'Level', operator: '=', value: 'error' }, + ]); + + await userEvent.click(screen.getAllByTestId('adhoc-condition-pill')[0]); + const valueInput = await screen.findByTestId(`${editorId}-value`); + expect(valueInput).toHaveValue('api'); + await userEvent.clear(valueInput); + await userEvent.type(valueInput, 'web'); + await userEvent.click(screen.getByTestId(`${editorId}-apply`)); + + expect(onChange).toHaveBeenCalledWith([ + { key: 'ServiceName', operator: '!=', value: 'web' }, + { key: 'Level', operator: '=', value: 'error' }, + ]); + }); + + it('warns instead of applying a value too long for the URL', async () => { + const onChange = renderFilter([existing]); + + await userEvent.click(screen.getAllByTestId('adhoc-condition-pill')[0]); + const valueInput = await screen.findByTestId(`${editorId}-value`); + await userEvent.clear(valueInput); + await userEvent.click(valueInput); + await userEvent.paste('x'.repeat(10001)); + await userEvent.click(screen.getByTestId(`${editorId}-apply`)); + + expect(onChange).not.toHaveBeenCalled(); + expect(notifications.show).toHaveBeenCalledWith( + expect.objectContaining({ title: 'Condition is too long' }), + ); + expect(valueInput).toBeInTheDocument(); + }); + + it('clears every condition', async () => { + const onChange = renderFilter([existing]); + + await userEvent.click(screen.getByTestId('adhoc-filter-clear-Conditions')); + + expect(onChange).toHaveBeenCalledWith([]); + }); +}); diff --git a/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx b/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx new file mode 100644 index 0000000000..11dd946059 --- /dev/null +++ b/packages/app/src/components/AdhocDashboardFilter/__tests__/useAdhocFilterOptions.test.tsx @@ -0,0 +1,225 @@ +import { ReactNode } from 'react'; +import { ColumnMeta, JSDataType } from '@hyperdx/common-utils/dist/clickhouse'; +import { + AdhocDashboardFilter, + SourceKind, + TSource, +} from '@hyperdx/common-utils/dist/types'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { renderHook, waitFor } from '@testing-library/react'; + +import { + getSourcesWithKey, + getSqlSourceColumnKeys, + getSqlSourceKeys, + getValueSuggestions, + useAdhocFilterOptions, +} from '@/components/AdhocDashboardFilter/useAdhocFilterOptions'; +import { + useMultipleAllFields, + useMultipleGetKeyValues, +} from '@/hooks/useMetadata'; +import { + fetchPromqlLabelNames, + fetchPromqlLabelValues, +} from '@/hooks/usePromqlMetadata'; + +let mockSources: Partial[] = []; + +jest.mock('@/source', () => ({ + useSources: () => ({ data: mockSources }), +})); +jest.mock('@/queryAttribution', () => ({ + useMetadataQueryAttribution: () => undefined, +})); +jest.mock('@/hooks/useMetadata', () => ({ + useMultipleAllFields: jest.fn(), + useMultipleGetKeyValues: jest.fn(() => ({ + data: undefined, + isFetching: false, + isError: false, + })), +})); +jest.mock('@/hooks/usePromqlMetadata', () => ({ + ...jest.requireActual('@/hooks/usePromqlMetadata'), + fetchPromqlLabelNames: jest.fn(), + fetchPromqlLabelValues: jest.fn(), +})); + +const column = (name: string, type: string): ColumnMeta => ({ + name, + type, + codec_expression: '', + comment: '', + default_expression: '', + default_type: '', + ttl_expression: '', +}); + +describe('getSqlSourceKeys', () => { + it('offers scalar columns and the keys inside map and JSON columns, not the whole columns', () => { + expect( + getSqlSourceKeys( + [ + { path: ['ServiceName'], type: 'String', jsType: JSDataType.String }, + { path: ['LogAttributes'], type: 'Map', jsType: JSDataType.Map }, + { path: ['LogAttributes', '1'], type: 'String', jsType: null }, + { path: ['Body'], type: 'JSON', jsType: JSDataType.JSON }, + { path: ['Body', 'user.id'], type: 'String', jsType: null }, + ], + [ + column('ServiceName', 'LowCardinality(String)'), + column('LogAttributes', 'Map(LowCardinality(String), String)'), + column('Body', 'JSON'), + ], + ), + ).toEqual(['ServiceName', "LogAttributes['1']", 'Body.`user`.`id`']); + }); +}); + +describe('getSqlSourceColumnKeys', () => { + it('offers only the scalar columns', () => { + expect( + getSqlSourceColumnKeys([ + column('ServiceName', 'LowCardinality(String)'), + column('LogAttributes', 'Map(LowCardinality(String), String)'), + ]), + ).toEqual(['ServiceName']); + }); +}); + +describe('getValueSuggestions', () => { + it('deduplicates scalar values and leaves out map and JSON values', () => { + const mapValue = JSON.parse('{"service.name":"api"}'); + expect( + getValueSuggestions([ + { key: 'ResourceAttributes', value: [mapValue, 'b', 'a'] }, + { key: 'ResourceAttributes', value: ['a', JSON.parse('null')] }, + ]), + ).toEqual(['a', 'b']); + }); +}); + +describe('getSourcesWithKey', () => { + const sources = [{ id: 'logs' }, { id: 'traces' }]; + + it('looks up a key no source is known to have in every source', () => { + expect( + getSourcesWithKey(sources, new Map([['logs', ['ServiceName']]]), 'typed'), + ).toEqual(sources); + }); +}); + +describe('useAdhocFilterOptions', () => { + const DATE_RANGE: [Date, Date] = [ + new Date('2026-01-01T00:00:00Z'), + new Date('2026-01-01T01:00:00Z'), + ]; + const source = (id: string, kind: SourceKind): Partial => ({ + id, + kind, + connection: 'conn', + from: { databaseName: 'default', tableName: id }, + timestampValueExpression: 'Timestamp', + }); + const filter = ( + sourceType: AdhocDashboardFilter['sourceType'], + sources: string[], + ): AdhocDashboardFilter => ({ + id: 'f', + type: 'ADHOC', + name: 'Conditions', + sourceType, + sources, + isVariableEnabled: true, + }); + + /** `useMultipleAllFields` with the given columns per source, by index. */ + const mockAllFields = ( + columnsBySource: ColumnMeta[][], + { isLoading = false } = {}, + ) => + jest.mocked(useMultipleAllFields).mockReturnValue({ + fieldsByTableConnection: new Map(), + columnsByTableConnection: new Map(columnsBySource.entries()), + isFieldMetadataDisabled: true, + isLoading, + isError: false, + } as unknown as ReturnType); + + const renderOptions = ( + adhocFilter: AdhocDashboardFilter, + key = 'SeverityText', + ) => { + const queryClient = new QueryClient(); + const wrapper = ({ children }: { children: ReactNode }) => ( + {children} + ); + return renderHook( + () => useAdhocFilterOptions(adhocFilter, key, DATE_RANGE), + { wrapper }, + ); + }; + + beforeEach(() => { + mockAllFields([]); + jest.mocked(fetchPromqlLabelNames).mockResolvedValue([]); + jest.mocked(fetchPromqlLabelValues).mockResolvedValue([]); + }); + + const lastSqlLookup = () => + jest.mocked(useMultipleGetKeyValues).mock.calls.at(-1)!; + + it('waits for keys, then looks up values only in the sources that have the key', () => { + mockSources = [ + source('logs', SourceKind.Log), + source('traces', SourceKind.Trace), + ]; + mockAllFields([], { isLoading: true }); + const { result, rerender } = renderOptions( + filter('sql', ['logs', 'traces']), + ); + + expect(lastSqlLookup()[0].keys).toEqual([]); + expect(result.current.values.isLoading).toBe(true); + + mockAllFields([ + [column('SeverityText', 'LowCardinality(String)')], + [column('SpanName', 'LowCardinality(String)')], + ]); + rerender(); + + expect(result.current.keys.data).toEqual(['SeverityText', 'SpanName']); + const [{ chartConfigs, keys }] = lastSqlLookup(); + expect(keys).toEqual(['SeverityText']); + expect(chartConfigs).toEqual([expect.objectContaining({ source: 'logs' })]); + }); + + it('quotes a map column name that is not a bare identifier', () => { + mockSources = [source('logs', SourceKind.Log)]; + mockAllFields([[column('log-attributes', 'Map(String, String)')]]); + renderOptions(filter('sql', ['logs']), "log-attributes['service']"); + + expect(lastSqlLookup()[0].keys).toEqual(["`log-attributes`['service']"]); + }); + + it('leaves the SQL lookup disabled for a PromQL filter', async () => { + mockSources = [source('prom', SourceKind.Promql)]; + jest + .mocked(fetchPromqlLabelNames) + .mockResolvedValue(['__name__', 'SeverityText']); + const { result } = renderOptions(filter('promql', ['prom'])); + + await waitFor(() => + expect(result.current.keys.data).toEqual(['SeverityText']), + ); + expect(lastSqlLookup()[1]).toEqual( + expect.objectContaining({ enabled: false }), + ); + await waitFor(() => + expect(fetchPromqlLabelValues).toHaveBeenCalledWith( + expect.objectContaining({ label: 'SeverityText', sourceId: 'prom' }), + ), + ); + }); +}); diff --git a/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts b/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts new file mode 100644 index 0000000000..e54151862d --- /dev/null +++ b/packages/app/src/components/AdhocDashboardFilter/useAdhocFilterOptions.ts @@ -0,0 +1,345 @@ +import { useCallback, useMemo } from 'react'; +import { + ColumnMeta, + convertCHDataTypeToJSType, + filterColumnMetaByType, + isJSDataTypeScalar, + JSDataType, + QueryAttribution, +} from '@hyperdx/common-utils/dist/clickhouse'; +import { Field, tcFromSource } from '@hyperdx/common-utils/dist/core/metadata'; +import { getAdhocFilterSourceKinds } from '@hyperdx/common-utils/dist/filters'; +import { + AdhocDashboardFilter, + TSource, +} from '@hyperdx/common-utils/dist/types'; +import { + keepPreviousData, + useQueries, + UseQueryOptions, + UseQueryResult, +} from '@tanstack/react-query'; + +import { toQuotedClickHouseKeyExpression } from '@/components/DBSearchPageFilters/utils'; +import { + Facet, + SINGLE_KEY_VALUES_LIMIT, + useMultipleAllFields, + useMultipleGetKeyValues, +} from '@/hooks/useMetadata'; +import { + fetchPromqlLabelNames, + fetchPromqlLabelValues, + toPromqlSeconds, +} from '@/hooks/usePromqlMetadata'; +import { useMetadataQueryAttribution } from '@/queryAttribution'; +import { pickSourceConfigFields } from '@/ServicesDashboardPage/helpers'; +import { useSources } from '@/source'; +import { mergePath } from '@/utils'; + +const STALE_TIME_MS = 1000 * 60 * 5; +const NO_SOURCES: TSource[] = []; +const NO_COLUMNS: Set = new Set(); + +const unionSorted = (lists: (string[] | undefined)[]) => + Array.from(new Set(lists.flatMap(list => list ?? []))).sort(); + +/** + * A SQL source's filterable keys, in the search sidebar's key format: scalar + * columns, and the keys inside its map and JSON columns. + */ +export function getSqlSourceKeys(fields: Field[], columns: ColumnMeta[]) { + const columnNames = (types: JSDataType[]) => + filterColumnMetaByType(columns, types)?.map(column => column.name) ?? []; + const jsonColumns = columnNames([JSDataType.JSON]); + const mapColumns = columnNames([JSDataType.Map]); + return fields + .filter(field => field.path.length > 1 || isJSDataTypeScalar(field.jsType)) + .map(field => mergePath(field.path, jsonColumns, mapColumns)); +} + +/** A SQL source's scalar columns, the only keys known without field metadata. */ +export function getSqlSourceColumnKeys(columns: ColumnMeta[]) { + return columns + .filter(column => + isJSDataTypeScalar(convertCHDataTypeToJSType(column.type)), + ) + .map(column => column.name); +} + +/** + * Suggestions from looked-up values. A key typed by hand can name a whole map + * or JSON column, whose values are objects rather than suggestions. + */ +export function getValueSuggestions(facets: Facet[] | undefined) { + return unionSorted( + (facets ?? []).map(({ value }) => + value + .filter((v: unknown) => v != null && typeof v !== 'object') + .map(String), + ), + ); +} + +/** + * The sources to look up `key`'s values in: those known to have it, so a source + * without the column doesn't fail its query, or every source when none are + * known to (a key typed by hand). + */ +export function getSourcesWithKey( + sources: T[], + keysBySourceId: ReadonlyMap, + key: string, +) { + const withKey = sources.filter(source => + keysBySourceId.get(source.id)?.includes(key), + ); + return withKey.length > 0 ? withKey : sources; +} + +/** The filter's sources, limited to the kinds its source type reads. */ +function useAdhocFilterSources(filter: AdhocDashboardFilter) { + const { data: allSources } = useSources(); + return useMemo(() => { + const kinds = getAdhocFilterSourceKinds(filter.sourceType); + return (allSources ?? []).filter( + source => + filter.sources.includes(source.id) && kinds.includes(source.kind), + ); + }, [allSources, filter.sources, filter.sourceType]); +} + +/** Suggestions for one input of the condition editor. */ +type AdhocFilterSuggestions = { + data: string[]; + isLoading: boolean; + isError: boolean; +}; + +type AdhocFilterKeys = AdhocFilterSuggestions & { + keysBySourceId: ReadonlyMap; + /** Top-level column names across SQL sources, for quoting keys in SQL. */ + columnNames: Set; +}; + +function useSqlSourceKeys( + sources: TSource[], + dateRange: [Date, Date], +): AdhocFilterKeys { + const tableConnections = useMemo(() => sources.map(tcFromSource), [sources]); + const { + fieldsByTableConnection, + columnsByTableConnection, + isFieldMetadataDisabled, + isLoading, + isError, + } = useMultipleAllFields(tableConnections, { dateRange }); + + return useMemo(() => { + const keysBySourceId = new Map(); + const columnNames = new Set(); + sources.forEach((source, index) => { + const columns = columnsByTableConnection.get(index); + if (!columns) return; + columns.forEach(column => columnNames.add(column.name)); + keysBySourceId.set( + source.id, + isFieldMetadataDisabled + ? getSqlSourceColumnKeys(columns) + : getSqlSourceKeys(fieldsByTableConnection.get(index) ?? [], columns), + ); + }); + return { + data: unionSorted([...keysBySourceId.values()]), + keysBySourceId, + columnNames, + isLoading, + isError, + }; + }, [ + sources, + fieldsByTableConnection, + columnsByTableConnection, + isFieldMetadataDisabled, + isLoading, + isError, + ]); +} + +const promqlLookupParams = ( + source: TSource, + { start, end }: { start: number; end: number }, + attribution: QueryAttribution, +) => ({ + sourceId: source.id, + start, + end, + attribution, +}); + +function usePromqlSourceKeys( + sources: TSource[], + dateRange: [Date, Date], +): AdhocFilterKeys { + const attribution = useMetadataQueryAttribution(); + const seconds = toPromqlSeconds(dateRange); + + const combineKeys = useCallback( + (results: UseQueryResult[]) => { + const keysBySourceId = new Map(); + sources.forEach((source, index) => { + const keys = results[index]?.data; + if (keys) keysBySourceId.set(source.id, keys); + }); + return { + data: unionSorted([...keysBySourceId.values()]), + keysBySourceId, + columnNames: NO_COLUMNS, + isLoading: results.some(result => result.isLoading), + isError: results.some(result => result.isError), + }; + }, + [sources], + ); + + return useQueries({ + queries: sources.map( + (source): UseQueryOptions => ({ + queryKey: [ + 'adhoc-filter-promql-keys', + source.id, + seconds.start, + seconds.end, + ], + queryFn: async () => { + const labels = await fetchPromqlLabelNames( + promqlLookupParams(source, seconds, attribution), + ); + return labels.filter(label => label !== '__name__'); + }, + staleTime: STALE_TIME_MS, + placeholderData: keepPreviousData, + }), + ), + combine: combineKeys, + }); +} + +function useSqlSourceValues( + sources: TSource[], + key: string, + dateRange: [Date, Date], + columnNames: Set, +): AdhocFilterSuggestions { + const chartConfigs = useMemo( + () => + sources.map(source => ({ + ...pickSourceConfigFields(source), + dateRange, + source: source.id, + select: '', + where: '', + whereLanguage: 'sql' as const, + })), + [sources, dateRange], + ); + // Quoted like the search sidebar does, so a column named e.g. + // `log-attributes` isn't read as a subtraction. + const keys = useMemo( + () => (key ? [toQuotedClickHouseKeyExpression(key, columnNames)] : []), + [key, columnNames], + ); + const { data, isFetching, isError } = 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 }; +} + +const combineValues = ( + results: UseQueryResult[], +): AdhocFilterSuggestions => ({ + data: unionSorted(results.map(result => result.data)), + isLoading: results.some(result => result.isFetching), + isError: results.some(result => result.isError), +}); + +function usePromqlSourceValues( + sources: TSource[], + key: string, + dateRange: [Date, Date], +): AdhocFilterSuggestions { + const attribution = useMetadataQueryAttribution(); + const seconds = toPromqlSeconds(dateRange); + + return useQueries({ + queries: sources.map( + (source): UseQueryOptions => ({ + queryKey: [ + 'adhoc-filter-promql-values', + source.id, + key, + seconds.start, + seconds.end, + ], + queryFn: () => + fetchPromqlLabelValues({ + label: key, + ...promqlLookupParams(source, seconds, attribution), + }), + enabled: !!key, + staleTime: STALE_TIME_MS, + retry: false, + }), + ), + combine: combineValues, + }); +} + +/** + * 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`. + */ +export function useAdhocFilterOptions( + filter: AdhocDashboardFilter, + key: string, + dateRange: [Date, Date], +): { keys: AdhocFilterSuggestions; values: AdhocFilterSuggestions } { + const sources = useAdhocFilterSources(filter); + const isPromql = filter.sourceType === 'promql'; + const sqlKeys = useSqlSourceKeys(isPromql ? NO_SOURCES : sources, dateRange); + const promqlKeys = usePromqlSourceKeys( + isPromql ? sources : NO_SOURCES, + dateRange, + ); + const keys = isPromql ? promqlKeys : sqlKeys; + + // Until keys load, every source would be queried, then the sources known to + // have `key` queried again. + const lookupKey = keys.isLoading ? '' : key; + const sourcesWithKey = useMemo( + () => getSourcesWithKey(sources, keys.keysBySourceId, lookupKey), + [sources, keys.keysBySourceId, lookupKey], + ); + const sqlValues = useSqlSourceValues( + isPromql ? NO_SOURCES : sourcesWithKey, + lookupKey, + dateRange, + keys.columnNames, + ); + const promqlValues = usePromqlSourceValues( + isPromql ? sourcesWithKey : NO_SOURCES, + lookupKey, + dateRange, + ); + const values = isPromql ? promqlValues : sqlValues; + return { + keys, + values: key && keys.isLoading ? { ...values, isLoading: true } : values, + }; +} diff --git a/packages/app/src/components/DBSearchPageFilters/hooks.ts b/packages/app/src/components/DBSearchPageFilters/hooks.ts index be73a26336..3c6b0d657c 100644 --- a/packages/app/src/components/DBSearchPageFilters/hooks.ts +++ b/packages/app/src/components/DBSearchPageFilters/hooks.ts @@ -16,6 +16,7 @@ import { import { Facet, + SINGLE_KEY_VALUES_LIMIT, useAllFields, useColumns, useDateTimeColumns, @@ -32,9 +33,6 @@ import { toQuotedClickHouseKeyExpression } from './utils'; const INITIAL_LOAD_LIMIT = 20; -/* The maximum number of values per filter to load when "Load More" is clicked */ -const LOAD_MORE_LOAD_LIMIT = 10000; - /** * Decide which table key-value discovery reads from. * @@ -230,7 +228,7 @@ function useFacets({ ), }, keys: [sqlKey], - limit: LOAD_MORE_LOAD_LIMIT, + limit: SINGLE_KEY_VALUES_LIMIT, disableRowLimit: true, source, }); @@ -255,7 +253,7 @@ function useFacets({ connectionId: tableConnection.connectionId, metadataMVs: tableConnection.metadataMVs, keyExpressions: [sqlKey], - maxValuesPerKey: LOAD_MORE_LOAD_LIMIT, + maxValuesPerKey: SINGLE_KEY_VALUES_LIMIT, dateRange, timestampValueExpression: source?.timestampValueExpression ?? diff --git a/packages/app/src/components/DashboardFilterLabel.tsx b/packages/app/src/components/DashboardFilterLabel.tsx new file mode 100644 index 0000000000..37f8927fbc --- /dev/null +++ b/packages/app/src/components/DashboardFilterLabel.tsx @@ -0,0 +1,61 @@ +import { ReactNode } from 'react'; +import { Group, Text, Tooltip } from '@mantine/core'; +import { IconAlertTriangle, IconHelp } from '@tabler/icons-react'; + +/** What a filter does with its value, from `getFilterEffect`. */ +export type FilterEffect = { hasEffect: boolean; tooltip: string }; + +/** One of the caution icons a filter's label row can carry, with its tooltip. */ +export const FilterCaution = ({ + label, + testId, + variant = 'warning', +}: { + label: string; + testId: string; + variant?: 'warning' | 'danger'; +}) => ( + + + +); + +/** + * The row above a dashboard filter's input: its name, what it does, and any + * cautions passed as children. + */ +export const DashboardFilterLabel = ({ + name, + effect, + children, +}: { + name: string; + effect: FilterEffect; + children?: ReactNode; +}) => ( + + + {name} + + + {effect.hasEffect ? ( + + ) : ( + + )} + + {children} + +); diff --git a/packages/app/src/components/DashboardFiltersModal/DashboardFiltersList.tsx b/packages/app/src/components/DashboardFiltersModal/DashboardFiltersList.tsx index 5f92373c35..22e99756ed 100644 --- a/packages/app/src/components/DashboardFiltersModal/DashboardFiltersList.tsx +++ b/packages/app/src/components/DashboardFiltersModal/DashboardFiltersList.tsx @@ -1,9 +1,6 @@ import { - getAdhocFilterAppliesToSourceIds, getFilterBroadcastTarget, getFilterVariableName, - isAdhocFilter, - isFilterBroadcastEnabled, isFilterGlobalRequirement, isFilterRequired, isFilterVariableEnabled, @@ -104,12 +101,6 @@ function getBroadcastTargetDisplay( filter: DashboardFilter, sources?: { id: string; name: string }[], ) { - // An ad hoc filter broadcasts to its own sources unless narrowed further - if (isAdhocFilter(filter)) { - if (!isFilterBroadcastEnabled(filter)) return undefined; - return getSourceNames(getAdhocFilterAppliesToSourceIds(filter), sources); - } - const broadcastTarget = getFilterBroadcastTarget(filter); if (!broadcastTarget) return undefined; diff --git a/packages/app/src/components/FilterPill/FilterAutocomplete.tsx b/packages/app/src/components/FilterPill/FilterAutocomplete.tsx index 60e3a1c499..4a68d8aeb0 100644 --- a/packages/app/src/components/FilterPill/FilterAutocomplete.tsx +++ b/packages/app/src/components/FilterPill/FilterAutocomplete.tsx @@ -26,7 +26,7 @@ export type FilterAutocompleteProps = { 'aria-label'?: string; 'data-testid'?: string; ref?: Ref; -} & Pick; +} & Pick; /** Suggestion input for filter keys and values that also accepts free text. */ export function FilterAutocomplete({ diff --git a/packages/app/src/components/FilterPill/FilterConditionEditor.stories.tsx b/packages/app/src/components/FilterPill/FilterConditionEditor.stories.tsx index 7db25cea43..c0ff2ad772 100644 --- a/packages/app/src/components/FilterPill/FilterConditionEditor.stories.tsx +++ b/packages/app/src/components/FilterPill/FilterConditionEditor.stories.tsx @@ -112,3 +112,16 @@ export const LoadingValues = () => ( export const NoSuggestions = () => ( ); + +export const KeysError = () => ( + +); + +export const ValuesError = () => ( + +); diff --git a/packages/app/src/components/FilterPill/FilterConditionEditor.tsx b/packages/app/src/components/FilterPill/FilterConditionEditor.tsx index e5486e544a..fbee6097c6 100644 --- a/packages/app/src/components/FilterPill/FilterConditionEditor.tsx +++ b/packages/app/src/components/FilterPill/FilterConditionEditor.tsx @@ -26,9 +26,11 @@ export type FilterConditionEditorProps = { initial?: FilterCondition; keyOptions: string[]; isLoadingKeys?: boolean; + isKeysError?: boolean; /** 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. */ onKeyChange?: (key: string) => void; keyLabel?: string; @@ -38,6 +40,7 @@ export type FilterConditionEditorProps = { }; const KEY_DEBOUNCE_MS = 300; +const SUGGESTIONS_ERROR = "Couldn't load suggestions"; /** Pick a key, an operator, and a value. Free text is accepted for both inputs. */ export function FilterConditionEditor({ @@ -45,8 +48,10 @@ export function FilterConditionEditor({ initial, keyOptions, isLoadingKeys, + isKeysError, valueOptions, isLoadingValues, + isValuesError, onKeyChange, keyLabel = 'Key', keyPlaceholder = 'Select a key', @@ -69,6 +74,7 @@ export function FilterConditionEditor({ // 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 selectedOperator = operators.find(op => op.value === operator); const canSubmit = !!trimmedKey && operator != null; @@ -84,6 +90,7 @@ export function FilterConditionEditor({ placeholder={keyPlaceholder} options={keyOptions} isLoading={isLoadingKeys} + error={!isLoadingKeys && isKeysError ? SUGGESTIONS_ERROR : undefined} value={key} onChange={setKey} // Enter moves on to the value instead of applying an empty-value filter. @@ -105,6 +112,7 @@ export function FilterConditionEditor({ placeholder={selectedOperator?.valuePlaceholder ?? 'Select a value'} options={valueOptions} isLoading={isValueLoading} + error={isValuesErrorShown ? SUGGESTIONS_ERROR : undefined} value={value} onChange={setValue} onSubmit={submit} diff --git a/packages/app/src/components/FilterPill/FilterPill.stories.tsx b/packages/app/src/components/FilterPill/FilterPill.stories.tsx index 0f16ab08ab..b872d92105 100644 --- a/packages/app/src/components/FilterPill/FilterPill.stories.tsx +++ b/packages/app/src/components/FilterPill/FilterPill.stories.tsx @@ -15,6 +15,7 @@ import { import { FilterLanguage, FilterOperator, + getFilterOperatorDisplay, getFilterOperators, } from './filterOperators'; import { FilterPill } from './FilterPill'; @@ -149,7 +150,7 @@ function ConditionEditorDemo() { language === 'promql' ? MOCK_PROMQL_VALUES : MOCK_SQL_VALUES; const values = useMockValues(mockValues, key); const operators = getFilterOperators(language); - const operator = operators.find(op => op.value === condition.operator); + const operator = getFilterOperatorDisplay(language, condition.operator); return ( @@ -171,9 +172,9 @@ function ConditionEditorDemo() { /> {}} renderPopover={close => ( { ).toBeInTheDocument(); }); + it('shows an error on the key input when keys fail to load', () => { + renderEditor({ isKeysError: true }); + expect(screen.getByText("Couldn't load suggestions")).toBeInTheDocument(); + }); + + it('shows a value error only once the key settles', async () => { + renderEditor({ isValuesError: true }); + expect(screen.queryByText("Couldn't load suggestions")).toBeNull(); + await userEvent.type(keyInput(), 'ServiceName'); + expect( + await screen.findByText("Couldn't load suggestions"), + ).toBeInTheDocument(); + }); + it('submits free-form text on Enter in the value input', async () => { const onSubmit = renderEditor({ isLoadingValues: true }); // `[[` types a literal `[`. diff --git a/packages/app/src/components/FilterPill/filterOperators.ts b/packages/app/src/components/FilterPill/filterOperators.ts index e766e247bd..62ba56ff5d 100644 --- a/packages/app/src/components/FilterPill/filterOperators.ts +++ b/packages/app/src/components/FilterPill/filterOperators.ts @@ -32,3 +32,12 @@ export function getFilterOperators( ): FilterOperatorOption[] { return language === 'promql' ? PROMQL_FILTER_OPERATORS : SQL_FILTER_OPERATORS; } + +/** How a pill shows `operator`: its label, and whether it excludes values. */ +export function getFilterOperatorDisplay( + language: FilterLanguage, + operator: string, +): { label: string; isExcluded: boolean } { + const option = getFilterOperators(language).find(op => op.value === operator); + return { label: option?.label ?? operator, isExcluded: !!option?.negated }; +} diff --git a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx index b3f9fffc39..1ea51eb2fc 100644 --- a/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/useDashboardFilters.test.tsx @@ -1,4 +1,7 @@ import { + ADHOC_FILTER_MAX_CONDITIONS, + AdhocDashboardFilter, + AdhocFilterCondition, DashboardFilter, DashboardFilterValue, Filter, @@ -988,6 +991,156 @@ describe('useDashboardFilters', () => { }); }); + describe('ad hoc filters', () => { + const adhocFilter: AdhocDashboardFilter = { + id: 'adhoc1', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs'], + isVariableEnabled: true, + variableName: 'conds', + }; + const condition: AdhocFilterCondition = { + key: 'ServiceName', + operator: '!=', + value: 'api', + }; + + it('reads conditions from an ad hoc entry with its variable name', () => { + mockState = [{ type: 'adhoc', name: 'conds', conditions: [condition] }]; + + const { result } = renderHook(() => useDashboardFilters([adhocFilter])); + + expect(result.current.adhocConditionsByFilterId.get('adhoc1')).toEqual([ + condition, + ]); + expect(result.current.ignoredVariableNames).toEqual([]); + }); + + it('writes and clears conditions without touching other entries', () => { + mockState = [{ type: 'sql', condition: "environment IN ('prod')" }]; + const filters = [mockFilters[0], adhocFilter]; + const { result } = renderHook(() => useDashboardFilters(filters)); + + act(() => { + result.current.setAdhocConditions('adhoc1', [condition]); + }); + expect(mockState).toEqual([ + { type: 'sql', condition: "environment IN ('prod')" }, + { type: 'adhoc', name: 'conds', conditions: [condition] }, + ]); + + act(() => { + result.current.setAdhocConditions('adhoc1', []); + }); + expect(mockState).toEqual([ + { type: 'sql', condition: "environment IN ('prod')" }, + ]); + }); + + it('drops repeated conditions on read and write', () => { + mockState = [ + { type: 'adhoc', name: 'conds', conditions: [condition, condition] }, + ]; + const { result } = renderHook(() => useDashboardFilters([adhocFilter])); + + expect(result.current.adhocConditionsByFilterId.get('adhoc1')).toEqual([ + condition, + ]); + + act(() => { + result.current.setAdhocConditions('adhoc1', [condition, condition]); + }); + expect(mockState).toEqual([ + { type: 'adhoc', name: 'conds', conditions: [condition] }, + ]); + }); + + it('ignores a write of more conditions than the URL entry can hold', () => { + mockState = [{ type: 'adhoc', name: 'conds', conditions: [condition] }]; + const { result } = renderHook(() => useDashboardFilters([adhocFilter])); + const conditionsUpTo = (count: number) => + Array.from({ length: count }, (_, i) => ({ + ...condition, + value: `${i}`, + })); + + act(() => { + result.current.setAdhocConditions( + 'adhoc1', + conditionsUpTo(ADHOC_FILTER_MAX_CONDITIONS + 1), + ); + }); + expect(mockState).toEqual([ + { type: 'adhoc', name: 'conds', conditions: [condition] }, + ]); + + act(() => { + result.current.setAdhocConditions( + 'adhoc1', + conditionsUpTo(ADHOC_FILTER_MAX_CONDITIONS), + ); + }); + expect(mockState).toEqual([ + { + type: 'adhoc', + name: 'conds', + conditions: conditionsUpTo(ADHOC_FILTER_MAX_CONDITIONS), + }, + ]); + }); + + it('ignores a write of a condition too long for the URL entry', () => { + mockState = [{ type: 'adhoc', name: 'conds', conditions: [condition] }]; + const { result } = renderHook(() => useDashboardFilters([adhocFilter])); + + act(() => { + result.current.setAdhocConditions('adhoc1', [ + condition, + { ...condition, value: 'x'.repeat(10001) }, + ]); + }); + expect(mockState).toEqual([ + { type: 'adhoc', name: 'conds', conditions: [condition] }, + ]); + }); + + it('keeps conditions across a write to another filter', () => { + mockState = [{ type: 'adhoc', name: 'conds', conditions: [condition] }]; + const filters = [mockFilters[0], adhocFilter]; + const { result } = renderHook(() => useDashboardFilters(filters)); + + act(() => { + result.current.setFilterValue('filter1', ['prod']); + }); + + expect(mockState).toEqual([ + { type: 'sql', condition: "environment IN ('prod')" }, + { type: 'adhoc', name: 'conds', conditions: [condition] }, + ]); + }); + + it('exposes no variable, selection, or broadcast query', () => { + mockState = [{ type: 'adhoc', name: 'conds', conditions: [condition] }]; + + const { result } = renderHook(() => useDashboardFilters([adhocFilter])); + + expect(result.current.variables).toEqual([]); + expect(result.current.selectionByFilterId.size).toBe(0); + expect(result.current.broadcastedFilters).toEqual([]); + }); + + it('lists an ad hoc entry naming no declared filter as ignored', () => { + mockState = [{ type: 'adhoc', name: 'renamed', conditions: [condition] }]; + + const { result } = renderHook(() => useDashboardFilters([adhocFilter])); + + expect(result.current.adhocConditionsByFilterId.size).toBe(0); + expect(result.current.ignoredVariableNames).toEqual(['renamed']); + }); + }); + describe('unsatisfiedRequiredFilters', () => { const required = ( overrides: Partial = {}, diff --git a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx index 2ccd430ffd..f3770007cc 100644 --- a/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx +++ b/packages/app/src/hooks/__tests__/usePresetDashboardFilters.test.tsx @@ -111,6 +111,8 @@ describe('usePresetDashboardFilters', () => { jest.mocked(useDashboardFilters).mockReturnValue({ selectionByFilterId: mockSelectionByFilterId, setFilterValue: mockSetFilterValue, + adhocConditionsByFilterId: new Map(), + setAdhocConditions: jest.fn(), broadcastedFilters: mockFilterQueries, setFilterValueEntries: jest.fn(), filterValueEntries: null, diff --git a/packages/app/src/hooks/useDashboardFilters.tsx b/packages/app/src/hooks/useDashboardFilters.tsx index e103ab81e3..547a7de988 100644 --- a/packages/app/src/hooks/useDashboardFilters.tsx +++ b/packages/app/src/hooks/useDashboardFilters.tsx @@ -1,4 +1,5 @@ import { useCallback, useMemo } from 'react'; +import { partition, uniqBy } from 'lodash'; import { useQueryState } from 'nuqs'; import { FilterSelection, @@ -8,6 +9,7 @@ import { ParsedDashboardFilterValues, resolveFilterSelection, serializeDashboardFilterValues, + VariableEntryValue, } from '@hyperdx/common-utils/dist/dashboardFilterValues'; import { doesFilterApplyToSource, @@ -15,10 +17,14 @@ import { filtersToQuery, getDashboardVariableFilters, getFilterExpression, + getFilterVariableName, + isAdhocFilter, isFilterBroadcastEnabled, isQueryExpressionFilter, } from '@hyperdx/common-utils/dist/filters'; import { + AdhocFilterCondition, + AdhocFilterValueSchema, ChartVariable, DashboardFilter, Filter, @@ -39,6 +45,10 @@ const hasSelection = (selection: FilterSelection): boolean => const isRepresentableAsVariable = (selection: FilterSelection): boolean => selection.excluded.size === 0 && selection.range == null; +/** Conditions are identified by content (e.g. pill keys), so drop repeats. */ +const uniqConditions = (conditions: AdhocFilterCondition[]) => + uniqBy(conditions, condition => JSON.stringify(condition)); + /** * Rebuild the whole state array from the selections every declared filter * currently resolves to. Anything that belongs to no declared filter is @@ -49,11 +59,12 @@ const rebuildEntries = ( parsed: ParsedDashboardFilterValues, ) => { const byExpression: FilterState = {}; - const byVariable = new Map(); + const byVariable = new Map(); const declaredExpressions = new Set(); const declaredVariableNames = new Set(); for (const filter of filters) { + if (isAdhocFilter(filter)) continue; const expression = getFilterExpression(filter); if (expression) declaredExpressions.add(expression); @@ -67,7 +78,10 @@ const rebuildEntries = ( if (!isRepresentableAsVariable(selection) && expression) { byExpression[expression] = selection; } else if (selection.included.size > 0) { - byVariable.set(key.name, Array.from(selection.included).map(String)); + byVariable.set(key.name, { + type: 'variable', + values: Array.from(selection.included).map(String), + }); } } @@ -83,9 +97,10 @@ const rebuildEntries = ( } } - // Pass through any entries that don't correspond to a declared filter. - for (const [name, values] of parsed.byVariable) { - if (!declaredVariableNames.has(name)) byVariable.set(name, values); + // Pass through any entries that don't correspond to a declared filter. Ad hoc + // filters are skipped above, so their conditions pass through here too. + for (const [name, value] of parsed.byVariable) { + if (!declaredVariableNames.has(name)) byVariable.set(name, value); } for (const [expression, selection] of Object.entries(parsed.byExpression)) { if (!declaredExpressions.has(expression)) { @@ -120,7 +135,7 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { const key = filterSelectionKey(target); if (key.kind === 'variable') { // Set even when empty to avoid falling back to a legacy expression-keyed value - parsed.byVariable.set(key.name, values); + parsed.byVariable.set(key.name, { type: 'variable', values }); } else if (values.length === 0) { delete parsed.byExpression[key.expression]; } else { @@ -136,28 +151,62 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { [setFilterValueEntries, filters], ); + const setAdhocConditions = useCallback( + (filterId: string, conditions: AdhocFilterCondition[]) => { + setFilterValueEntries(prev => { + const target = filters.find(f => f.id === filterId); + const name = target && getFilterVariableName(target); + // The URL parser drops an invalid entry whole, losing every condition + const unique = uniqConditions(conditions); + const isStorable = + AdhocFilterValueSchema.shape.conditions.safeParse(unique).success; + if (!name || !isStorable) return prev; + + const parsed = parseDashboardFilterValues(prev ?? []); + parsed.byVariable.set(name, { type: 'adhoc', conditions: unique }); + return rebuildEntries(filters, parsed); + }); + }, + [setFilterValueEntries, filters], + ); + const { selectionByFilterId, + adhocConditionsByFilterId, ignoredExpressions, ignoredVariableNames, variables, unsatisfiedRequiredFilters, } = useMemo<{ selectionByFilterId: ReadonlyMap; + adhocConditionsByFilterId: ReadonlyMap; ignoredExpressions: string[]; ignoredVariableNames: string[]; variables: ChartVariable[]; unsatisfiedRequiredFilters: DashboardFilter[]; }>(() => { const parsed = parseDashboardFilterValues(filterValueEntries ?? []); + // Ad hoc filters aren't exposed as variables yet + const [adhocFilters, nonAdhocFilters] = partition(filters, isAdhocFilter); // Keyed by filter ID: two definitions may share one expression const selectionByFilterId = new Map(); - for (const filter of filters) { + for (const filter of nonAdhocFilters) { const selection = resolveFilterSelection(filter, parsed); if (selection) selectionByFilterId.set(filter.id, selection); } + const adhocConditionsByFilterId = new Map(); + for (const filter of adhocFilters) { + const value = parsed.byVariable.get(getFilterVariableName(filter) ?? ''); + if (value?.type === 'adhoc') { + adhocConditionsByFilterId.set( + filter.id, + uniqConditions(value.conditions), + ); + } + } + // Find state that doesn't correspond to any declared filter, so the caller can surface a warning. const knownExpressions = new Set( filters.filter(isQueryExpressionFilter).map(f => f.expression), @@ -166,8 +215,11 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { expression => !knownExpressions.has(expression), ); - const variableFilters = getDashboardVariableFilters(filters); - const declaredVariableNames = new Set(variableFilters.map(v => v.name)); + const variableFilters = getDashboardVariableFilters(nonAdhocFilters); + const declaredVariableNames = new Set([ + ...variableFilters.map(v => v.name), + ...adhocFilters.flatMap(filter => getFilterVariableName(filter) ?? []), + ]); const ignoredVariableNames = Array.from(parsed.byVariable.keys()).filter( name => !declaredVariableNames.has(name), ); @@ -191,6 +243,7 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { return { selectionByFilterId, + adhocConditionsByFilterId, variables, ignoredExpressions, ignoredVariableNames, @@ -260,6 +313,10 @@ const useDashboardFilters = (filters: DashboardFilter[]) => { broadcastedFilters, /** Set the selected values for a filter, by its ID */ setFilterValue, + /** Each ad hoc filter's current conditions, keyed by `filter.id`. */ + adhocConditionsByFilterId, + /** Replace the conditions of an ad hoc filter, by its ID */ + setAdhocConditions, /** Set the raw filter value state */ setFilterValueEntries, /** The raw persisted entries, as they appear in the URL param. */ diff --git a/packages/app/src/hooks/useMetadata.tsx b/packages/app/src/hooks/useMetadata.tsx index fd265c1af0..c55ac4cc8c 100644 --- a/packages/app/src/hooks/useMetadata.tsx +++ b/packages/app/src/hooks/useMetadata.tsx @@ -39,6 +39,12 @@ import { toArray } from '@/utils'; export type Facet = { key: string; value: string[] }; +/** + * The most values to load when one key's values are looked up in full, e.g. + * the search sidebar's "Load more". + */ +export const SINGLE_KEY_VALUES_LIMIT = 10000; + // Hook to get metadata with proper settings applied export function useMetadataWithSettings() { const [metadata, setMetadata] = useState(getMetadata()); diff --git a/packages/app/src/hooks/usePromqlLabelFilterValues.tsx b/packages/app/src/hooks/usePromqlLabelFilterValues.tsx index ed66bd5384..b1cfdeeed2 100644 --- a/packages/app/src/hooks/usePromqlLabelFilterValues.tsx +++ b/packages/app/src/hooks/usePromqlLabelFilterValues.tsx @@ -14,7 +14,10 @@ import { UseQueryResult, } from '@tanstack/react-query'; -import { prometheusApi } from '@/api'; +import { + fetchPromqlLabelValues, + toPromqlSeconds, +} from '@/hooks/usePromqlMetadata'; import { useMetadataQueryAttribution } from '@/queryAttribution'; import { useSources } from '@/source'; import { mapKeyBy } from '@/utils'; @@ -42,9 +45,7 @@ export function usePromqlLabelFilterValues({ const { data: sources, isLoading: isLoadingSources } = useSources(); const sourcesById = useMemo(() => mapKeyBy(sources ?? [], 'id'), [sources]); - // Round the date range, since the API accepts whole seconds - const startSec = Math.floor(dateRange[0].getTime() / 1000); - const endSec = Math.ceil(dateRange[1].getTime() / 1000); + const { start: startSec, end: endSec } = toPromqlSeconds(dateRange); // A filter's selector may reference the dashboard's variables. Expand them // here so react-query keys on the resolved selector rather than the template. @@ -113,20 +114,15 @@ export function usePromqlLabelFilterValues({ return cached[0]?.data; }, staleTime: 1000 * 60 * 5, - queryFn: async (): Promise => { - const resp = await prometheusApi.labelValues({ + queryFn: () => + fetchPromqlLabelValues({ label: call.label, sourceId: call.sourceId, start: startSec, end: endSec, match: call.match, attribution, - }); - if (resp.status === 'error') { - throw new Error(resp.error ?? 'Label values query failed'); - } - return resp.data ?? []; - }, + }), }; }), }); diff --git a/packages/app/src/hooks/usePromqlMetadata.ts b/packages/app/src/hooks/usePromqlMetadata.ts index b8102ac745..045054ace0 100644 --- a/packages/app/src/hooks/usePromqlMetadata.ts +++ b/packages/app/src/hooks/usePromqlMetadata.ts @@ -1,20 +1,53 @@ +import { QueryAttribution } from '@hyperdx/common-utils/dist/clickhouse'; import { useQuery } from '@tanstack/react-query'; import { prometheusApi } from '@/api'; import { useMetadataQueryAttribution } from '@/queryAttribution'; +type PromqlLabelLookup = { + sourceId: string; + start?: number; + end?: number; + attribution?: QueryAttribution; +}; + +/** The label lookup APIs accept whole seconds. */ +export const toPromqlSeconds = (dateRange: [Date, Date]) => ({ + start: Math.floor(dateRange[0].getTime() / 1000), + end: Math.ceil(dateRange[1].getTime() / 1000), +}); + +export async function fetchPromqlLabelNames( + params: PromqlLabelLookup, +): Promise { + const resp = await prometheusApi.labels(params); + if (resp.status === 'error') { + throw new Error(resp.error ?? 'Label names query failed'); + } + return resp.data ?? []; +} + +export async function fetchPromqlLabelValues( + params: PromqlLabelLookup & { label: string; match?: string }, +): Promise { + const resp = await prometheusApi.labelValues(params); + if (resp.status === 'error') { + throw new Error(resp.error ?? 'Label values query failed'); + } + return resp.data ?? []; +} + export function usePromqlMetricNames(sourceId: string | undefined) { const attribution = useMetadataQueryAttribution(); return useQuery({ queryKey: ['promql-metric-names', sourceId], queryFn: async () => { if (!sourceId) return []; - const resp = await prometheusApi.labelValues({ + return fetchPromqlLabelValues({ label: '__name__', sourceId, attribution, }); - return resp.data ?? []; }, enabled: !!sourceId, staleTime: 60_000, @@ -28,8 +61,7 @@ export function usePromqlLabelNames(sourceId: string | undefined) { queryKey: ['promql-label-names', sourceId], queryFn: async () => { if (!sourceId) return []; - const resp = await prometheusApi.labels({ sourceId, attribution }); - return resp.data ?? []; + return fetchPromqlLabelNames({ sourceId, attribution }); }, enabled: !!sourceId, staleTime: 60_000, 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 e64ed257cc..4c2bfa9172 100644 --- a/packages/app/tests/e2e/features/dashboard-adhoc-filters.spec.ts +++ b/packages/app/tests/e2e/features/dashboard-adhoc-filters.spec.ts @@ -1,9 +1,11 @@ /** * An `ADHOC` dashboard filter's conditions (`key operator value`) are picked - * from the keys of its sources. For now the filters modal only lists, creates, - * and edits them; they are not rendered or applied on the dashboard. + * from the keys of its sources. The filters modal creates and edits them, and + * the dashboard renders their conditions and keeps them in the URL; they are not + * applied to tiles yet. */ import { DashboardPage } from '../page-objects/DashboardPage'; +import { SERVICES } from '../seed-clickhouse'; import { expect, test } from '../utils/base-test'; import { DEFAULT_LOGS_SOURCE_NAME, @@ -11,6 +13,7 @@ import { DEFAULT_TRACES_SOURCE_NAME, PROMQL_SOURCE_NAME, } from '../utils/constants'; +import { expectFiltersParam, filtersParam } from '../utils/filters-param'; test.describe( 'Ad hoc dashboard filters', @@ -23,6 +26,16 @@ test.describe( await dashboardPage.goto(); }); + /** Create a dashboard holding one ad hoc filter, and close the modal. */ + const createDashboardWithAdhocFilter = async ( + ...args: Parameters + ) => { + await dashboardPage.createNewDashboard(); + await dashboardPage.openEditFiltersModal(); + await dashboardPage.addAdhocFilterToDashboard(...args); + await dashboardPage.closeFiltersModal(); + }; + test('authors a SQL filter over several sources', async ({ page }) => { await test.step('Open the add-filter form and pick the ad hoc type', async () => { await dashboardPage.createNewDashboard(); @@ -257,6 +270,200 @@ test.describe( ).toBeVisible(); }); + test('picks conditions from the keys and values of one SQL source', async ({ + page, + }) => { + const filterName = 'Conditions'; + const pills = dashboardPage.getAdhocConditionPills(filterName); + + await test.step('Create the filter', async () => { + await createDashboardWithAdhocFilter( + filterName, + [DEFAULT_LOGS_SOURCE_NAME], + { variableName: 'conds' }, + ); + await expect(dashboardPage.getAdhocFilter(filterName)).toBeVisible(); + }); + + await test.step('Keys are suggested, values only once a key is picked', async () => { + const editor = dashboardPage.getAdhocConditionEditor(filterName); + const valueInput = dashboardPage.getAdhocConditionInput( + filterName, + 'value', + ); + const serviceNameOption = dashboardPage.getAdhocConditionOption( + filterName, + 'ServiceName', + ); + const accountingOption = dashboardPage.getAdhocConditionOption( + filterName, + 'accounting', + ); + + await dashboardPage.openAddAdhocCondition(filterName); + await valueInput.click(); + await expect(editor.getByRole('option')).toHaveCount(0); + + await dashboardPage.getAdhocConditionInput(filterName, 'key').click(); + await expect(serviceNameOption).toBeVisible({ timeout: 30000 }); + await serviceNameOption.click(); + + await valueInput.click(); + await expect(accountingOption).toBeVisible({ timeout: 30000 }); + await accountingOption.click(); + await valueInput.press('Enter'); + await expect(editor).toBeHidden(); + }); + + await test.step('The condition is shown and kept in the URL', async () => { + await expect(pills).toHaveCount(1); + await expect(pills).toHaveText('ServiceName = accounting'); + await expectFiltersParam(page, [ + { + type: 'adhoc', + name: 'conds', + conditions: [ + { key: 'ServiceName', operator: '=', value: 'accounting' }, + ], + }, + ]); + }); + + await test.step('Editing the pill changes its operator', async () => { + await dashboardPage.editAdhocCondition( + filterName, + 'ServiceName = accounting', + { operatorLabel: '!=' }, + ); + await expect(pills).toHaveText('ServiceName != accounting'); + await expectFiltersParam(page, [ + { + type: 'adhoc', + name: 'conds', + conditions: [ + { key: 'ServiceName', operator: '!=', value: 'accounting' }, + ], + }, + ]); + }); + + await test.step('The condition survives a reload', async () => { + await page.reload(); + await dashboardPage.waitForLoaded(); + await expect(pills).toHaveText('ServiceName != accounting'); + }); + + await test.step('Removing the pill clears it from the URL', async () => { + await pills.getByRole('button', { name: 'Remove filter' }).click(); + await expect(pills).toHaveCount(0); + await expect.poll(() => filtersParam(page) ?? []).toEqual([]); + }); + }); + + test('offers deduplicated keys and values across several SQL sources', async ({ + page, + }) => { + const filterName = 'Both'; + const pills = dashboardPage.getAdhocConditionPills(filterName); + + await test.step('Create a filter over logs and traces', async () => { + await createDashboardWithAdhocFilter( + filterName, + [DEFAULT_LOGS_SOURCE_NAME, DEFAULT_TRACES_SOURCE_NAME], + { variableName: 'both' }, + ); + }); + + const keyInput = dashboardPage.getAdhocConditionInput(filterName, 'key'); + + await test.step('A key both sources have is listed once', async () => { + await dashboardPage.openAddAdhocCondition(filterName); + // Typing narrows the list, which renders at most 200 suggestions. + await keyInput.fill('ServiceName'); + await expect( + dashboardPage.getAdhocConditionOption(filterName, 'ServiceName'), + ).toHaveCount(1, { timeout: 30000 }); + }); + + await test.step('A key only one source has is offered too', async () => { + await keyInput.fill('SpanName'); + await expect( + dashboardPage.getAdhocConditionOption(filterName, 'SpanName'), + ).toHaveCount(1); + }); + + await test.step("The key's values are listed once each", async () => { + await keyInput.fill('ServiceName'); + await dashboardPage.getAdhocConditionInput(filterName, 'value').click(); + await expect( + dashboardPage.getAdhocConditionOption(filterName, 'accounting'), + ).toHaveCount(1, { timeout: 30000 }); + const options = await dashboardPage.getOpenFilterDropdownOptions(); + expect(new Set(options).size).toBe(options.length); + expect(options).toEqual(expect.arrayContaining([...SERVICES])); + await dashboardPage.fillAdhocConditionEditor(filterName, { + value: 'accounting', + }); + }); + + await test.step('A key only one source has gets its values', async () => { + await dashboardPage.openAddAdhocCondition(filterName); + await keyInput.fill('SeverityText'); + await dashboardPage.getAdhocConditionInput(filterName, 'value').click(); + await expect( + dashboardPage.getAdhocConditionOption(filterName, 'error'), + ).toBeVisible({ timeout: 30000 }); + await dashboardPage.fillAdhocConditionEditor(filterName, { + value: 'error', + }); + }); + + await test.step('Both conditions survive a reload', async () => { + await expect(pills).toHaveText([ + 'ServiceName = accounting', + 'SeverityText = error', + ]); + await page.reload(); + await dashboardPage.waitForLoaded(); + await expect(pills).toHaveText([ + 'ServiceName = accounting', + 'SeverityText = error', + ]); + }); + }); + + test('picks conditions from the labels of a PromQL source', async () => { + const filterName = 'Labels'; + + await createDashboardWithAdhocFilter(filterName, [PROMQL_SOURCE_NAME], { + sourceType: 'Prometheus', + }); + + await dashboardPage.openAddAdhocCondition(filterName); + await dashboardPage.getAdhocConditionInput(filterName, 'key').click(); + await expect( + dashboardPage.getAdhocConditionOption(filterName, 'service'), + ).toBeVisible({ timeout: 30000 }); + await expect( + dashboardPage.getAdhocConditionOption(filterName, '__name__'), + ).toHaveCount(0); + + await dashboardPage + .getAdhocConditionInput(filterName, 'key') + .fill('service'); + await dashboardPage.getAdhocConditionInput(filterName, 'value').click(); + await expect( + dashboardPage.getAdhocConditionOption(filterName, 'accounting'), + ).toBeVisible({ timeout: 30000 }); + await dashboardPage.fillAdhocConditionEditor(filterName, { + operatorLabel: '=~', + value: 'acc.*', + }); + await expect(dashboardPage.getAdhocConditionPills(filterName)).toHaveText( + 'service =~ acc.*', + ); + }); + test("lives alongside the dashboard's other filters", async ({ page }) => { await test.step('Create an ad hoc and a static filter', async () => { await dashboardPage.createNewDashboard(); @@ -285,11 +492,12 @@ test.describe( await page.getByRole('button', { name: 'Cancel' }).click(); }); - await test.step('Only the other filter is rendered on the dashboard', async () => { + await test.step('Both filters are rendered on the dashboard', async () => { await dashboardPage.closeFiltersModal(); await expect( dashboardPage.getFilterSelectByName('Environment'), ).toBeVisible(); + await expect(dashboardPage.getAdhocFilter('Conditions')).toBeVisible(); await expect( dashboardPage.getFilterSelectByName('Conditions'), ).toHaveCount(0); diff --git a/packages/app/tests/e2e/page-objects/DashboardPage.ts b/packages/app/tests/e2e/page-objects/DashboardPage.ts index 974646c7d1..fcfcdaf9f3 100644 --- a/packages/app/tests/e2e/page-objects/DashboardPage.ts +++ b/packages/app/tests/e2e/page-objects/DashboardPage.ts @@ -1569,11 +1569,18 @@ export class DashboardPage { async addAdhocFilterToDashboard( name: string, sourceNames: string[], - options?: { variableName?: string }, + options?: { + variableName?: string; + sourceType?: 'ClickHouse' | 'Prometheus'; + }, ) { await this.addFiltersButton.click(); await this.selectFilterType('Ad hoc keys and values'); await this.getFilterNameInput().fill(name); + // Before the sources, since switching the type clears them. + if (options?.sourceType) { + await this.selectAdhocFilterSourceType(options.sourceType); + } await this.selectSourcesInMultiSelect( this.getAdhocFilterSourcesInput(), sourceNames, @@ -1588,6 +1595,85 @@ export class DashboardPage { }); } + /** The filter bar control of an ad hoc filter. */ + getAdhocFilter(filterName: string): Locator { + return this.page.getByTestId(`adhoc-filter-${filterName}`); + } + + /** The condition pills of an ad hoc filter in the filter bar. */ + getAdhocConditionPills(filterName: string): Locator { + return this.getAdhocFilter(filterName).getByTestId('adhoc-condition-pill'); + } + + /** The open condition editor of an ad hoc filter. */ + getAdhocConditionEditor(filterName: string): Locator { + return this.page.getByTestId(`adhoc-condition-editor-${filterName}`); + } + + /** The key, operator, or value input of an ad hoc filter's open editor. */ + getAdhocConditionInput( + filterName: string, + input: 'key' | 'operator' | 'value', + ): Locator { + return this.page.getByTestId( + `adhoc-condition-editor-${filterName}-${input}`, + ); + } + + /** A suggestion, matched exactly, in an ad hoc filter's open editor. */ + getAdhocConditionOption(filterName: string, name: string): Locator { + return this.getAdhocConditionEditor(filterName).getByRole('option', { + name, + exact: true, + }); + } + + /** Open the editor that adds a condition to an ad hoc filter. */ + async openAddAdhocCondition(filterName: string) { + await this.page.getByTestId(`adhoc-filter-add-${filterName}`).click(); + await this.getAdhocConditionEditor(filterName).waitFor({ + state: 'visible', + }); + } + + /** Click a condition's pill and change any part of it in the editor. */ + async editAdhocCondition( + filterName: string, + pillText: string, + changes: { key?: string; operatorLabel?: string; value?: string }, + ) { + await this.getAdhocConditionPills(filterName) + .filter({ hasText: pillText }) + .click(); + await this.fillAdhocConditionEditor(filterName, changes); + } + + /** Fill the open editor's inputs, then apply it. */ + async fillAdhocConditionEditor( + filterName: string, + { + key, + operatorLabel, + value, + }: { key?: string; operatorLabel?: string; value?: string }, + ) { + if (key !== undefined) { + await this.getAdhocConditionInput(filterName, 'key').fill(key); + } + if (operatorLabel !== undefined) { + await this.getAdhocConditionInput(filterName, 'operator').click(); + await this.getAdhocConditionOption(filterName, operatorLabel).click(); + } + const valueInput = this.getAdhocConditionInput(filterName, 'value'); + if (value !== undefined) { + await valueInput.fill(value); + } + // Enter applies. Escape would close the whole popover, and the value + // suggestions can cover the Apply button. + await valueInput.press('Enter'); + await this.getAdhocConditionEditor(filterName).waitFor({ state: 'hidden' }); + } + /** * The options offered by the dashboard filter dropdown that is currently * open, in render order. Scoped to visible nodes: a just-closed dropdown's diff --git a/packages/app/tests/e2e/utils/filters-param.ts b/packages/app/tests/e2e/utils/filters-param.ts index 17a26c7851..a131150672 100644 --- a/packages/app/tests/e2e/utils/filters-param.ts +++ b/packages/app/tests/e2e/utils/filters-param.ts @@ -1,13 +1,19 @@ /** * The `filters=` URL param carries dashboard filter selections in one of two - * shapes: legacy, keyed by SQL expression, or variable-keyed by variable name. - * These helpers are the one definition of that contract for the E2E suite. + * shapes: legacy, keyed by SQL expression, or variable-keyed by variable name + * (selected values, or an ad hoc filter's conditions). These helpers are the + * one definition of that contract for the E2E suite. */ import { expect, type Page } from '@playwright/test'; export type FilterEntry = | { type: 'sql'; condition: string } - | { type: 'variable'; name: string; values: string[] }; + | { type: 'variable'; name: string; values: string[] } + | { + type: 'adhoc'; + name: string; + conditions: { key: string; operator: string; value: string }[]; + }; /** The raw, still-encoded `filters=` param. `null` when the param is absent. */ export const rawFiltersParam = (page: Page): string | null => diff --git a/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts b/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts index 69652d0bea..669c190fa4 100644 --- a/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts +++ b/packages/common-utils/src/__tests__/dashboardFilterValues.test.ts @@ -7,9 +7,11 @@ import { parseDashboardFilterValues, resolveFilterSelection, serializeDashboardFilterValues, + VariableEntryValue, } from '@/dashboardFilterValues'; import { FilterState, filtersToQuery } from '@/filters'; import type { + AdhocFilterCondition, ChartConfigWithOptDateRange, DashboardFilter, DashboardFilterValue, @@ -56,6 +58,17 @@ const promqlFilter = ( ...overrides, }); +const condition: AdhocFilterCondition = { + key: 'ServiceName', + operator: '=', + value: 'api', +}; + +const variableValue = (...values: string[]): VariableEntryValue => ({ + type: 'variable', + values, +}); + const included = (...values: (string | boolean)[]) => ({ included: new Set(values), excluded: new Set(), @@ -79,7 +92,9 @@ describe('dashboardFilterValues', () => { ]); expect(parsed.byExpression).toEqual({ Env: included('prod') }); - expect(Array.from(parsed.byVariable)).toEqual([['svc', ['accounting']]]); + expect(Array.from(parsed.byVariable)).toEqual([ + ['svc', variableValue('accounting')], + ]); expect(parsed.passthrough).toEqual([]); }); @@ -91,8 +106,8 @@ describe('dashboardFilterValues', () => { expect(parsed.byExpression).toEqual({}); expect(Array.from(parsed.byVariable)).toEqual([ - ['svc', ['a', 'b']], - ['env', []], + ['svc', variableValue('a', 'b')], + ['env', variableValue()], ]); }); @@ -114,7 +129,9 @@ describe('dashboardFilterValues', () => { { type: 'variable', name: 'svc', values: ['second'] }, ]); - expect(Array.from(parsed.byVariable)).toEqual([['svc', ['first']]]); + expect(Array.from(parsed.byVariable)).toEqual([ + ['svc', variableValue('first')], + ]); }); it('routes non-sql entries to passthrough', () => { @@ -151,9 +168,14 @@ describe('dashboardFilterValues', () => { { type: 'variable', name: 'env', values: ['prod'] }, ]); + expect(Array.from(parsed.byVariable)).toEqual([ + ['adhoc', { type: 'adhoc', conditions: adhoc.conditions }], + ['env', variableValue('prod')], + ]); + expect(parsed.passthrough).toEqual([]); expect(serializeDashboardFilterValues(parsed)).toEqual([ - { type: 'variable', name: 'env', values: ['prod'] }, adhoc, + { type: 'variable', name: 'env', values: ['prod'] }, ]); }); @@ -189,8 +211,8 @@ describe('dashboardFilterValues', () => { serializeDashboardFilterValues({ byExpression: { Env: included('prod'), Region: included('us') }, byVariable: new Map([ - ['svc', ['accounting']], - ['team', ['platform']], + ['svc', variableValue('accounting')], + ['team', variableValue('platform')], ]), passthrough: [passthrough], }), @@ -210,9 +232,10 @@ describe('dashboardFilterValues', () => { Env: included(), Region: included('us'), }, - byVariable: new Map([ - ['svc', []], - ['team', ['platform']], + byVariable: new Map([ + ['svc', variableValue()], + ['team', variableValue('platform')], + ['conds', { type: 'adhoc', conditions: [] }], ]), }), ).toEqual([ @@ -264,7 +287,7 @@ describe('dashboardFilterValues', () => { 'preserves %s exactly on the variable path', (_label, value) => { const entries = serializeDashboardFilterValues({ - byVariable: new Map([['svc', [value]]]), + byVariable: new Map([['svc', variableValue(value)]]), }); expect(entries).toEqual([ { type: 'variable', name: 'svc', values: [value] }, @@ -275,7 +298,7 @@ describe('dashboardFilterValues', () => { const parsed = parseDashboardFilterValues( JSON.parse(JSON.stringify(entries)), ); - expect(parsed.byVariable.get('svc')).toEqual([value]); + expect(parsed.byVariable.get('svc')).toEqual(variableValue(value)); expect(parsed.byExpression).toEqual({}); expect(parsed.passthrough).toEqual([]); }, @@ -319,7 +342,7 @@ describe('dashboardFilterValues', () => { ]); expect( serializeDashboardFilterValues({ - byVariable: new Map([['svc', ['true']]]), + byVariable: new Map([['svc', variableValue('true')]]), }), ).toEqual([{ type: 'variable', name: 'svc', values: ['true'] }]); }); @@ -430,6 +453,14 @@ describe('dashboardFilterValues', () => { ); }); + it('does not read an ad hoc entry as a selection', () => { + const parsed = parseDashboardFilterValues([ + { type: 'adhoc', name: 'svc', conditions: [condition] }, + ]); + + expect(resolveFilterSelection(variableFilter, parsed)).toBeUndefined(); + }); + it('falls back to the expression entry for back-compat', () => { const parsed = parseDashboardFilterValues([ { type: 'sql', condition: "ServiceName IN ('legacy')" }, diff --git a/packages/common-utils/src/__tests__/filters.test.ts b/packages/common-utils/src/__tests__/filters.test.ts index aff7b0326d..050834af73 100644 --- a/packages/common-utils/src/__tests__/filters.test.ts +++ b/packages/common-utils/src/__tests__/filters.test.ts @@ -28,6 +28,7 @@ import { validateVariableName, } from '@/filters'; import type { + AdhocDashboardFilter, ChartVariable, DashboardFilter, Filter, @@ -1611,14 +1612,30 @@ describe('filters', () => { ...overrides, }); - it('reports the expression and scope for a broadcasting filter', () => { + it('reports the scope for a broadcasting filter', () => { expect(getFilterBroadcastTarget(filter({}))).toEqual({ - expression: 'ServiceName', appliesToSourceIds: undefined, }); expect( getFilterBroadcastTarget(filter({ appliesToSourceIds: ['logs'] })), - ).toEqual({ expression: 'ServiceName', appliesToSourceIds: ['logs'] }); + ).toEqual({ appliesToSourceIds: ['logs'] }); + }); + + it('scopes an ad hoc filter to the sources it applies to', () => { + const adhoc: AdhocDashboardFilter = { + id: 'f4', + type: 'ADHOC', + name: 'Conditions', + sourceType: 'sql', + sources: ['logs', 'traces'], + isVariableEnabled: true, + }; + expect(getFilterBroadcastTarget(adhoc)).toEqual({ + appliesToSourceIds: ['logs', 'traces'], + }); + expect( + getFilterBroadcastTarget({ ...adhoc, isBroadcastEnabled: false }), + ).toBeUndefined(); }); it('returns undefined when broadcasting is off', () => { diff --git a/packages/common-utils/src/clickhouse/__tests__/index.test.ts b/packages/common-utils/src/clickhouse/__tests__/index.test.ts index 69e70a306d..50b67a3dbe 100644 --- a/packages/common-utils/src/clickhouse/__tests__/index.test.ts +++ b/packages/common-utils/src/clickhouse/__tests__/index.test.ts @@ -3,6 +3,7 @@ import { convertCHDataTypeToJSType, extractColumnReferencesFromKey, isAccessDeniedError, + isJSDataTypeScalar, isMissingColumnError, JSDataType, } from '@/clickhouse'; @@ -182,6 +183,28 @@ describe('convertCHDataTypeToJSType', () => { }); }); +describe('isJSDataTypeScalar', () => { + it.each([ + JSDataType.String, + JSDataType.Number, + JSDataType.Bool, + JSDataType.Date, + JSDataType.Dynamic, + null, + ])('treats %s as scalar', dataType => { + expect(isJSDataTypeScalar(dataType)).toBe(true); + }); + + it.each([ + JSDataType.Map, + JSDataType.JSON, + JSDataType.Array, + JSDataType.Tuple, + ])('treats %s as non-scalar', dataType => { + expect(isJSDataTypeScalar(dataType)).toBe(false); + }); +}); + // logQuery is protected on the base class; this subclass exposes it so the // tests can drive it without reaching through the client's public query path. class TestClickhouseClient extends ClickhouseClient { diff --git a/packages/common-utils/src/clickhouse/index.ts b/packages/common-utils/src/clickhouse/index.ts index 0c6dcde01a..2a9f3d2054 100644 --- a/packages/common-utils/src/clickhouse/index.ts +++ b/packages/common-utils/src/clickhouse/index.ts @@ -170,6 +170,21 @@ export const isJSDataTypeJSONStringifiable = ( ); }; +/** Types that hold structure rather than a filterable value. */ +const NON_SCALAR_JS_DATA_TYPES = [ + JSDataType.Map, + JSDataType.JSON, + JSDataType.Array, + JSDataType.Tuple, +]; + +/** + * Unlike !isJSDataTypeJSONStringifiable, Dynamic (JSON subfields) and + * unrecognized (null) types count as scalar. + */ +export const isJSDataTypeScalar = (dataType: JSDataType | null) => + dataType == null || !NON_SCALAR_JS_DATA_TYPES.includes(dataType); + export const extractInnerCHArrayJSType = ( dataType: string, ): JSDataType | null => { diff --git a/packages/common-utils/src/dashboardFilterValues.ts b/packages/common-utils/src/dashboardFilterValues.ts index f060de0ee3..33b5f6180f 100644 --- a/packages/common-utils/src/dashboardFilterValues.ts +++ b/packages/common-utils/src/dashboardFilterValues.ts @@ -5,6 +5,7 @@ import { getDashboardVariableFilters, getFilterExpression, getFilterVariableName, + isAdhocFilterValue, isFilterGlobalRequirement, isFilterRequired, isFilterVariableEnabled, @@ -12,6 +13,7 @@ import { } from '@/filters'; import { isMissingFiltersMacro } from '@/macros'; import { + AdhocFilterValue, ChartConfigWithOptDateRange, DashboardFilter, DashboardFilterValue, @@ -27,6 +29,7 @@ import { isFilterSingleSelect } from '@/variables'; * * - legacy, keyed by SQL expression: `{ type: 'sql', condition: "E IN ('a')" }` * - variable-keyed: `{ type: 'variable', name: 'svc', values: ['a'] }` + * `{ type: 'adhoc', name: 'conds', conditions: [...] }` * * Expression keying cannot represent two filters that share an `expression`, * and has nowhere to put a selection for a filter with no expression at all. @@ -39,11 +42,16 @@ import { isFilterSingleSelect } from '@/variables'; /** One filter's selection: what a `FilterState` holds per key. */ export type FilterSelection = FilterState[string]; +/** A variable-keyed entry, without the name it is keyed by. */ +export type VariableEntryValue = + | Omit + | Omit; + export type ParsedDashboardFilterValues = { /** Selections addressed by SQL expression, parsed via `parseQuery`. */ byExpression: FilterState; - /** Selections addressed by dashboard variable name. */ - byVariable: Map; + /** Selected values and ad hoc conditions addressed by dashboard variable name. */ + byVariable: Map; /** * Non-`sql` entries (`lucene`, `sql_ast`), carried verbatim so a write doesn't * destroy them. @@ -59,15 +67,16 @@ const isVariableEntry = ( export function parseDashboardFilterValues( entries: DashboardFilterValue[] | undefined, ): ParsedDashboardFilterValues { - const byVariable = new Map(); + const byVariable = new Map(); const passthrough: DashboardFilterValue[] = []; const sqlEntries: Filter[] = []; for (const entry of entries ?? []) { - if (isVariableEntry(entry)) { + if (isVariableEntry(entry) || isAdhocFilterValue(entry)) { // There shouldn't be duplicate names, but if there are the first wins. if (!byVariable.has(entry.name)) { - byVariable.set(entry.name, entry.values); + const { name, ...value } = entry; + byVariable.set(name, value); } continue; } @@ -99,7 +108,7 @@ export function parseDashboardFilterValues( */ export function serializeDashboardFilterValues(input: { byExpression?: FilterState; - byVariable?: ReadonlyMap; + byVariable?: ReadonlyMap; passthrough?: DashboardFilterValue[]; }): DashboardFilterValue[] { const entries: DashboardFilterValue[] = [ @@ -107,9 +116,13 @@ export function serializeDashboardFilterValues(input: { ...filtersToQuery(input.byExpression ?? {}, { stringifyKeys: false }), ]; - for (const [name, values] of input.byVariable ?? []) { - if (values.length === 0) continue; // Empty selections are omitted. - entries.push({ type: 'variable', name, values }); + for (const [name, value] of input.byVariable ?? []) { + // Empty selections are omitted. + const isEmpty = + value.type === 'adhoc' + ? value.conditions.length === 0 + : value.values.length === 0; + if (!isEmpty) entries.push({ name, ...value }); } entries.push(...(input.passthrough ?? [])); @@ -149,7 +162,7 @@ export function filterSelectionKey( export function resolveFilterSelection( filter: DashboardFilter, parsed: Pick & { - byVariable: ReadonlyMap; + byVariable: ReadonlyMap; }, ): FilterSelection | undefined { const selection = resolveUncappedFilterSelection(filter, parsed); @@ -165,15 +178,15 @@ export function resolveFilterSelection( function resolveUncappedFilterSelection( filter: DashboardFilter, parsed: Pick & { - byVariable: ReadonlyMap; + byVariable: ReadonlyMap; }, ): FilterSelection | undefined { const key = filterSelectionKey(filter); if (key.kind === 'variable') { const values = parsed.byVariable.get(key.name); - if (values) { + if (values?.type === 'variable') { return { - included: new Set(values), + included: new Set(values.values), excluded: new Set(), }; } diff --git a/packages/common-utils/src/filters.ts b/packages/common-utils/src/filters.ts index 0ad3dc1ae6..9fa6b71c35 100644 --- a/packages/common-utils/src/filters.ts +++ b/packages/common-utils/src/filters.ts @@ -950,16 +950,16 @@ export function getFilterExpression( return isQueryExpressionFilter(filter) ? filter.expression : undefined; } -/** What the filter broadcasts, or undefined when it cannot broadcast. */ +/** Where the filter broadcasts, or undefined when it cannot broadcast. */ export function getFilterBroadcastTarget( filter: DashboardFilter, -): { expression: string; appliesToSourceIds?: string[] } | undefined { - if (!isQueryExpressionFilter(filter) || !isFilterBroadcastEnabled(filter)) - return undefined; - return { - expression: filter.expression, - appliesToSourceIds: filter.appliesToSourceIds, - }; +): { appliesToSourceIds?: string[] } | undefined { + if (!isFilterBroadcastEnabled(filter)) return undefined; + if (isAdhocFilter(filter)) { + return { appliesToSourceIds: getAdhocFilterAppliesToSourceIds(filter) }; + } + if (!isQueryExpressionFilter(filter)) return undefined; + return { appliesToSourceIds: filter.appliesToSourceIds }; } /**