Skip to content

Commit

Permalink
Remove the advanced sorting switch from the rules management page
Browse files Browse the repository at this point in the history
  • Loading branch information
xcrzx committed Feb 1, 2023
1 parent 0d6c113 commit cd5e413
Show file tree
Hide file tree
Showing 38 changed files with 242 additions and 342 deletions.
7 changes: 7 additions & 0 deletions x-pack/plugins/alerting/common/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,12 @@ export type RuleExecutionStatuses = typeof RuleExecutionStatusValues[number];
export const RuleLastRunOutcomeValues = ['succeeded', 'warning', 'failed'] as const;
export type RuleLastRunOutcomes = typeof RuleLastRunOutcomeValues[number];

export const RuleLastRunOutcomeOrderMap: Record<RuleLastRunOutcomes, number> = {
succeeded: 0,
warning: 10,
failed: 20,
};

export enum RuleExecutionStatusErrorReasons {
Read = 'read',
Decrypt = 'decrypt',
Expand Down Expand Up @@ -93,6 +99,7 @@ export interface RuleAggregations {

export interface RuleLastRun {
outcome: RuleLastRunOutcomes;
outcomeOrder: number;
warning?: RuleExecutionStatusErrorReasons | RuleExecutionStatusWarningReasons | null;
outcomeMsg?: string[] | null;
alertsCount: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ describe('common_transformations', () => {
},
last_run: {
outcome: RuleLastRunOutcomeValues[2],
outcome_order: 20,
outcome_msg: ['this is just a test'],
warning: RuleExecutionStatusErrorReasons.Unknown,
alerts_count: {
Expand Down Expand Up @@ -137,6 +138,7 @@ describe('common_transformations', () => {
"outcomeMsg": Array [
"this is just a test",
],
"outcomeOrder": 20,
"warning": "unknown",
},
"monitoring": Object {
Expand Down Expand Up @@ -255,6 +257,7 @@ describe('common_transformations', () => {
},
last_run: {
outcome: 'failed',
outcome_order: 20,
outcome_msg: ['this is just a test'],
warning: RuleExecutionStatusErrorReasons.Unknown,
alerts_count: {
Expand Down Expand Up @@ -300,6 +303,7 @@ describe('common_transformations', () => {
"outcomeMsg": Array [
"this is just a test",
],
"outcomeOrder": 20,
"warning": "unknown",
},
"monitoring": Object {
Expand Down
8 changes: 7 additions & 1 deletion x-pack/plugins/alerting/public/lib/common_transformations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,16 @@ function transformMonitoring(input: RuleMonitoring): RuleMonitoring {
}

function transformLastRun(input: AsApiContract<RuleLastRun>): RuleLastRun {
const { outcome_msg: outcomeMsg, alerts_count: alertsCount, ...rest } = input;
const {
outcome_msg: outcomeMsg,
alerts_count: alertsCount,
outcome_order: outcomeOrder,
...rest
} = input;
return {
outcomeMsg,
alertsCount,
outcomeOrder,
...rest,
};
}
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/alerting/server/lib/last_run_status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import { RuleTaskStateAndMetrics } from '../task_runner/types';
import { getReasonFromError } from './error_with_reason';
import { getEsErrorMessage } from './errors';
import { ActionsCompletion, RuleLastRunOutcomes } from '../../common';
import { ActionsCompletion, RuleLastRunOutcomeOrderMap, RuleLastRunOutcomes } from '../../common';
import {
RuleLastRunOutcomeValues,
RuleExecutionStatusWarningReasons,
Expand Down Expand Up @@ -65,6 +65,7 @@ export const lastRunFromState = (
return {
lastRun: {
outcome,
outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
outcomeMsg: outcomeMsg.length > 0 ? outcomeMsg : null,
warning: warning || null,
alertsCount: {
Expand All @@ -80,9 +81,11 @@ export const lastRunFromState = (

export const lastRunFromError = (error: Error): ILastRun => {
const esErrorMessage = getEsErrorMessage(error);
const outcome = RuleLastRunOutcomeValues[2];
return {
lastRun: {
outcome: RuleLastRunOutcomeValues[2],
outcome,
outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
warning: getReasonFromError(error),
outcomeMsg: esErrorMessage ? [esErrorMessage] : null,
alertsCount: {},
Expand All @@ -104,5 +107,6 @@ export const lastRunToRaw = (lastRun: ILastRun['lastRun']): RawRuleLastRun => {
},
warning: warning ?? null,
outcomeMsg: outcomeMsg && !Array.isArray(outcomeMsg) ? [outcomeMsg] : outcomeMsg,
outcomeOrder: RuleLastRunOutcomeOrderMap[lastRun.outcome],
};
};
3 changes: 2 additions & 1 deletion x-pack/plugins/alerting/server/routes/lib/rewrite_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import { omit } from 'lodash';
import { RuleTypeParams, SanitizedRule, RuleLastRun } from '../../types';

export const rewriteRuleLastRun = (lastRun: RuleLastRun) => {
const { outcomeMsg, alertsCount, ...rest } = lastRun;
const { outcomeMsg, outcomeOrder, alertsCount, ...rest } = lastRun;
return {
alerts_count: alertsCount,
outcome_msg: outcomeMsg,
outcome_order: outcomeOrder,
...rest,
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
isLogThresholdRuleType,
pipeMigrations,
} from '../utils';
import { RawRule } from '../../../types';
import { RawRule, RuleLastRunOutcomeOrderMap } from '../../../types';

function addGroupByToEsQueryRule(
doc: SavedObjectUnsanitizedDoc<RawRule>
Expand Down Expand Up @@ -70,9 +70,29 @@ function addLogViewRefToLogThresholdRule(
return doc;
}

function migrateOutcomeOrder(
doc: SavedObjectUnsanitizedDoc<RawRule>
): SavedObjectUnsanitizedDoc<RawRule> {
if (!doc.attributes.lastRun) {
return doc;
}

const outcome = doc.attributes.lastRun.outcome;
return {
...doc,
attributes: {
...doc.attributes,
lastRun: {
...doc.attributes.lastRun,
outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
},
},
};
}

export const getMigrations870 = (encryptedSavedObjects: EncryptedSavedObjectsPluginSetup) =>
createEsoMigration(
encryptedSavedObjects,
(doc: SavedObjectUnsanitizedDoc<RawRule>): doc is SavedObjectUnsanitizedDoc<RawRule> => true,
pipeMigrations(addGroupByToEsQueryRule, addLogViewRefToLogThresholdRule)
pipeMigrations(addGroupByToEsQueryRule, addLogViewRefToLogThresholdRule, migrateOutcomeOrder)
);
Original file line number Diff line number Diff line change
Expand Up @@ -2606,6 +2606,21 @@ describe('successful migrations', () => {
expect(migratedAlert870.references).toEqual([]);
});
});

test('migrates last run outcome order', () => {
const migration870 = getMigrations(encryptedSavedObjectsSetup, {}, isPreconfigured)['8.7.0'];
const rule = getMockData({
lastRun: {
outcome: 'failed',
},
});
const migratedAlert870 = migration870(rule, migrationContext);

expect(migratedAlert870.attributes.lastRun).toEqual({
outcome: 'failed',
outcomeOrder: 20,
});
});
});

describe('Metrics Inventory Threshold rule', () => {
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/alerting/server/task_runner/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,14 @@
*/

import { TaskStatus } from '@kbn/task-manager-plugin/server';
import { Rule, RuleTypeParams, RecoveredActionGroup, RuleMonitoring } from '../../common';
import {
Rule,
RuleTypeParams,
RecoveredActionGroup,
RuleMonitoring,
RuleLastRunOutcomeOrderMap,
RuleLastRunOutcomes,
} from '../../common';
import { getDefaultMonitoring } from '../lib/monitoring';
import { UntypedNormalizedRuleType } from '../rule_type_registry';
import { EVENT_LOG_ACTIONS } from '../plugin';
Expand Down Expand Up @@ -74,7 +81,7 @@ export const generateSavedObjectParams = ({
error?: null | { reason: string; message: string };
warning?: null | { reason: string; message: string };
status?: string;
outcome?: string;
outcome?: RuleLastRunOutcomes;
nextRun?: string | null;
successRatio?: number;
history?: RuleMonitoring['run']['history'];
Expand Down Expand Up @@ -110,6 +117,7 @@ export const generateSavedObjectParams = ({
},
lastRun: {
outcome,
outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
outcomeMsg:
(error?.message && [error?.message]) || (warning?.message && [warning?.message]) || null,
warning: error?.reason || warning?.reason || null,
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/alerting/server/task_runner/task_runner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
3,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
4,
Expand Down Expand Up @@ -355,7 +355,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
4,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
5,
Expand Down Expand Up @@ -442,7 +442,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
5,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":1,"recovered":0,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
6,
Expand Down Expand Up @@ -622,7 +622,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
5,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":2,"new":2,"recovered":0,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":2,"new":2,"recovered":0,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
6,
Expand Down Expand Up @@ -1061,7 +1061,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
5,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
6,
Expand Down Expand Up @@ -1187,7 +1187,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
5,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":1,"new":0,"recovered":1,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
6,
Expand Down Expand Up @@ -2325,7 +2325,7 @@ describe('Task Runner', () => {
);
expect(logger.debug).nthCalledWith(
3,
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
'ruleRunStatus for test:1: {"outcome":"succeeded","outcomeOrder":0,"outcomeMsg":null,"warning":null,"alertsCount":{"active":0,"new":0,"recovered":0,"ignored":0}}'
);
expect(logger.debug).nthCalledWith(
4,
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/alerting/server/task_runner/task_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import {
RuleTypeState,
parseDuration,
RawAlertInstance,
RuleLastRunOutcomeOrderMap,
} from '../../common';
import { NormalizedRuleType, UntypedNormalizedRuleType } from '../rule_type_registry';
import { getEsErrorMessage } from '../lib/errors';
Expand Down Expand Up @@ -817,10 +818,12 @@ export class TaskRunner<
this.logger.debug(
`Updating rule task for ${this.ruleType.id} rule with id ${ruleId} - execution error due to timeout`
);
const outcome = 'failed';
await this.updateRuleSavedObjectPostRun(ruleId, namespace, {
executionStatus: ruleExecutionStatusToRaw(executionStatus),
lastRun: {
outcome: 'failed',
outcome,
outcomeOrder: RuleLastRunOutcomeOrderMap[outcome],
warning: RuleExecutionStatusErrorReasons.Timeout,
outcomeMsg,
alertsCount: {},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ describe('Task Runner Cancel', () => {
outcomeMsg: [
'test:1: execution cancelled due to timeout - exceeded rule type timeout of 5m',
],
outcomeOrder: 20,
warning: 'timeout',
},
monitoring: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe('Find rules request schema', () => {
const payload: FindRulesRequestQuery = {
per_page: 5,
page: 1,
sort_field: 'some field',
sort_field: 'name',
fields: ['field 1', 'field 2'],
filter: 'some filter',
sort_order: 'asc',
Expand Down Expand Up @@ -80,14 +80,14 @@ describe('Find rules request schema', () => {

test('sort_field validates', () => {
const payload: FindRulesRequestQuery = {
sort_field: 'value',
sort_field: 'name',
};

const decoded = FindRulesRequestQuery.decode(payload);
const checked = exactCheck(payload, decoded);
const message = pipe(checked, foldLeftRight);
expect(getPaths(left(message.errors))).toEqual([]);
expect((message.schema as FindRulesRequestQuery).sort_field).toEqual('value');
expect((message.schema as FindRulesRequestQuery).sort_field).toEqual('name');
});

test('fields validates with a string', () => {
Expand Down Expand Up @@ -173,7 +173,7 @@ describe('Find rules request schema', () => {
test('sort_order validates with desc and sort_field', () => {
const payload: FindRulesRequestQuery = {
sort_order: 'desc',
sort_field: 'some field',
sort_field: 'name',
};

const decoded = FindRulesRequestQuery.decode(payload);
Expand All @@ -187,7 +187,7 @@ describe('Find rules request schema', () => {
test('sort_order does not validate with a string other than asc and desc', () => {
const payload: Omit<FindRulesRequestQuery, 'sort_order'> & { sort_order: string } = {
sort_order: 'some other string',
sort_field: 'some field',
sort_field: 'name',
};

const decoded = FindRulesRequestQuery.decode(payload);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('Find rules request schema, additional validation', () => {

test('You can have both a sort_field and and a sort_order', () => {
const schema: FindRulesRequestQuery = {
sort_field: 'some field',
sort_field: 'name',
sort_order: 'asc',
};
const errors = validateFindRulesRequestQuery(schema);
Expand All @@ -27,7 +27,7 @@ describe('Find rules request schema, additional validation', () => {

test('You cannot have sort_field without sort_order', () => {
const schema: FindRulesRequestQuery = {
sort_field: 'some field',
sort_field: 'name',
};
const errors = validateFindRulesRequestQuery(schema);
expect(errors).toEqual([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,23 @@ import type { Either } from 'fp-ts/lib/Either';
import { capitalize } from 'lodash';

export type SortField = t.TypeOf<typeof SortField>;
export const SortField = t.string;
export const SortField = t.union([
t.literal('created_at'),
t.literal('createdAt'), // Legacy notation, keeping for backwards compatibility
t.literal('enabled'),
t.literal('execution_summary.last_execution.date'),
t.literal('execution_summary.last_execution.metrics.execution_gap_duration_s'),
t.literal('execution_summary.last_execution.metrics.total_indexing_duration_ms'),
t.literal('execution_summary.last_execution.metrics.total_search_duration_ms'),
t.literal('execution_summary.last_execution.status'),
t.literal('name'),
t.literal('risk_score'),
t.literal('riskScore'), // Legacy notation, keeping for backwards compatibility
t.literal('severity'),
t.literal('updated_at'),
t.literal('updatedAt'), // Legacy notation, keeping for backwards compatibility
t.literal('version'),
]);

export type SortFieldOrUndefined = t.TypeOf<typeof SortFieldOrUndefined>;
export const SortFieldOrUndefined = t.union([SortField, t.undefined]);
Expand Down

0 comments on commit cd5e413

Please sign in to comment.