From 735954386f81f9761aa481785151ca13cea74a5a Mon Sep 17 00:00:00 2001 From: Tom Ratcliffe Date: Tue, 13 Aug 2024 12:56:13 +0100 Subject: [PATCH] Alerting: Consolidate contact points dropdown and add filter in alert rules (#91690) Co-authored-by: Gilles De Mey Co-authored-by: Konrad Lalik Co-authored-by: Sonia Aguilar --- .betterer.results | 3 - .../unified/NotificationPolicies.test.tsx | 6 +- .../alerting/unified/NotificationPolicies.tsx | 4 +- .../alert-groups/AlertGroupFilter.tsx | 1 + .../contact-points/ContactPoint.tsx | 30 +++- .../ContactPointSelector.tsx | 133 +++++++++++----- .../EditDefaultPolicyForm.test.tsx | 41 +++-- .../EditDefaultPolicyForm.tsx | 29 ++-- .../EditNotificationPolicyForm.test.tsx | 27 ++-- .../EditNotificationPolicyForm.tsx | 41 ++--- .../notification-policies/Filters.tsx | 37 ++--- .../notification-policies/Modals.tsx | 20 +-- .../components/notification-policies/utils.ts | 19 +++ .../grafanaAppReceivers/grafanaApp.ts | 40 ----- .../receivers/grafanaAppReceivers/types.ts | 6 - .../simplifiedRouting/AlertManagerRouting.tsx | 30 +--- .../contactPoint/ContactPointSelector.tsx | 148 +++--------------- .../unified/components/rules/RulesFilter.tsx | 45 +++++- .../unified/hooks/useFilteredRules.ts | 22 ++- .../mocks/server/handlers/alertmanagers.ts | 21 ++- .../unified/search/rulesSearchParser.test.ts | 20 ++- .../unified/search/rulesSearchParser.ts | 6 + .../alerting/unified/search/search.grammar | 7 +- .../alerting/unified/search/search.js | 35 +++-- .../alerting/unified/search/search.terms.js | 6 +- .../alerting/unified/search/searchParser.ts | 2 + .../alerting/unified/utils/amroutes.ts | 13 -- public/locales/en-US/grafana.json | 3 + public/locales/pseudo-LOCALE/grafana.json | 3 + 29 files changed, 378 insertions(+), 420 deletions(-) create mode 100644 public/app/features/alerting/unified/components/notification-policies/utils.ts delete mode 100644 public/app/features/alerting/unified/components/receivers/grafanaAppReceivers/grafanaApp.ts diff --git a/.betterer.results b/.betterer.results index 576dfc83f2e..f6d6df2a211 100644 --- a/.betterer.results +++ b/.betterer.results @@ -1769,9 +1769,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] ], - "public/app/features/alerting/unified/components/notification-policies/ContactPointSelector.tsx:5381": [ - [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] - ], "public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] diff --git a/public/app/features/alerting/unified/NotificationPolicies.test.tsx b/public/app/features/alerting/unified/NotificationPolicies.test.tsx index 466e034cd4c..f9cc962e05e 100644 --- a/public/app/features/alerting/unified/NotificationPolicies.test.tsx +++ b/public/app/features/alerting/unified/NotificationPolicies.test.tsx @@ -4,6 +4,7 @@ import { byLabelText, byRole, byTestId, byText } from 'testing-library-selector' import { DataSourceSrv, setDataSourceSrv } from '@grafana/runtime'; import { contextSrv } from 'app/core/services/context_srv'; +import { setupMswServer } from 'app/features/alerting/unified/mockApi'; import { AlertManagerCortexConfig, AlertManagerDataSourceJsonData, @@ -19,7 +20,6 @@ import NotificationPolicies, { findRoutesMatchingFilters } from './NotificationP import { fetchAlertManagerConfig, fetchStatus, updateAlertManagerConfig } from './api/alertmanager'; import { alertmanagerApi } from './api/alertmanagerApi'; import { discoverAlertmanagerFeatures } from './api/buildInfo'; -import * as grafanaApp from './components/receivers/grafanaAppReceivers/grafanaApp'; import { MockDataSourceSrv, mockDataSource, someCloudAlertManagerConfig, someCloudAlertManagerStatus } from './mocks'; import { defaultGroupBy } from './utils/amroutes'; import { getAllDataSources } from './utils/config'; @@ -45,7 +45,8 @@ const mocks = { }, contextSrv: jest.mocked(contextSrv), }; -const useGetGrafanaReceiverTypeCheckerMock = jest.spyOn(grafanaApp, 'useGetGrafanaReceiverTypeChecker'); + +setupMswServer(); const renderNotificationPolicies = (alertManagerSourceName?: string) => { return render(, { @@ -195,7 +196,6 @@ describe('NotificationPolicies', () => { mocks.contextSrv.evaluatePermission.mockImplementation(() => []); mocks.api.discoverAlertmanagerFeatures.mockResolvedValue({ lazyConfigInit: false }); setDataSourceSrv(new MockDataSourceSrv(dataSources)); - useGetGrafanaReceiverTypeCheckerMock.mockReturnValue(() => undefined); }); afterEach(() => { diff --git a/public/app/features/alerting/unified/NotificationPolicies.tsx b/public/app/features/alerting/unified/NotificationPolicies.tsx index e58f634d258..ba1f5025330 100644 --- a/public/app/features/alerting/unified/NotificationPolicies.tsx +++ b/public/app/features/alerting/unified/NotificationPolicies.tsx @@ -193,10 +193,9 @@ const AmRoutes = () => { } // edit, add, delete modals - const [addModal, openAddModal, closeAddModal] = useAddPolicyModal(receivers, handleAdd, updatingTree); + const [addModal, openAddModal, closeAddModal] = useAddPolicyModal(handleAdd, updatingTree); const [editModal, openEditModal, closeEditModal] = useEditPolicyModal( selectedAlertmanager ?? '', - receivers, handleSave, updatingTree ); @@ -253,7 +252,6 @@ const AmRoutes = () => { {rootRoute && ( { groupBy: null, queryString: null, alertState: null, + contactPoint: null, }); setTimeout(() => setFilterKey(filterKey + 1), 100); }; diff --git a/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx b/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx index f8b353a1059..60d2663d918 100644 --- a/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx +++ b/public/app/features/alerting/unified/components/contact-points/ContactPoint.tsx @@ -8,7 +8,6 @@ import { Trans } from 'app/core/internationalization'; import { PrimaryText } from 'app/features/alerting/unified/components/common/TextVariants'; import { ContactPointHeader } from 'app/features/alerting/unified/components/contact-points/ContactPointHeader'; import { receiverTypeNames } from 'app/plugins/datasource/alertmanager/consts'; -import { GrafanaManagedReceiverConfig } from 'app/plugins/datasource/alertmanager/types'; import { GrafanaNotifierType, NotifierStatus } from 'app/types/alerting'; import { INTEGRATION_ICONS } from '../../types/contact-points'; @@ -152,37 +151,56 @@ interface ContactPointReceiverMetadata { } type ContactPointReceiverSummaryProps = { - receivers: GrafanaManagedReceiverConfig[]; + receivers: ReceiverConfigWithMetadata[]; + limit?: number; }; /** * This summary is used when we're dealing with non-Grafana managed alertmanager since they * don't have any metadata worth showing other than a summary of what types are configured for the contact point */ -export const ContactPointReceiverSummary = ({ receivers }: ContactPointReceiverSummaryProps) => { +export const ContactPointReceiverSummary = ({ receivers, limit }: ContactPointReceiverSummaryProps) => { + // limit for how many integrations are rendered + const INTEGRATIONS_LIMIT = limit ?? Number.MAX_VALUE; const countByType = groupBy(receivers, (receiver) => receiver.type); + const numberOfUniqueIntegrations = size(countByType); + const integrationsShown = Object.entries(countByType).slice(0, INTEGRATIONS_LIMIT); + const numberOfIntegrationsNotShown = numberOfUniqueIntegrations - INTEGRATIONS_LIMIT; + return ( - {Object.entries(countByType).map(([type, receivers], index) => { + {integrationsShown.map(([type, receivers], index) => { const iconName = INTEGRATION_ICONS[type]; const receiverName = receiverTypeNames[type] ?? upperFirst(type); const isLastItem = size(countByType) - 1 === index; + // Pick the first integration of the grouped receivers, since they should all be the same type + // e.g. if we have multiple Oncall, they _should_ all have the same plugin metadata, + // so we can just use the first one for additional display purposes + const receiver = receivers[0]; return ( + {receiver[RECEIVER_PLUGIN_META_KEY]?.icon && ( + {receiver[RECEIVER_PLUGIN_META_KEY]?.title} + )} {iconName && } - + {receiverName} {receivers.length > 1 && receivers.length} - + {!isLastItem && '⋅'} ); })} + {numberOfIntegrationsNotShown > 0 && {`+${numberOfIntegrationsNotShown} more`}} ); diff --git a/public/app/features/alerting/unified/components/notification-policies/ContactPointSelector.tsx b/public/app/features/alerting/unified/components/notification-policies/ContactPointSelector.tsx index 29d496d1eec..bc0a9a6ec0f 100644 --- a/public/app/features/alerting/unified/components/notification-policies/ContactPointSelector.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/ContactPointSelector.tsx @@ -1,52 +1,115 @@ -import { SelectableValue } from '@grafana/data'; -import { Select, SelectCommonProps, Text, Stack } from '@grafana/ui'; +import { css, cx, keyframes } from '@emotion/css'; +import { useMemo, useState } from 'react'; + +import { GrafanaTheme2, SelectableValue } from '@grafana/data'; +import { Select, SelectCommonProps, Stack, Alert, IconButton, Text, useStyles2 } from '@grafana/ui'; +import { ContactPointReceiverSummary } from 'app/features/alerting/unified/components/contact-points/ContactPoint'; import { useAlertmanager } from 'app/features/alerting/unified/state/AlertmanagerContext'; -import { RECEIVER_META_KEY, RECEIVER_PLUGIN_META_KEY } from '../contact-points/constants'; import { useContactPointsWithStatus } from '../contact-points/useContactPoints'; -import { ReceiverConfigWithMetadata } from '../contact-points/utils'; +import { ContactPointWithMetadata } from '../contact-points/utils'; -export const ContactPointSelector = (props: SelectCommonProps) => { +const MAX_CONTACT_POINTS_RENDERED = 500; + +// Mock sleep method, as fetching receivers is very fast and may seem like it hasn't occurred +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); +const LOADING_SPINNER_DURATION = 1000; + +type ContactPointSelectorProps = { + selectProps: SelectCommonProps; + showRefreshButton?: boolean; + /** Name of a contact point to optionally find and set as the preset value on the dropdown */ + selectedContactPointName?: string | null; +}; + +export const ContactPointSelector = ({ + selectProps, + showRefreshButton, + selectedContactPointName, +}: ContactPointSelectorProps) => { const { selectedAlertmanager } = useAlertmanager(); - const { contactPoints, isLoading, error } = useContactPointsWithStatus({ alertmanager: selectedAlertmanager! }); + const { contactPoints, isLoading, error, refetch } = useContactPointsWithStatus({ + alertmanager: selectedAlertmanager!, + }); + const [loaderSpinning, setLoaderSpinning] = useState(false); + const styles = useStyles2(getStyles); - // TODO error handling - if (error) { - return Failed to load contact points; - } - - const options: Array> = contactPoints.map((contactPoint) => { + const options: Array> = contactPoints.map((contactPoint) => { return { label: contactPoint.name, - value: contactPoint.name, - component: () => , + value: contactPoint, + component: () => ( + + + + ), }; }); - return MAX_CONTACT_POINTS_RENDERED} + options={options} + value={matchedContactPoint} + {...selectProps} + isLoading={isLoading} + /> + {showRefreshButton && ( + + )} ); }; + +const rotation = keyframes({ + from: { + transform: 'rotate(0deg)', + }, + to: { + transform: 'rotate(720deg)', + }, +}); + +const getStyles = (theme: GrafanaTheme2) => ({ + refreshButton: css({ + color: theme.colors.text.secondary, + cursor: 'pointer', + borderRadius: theme.shape.radius.circle, + overflow: 'hidden', + }), + loading: css({ + pointerEvents: 'none', + [theme.transitions.handleMotion('no-preference')]: { + animation: `${rotation} 2s infinite linear`, + }, + [theme.transitions.handleMotion('reduce')]: { + animation: `${rotation} 6s infinite linear`, + }, + }), +}); diff --git a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx index 3c16817ebb2..e7ed08151a5 100644 --- a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.test.tsx @@ -3,12 +3,14 @@ import { render } from 'test/test-utils'; import { byRole } from 'testing-library-selector'; import { Button } from '@grafana/ui'; +import { setupMswServer } from 'app/features/alerting/unified/mockApi'; +import { grantUserPermissions } from 'app/features/alerting/unified/mocks'; +import { AlertmanagerProvider } from 'app/features/alerting/unified/state/AlertmanagerContext'; +import { AccessControlAction } from 'app/types'; import { RouteWithID } from '../../../../../plugins/datasource/alertmanager/types'; -import * as grafanaApp from '../../components/receivers/grafanaAppReceivers/grafanaApp'; import { FormAmRoute } from '../../types/amroutes'; import { GRAFANA_RULES_SOURCE_NAME } from '../../utils/datasource'; -import { AmRouteReceiver } from '../receivers/grafanaAppReceivers/types'; import { AmRootRouteForm } from './EditDefaultPolicyForm'; @@ -20,12 +22,15 @@ const ui = { groupIntervalInput: byRole('textbox', { name: /Group interval/ }), repeatIntervalInput: byRole('textbox', { name: /Repeat interval/ }), }; - -const useGetGrafanaReceiverTypeCheckerMock = jest.spyOn(grafanaApp, 'useGetGrafanaReceiverTypeChecker'); -useGetGrafanaReceiverTypeCheckerMock.mockReturnValue(() => undefined); - +setupMswServer(); // TODO Default and Notification policy form should be unified so we don't need to maintain two almost identical forms describe('EditDefaultPolicyForm', function () { + beforeEach(() => { + grantUserPermissions([ + AccessControlAction.AlertingNotificationsRead, + AccessControlAction.AlertingNotificationsWrite, + ]); + }); describe('Timing options', function () { it('should render prometheus duration strings in form inputs', async function () { const { user } = renderRouteForm({ @@ -47,7 +52,6 @@ describe('EditDefaultPolicyForm', function () { id: '0', receiver: 'default', }, - [{ value: 'default', label: 'Default' }], onSubmit ); @@ -78,7 +82,6 @@ describe('EditDefaultPolicyForm', function () { id: '0', receiver: 'default', }, - [{ value: 'default', label: 'Default' }], onSubmit ); @@ -105,7 +108,6 @@ describe('EditDefaultPolicyForm', function () { group_interval: '2d4h30m35s', repeat_interval: '1w2d6h', }, - [{ value: 'default', label: 'Default' }], onSubmit ); @@ -128,18 +130,15 @@ describe('EditDefaultPolicyForm', function () { }); }); -function renderRouteForm( - route: RouteWithID, - receivers: AmRouteReceiver[] = [], - onSubmit: (route: Partial) => void = noop -) { +function renderRouteForm(route: RouteWithID, onSubmit: (route: Partial) => void = noop) { return render( - Update default policy} - onSubmit={onSubmit} - receivers={receivers} - route={route} - /> + + Update default policy} + onSubmit={onSubmit} + route={route} + /> + ); } diff --git a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx index 5f75bfcde1c..bfba851da73 100644 --- a/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/EditDefaultPolicyForm.tsx @@ -1,7 +1,9 @@ import { ReactNode, useState } from 'react'; import { useForm, Controller } from 'react-hook-form'; -import { Collapse, Field, Link, MultiSelect, Select, useStyles2 } from '@grafana/ui'; +import { Collapse, Field, Link, MultiSelect, useStyles2 } from '@grafana/ui'; +import { ContactPointSelector } from 'app/features/alerting/unified/components/notification-policies/ContactPointSelector'; +import { handleContactPointSelect } from 'app/features/alerting/unified/components/notification-policies/utils'; import { RouteWithID } from 'app/plugins/datasource/alertmanager/types'; import { FormAmRoute } from '../../types/amroutes'; @@ -9,14 +11,12 @@ import { amRouteToFormAmRoute, commonGroupByOptions, mapMultiSelectValueToStrings, - mapSelectValueToString, promDurationValidator, repeatIntervalValidator, stringsToSelectableValues, stringToSelectableValue, } from '../../utils/amroutes'; import { makeAMLink } from '../../utils/misc'; -import { AmRouteReceiver } from '../receivers/grafanaAppReceivers/types'; import { PromDurationInput } from './PromDurationInput'; import { getFormStyles } from './formStyles'; @@ -26,17 +26,10 @@ export interface AmRootRouteFormProps { alertManagerSourceName: string; actionButtons: ReactNode; onSubmit: (route: Partial) => void; - receivers: AmRouteReceiver[]; route: RouteWithID; } -export const AmRootRouteForm = ({ - actionButtons, - alertManagerSourceName, - onSubmit, - receivers, - route, -}: AmRootRouteFormProps) => { +export const AmRootRouteForm = ({ actionButtons, alertManagerSourceName, onSubmit, route }: AmRootRouteFormProps) => { const styles = useStyles2(getFormStyles); const [isTimingOptionsExpanded, setIsTimingOptionsExpanded] = useState(false); const [groupByOptions, setGroupByOptions] = useState(stringsToSelectableValues(route.group_by)); @@ -62,13 +55,13 @@ export const AmRootRouteForm = ({ <>
( - onChange(mapSelectValueToString(value))} - options={receiversWithOnCallOnTop} - isClearable + render={({ field: { onChange, ref, value, ...field } }) => ( + handleContactPointSelect(value, onChange), + isClearable: true, + }} + selectedContactPointName={value} /> )} control={control} @@ -298,14 +289,6 @@ export const AmRoutesExpandedForm = ({ ); }; -function onCallFirst(receiver: AmRouteReceiver) { - if (receiver.grafanaAppReceiverType === SupportedPlugin.OnCall) { - return -1; - } else { - return 0; - } -} - const getStyles = (theme: GrafanaTheme2) => { const commonSpacing = theme.spacing(3.5); diff --git a/public/app/features/alerting/unified/components/notification-policies/Filters.tsx b/public/app/features/alerting/unified/components/notification-policies/Filters.tsx index 4989cf15a83..5fb6c57459e 100644 --- a/public/app/features/alerting/unified/components/notification-policies/Filters.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/Filters.tsx @@ -2,9 +2,9 @@ import { css } from '@emotion/css'; import { debounce, isEqual } from 'lodash'; import { useCallback, useEffect, useRef } from 'react'; -import { SelectableValue } from '@grafana/data'; -import { Button, Field, Icon, Input, Label, Select, Stack, Text, Tooltip, useStyles2 } from '@grafana/ui'; -import { ObjectMatcher, Receiver, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; +import { Button, Field, Icon, Input, Label, Stack, Text, Tooltip, useStyles2 } from '@grafana/ui'; +import { ContactPointSelector } from 'app/features/alerting/unified/components/notification-policies/ContactPointSelector'; +import { ObjectMatcher, RouteWithID } from 'app/plugins/datasource/alertmanager/types'; import { useURLSearchParams } from '../../hooks/useURLSearchParams'; import { matcherToObjectMatcher } from '../../utils/alertmanager'; @@ -15,14 +15,12 @@ import { } from '../../utils/matchers'; interface NotificationPoliciesFilterProps { - receivers: Receiver[]; onChangeMatchers: (labels: ObjectMatcher[]) => void; onChangeReceiver: (receiver: string | undefined) => void; matchingCount: number; } const NotificationPoliciesFilter = ({ - receivers, onChangeReceiver, onChangeMatchers, matchingCount, @@ -47,12 +45,9 @@ const NotificationPoliciesFilter = ({ if (searchInputRef.current) { searchInputRef.current.value = ''; } - setSearchParams({ contactPoint: undefined, queryString: undefined }); + setSearchParams({ contactPoint: '', queryString: undefined }); }, [setSearchParams]); - const receiverOptions: Array> = receivers.map(toOption); - const selectedContactPoint = receiverOptions.find((option) => option.value === contactPoint) ?? null; - const hasFilters = queryString || contactPoint; let inputValid = Boolean(queryString && queryString.length > 3); @@ -103,16 +98,17 @@ const NotificationPoliciesFilter = ({ /> -