Alerting: Simplify notification step (#96430)

* Move evaluation outside folder section, and move labels in instead

* rename file

* update translations

* refactor

* rename file and component

* refactor

* fix test

* refactor

* rename files and components

* update translations

* fix style

* update translations

* Add feature toggle for simplified mode in notifications step

* WIP

* Use useAppNotification for toasts

* update label when group can not be selected yet

* update translations

* WIP

* update some texts and add comment

* update translations

* remove duplicated code

* fix typo

* update translations

* update styles and remove label

* update simplified_notifications_section name according BE changes

* remove FolderWithoutGroup

* remove commented code

* prettier

* remove SIMPLIFIED_NOTIFICATION_STEP_KEY and use MANUAL_ROUTING_KEY instead

* styles cleanup

* Update docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md

Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>

* merge main and prettier doc

---------

Co-authored-by: Gilles De Mey <gilles.de.mey@gmail.com>
This commit is contained in:
Sonia Aguilar 2024-11-22 12:07:45 +01:00 committed by GitHub
parent 68c61514b0
commit 977184b878
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
14 changed files with 98 additions and 62 deletions

View File

@ -1378,8 +1378,7 @@ exports[`better eslint`] = {
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "8"], [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "8"],
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "9"], [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "9"],
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "10"], [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "10"],
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "11"], [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "11"]
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "12"]
], ],
"public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx:5381": [ "public/app/features/alerting/unified/components/rule-editor/PreviewRule.tsx:5381": [
[0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"], [0, 0, 0, "No untranslated strings. Wrap text with <Trans />", "0"],

View File

@ -226,6 +226,7 @@ Experimental features might be changed or removed without prior notice.
| `enableSCIM` | Enables SCIM support for user and group management | | `enableSCIM` | Enables SCIM support for user and group management |
| `crashDetection` | Enables browser crash detection reporting to Faro. | | `crashDetection` | Enables browser crash detection reporting to Faro. |
| `jaegerBackendMigration` | Enables querying the Jaeger data source without the proxy | | `jaegerBackendMigration` | Enables querying the Jaeger data source without the proxy |
| `alertingNotificationsStepMode` | Enables simplified step mode in the notifications section |
## Development feature toggles ## Development feature toggles

View File

@ -241,4 +241,5 @@ export interface FeatureToggles {
jaegerBackendMigration?: boolean; jaegerBackendMigration?: boolean;
reportingUseRawTimeRange?: boolean; reportingUseRawTimeRange?: boolean;
alertingUIOptimizeReducer?: boolean; alertingUIOptimizeReducer?: boolean;
alertingNotificationsStepMode?: boolean;
} }

View File

@ -1666,6 +1666,13 @@ var (
Owner: grafanaAlertingSquad, Owner: grafanaAlertingSquad,
Expression: "true", // enabled by default Expression: "true", // enabled by default
}, },
{
Name: "alertingNotificationsStepMode",
Description: "Enables simplified step mode in the notifications section",
Stage: FeatureStageExperimental,
Owner: grafanaAlertingSquad,
FrontendOnly: true,
},
} }
) )

View File

@ -222,3 +222,4 @@ crashDetection,experimental,@grafana/observability-traces-and-profiling,false,fa
jaegerBackendMigration,experimental,@grafana/oss-big-tent,false,false,false jaegerBackendMigration,experimental,@grafana/oss-big-tent,false,false,false
reportingUseRawTimeRange,preview,@grafana/sharing-squad,false,false,false reportingUseRawTimeRange,preview,@grafana/sharing-squad,false,false,false
alertingUIOptimizeReducer,GA,@grafana/alerting-squad,false,false,true alertingUIOptimizeReducer,GA,@grafana/alerting-squad,false,false,true
alertingNotificationsStepMode,experimental,@grafana/alerting-squad,false,false,true

1 Name Stage Owner requiresDevMode RequiresRestart FrontendOnly
222 jaegerBackendMigration experimental @grafana/oss-big-tent false false false
223 reportingUseRawTimeRange preview @grafana/sharing-squad false false false
224 alertingUIOptimizeReducer GA @grafana/alerting-squad false false true
225 alertingNotificationsStepMode experimental @grafana/alerting-squad false false true

View File

@ -898,4 +898,8 @@ const (
// FlagAlertingUIOptimizeReducer // FlagAlertingUIOptimizeReducer
// Enables removing the reducer from the alerting UI when creating a new alert rule and using instant query // Enables removing the reducer from the alerting UI when creating a new alert rule and using instant query
FlagAlertingUIOptimizeReducer = "alertingUIOptimizeReducer" FlagAlertingUIOptimizeReducer = "alertingUIOptimizeReducer"
// FlagAlertingNotificationsStepMode
// Enables simplified step mode in the notifications section
FlagAlertingNotificationsStepMode = "alertingNotificationsStepMode"
) )

View File

@ -241,6 +241,22 @@
"hideFromAdminPage": true "hideFromAdminPage": true
} }
}, },
{
"metadata": {
"name": "alertingNotificationsStepMode",
"resourceVersion": "1732272457848",
"creationTimestamp": "2024-11-06T09:35:49Z",
"annotations": {
"grafana.app/updatedTimestamp": "2024-11-22 10:47:37.848635 +0000 UTC"
}
},
"spec": {
"description": "Enables simplified step mode in the notifications section",
"stage": "experimental",
"codeowner": "@grafana/alerting-squad",
"frontend": true
}
},
{ {
"metadata": { "metadata": {
"name": "alertingPrometheusRulesPrimary", "name": "alertingPrometheusRulesPrimary",

View File

@ -41,12 +41,13 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => {
const { watch, getValues, setValue } = useFormContext<RuleFormValues>(); const { watch, getValues, setValue } = useFormContext<RuleFormValues>();
const styles = useStyles2(getStyles); const styles = useStyles2(getStyles);
const [type] = watch(['type', 'labels', 'queries', 'condition', 'folder', 'name', 'manualRouting']); const [type, manualRouting] = watch(['type', 'manualRouting']);
const [showLabelsEditor, setShowLabelsEditor] = useState(false); const [showLabelsEditor, setShowLabelsEditor] = useState(false);
const dataSourceName = watch('dataSourceName') ?? GRAFANA_RULES_SOURCE_NAME; const dataSourceName = watch('dataSourceName') ?? GRAFANA_RULES_SOURCE_NAME;
const isGrafanaManaged = isGrafanaManagedRuleByType(type); const isGrafanaManaged = isGrafanaManagedRuleByType(type);
const simplifiedRoutingToggleEnabled = config.featureToggles.alertingSimplifiedRouting ?? false; const simplifiedRoutingToggleEnabled = config.featureToggles.alertingSimplifiedRouting ?? false;
const simplifiedModeInNotificationsStepEnabled = config.featureToggles.alertingNotificationsStepMode ?? false;
const shouldRenderpreview = type === RuleFormType.grafana; const shouldRenderpreview = type === RuleFormType.grafana;
const hasInternalAlertmanagerEnabled = useHasInternalAlertmanagerEnabled(); const hasInternalAlertmanagerEnabled = useHasInternalAlertmanagerEnabled();
@ -66,6 +67,16 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => {
const step = !isGrafanaManaged ? 4 : 5; const step = !isGrafanaManaged ? 4 : 5;
const switchMode =
isGrafanaManaged && simplifiedModeInNotificationsStepEnabled
? {
isAdvancedMode: !manualRouting,
setAdvancedMode: (isAdvanced: boolean) => {
setValue('editorSettings.simplifiedNotificationEditor', !isAdvanced);
setValue('manualRouting', !isAdvanced);
},
}
: undefined;
const title = isRecordingRuleByType(type) const title = isRecordingRuleByType(type)
? 'Add labels' ? 'Add labels'
: isGrafanaManaged : isGrafanaManaged
@ -91,6 +102,7 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => {
)} )}
</Stack> </Stack>
} }
switchMode={switchMode}
fullWidth fullWidth
> >
{!isGrafanaManaged && ( {!isGrafanaManaged && (
@ -106,14 +118,16 @@ export const NotificationsStep = ({ alertUid }: NotificationsStepProps) => {
)} )}
{shouldAllowSimplifiedRouting && ( {shouldAllowSimplifiedRouting && (
<div className={styles.configureNotifications}> <div className={styles.configureNotifications}>
<Text element="h5">Notifications</Text> <Text element="h5">Recipient</Text>
<Text variant="bodySmall" color="secondary">
Select who should receive a notification when an alert rule fires.
</Text>
</div> </div>
)} )}
{shouldAllowSimplifiedRouting ? ( // when simplified routing is enabled and is grafana rule {shouldAllowSimplifiedRouting ? ( // when simplified routing is enabled and is grafana rule
simplifiedModeInNotificationsStepEnabled ? ( // simplified mode is enabled
<ManualAndAutomaticRoutingSimplified alertUid={alertUid} />
) : (
// simplified mode is disabled
<ManualAndAutomaticRouting alertUid={alertUid} /> <ManualAndAutomaticRouting alertUid={alertUid} />
)
) : // when simplified routing is not enabled, render the notification preview as we did before ) : // when simplified routing is not enabled, render the notification preview as we did before
shouldRenderpreview ? ( shouldRenderpreview ? (
<AutomaticRooting alertUid={alertUid} /> <AutomaticRooting alertUid={alertUid} />
@ -167,6 +181,32 @@ function ManualAndAutomaticRouting({ alertUid }: { alertUid?: string }) {
); );
} }
/**
* Preconditions:
* - simplified routing is enabled
* - simple mode for notifications step is enabled
* - the alert rule is a grafana rule
*
* This component will render the switch between the select contact point routing and the notification policy routing.
* It also renders the section body of the NotificationsStep, depending on the routing option selected.
* If select contact point routing is selected, it will render the SimplifiedRouting component.
* If notification policy routing is selected, it will render the AutomaticRouting component.
*
*/
function ManualAndAutomaticRoutingSimplified({ alertUid }: { alertUid?: string }) {
const { watch } = useFormContext<RuleFormValues>();
const [manualRouting] = watch(['manualRouting']);
return (
<Stack direction="column" gap={2}>
<RoutingOptionDescription manualRouting={manualRouting} />
{manualRouting ? <SimplifiedRouting /> : <AutomaticRooting alertUid={alertUid} />}
</Stack>
);
}
interface AutomaticRootingProps { interface AutomaticRootingProps {
alertUid?: string; alertUid?: string;
} }

View File

@ -1,11 +1,8 @@
import { css } from '@emotion/css';
import { compact } from 'lodash'; import { compact } from 'lodash';
import { lazy, Suspense } from 'react'; import { lazy, Suspense } from 'react';
import { GrafanaTheme2 } from '@grafana/data'; import { Button, LoadingPlaceholder, Stack, Text } from '@grafana/ui';
import { Button, LoadingPlaceholder, Text, useStyles2 } from '@grafana/ui';
import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi'; import { alertRuleApi } from 'app/features/alerting/unified/api/alertRuleApi';
import { Stack } from 'app/plugins/datasource/parca/QueryEditor/Stack';
import { AlertQuery } from 'app/types/unified-alerting-dto'; import { AlertQuery } from 'app/types/unified-alerting-dto';
import { Folder, KBObjectArray } from '../../../types/rule-form'; import { Folder, KBObjectArray } from '../../../types/rule-form';
@ -32,7 +29,6 @@ export const NotificationPreview = ({
alertName, alertName,
alertUid, alertUid,
}: NotificationPreviewProps) => { }: NotificationPreviewProps) => {
const styles = useStyles2(getStyles);
const disabled = !condition || !folder; const disabled = !condition || !folder;
const previewEndpoint = alertRuleApi.endpoints.preview; const previewEndpoint = alertRuleApi.endpoints.preview;
@ -66,8 +62,8 @@ export const NotificationPreview = ({
return ( return (
<Stack direction="column"> <Stack direction="column">
<div className={styles.routePreviewHeaderRow}> <Stack direction="row" alignItems="flex-start" justifyContent="space-between">
<div className={styles.previewHeader}> <Stack direction="column" gap={1}>
<Text element="h5">Alert instance routing preview</Text> <Text element="h5">Alert instance routing preview</Text>
{isLoading && previewUninitialized && ( {isLoading && previewUninitialized && (
<Text color="secondary" variant="bodySmall"> <Text color="secondary" variant="bodySmall">
@ -85,13 +81,11 @@ export const NotificationPreview = ({
notification policy below to view more details. notification policy below to view more details.
</Text> </Text>
)} )}
</div> </Stack>
<div className={styles.button}>
<Button icon="sync" variant="secondary" type="button" onClick={onPreview} disabled={disabled}> <Button icon="sync" variant="secondary" type="button" onClick={onPreview} disabled={disabled}>
Preview routing Preview routing
</Button> </Button>
</div> </Stack>
</div>
{!isLoading && !previewUninitialized && potentialInstances.length > 0 && ( {!isLoading && !previewUninitialized && potentialInstances.length > 0 && (
<Suspense fallback={<LoadingPlaceholder text="Loading preview..." />}> <Suspense fallback={<LoadingPlaceholder text="Loading preview..." />}>
{alertManagerDataSources.map((alertManagerSource) => ( {alertManagerDataSources.map((alertManagerSource) => (
@ -107,36 +101,3 @@ export const NotificationPreview = ({
</Stack> </Stack>
); );
}; };
const getStyles = (theme: GrafanaTheme2) => ({
collapsableSection: css({
width: 'auto',
border: 0,
}),
previewHeader: css({
margin: 0,
}),
routePreviewHeaderRow: css({
display: 'flex',
flexDirection: 'row',
justifyContent: 'space-between',
alignItems: 'flex-start',
marginTop: theme.spacing(1),
}),
collapseLabel: css({
flex: 1,
}),
button: css({
justifyContent: 'flex-end',
}),
tagsInDetails: css({
display: 'flex',
justifyContent: 'flex-start',
flexWrap: 'wrap',
}),
policyPathItemMatchers: css({
display: 'flex',
flexDirection: 'row',
gap: theme.spacing(1),
}),
});

View File

@ -484,7 +484,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
return; return;
} }
} }
setValue('editorSettings', { simplifiedQueryEditor: !isAdvanced }); setValue('editorSettings.simplifiedQueryEditor', !isAdvanced);
}, },
} }
: undefined; : undefined;
@ -688,7 +688,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
confirmText="Deactivate" confirmText="Deactivate"
icon="exclamation-triangle" icon="exclamation-triangle"
onConfirm={() => { onConfirm={() => {
setValue('editorSettings', { simplifiedQueryEditor: true }); setValue('editorSettings.simplifiedNotificationEditor', true);
setShowResetModal(false); setShowResetModal(false);
dispatch(resetToSimpleCondition()); dispatch(resetToSimpleCondition());
}} }}

View File

@ -9,7 +9,7 @@ const expressionQueries = [reduceExpression, thresholdExpression];
describe('determineAdvancedMode', () => { describe('determineAdvancedMode', () => {
it('should return true if simplifiedQueryEditor is false', () => { it('should return true if simplifiedQueryEditor is false', () => {
const editorSettings = { simplifiedQueryEditor: false }; const editorSettings = { simplifiedQueryEditor: false, simplifiedNotificationEditor: true };
const isGrafanaAlertingType = true; const isGrafanaAlertingType = true;
const isNewFromQueryParams = false; const isNewFromQueryParams = false;
@ -25,7 +25,7 @@ describe('determineAdvancedMode', () => {
}); });
it('should return true if isGrafanaAlertingType is false', () => { it('should return true if isGrafanaAlertingType is false', () => {
const editorSettings = { simplifiedQueryEditor: true }; const editorSettings = { simplifiedQueryEditor: true, simplifiedNotificationEditor: true };
const isGrafanaAlertingType = false; const isGrafanaAlertingType = false;
const isNewFromQueryParams = false; const isNewFromQueryParams = false;
@ -40,8 +40,8 @@ describe('determineAdvancedMode', () => {
expect(result).toBe(true); expect(result).toBe(true);
}); });
const editorSettings = { simplifiedQueryEditor: true, simplifiedNotificationEditor: true };
it('should return true if isNewFromQueryParams is true and queries are not transformable', () => { it('should return true if isNewFromQueryParams is true and queries are not transformable', () => {
const editorSettings = { simplifiedQueryEditor: true };
const isGrafanaAlertingType = true; const isGrafanaAlertingType = true;
const isNewFromQueryParams = true; const isNewFromQueryParams = true;
@ -61,7 +61,6 @@ describe('determineAdvancedMode', () => {
}); });
it('should return false if all conditions are false', () => { it('should return false if all conditions are false', () => {
const editorSettings = { simplifiedQueryEditor: true };
const isGrafanaAlertingType = true; const isGrafanaAlertingType = true;
const isNewFromQueryParams = false; const isNewFromQueryParams = false;

View File

@ -25,6 +25,7 @@ export interface AlertManagerManualRouting {
export interface SimplifiedEditor { export interface SimplifiedEditor {
simplifiedQueryEditor: boolean; simplifiedQueryEditor: boolean;
simplifiedNotificationEditor: boolean;
} }
export type KVObject = { key: string; value: string }; export type KVObject = { key: string; value: string };

View File

@ -136,10 +136,12 @@ function getDefaultEditorSettings() {
if (!editorSettingsEnabled) { if (!editorSettingsEnabled) {
return undefined; return undefined;
} }
//then, check in local storage if the user has saved last rule with simplified query editor //then, check in local storage if the user has saved last rule with sections simplified
const queryEditorSettings = localStorage.getItem(SIMPLIFIED_QUERY_EDITOR_KEY); const queryEditorSettings = localStorage.getItem(SIMPLIFIED_QUERY_EDITOR_KEY);
const notificationStepSettings = localStorage.getItem(MANUAL_ROUTING_KEY);
return { return {
simplifiedQueryEditor: queryEditorSettings !== 'false', simplifiedQueryEditor: queryEditorSettings !== 'false',
simplifiedNotificationEditor: notificationStepSettings !== 'false',
}; };
} }
@ -229,6 +231,7 @@ export function getNotificationSettingsForDTO(
function getEditorSettingsForDTO(simplifiedEditor: SimplifiedEditor) { function getEditorSettingsForDTO(simplifiedEditor: SimplifiedEditor) {
return { return {
simplified_query_and_expressions_section: simplifiedEditor.simplifiedQueryEditor, simplified_query_and_expressions_section: simplifiedEditor.simplifiedQueryEditor,
simplified_notifications_section: simplifiedEditor.simplifiedNotificationEditor,
}; };
} }
export function formValuesToRulerGrafanaRuleDTO(values: RuleFormValues): PostableRuleGrafanaRuleDTO { export function formValuesToRulerGrafanaRuleDTO(values: RuleFormValues): PostableRuleGrafanaRuleDTO {
@ -348,11 +351,13 @@ function getEditorSettingsFromDTO(ga: GrafanaRuleDefinition) {
if (ga.metadata?.editor_settings) { if (ga.metadata?.editor_settings) {
return { return {
simplifiedQueryEditor: ga.metadata.editor_settings.simplified_query_and_expressions_section, simplifiedQueryEditor: ga.metadata.editor_settings.simplified_query_and_expressions_section,
simplifiedNotificationEditor: ga.metadata.editor_settings.simplified_notifications_section,
}; };
} }
return { return {
simplifiedQueryEditor: false, simplifiedQueryEditor: false,
simplifiedNotificationEditor: Boolean(ga.notification_settings), // in case this rule was created before the new field was added, we'll default to current routing settings
}; };
} }

View File

@ -224,6 +224,7 @@ export interface GrafanaNotificationSettings {
export interface GrafanaEditorSettings { export interface GrafanaEditorSettings {
simplified_query_and_expressions_section: boolean; simplified_query_and_expressions_section: boolean;
simplified_notifications_section: boolean;
} }
export interface PostableGrafanaRuleDefinition { export interface PostableGrafanaRuleDefinition {
uid?: string; uid?: string;