Skip to content

Commit ef8c3f3

Browse files
authored
Feat: Alerts per check (#1011)
* feat: define types for CheckAlert * feat: add AlertsPerCheck and AlertCard components in Check form * feat: add schema for new alert fields * feat: add useCheckAlert hook and datasource methods to fetch and update alerts * feat: fetch alerts when creating/updating checks and format payload for api * feat: fetch alerts when editing a check, and format response for form * chore: mock API response (only for testing purposes) * fix: filter alerts according to the supported checktypes * fix: make current tests pass - adjust schema validation to make it optional - add test handlers for new alerts requests * fix: remove mocked response for testing with dev API * fix: set correct alert id to payload - Also, invalidate cache for fetching fresh check alerts * fix: remove alert mocks as the API is available in dev * test: add tests for creating a check with alerts per check * fix: remove percentiles dropdown - Instead of grouping by alert type, I'm displaying all alerts as separate ones - Grouping by type didn't allow to specify threshold for different percentiles * refactor: introduce new CheckAlert types: Base, Draft and Published * test: add mocks back temporarily - since the alerts API is no longer in dev, I'm adding the mocks back to be able to test it - This should be improved by #850 * refactor: avoid passing unneeded props to AlertsPerCheck - Also, adding loading and error states * fix: remove emtpy response from listAlertsforCheck * refactor: move predefined alerts to external file - Add specific predefined alerts according to the check type in a single constants file - Display threshold units * fix: tests * fix: rebase conflicts * refactor: display new alerts in list format * chore: remove unneeded AlertCard component * fix: tests * fix: humanize alert labels * fix: tests * feat: add feature flag - To enable the feature, set sm-alerts-per-check=true * fix: review comments * fix: add comment on query key * fix: change layout * feat: add default values for check alerts * fix: address review comments * fix: setting submitting form state when alerts request fails * fix: allow to delete input values * fix: set default value when generating form values * fix: tests * chore: remove checkAlert id concept (#1041) The API is removing the id property for check alerts as they can be identified just by the name * fix: center header and tweak description text * chore: remove mocked data as API is available in dev * fix: avoid setting 0 when there's no threshold value * fix: remove success message when alerts are saved * fix: add validation on percentage values * fix: when alerts load, add them to form defaults This prevents show the unsaved changes warning when there are no changes * fix: address review comments * fix: review comments
1 parent d37081e commit ef8c3f3

File tree

25 files changed

+1038
-25
lines changed

25 files changed

+1038
-25
lines changed
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { CheckAlertFormRecord, CheckAlertType } from 'types';
2+
import { CheckAlertsResponse } from 'datasource/responses.types';
3+
import { ALL_PREDEFINED_ALERTS } from 'components/CheckForm/AlertsPerCheck/AlertsPerCheck.constants';
4+
5+
export function getAlertCheckFormValues(data: CheckAlertsResponse): CheckAlertFormRecord {
6+
return Object.keys(CheckAlertType).reduce<CheckAlertFormRecord>((acc, alertTypeKey) => {
7+
const alertType = CheckAlertType[alertTypeKey as keyof typeof CheckAlertType];
8+
9+
const existingAlert = data.alerts.find((alert) => alert.name.includes(alertType));
10+
11+
if (existingAlert) {
12+
acc[alertType] = {
13+
threshold: existingAlert.threshold,
14+
isSelected: true,
15+
};
16+
} else {
17+
acc[alertType] = {
18+
threshold: ALL_PREDEFINED_ALERTS.find((alert) => alert.type === alertType)?.default || 0,
19+
isSelected: false,
20+
};
21+
}
22+
23+
return acc;
24+
}, {});
25+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
import { CheckAlertDraft, CheckAlertFormRecord, CheckAlertType } from 'types';
2+
3+
export function getAlertsPayload(formValues?: CheckAlertFormRecord, checkId?: number): CheckAlertDraft[] {
4+
if (!checkId || !formValues) {
5+
return [];
6+
}
7+
8+
return Object.entries(formValues).reduce<CheckAlertDraft[]>((alerts, [alertType, alert]) => {
9+
if (alert.isSelected) {
10+
alerts.push({
11+
name: alertType as CheckAlertType,
12+
threshold: alert.threshold!!,
13+
});
14+
}
15+
return alerts;
16+
}, []);
17+
}
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
import React from 'react';
2+
import { Controller, useFormContext } from 'react-hook-form';
3+
import { GrafanaTheme2 } from '@grafana/data';
4+
import { Checkbox, Field, Input, Label, useStyles2 } from '@grafana/ui';
5+
import { css } from '@emotion/css';
6+
7+
import { CheckAlertType, CheckFormValues } from 'types';
8+
9+
import { useCheckFormContext } from '../CheckFormContext/CheckFormContext';
10+
import { PredefinedAlertInterface } from './AlertsPerCheck.constants';
11+
12+
export const AlertItem = ({
13+
alert,
14+
selected,
15+
onSelectionChange,
16+
}: {
17+
alert: PredefinedAlertInterface;
18+
selected: boolean;
19+
onSelectionChange: (type: CheckAlertType) => void;
20+
}) => {
21+
const styles = useStyles2(getStyles);
22+
23+
const { control, formState, getValues } = useFormContext<CheckFormValues>();
24+
const { isFormDisabled } = useCheckFormContext();
25+
26+
const handleToggleAlert = (type: CheckAlertType) => {
27+
onSelectionChange(type);
28+
};
29+
30+
const threshold: number = getValues(`alerts.${alert.type}.threshold`);
31+
const thresholdError = formState.errors?.alerts?.[alert.type]?.threshold?.message;
32+
33+
return (
34+
<div key={alert.type} className={styles.item}>
35+
<div className={styles.itemInfo}>
36+
<Checkbox id={`alert-${alert.type}`} onClick={() => handleToggleAlert(alert.type)} checked={selected} />
37+
<Label htmlFor={`alert-${alert.type}`} className={styles.columnLabel}>
38+
{alert.name}
39+
</Label>
40+
</div>
41+
<div className={styles.thresholdInput}>
42+
<Field
43+
label="Threshold"
44+
htmlFor={`alert-threshold-${alert.type}`}
45+
invalid={!!thresholdError}
46+
error={thresholdError}
47+
>
48+
<Controller
49+
name={`alerts.${alert.type}.threshold`}
50+
control={control}
51+
render={({ field }) => (
52+
<Input
53+
aria-disabled={!selected}
54+
suffix={alert.unit}
55+
type="number"
56+
step="any"
57+
id={`alert-threshold-${alert.type}`}
58+
value={field.value !== undefined ? field.value : threshold}
59+
onChange={(e) => {
60+
const value = e.currentTarget.value;
61+
return field.onChange(value !== '' ? Number(value) : undefined);
62+
}}
63+
width={10}
64+
disabled={!selected || isFormDisabled}
65+
/>
66+
)}
67+
/>
68+
</Field>
69+
</div>
70+
</div>
71+
);
72+
};
73+
74+
const getStyles = (theme: GrafanaTheme2) => ({
75+
item: css({
76+
display: `flex`,
77+
gap: theme.spacing(1),
78+
marginLeft: theme.spacing(1),
79+
}),
80+
81+
itemInfo: css({
82+
display: 'flex',
83+
alignItems: 'center',
84+
gap: theme.spacing(1),
85+
width: '50%',
86+
textWrap: 'wrap',
87+
}),
88+
89+
columnLabel: css({
90+
fontWeight: theme.typography.fontWeightLight,
91+
fontSize: theme.typography.h6.fontSize,
92+
lineHeight: theme.typography.body.lineHeight,
93+
marginBottom: '0',
94+
}),
95+
96+
thresholdInput: css({
97+
marginLeft: '22px',
98+
}),
99+
});
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
import React from 'react';
2+
import { GrafanaTheme2 } from '@grafana/data';
3+
import { Label, Stack, Text, useStyles2 } from '@grafana/ui';
4+
import { css } from '@emotion/css';
5+
6+
import { CheckAlertFormValues, CheckAlertType } from 'types';
7+
8+
import { AlertItem } from './AlertItem';
9+
import { PredefinedAlertInterface } from './AlertsPerCheck.constants';
10+
11+
export const AlertsList = ({
12+
title,
13+
alerts,
14+
selectedAlerts,
15+
onSelectionChange,
16+
}: {
17+
title: string;
18+
alerts: PredefinedAlertInterface[];
19+
selectedAlerts?: Partial<Record<CheckAlertType, CheckAlertFormValues>>;
20+
onSelectionChange: (type: CheckAlertType) => void;
21+
}) => {
22+
const styles = useStyles2(getStyles);
23+
24+
const handleToggleAlert = (type: CheckAlertType) => {
25+
onSelectionChange(type);
26+
};
27+
28+
return (
29+
<div className={styles.column}>
30+
<div className={styles.sectionHeader}>
31+
<Label htmlFor={`header-${title}`} className={styles.headerLabel}>
32+
<Stack>
33+
<Text>{title}</Text>
34+
</Stack>
35+
</Label>
36+
</div>
37+
<div className={styles.list}>
38+
{alerts.map((alert: PredefinedAlertInterface) => (
39+
<AlertItem
40+
key={alert.type}
41+
alert={alert}
42+
selected={!!selectedAlerts?.[alert.type]?.isSelected}
43+
onSelectionChange={() => handleToggleAlert(alert.type)}
44+
/>
45+
))}
46+
</div>
47+
</div>
48+
);
49+
};
50+
51+
const getStyles = (theme: GrafanaTheme2) => ({
52+
column: css({
53+
fontSize: theme.typography.h6.fontSize,
54+
fontWeight: theme.typography.fontWeightLight,
55+
flex: 1,
56+
}),
57+
58+
list: css({
59+
display: 'flex',
60+
flexDirection: 'column',
61+
whiteSpace: 'nowrap',
62+
overflowY: 'auto',
63+
}),
64+
65+
sectionHeader: css({
66+
display: 'flex',
67+
border: `1px solid ${theme.colors.border.weak}`,
68+
backgroundColor: `${theme.colors.background.secondary}`,
69+
padding: theme.spacing(1),
70+
marginTop: theme.spacing(1),
71+
marginBottom: theme.spacing(1),
72+
gap: theme.spacing(1),
73+
verticalAlign: 'middle',
74+
alignItems: 'center',
75+
}),
76+
77+
headerLabel: css({
78+
fontWeight: theme.typography.fontWeightLight,
79+
fontSize: theme.typography.h5.fontSize,
80+
color: theme.colors.text.primary,
81+
marginBottom: 0,
82+
}),
83+
});
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import { CheckAlertCategory, CheckAlertType, CheckType, ThresholdUnit } from 'types';
2+
3+
export interface PredefinedAlertInterface {
4+
type: CheckAlertType;
5+
name: string;
6+
unit: ThresholdUnit;
7+
category: CheckAlertCategory;
8+
default?: number;
9+
}
10+
11+
const GLOBAL_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
12+
{
13+
type: CheckAlertType.ProbeFailedExecutionsTooHigh,
14+
name: 'Probe Failed Executions Too High',
15+
unit: '%',
16+
category: CheckAlertCategory.SystemHealth,
17+
default: 10,
18+
},
19+
];
20+
21+
const HTTP_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
22+
{
23+
type: CheckAlertType.HTTPRequestDurationTooHighP50,
24+
name: 'HTTP Request Duration Too High (P50)',
25+
unit: 'ms',
26+
category: CheckAlertCategory.RequestDuration,
27+
default: 300,
28+
},
29+
{
30+
type: CheckAlertType.HTTPRequestDurationTooHighP90,
31+
name: 'HTTP Request Duration Too High (P90)',
32+
unit: 'ms',
33+
category: CheckAlertCategory.RequestDuration,
34+
default: 500,
35+
},
36+
{
37+
type: CheckAlertType.HTTPRequestDurationTooHighP95,
38+
name: 'HTTP Request Duration Too High (P95)',
39+
unit: 'ms',
40+
category: CheckAlertCategory.RequestDuration,
41+
default: 800,
42+
},
43+
{
44+
type: CheckAlertType.HTTPRequestDurationTooHighP99,
45+
name: 'HTTP Request Duration Too High (P99)',
46+
unit: 'ms',
47+
category: CheckAlertCategory.RequestDuration,
48+
default: 1500,
49+
},
50+
{
51+
type: CheckAlertType.HTTPTargetCertificateCloseToExpiring,
52+
name: 'HTTP Target Certificate Close To Expiring',
53+
unit: 'd',
54+
category: CheckAlertCategory.SystemHealth,
55+
default: 60,
56+
},
57+
];
58+
59+
const PING_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
60+
{
61+
type: CheckAlertType.PingICMPDurationTooHighP50,
62+
name: 'Ping ICMP Duration Too High (P50)',
63+
unit: 'ms',
64+
category: CheckAlertCategory.RequestDuration,
65+
default: 50,
66+
},
67+
{
68+
type: CheckAlertType.PingICMPDurationTooHighP90,
69+
name: 'Ping ICMP Duration Too High (P90)',
70+
unit: 'ms',
71+
category: CheckAlertCategory.RequestDuration,
72+
default: 100,
73+
},
74+
{
75+
type: CheckAlertType.PingICMPDurationTooHighP95,
76+
name: 'Ping ICMP Duration Too High (P95)',
77+
unit: 'ms',
78+
category: CheckAlertCategory.RequestDuration,
79+
default: 200,
80+
},
81+
{
82+
type: CheckAlertType.PingICMPDurationTooHighP99,
83+
name: 'Ping ICMP Duration Too High (P99)',
84+
unit: 'ms',
85+
category: CheckAlertCategory.RequestDuration,
86+
default: 400,
87+
},
88+
];
89+
90+
export const PREDEFINED_ALERTS: Record<CheckType, PredefinedAlertInterface[]> = Object.fromEntries(
91+
Object.values(CheckType).map((checkType) => [
92+
checkType,
93+
[
94+
...GLOBAL_PREDEFINED_ALERTS,
95+
...(checkType === CheckType.HTTP ? HTTP_PREDEFINED_ALERTS : []),
96+
...(checkType === CheckType.PING ? PING_PREDEFINED_ALERTS : []),
97+
],
98+
])
99+
) as Record<CheckType, PredefinedAlertInterface[]>;
100+
101+
export const ALL_PREDEFINED_ALERTS: PredefinedAlertInterface[] = [
102+
...GLOBAL_PREDEFINED_ALERTS,
103+
...HTTP_PREDEFINED_ALERTS,
104+
...PING_PREDEFINED_ALERTS,
105+
];

0 commit comments

Comments
 (0)