From 5179a830efaafd0eb105ba510fef5b5a526058cc Mon Sep 17 00:00:00 2001 From: Konrad Lalik Date: Thu, 9 Mar 2023 16:24:32 +0100 Subject: [PATCH] Alerting: Add fuzzy search to alert list view (#63931) * Add basic fuzzy search * Add fuzzy search to rule name, group and namespace filters * Add tests * Apply sort order when filtering * Filter rules on Enter instead of onChange * Add minor rule stats performance improvements * Fix tests * Remove unused code, add ufuzzy inline docs * Use form submit to set query string, add debounce docs --- .betterer.results | 3 - .../alerting/unified/RuleList.test.tsx | 12 +- .../features/alerting/unified/RuleList.tsx | 6 +- .../unified/components/rules/RuleStats.tsx | 70 ++++++----- .../unified/components/rules/RulesFilter.tsx | 85 ++++++++------ .../hooks/useCombinedRuleNamespaces.ts | 78 ++++++------- .../unified/hooks/useFilteredRules.test.ts | 52 ++++++--- .../unified/hooks/useFilteredRules.ts | 110 +++++++++++------- 8 files changed, 249 insertions(+), 167 deletions(-) diff --git a/.betterer.results b/.betterer.results index 9ffb427a6de..e06e9ab5725 100644 --- a/.betterer.results +++ b/.betterer.results @@ -2688,9 +2688,6 @@ exports[`better eslint`] = { "public/app/features/alerting/unified/components/rules/RuleDetailsDataSources.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] ], - "public/app/features/alerting/unified/components/rules/RulesFilter.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] - ], "public/app/features/alerting/unified/components/silences/SilencesEditor.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] ], diff --git a/public/app/features/alerting/unified/RuleList.test.tsx b/public/app/features/alerting/unified/RuleList.test.tsx index ca7bd53c184..3c9041b39f9 100644 --- a/public/app/features/alerting/unified/RuleList.test.tsx +++ b/public/app/features/alerting/unified/RuleList.test.tsx @@ -322,6 +322,9 @@ describe('RuleList', () => { const groups = await ui.ruleGroup.findAll(); expect(groups).toHaveLength(2); + + await waitFor(() => expect(groups[0]).toHaveTextContent(/firing|pending|normal/)); + expect(groups[0]).toHaveTextContent('1 firing'); expect(groups[1]).toHaveTextContent('1 firing'); expect(groups[1]).toHaveTextContent('1 pending'); @@ -489,11 +492,12 @@ describe('RuleList', () => { }); await renderRuleList(); + const groups = await ui.ruleGroup.findAll(); expect(groups).toHaveLength(2); const filterInput = ui.rulesFilterInput.get(); - await userEvent.type(filterInput, 'label:foo=bar'); + await userEvent.type(filterInput, 'label:foo=bar{Enter}'); // Input is debounced so wait for it to be visible await waitFor(() => expect(filterInput).toHaveValue('label:foo=bar')); @@ -512,17 +516,17 @@ describe('RuleList', () => { // Check for different label matchers await userEvent.clear(filterInput); - await userEvent.type(filterInput, 'label:foo!=bar label:foo!=baz'); + await userEvent.type(filterInput, 'label:foo!=bar label:foo!=baz{Enter}'); // Group doesn't contain matching labels await waitFor(() => expect(ui.ruleGroup.queryAll()).toHaveLength(1)); await waitFor(() => expect(ui.ruleGroup.get()).toHaveTextContent('group-2')); await userEvent.clear(filterInput); - await userEvent.type(filterInput, 'label:"foo=~b.+"'); + await userEvent.type(filterInput, 'label:"foo=~b.+"{Enter}'); await waitFor(() => expect(ui.ruleGroup.queryAll()).toHaveLength(2)); await userEvent.clear(filterInput); - await userEvent.type(filterInput, 'label:region=US'); + await userEvent.type(filterInput, 'label:region=US{Enter}'); await waitFor(() => expect(ui.ruleGroup.queryAll()).toHaveLength(1)); await waitFor(() => expect(ui.ruleGroup.get()).toHaveTextContent('group-2')); }); diff --git a/public/app/features/alerting/unified/RuleList.tsx b/public/app/features/alerting/unified/RuleList.tsx index feb0c0d79bf..ca3236c7be4 100644 --- a/public/app/features/alerting/unified/RuleList.tsx +++ b/public/app/features/alerting/unified/RuleList.tsx @@ -1,5 +1,5 @@ import { css } from '@emotion/css'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { useLocation } from 'react-router-dom'; import { useAsyncFn, useInterval } from 'react-use'; @@ -42,6 +42,8 @@ const RuleList = withErrorBoundary( const location = useLocation(); const [expandAll, setExpandAll] = useState(false); + const onFilterCleared = useCallback(() => setExpandAll(false), []); + const [queryParams] = useQueryParams(); const { filterState, hasActiveFilters } = useRulesFilter(); @@ -90,7 +92,7 @@ const RuleList = withErrorBoundary( // We show separate indicators for Grafana-managed and Cloud rules - setExpandAll(false)} /> + {!hasNoAlertRulesCreatedYet && ( <>
diff --git a/public/app/features/alerting/unified/components/rules/RuleStats.tsx b/public/app/features/alerting/unified/components/rules/RuleStats.tsx index 0655643d5ca..7a104e44d93 100644 --- a/public/app/features/alerting/unified/components/rules/RuleStats.tsx +++ b/public/app/features/alerting/unified/components/rules/RuleStats.tsx @@ -1,5 +1,6 @@ import pluralize from 'pluralize'; -import React, { FC, Fragment, useMemo } from 'react'; +import React, { FC, Fragment, useState } from 'react'; +import { useDebounce } from 'react-use'; import { Stack } from '@grafana/experimental'; import { Badge } from '@grafana/ui'; @@ -26,39 +27,48 @@ const emptyStats = { export const RuleStats: FC = ({ group, namespaces, includeTotal }) => { const evaluationInterval = group?.interval; - - const calculated = useMemo(() => { - const stats = { ...emptyStats }; - - const calcRule = (rule: CombinedRule) => { - if (rule.promRule && isAlertingRule(rule.promRule)) { - if (isGrafanaRulerRulePaused(rule)) { - stats.paused += 1; + const [calculated, setCalculated] = useState(emptyStats); + + // Performance optimization allowing reducing number of stats calculation + // The problem occurs when we load many data sources. + // Then redux store gets updated multiple times in a pretty short period, triggering calculating stats many times. + // debounce allows to skip calculations which results would be abandoned in milliseconds + useDebounce( + () => { + const stats = { ...emptyStats }; + + const calcRule = (rule: CombinedRule) => { + if (rule.promRule && isAlertingRule(rule.promRule)) { + if (isGrafanaRulerRulePaused(rule)) { + stats.paused += 1; + } + stats[rule.promRule.state] += 1; } - stats[rule.promRule.state] += 1; - } - if (ruleHasError(rule)) { - stats.error += 1; - } - if ( - (rule.promRule && isRecordingRule(rule.promRule)) || - (rule.rulerRule && isRecordingRulerRule(rule.rulerRule)) - ) { - stats.recording += 1; - } - stats.total += 1; - }; + if (ruleHasError(rule)) { + stats.error += 1; + } + if ( + (rule.promRule && isRecordingRule(rule.promRule)) || + (rule.rulerRule && isRecordingRulerRule(rule.rulerRule)) + ) { + stats.recording += 1; + } + stats.total += 1; + }; - if (group) { - group.rules.forEach(calcRule); - } + if (group) { + group.rules.forEach(calcRule); + } - if (namespaces) { - namespaces.forEach((namespace) => namespace.groups.forEach((group) => group.rules.forEach(calcRule))); - } + if (namespaces) { + namespaces.forEach((namespace) => namespace.groups.forEach((group) => group.rules.forEach(calcRule))); + } - return stats; - }, [group, namespaces]); + setCalculated(stats); + }, + 400, + [group, namespaces] + ); const statsComponents: React.ReactNode[] = []; diff --git a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx index 66b0d43f653..e2ec4b7103c 100644 --- a/public/app/features/alerting/unified/components/rules/RulesFilter.tsx +++ b/public/app/features/alerting/unified/components/rules/RulesFilter.tsx @@ -1,6 +1,6 @@ import { css } from '@emotion/css'; -import { debounce } from 'lodash'; -import React, { FormEvent, useState } from 'react'; +import React, { useRef, useState } from 'react'; +import { useForm } from 'react-hook-form'; import { DataSourceInstanceSettings, GrafanaTheme2, SelectableValue } from '@grafana/data'; import { Stack } from '@grafana/experimental'; @@ -54,22 +54,21 @@ interface RulesFilerProps { onFilterCleared?: () => void; } +const RuleStateOptions = Object.entries(PromAlertingRuleState).map(([key, value]) => ({ + label: alertStateToReadable(value), + value, +})); + const RulesFilter = ({ onFilterCleared = () => undefined }: RulesFilerProps) => { + const styles = useStyles2(getStyles); const [queryParams, setQueryParams] = useQueryParams(); + const { filterState, hasActiveFilters, searchQuery, setSearchQuery, updateFilters } = useRulesFilter(); // This key is used to force a rerender on the inputs when the filters are cleared const [filterKey, setFilterKey] = useState(Math.floor(Math.random() * 100)); const dataSourceKey = `dataSource-${filterKey}`; const queryStringKey = `queryString-${filterKey}`; - const { filterState, hasActiveFilters, searchQuery, setSearchQuery, updateFilters } = useRulesFilter(); - - const styles = useStyles2(getStyles); - const stateOptions = Object.entries(PromAlertingRuleState).map(([key, value]) => ({ - label: alertStateToReadable(value), - value, - })); - const handleDataSourceChange = (dataSourceValue: DataSourceInstanceSettings) => { updateFilters({ ...filterState, dataSourceName: dataSourceValue.name }); setFilterKey((key) => key + 1); @@ -80,11 +79,6 @@ const RulesFilter = ({ onFilterCleared = () => undefined }: RulesFilerProps) => setFilterKey((key) => key + 1); }; - const handleQueryStringChange = debounce((e: FormEvent) => { - const target = e.target as HTMLInputElement; - setSearchQuery(target.value); - }, 600); - const handleAlertStateChange = (value: PromAlertingRuleState) => { logInfo(LogMessages.clickingAlertStateFilters); updateFilters({ ...filterState, ruleState: value }); @@ -112,6 +106,10 @@ const RulesFilter = ({ onFilterCleared = () => undefined }: RulesFilerProps) => setTimeout(() => setFilterKey(filterKey + 1), 100); }; + const searchQueryRef = useRef(null); + const { handleSubmit, register } = useForm<{ searchQuery: string }>({ defaultValues: { searchQuery } }); + const { ref, ...rest } = register('searchQuery'); + const searchIcon = ; return (
@@ -130,7 +128,11 @@ const RulesFilter = ({ onFilterCleared = () => undefined }: RulesFilerProps) =>
- +
@@ -147,28 +149,39 @@ const RulesFilter = ({ onFilterCleared = () => undefined }: RulesFilerProps) => - - - Search - }> - - - - - } + onSubmit={handleSubmit((data) => { + setSearchQuery(data.searchQuery); + searchQueryRef.current?.blur(); + })} > - - + + + Search + }> + + + + + } + > + { + ref(e); + searchQueryRef.current = e; + }} + {...rest} + placeholder="Search" + data-testid="search-query-input" + /> + + +
- rulesSources - .map((rulesSource): CombinedRuleNamespace[] => { - const rulesSourceName = isCloudRulesSource(rulesSource) ? rulesSource.name : rulesSource; - const promRules = promRulesResponses[rulesSourceName]?.result; - const rulerRules = rulerRulesResponses[rulesSourceName]?.result; - - const cached = cache.current[rulesSourceName]; - if (cached && cached.promRules === promRules && cached.rulerRules === rulerRules) { - return cached.result; - } - const namespaces: Record = {}; - - // first get all the ruler rules in - Object.entries(rulerRules || {}).forEach(([namespaceName, groups]) => { - const namespace: CombinedRuleNamespace = { - rulesSource, - name: namespaceName, - groups: [], - }; - namespaces[namespaceName] = namespace; - addRulerGroupsToCombinedNamespace(namespace, groups); + return useMemo(() => { + return rulesSources + .map((rulesSource): CombinedRuleNamespace[] => { + const rulesSourceName = isCloudRulesSource(rulesSource) ? rulesSource.name : rulesSource; + const promRules = promRulesResponses[rulesSourceName]?.result; + const rulerRules = rulerRulesResponses[rulesSourceName]?.result; + + const cached = cache.current[rulesSourceName]; + if (cached && cached.promRules === promRules && cached.rulerRules === rulerRules) { + return cached.result; + } + const namespaces: Record = {}; + + // first get all the ruler rules in + Object.entries(rulerRules || {}).forEach(([namespaceName, groups]) => { + const namespace: CombinedRuleNamespace = { + rulesSource, + name: namespaceName, + groups: [], + }; + namespaces[namespaceName] = namespace; + addRulerGroupsToCombinedNamespace(namespace, groups); + }); + + // then correlate with prometheus rules + promRules?.forEach(({ name: namespaceName, groups }) => { + const ns = (namespaces[namespaceName] = namespaces[namespaceName] || { + rulesSource, + name: namespaceName, + groups: [], }); - // then correlate with prometheus rules - promRules?.forEach(({ name: namespaceName, groups }) => { - const ns = (namespaces[namespaceName] = namespaces[namespaceName] || { - rulesSource, - name: namespaceName, - groups: [], - }); + addPromGroupsToCombinedNamespace(ns, groups); + }); - addPromGroupsToCombinedNamespace(ns, groups); - }); - - const result = Object.values(namespaces); + const result = Object.values(namespaces); - cache.current[rulesSourceName] = { promRules, rulerRules, result }; - return result; - }) - .flat(), - [promRulesResponses, rulerRulesResponses, rulesSources] - ); + cache.current[rulesSourceName] = { promRules, rulerRules, result }; + return result; + }) + .flat(); + }, [promRulesResponses, rulerRulesResponses, rulesSources]); } // merge all groups in case of grafana managed, essentially treating namespaces (folders) as groups diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts index a809e148a61..2bda8046d5b 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.test.ts @@ -26,32 +26,37 @@ beforeAll(() => { }); describe('filterRules', function () { - it('should filter out rules by name filter', function () { + // Typos there are deliberate to test the fuzzy search + it.each(['cpu', 'hi usage', 'usge'])('should filter out rules by name filter = "%s"', function (nameFilter) { const rules = [mockCombinedRule({ name: 'High CPU usage' }), mockCombinedRule({ name: 'Memory too low' })]; const ns = mockCombinedRuleNamespace({ groups: [mockCombinedRuleGroup('Resources usage group', rules)], }); - const filtered = filterRules([ns], getFilter({ ruleName: 'cpu' })); + const filtered = filterRules([ns], getFilter({ ruleName: nameFilter })); expect(filtered[0].groups[0].rules).toHaveLength(1); expect(filtered[0].groups[0].rules[0].name).toBe('High CPU usage'); }); - it('should filter out rules by evaluation group name', function () { - const ns = mockCombinedRuleNamespace({ - groups: [ - mockCombinedRuleGroup('Performance group', [mockCombinedRule({ name: 'High CPU usage' })]), - mockCombinedRuleGroup('Availability group', [mockCombinedRule({ name: 'Memory too low' })]), - ], - }); + // Typos there are deliberate to test the fuzzy search + it.each(['availability', 'avialability', 'avail group'])( + 'should filter out rules by evaluation group name = "%s"', + function (groupFilter) { + const ns = mockCombinedRuleNamespace({ + groups: [ + mockCombinedRuleGroup('Performance group', [mockCombinedRule({ name: 'High CPU usage' })]), + mockCombinedRuleGroup('Availability group', [mockCombinedRule({ name: 'Memory too low' })]), + ], + }); - const filtered = filterRules([ns], getFilter({ groupName: 'availability' })); + const filtered = filterRules([ns], getFilter({ groupName: groupFilter })); - expect(filtered[0].groups).toHaveLength(1); - expect(filtered[0].groups[0].rules[0].name).toBe('Memory too low'); - }); + expect(filtered[0].groups).toHaveLength(1); + expect(filtered[0].groups[0].rules[0].name).toBe('Memory too low'); + } + ); it('should filter out rules by label filter', function () { const rules = [ @@ -160,4 +165,25 @@ describe('filterRules', function () { expect(filtered[0].groups[0].rules).toHaveLength(1); expect(filtered[0].groups[0].rules[0].name).toBe('Memory too low'); }); + + // Typos there are deliberate to test the fuzzy search + it.each(['nasa', 'alrt rul', 'nasa ruls'])('should filter out rules by namespace = "%s"', (namespaceFilter) => { + const cpuRule = mockCombinedRule({ name: 'High CPU usage' }); + const memoryRule = mockCombinedRule({ name: 'Memory too low' }); + + const teamEmeaNs = mockCombinedRuleNamespace({ + name: 'EMEA Alerting', + groups: [mockCombinedRuleGroup('CPU group', [cpuRule])], + }); + + const teamNasaNs = mockCombinedRuleNamespace({ + name: 'NASA Alert Rules', + groups: [mockCombinedRuleGroup('Memory group', [memoryRule])], + }); + + const filtered = filterRules([teamEmeaNs, teamNasaNs], getFilter({ namespace: namespaceFilter })); + + expect(filtered[0].groups[0].rules).toHaveLength(1); + expect(filtered[0].groups[0].rules[0].name).toBe('Memory too low'); + }); }); diff --git a/public/app/features/alerting/unified/hooks/useFilteredRules.ts b/public/app/features/alerting/unified/hooks/useFilteredRules.ts index 80666cb91ed..f7b259706a6 100644 --- a/public/app/features/alerting/unified/hooks/useFilteredRules.ts +++ b/public/app/features/alerting/unified/hooks/useFilteredRules.ts @@ -1,3 +1,4 @@ +import uFuzzy from '@leeoniya/ufuzzy'; import produce from 'immer'; import { compact, isEmpty } from 'lodash'; import { useCallback, useEffect, useMemo } from 'react'; @@ -18,8 +19,8 @@ export function useRulesFilter() { const [queryParams, updateQueryParams] = useURLSearchParams(); const searchQuery = queryParams.get('search') ?? ''; - const filterState = getSearchFilterFromQuery(searchQuery); - const hasActiveFilters = Object.values(filterState).some((filter) => !isEmpty(filter)); + const filterState = useMemo(() => getSearchFilterFromQuery(searchQuery), [searchQuery]); + const hasActiveFilters = useMemo(() => Object.values(filterState).some((filter) => !isEmpty(filter)), [filterState]); const updateFilters = useCallback( (newFilter: RulesFilter) => { @@ -76,37 +77,67 @@ export const useFilteredRules = (namespaces: CombinedRuleNamespace[], filterStat return useMemo(() => filterRules(namespaces, filterState), [namespaces, filterState]); }; +// Options details can be found here https://github.com/leeoniya/uFuzzy#options +// The following configuration complies with Damerau-Levenshtein distance +// https://en.wikipedia.org/wiki/Damerau%E2%80%93Levenshtein_distance +const ufuzzy = new uFuzzy({ + intraMode: 1, + intraIns: 1, + intraSub: 1, + intraTrn: 1, + intraDel: 1, +}); + export const filterRules = ( namespaces: CombinedRuleNamespace[], filterState: RulesFilter = { labels: [], freeFormWords: [] } ): CombinedRuleNamespace[] => { - return ( - namespaces - .filter((ns) => - filterState.namespace ? ns.name.toLowerCase().includes(filterState.namespace.toLowerCase()) : true - ) - .filter(({ rulesSource }) => - filterState.dataSourceName && isCloudRulesSource(rulesSource) - ? rulesSource.name === filterState.dataSourceName - : true - ) - // If a namespace and group have rules that match the rules filters then keep them. - .reduce(reduceNamespaces(filterState), [] as CombinedRuleNamespace[]) - ); + let filteredNamespaces = namespaces; + + const dataSourceFilter = filterState.dataSourceName; + if (dataSourceFilter) { + filteredNamespaces = filteredNamespaces.filter(({ rulesSource }) => + isCloudRulesSource(rulesSource) ? rulesSource.name === dataSourceFilter : true + ); + } + + const namespaceFilter = filterState.namespace; + if (namespaceFilter) { + const namespaceHaystack = filteredNamespaces.map((ns) => ns.name); + + const [idxs, info, order] = ufuzzy.search(namespaceHaystack, namespaceFilter); + if (info && order) { + filteredNamespaces = order.map((idx) => filteredNamespaces[info.idx[idx]]); + } else { + filteredNamespaces = idxs.map((idx) => filteredNamespaces[idx]); + } + } + + // If a namespace and group have rules that match the rules filters then keep them. + return filteredNamespaces.reduce(reduceNamespaces(filterState), [] as CombinedRuleNamespace[]); }; -const reduceNamespaces = (filterStateFilters: RulesFilter) => { +const reduceNamespaces = (filterState: RulesFilter) => { return (namespaceAcc: CombinedRuleNamespace[], namespace: CombinedRuleNamespace) => { - const groups = namespace.groups - .filter((g) => - filterStateFilters.groupName ? g.name.toLowerCase().includes(filterStateFilters.groupName.toLowerCase()) : true - ) - .reduce(reduceGroups(filterStateFilters), [] as CombinedRuleGroup[]); + const groupNameFilter = filterState.groupName; + let filteredGroups = namespace.groups; + + if (groupNameFilter) { + const groupsHaystack = filteredGroups.map((g) => g.name); + const [idxs, info, order] = ufuzzy.search(groupsHaystack, groupNameFilter); + if (info && order) { + filteredGroups = order.map((idx) => filteredGroups[info.idx[idx]]); + } else { + filteredGroups = idxs.map((idx) => filteredGroups[idx]); + } + } + + filteredGroups = filteredGroups.reduce(reduceGroups(filterState), [] as CombinedRuleGroup[]); - if (groups.length) { + if (filteredGroups.length) { namespaceAcc.push({ ...namespace, - groups, + groups: filteredGroups, }); } @@ -116,8 +147,22 @@ const reduceNamespaces = (filterStateFilters: RulesFilter) => { // Reduces groups to only groups that have rules matching the filters const reduceGroups = (filterState: RulesFilter) => { + const ruleNameQuery = filterState.ruleName ?? filterState.freeFormWords.join(' '); + return (groupAcc: CombinedRuleGroup[], group: CombinedRuleGroup) => { - const rules = group.rules.filter((rule) => { + let filteredRules = group.rules; + + if (ruleNameQuery) { + const rulesHaystack = filteredRules.map((r) => r.name); + const [idxs, info, order] = ufuzzy.search(rulesHaystack, ruleNameQuery); + if (info && order) { + filteredRules = order.map((idx) => filteredRules[info.idx[idx]]); + } else { + filteredRules = idxs.map((idx) => filteredRules[idx]); + } + } + + filteredRules = filteredRules.filter((rule) => { if (filterState.ruleType && filterState.ruleType !== rule.promRule?.type) { return false; } @@ -127,19 +172,6 @@ const reduceGroups = (filterState: RulesFilter) => { return false; } - const ruleNameLc = rule.name?.toLocaleLowerCase(); - // Free Form Query is used to filter by rule name - if ( - filterState.freeFormWords.length > 0 && - !filterState.freeFormWords.every((w) => ruleNameLc.includes(w.toLocaleLowerCase())) - ) { - return false; - } - - if (filterState.ruleName && !rule.name?.toLocaleLowerCase().includes(filterState.ruleName.toLocaleLowerCase())) { - return false; - } - if (filterState.ruleHealth && rule.promRule) { const ruleHealth = getRuleHealth(rule.promRule.health); return filterState.ruleHealth === ruleHealth; @@ -171,10 +203,10 @@ const reduceGroups = (filterState: RulesFilter) => { return true; }); // Add rules to the group that match the rule list filters - if (rules.length) { + if (filteredRules.length) { groupAcc.push({ ...group, - rules, + rules: filteredRules, }); } return groupAcc;