IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2024-12-01 13:09:22 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Fix eval interval not being saved when creating a new group (#93821)

Browse Source
This commit is contained in:
Tom Ratcliffe 2024-09-30 09:19:01 +01:00 committed by GitHub
parent 1aed1d8017
commit e4698d9c52
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 148 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
public/app/features/alerting/unified/RuleEditorExisting.test.tsx
View File

@ -3,44 +3,25 @@ import { ui } from 'test/helpers/alertingRuleEditor';
import { render, screen } from 'test/test-utils';
import { contextSrv } from 'app/core/services/context_srv';
import { DashboardSearchHit, DashboardSearchItemType } from 'app/features/search/types';
import { setFolderResponse } from 'app/features/alerting/unified/mocks/server/configure';
import { captureRequests } from 'app/features/alerting/unified/mocks/server/events';
import { DashboardSearchItemType } from 'app/features/search/types';
import { searchFolders } from '../../../../app/features/manage-dashboards/state/actions';
import { backendSrv } from '../../../core/services/backend_srv';
import { AccessControlAction } from '../../../types';
import RuleEditor from './RuleEditor';
import { ExpressionEditorProps } from './components/rule-editor/ExpressionEditor';
import { setupMswServer } from './mockApi';
import { grantUserPermissions, mockDataSource, mockFolder } from './mocks';
import { grafanaRulerRule } from './mocks/grafanaRulerApi';
import { setupDataSources } from './testSetup/datasources';
import { Annotation } from './utils/constants';
jest.mock('./components/rule-editor/ExpressionEditor', () => ({
ExpressionEditor: ({ value, onChange }: ExpressionEditorProps) => (
<input value={value} data-testid="expr" onChange={(e) => onChange(e.target.value)} />
),
}));
jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({
AppChromeUpdate: ({ actions }: { actions: React.ReactNode }) => <div>{actions}</div>,
}));
jest.mock('../../../../app/features/manage-dashboards/state/actions');
// there's no angular scope in test and things go terribly wrong when trying to render the query editor row.
// lets just skip it
jest.mock('app/features/query/components/QueryEditorRow', () => ({
QueryEditorRow: () => <p>hi</p>,
}));
jest.setTimeout(60 * 1000);
const mocks = {
searchFolders: jest.mocked(searchFolders),
};
setupMswServer();
function renderRuleEditor(identifier: string) {
@ -55,6 +36,24 @@ function renderRuleEditor(identifier: string) {
}
describe('RuleEditor grafana managed rules', () => {
const folder = {
title: 'Folder A',
uid: grafanaRulerRule.grafana_alert.namespace_uid,
id: 1,
type: DashboardSearchItemType.DashDB,
accessControl: {
[AccessControlAction.AlertingRuleUpdate]: true,
},
};
const slashedFolder = {
title: 'Folder with /',
uid: 'abcde',
id: 2,
accessControl: {
[AccessControlAction.AlertingRuleUpdate]: true,
},
};
beforeEach(() => {
jest.clearAllMocks();
contextSrv.isEditor = true;
@ -73,21 +72,6 @@ describe('RuleEditor grafana managed rules', () => {
AccessControlAction.AlertingRuleExternalRead,
AccessControlAction.AlertingRuleExternalWrite,
]);
});
it('can edit grafana managed rule', async () => {
const folder = {
title: 'Folder A',
uid: grafanaRulerRule.grafana_alert.namespace_uid,
id: 1,
type: DashboardSearchItemType.DashDB,
};
const slashedFolder = {
title: 'Folder with /',
uid: 'abcde',
id: 2,
};
const dataSources = {
default: mockDataSource(
@ -99,18 +83,12 @@ describe('RuleEditor grafana managed rules', () => {
{ alerting: false }
),
};
jest.spyOn(backendSrv, 'getFolderByUid').mockResolvedValue({
...mockFolder(),
accessControl: {
[AccessControlAction.AlertingRuleUpdate]: true,
},
});
setupDataSources(dataSources.default);
setFolderResponse(mockFolder(folder));
setFolderResponse(mockFolder(slashedFolder));
});
// mocks.api.fetchRulerRulesNamespace.mockResolvedValue([]);
mocks.searchFolders.mockResolvedValue([folder, slashedFolder] as DashboardSearchHit[]);
it('can edit grafana managed rule', async () => {
const { user } = renderRuleEditor(grafanaRulerRule.grafana_alert.uid);
// check that it's filled in
@ -141,7 +119,36 @@ describe('RuleEditor grafana managed rules', () => {
// save and check what was sent to backend
await user.click(ui.buttons.save.get());
mocks.searchFolders.mockResolvedValue([] as DashboardSearchHit[]);
expect(screen.getByText('New folder')).toBeInTheDocument();
});
it('saves evaluation interval correctly', async () => {
const { user } = renderRuleEditor(grafanaRulerRule.grafana_alert.uid);
await user.click(await screen.findByRole('button', { name: /new evaluation group/i }));
await screen.findByRole('dialog');
await user.type(screen.getByLabelText(/evaluation group name/i), 'new group');
const evalInterval = screen.getByLabelText(/^evaluation interval/i);
await user.clear(evalInterval);
await user.type(evalInterval, '12m');
await user.click(screen.getByRole('button', { name: /create/i }));
// Update the pending period as well, otherwise we'll get a form validation error
// and the rule won't try and save
await user.type(screen.getByLabelText(/pending period/i), '12m');
const capture = captureRequests(
(req) => req.method === 'POST' && req.url.includes('/api/ruler/grafana/api/v1/rules/uuid020c61ef')
);
await user.click(ui.buttons.save.get());
const [request] = await capture;
const postBody = await request.json();
expect(postBody.name).toBe('new group');
expect(postBody.interval).toBe('12m');
});
});

41
public/app/features/alerting/unified/components/rule-editor/FolderAndGroup.tsx
View File

@ -211,6 +211,7 @@ export function FolderAndGroup({
className={styles.formInput}
error={errors.group?.message}
invalid={!!errors.group?.message}
htmlFor="group"
>
<Controller
render={({ field: { ref, ...field }, fieldState }) => (
@ -356,6 +357,7 @@ function EvaluationGroupCreationModal({
const { watch } = useFormContext<RuleFormValues>();
const evaluateEveryId = 'eval-every-input';
const evaluationGroupNameId = 'new-eval-group-name';
const [groupName, folderName] = watch(['group', 'folder.title']);
const groupRules =
@ -392,8 +394,11 @@ function EvaluationGroupCreationModal({
<form onSubmit={handleSubmit(() => onSubmit())}>
<Field
label={
<Label htmlFor={'group'} description="A group evaluates all its rules over the same evaluation interval.">
Evaluation group
<Label
htmlFor={evaluationGroupNameId}
description="A group evaluates all its rules over the same evaluation interval."
>
Evaluation group name
</Label>
}
error={formState.errors.group?.message}
@ -403,7 +408,7 @@ function EvaluationGroupCreationModal({
data-testid={selectors.components.AlertRules.newEvaluationGroupName}
className={styles.formInput}
autoFocus={true}
id={'group'}
id={evaluationGroupNameId}
placeholder="Enter a name"
{...register('group', { required: { value: true, message: 'Required.' } })}
/>
@ -411,29 +416,27 @@ function EvaluationGroupCreationModal({
<Field
error={formState.errors.evaluateEvery?.message}
invalid={Boolean(formState.errors.evaluateEvery) ? true : undefined}
label={
<Label htmlFor={evaluateEveryId} description="How often all rules in the group are evaluated.">
Evaluation interval
</Label>
}
invalid={Boolean(formState.errors.evaluateEvery)}
>
<Stack direction="column">
<Input
data-testid={selectors.components.AlertRules.newEvaluationGroupInterval}
className={styles.formInput}
id={evaluateEveryId}
placeholder={DEFAULT_GROUP_EVALUATION_INTERVAL}
{...register(
'evaluateEvery',
evaluateEveryValidationOptions<{ group: string; evaluateEvery: string }>(groupRules)
)}
/>
<Stack direction="row" alignItems="flex-end">
<EvaluationGroupQuickPick currentInterval={evaluationInterval} onSelect={setEvaluationInterval} />
</Stack>
</Stack>
<Input
data-testid={selectors.components.AlertRules.newEvaluationGroupInterval}
className={styles.formInput}
id={evaluateEveryId}
placeholder={DEFAULT_GROUP_EVALUATION_INTERVAL}
{...register(
'evaluateEvery',
evaluateEveryValidationOptions<{ group: string; evaluateEvery: string }>(groupRules)
)}
/>
</Field>
<EvaluationGroupQuickPick currentInterval={evaluationInterval} onSelect={setEvaluationInterval} />
<Modal.ButtonRow>
<Button variant="secondary" type="button" onClick={onCancel}>
Cancel

19
public/app/features/alerting/unified/components/rule-editor/GrafanaEvaluationBehavior.tsx
View File

@ -115,7 +115,6 @@ function FolderGroupAndEvaluationInterval({
const onOpenEditGroupModal = () => setIsEditingGroup(true);
const editGroupDisabled = groupfoldersForGrafana?.loading || isNewGroup || !folderUid || !groupName;
const emptyNamespace: CombinedRuleNamespace = {
name: folderName,
rulesSource: GRAFANA_RULES_SOURCE_NAME,
@ -185,11 +184,11 @@ function ForInput({ evaluateEvery }: { evaluateEvery: string }) {
};
return (
<Stack direction="row" justify-content="flex-start" align-items="flex-start">
<Stack direction="column" justify-content="flex-start" align-items="flex-start">
<Field
label={
<Label
htmlFor="evaluateFor"
htmlFor={evaluateForId}
description='Period the threshold condition must be met to trigger the alert. Selecting "None" triggers the alert immediately once the condition is met.'
>
<Trans i18nKey="alert-rule-form.evaluation-behaviour.pending-period">Pending period</Trans>
@ -200,15 +199,13 @@ function ForInput({ evaluateEvery }: { evaluateEvery: string }) {
invalid={Boolean(errors.evaluateFor?.message) ? true : undefined}
validationMessageHorizontalOverflow={true}
>
<Stack direction="row" alignItems="center">
<Input id={evaluateForId} width={8} {...register('evaluateFor', forValidationOptions(evaluateEvery))} />
<PendingPeriodQuickPick
selectedPendingPeriod={currentPendingPeriod}
groupEvaluationInterval={evaluateEvery}
onSelect={setPendingPeriod}
/>
</Stack>
<Input id={evaluateForId} width={8} {...register('evaluateFor', forValidationOptions(evaluateEvery))} />
</Field>
<PendingPeriodQuickPick
selectedPendingPeriod={currentPendingPeriod}
groupEvaluationInterval={evaluateEvery}
onSelect={setPendingPeriod}
/>
</Stack>
);
}

7
public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx
View File

@ -147,21 +147,20 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
? getRuleGroupLocationFromRuleWithLocation(existing)
: getRuleGroupLocationFromFormValues(values);
// @TODO what is "evaluateEvery" being used for?
// @TODO move this to a hook too to make sure the logic here is tested for regressions?
if (!existing) {
// when creating a new rule, we save the manual routing setting , and editorSettings.simplifiedQueryEditor to the local storage
storeInLocalStorageValues(values);
await addRuleToRuleGroup.execute(ruleGroupIdentifier, ruleDefinition, values.evaluateEvery);
await addRuleToRuleGroup.execute(ruleGroupIdentifier, ruleDefinition, evaluateEvery);
} else {
const ruleIdentifier = fromRulerRuleAndRuleGroupIdentifier(ruleGroupIdentifier, existing.rule);
const targetRuleGroupIdentifier = getRuleGroupLocationFromFormValues(values);
await updateRuleInRuleGroup.execute(
ruleGroupIdentifier,
ruleIdentifier,
ruleDefinition,
targetRuleGroupIdentifier
targetRuleGroupIdentifier,
evaluateEvery
);
}

4
public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/SimplifiedRuleEditor.test.tsx
View File

@ -19,7 +19,7 @@ import { DataSourceType, GRAFANA_DATASOURCE_NAME } from 'app/features/alerting/u
import { AlertmanagerChoice } from 'app/plugins/datasource/alertmanager/types';
import { AccessControlAction } from 'app/types';
import { grafanaRulerEmptyGroup } from '../../../../mocks/grafanaRulerApi';
import { grafanaRulerGroup } from '../../../../mocks/grafanaRulerApi';
import { setupDataSources } from '../../../../testSetup/datasources';
jest.mock('app/core/components/AppChrome/AppChromeUpdate', () => ({
@ -52,7 +52,7 @@ const selectFolderAndGroup = async () => {
await clickSelectOption(folderInput, FOLDER_TITLE_HAPPY_PATH);
const groupInput = await ui.inputs.group.find();
await user.click(await byRole('combobox').find(groupInput));
await clickSelectOption(groupInput, grafanaRulerEmptyGroup.name);
await clickSelectOption(groupInput, grafanaRulerGroup.name);
};
describe('Can create a new grafana managed alert using simplified routing', () => {

44
public/app/features/alerting/unified/components/rule-editor/alert-rule-form/simplifiedRouting/__snapshots__/SimplifiedRuleEditor.test.tsx.snap
View File

@ -5,8 +5,48 @@ exports[`Can create a new grafana managed alert using simplified routing can cre
{
"body": {
"interval": "1m",
"name": "empty-group",
"name": "grafana-group-1",
"rules": [
{
"annotations": {
"summary": "Test alert",
},
"for": "5m",
"grafana_alert": {
"condition": "A",
"data": [
{
"datasourceUid": "datasource-uid",
"model": {
"datasource": {
"type": "prometheus",
"uid": "datasource-uid",
},
"expression": "vector(1)",
"queryType": "alerting",
"refId": "A",
},
"queryType": "alerting",
"refId": "A",
"relativeTimeRange": {
"from": 1000,
"to": 2000,
},
},
],
"exec_err_state": "Error",
"is_paused": false,
"namespace_uid": "uuid020c61ef",
"no_data_state": "NoData",
"rule_group": "grafana-group-1",
"title": "Grafana-rule",
"uid": "4d7125fee983",
},
"labels": {
"region": "nasa",
"severity": "critical",
},
},
{
"annotations": {},
"for": "1m",
@ -110,7 +150,7 @@ exports[`Can create a new grafana managed alert using simplified routing can cre
],
],
"method": "POST",
"url": "http://localhost/api/ruler/grafana/api/v1/rules/6abdb25bc1eb?subtype=cortex",
"url": "http://localhost/api/ruler/grafana/api/v1/rules/uuid020c61ef?subtype=cortex",
},
]
`;

10
public/app/features/alerting/unified/hooks/ruleGroup/useUpsertRuleFromRuleGroup.ts
View File

@ -55,7 +55,8 @@ export function useUpdateRuleInRuleGroup() {
ruleGroup: RuleGroupIdentifier,
ruleIdentifier: EditableRuleIdentifier,
ruleDefinition: PostableRuleDTO,
targetRuleGroup?: RuleGroupIdentifier
targetRuleGroup?: RuleGroupIdentifier,
interval?: string
) => {
const { namespaceName } = ruleGroup;
const finalRuleDefinition = copyGrafanaUID(ruleIdentifier, ruleDefinition);
@ -63,7 +64,7 @@ export function useUpdateRuleInRuleGroup() {
// check if the existing rule and the form values have the same rule group identifier
const sameTargetRuleGroup = isEqual(ruleGroup, targetRuleGroup);
if (targetRuleGroup && !sameTargetRuleGroup) {
const result = moveRuleToGroup.execute(ruleGroup, targetRuleGroup, ruleIdentifier, ruleDefinition);
const result = moveRuleToGroup.execute(ruleGroup, targetRuleGroup, ruleIdentifier, ruleDefinition, interval);
return result;
}
@ -96,12 +97,13 @@ export function useMoveRuleToRuleGroup() {
currentRuleGroup: RuleGroupIdentifier,
targetRuleGroup: RuleGroupIdentifier,
ruleIdentifier: EditableRuleIdentifier,
ruleDefinition: PostableRuleDTO
ruleDefinition: PostableRuleDTO,
interval?: string
) => {
const finalRuleDefinition = copyGrafanaUID(ruleIdentifier, ruleDefinition);
// 1. add the rule to the new namespace / group / ruler target
const addRuleToGroup = addRuleAction({ rule: finalRuleDefinition });
const addRuleToGroup = addRuleAction({ rule: finalRuleDefinition, interval });
const { newRuleGroupDefinition: newTargetGroup, rulerConfig: targetGroupRulerConfig } = await produceNewRuleGroup(
targetRuleGroup,
addRuleToGroup

8
public/app/features/alerting/unified/mocks/server/configure.ts
View File

@ -52,6 +52,14 @@ export const setFolderAccessControl = (accessControl: FolderDTO['accessControl']
server.use(getFolderHandler(mockFolder({ hasAcl: true, accessControl })));
};
/**
* Makes the mock server respond with different folder response, for just the folder in question
*/
export const setFolderResponse = (response: Partial<FolderDTO>) => {
const handler = http.get<{ folderUid: string }>(`/api/folders/${response.uid}`, () => HttpResponse.json(response));
server.use(handler);
};
/**
* Makes the mock server respond with different Grafana Alertmanager config
*/

4
public/app/features/alerting/unified/mocks/server/handlers/search.ts
View File

@ -1,6 +1,6 @@
import { HttpResponse, http } from 'msw';
import { grafanaRulerNamespace2 } from 'app/features/alerting/unified/mocks/grafanaRulerApi';
import { grafanaRulerNamespace } from 'app/features/alerting/unified/mocks/grafanaRulerApi';
import { DashboardSearchItemType } from 'app/features/search/types';
export const FOLDER_TITLE_HAPPY_PATH = 'Folder A';
@ -10,7 +10,7 @@ export const FOLDER_TITLE_HAPPY_PATH = 'Folder A';
const defaultSearchResponse = [
{
title: FOLDER_TITLE_HAPPY_PATH,
uid: grafanaRulerNamespace2.uid,
uid: grafanaRulerNamespace.uid,
id: 1,
type: DashboardSearchItemType.DashFolder,
},