Skip to content

Commit

Permalink
Not selecting any days in alerts filter maps to all weekdays (#156913)
Browse files Browse the repository at this point in the history
Fixes: #156878

As discussed in the issue just turning `if alert is generated within
timeframe` on should not change the filter scope.
And the UI should not confuse the users by setting some days as default.

And as discussed in #154680, default hours filter should cover the whole
day.

Therefore, this PR sets default alerts filter options as:
```
  {
    days: [],
    hours: {
      start: '00:00',
      end: '00:00',
    },
  };
```

empty days array maps to all weekdays  `[1,2,3,4,5,6,7]`
and hours `00:00 -> 00:00` maps to `00:00 -> 24:00`
  • Loading branch information
ersin-erdal authored May 9, 2023
1 parent 0676f49 commit d157389
Show file tree
Hide file tree
Showing 11 changed files with 191 additions and 16 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/alerting/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export * from './rrule_type';
export * from './maintenance_window';
export * from './default_rule_aggregation';
export * from './rule_tags_aggregation';
export * from './iso_weekdays';

export {
mappingFromFieldMap,
Expand Down
9 changes: 9 additions & 0 deletions x-pack/plugins/alerting/common/iso_weekdays.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export type IsoWeekday = 1 | 2 | 3 | 4 | 5 | 6 | 7;
export const ISO_WEEKDAYS: IsoWeekday[] = [1, 2, 3, 4, 5, 6, 7];
2 changes: 1 addition & 1 deletion x-pack/plugins/alerting/common/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
SavedObjectsResolveResponse,
} from '@kbn/core/server';
import type { Filter, KueryNode } from '@kbn/es-query';
import { IsoWeekday } from './iso_weekdays';
import { RuleNotifyWhenType } from './rule_notify_when_type';
import { RuleSnooze } from './rule_snooze_type';

Expand Down Expand Up @@ -83,7 +84,6 @@ export interface RuleActionFrequency extends SavedObjectAttributes {
throttle: string | null;
}

export type IsoWeekday = 1 | 2 | 3 | 4 | 5 | 6 | 7;
export interface AlertsFilterTimeframe extends SavedObjectAttributes {
days: IsoWeekday[];
timezone: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { invert, mapValues } from 'lodash';
import moment from 'moment';
import * as i18n from './translations';
import { MaintenanceWindowStatus } from '../../../common';
import { ISO_WEEKDAYS, MaintenanceWindowStatus } from '../../../common';

// TODO - consolidate enum with backend
export enum Frequency {
Expand Down Expand Up @@ -86,8 +86,6 @@ export const CREATE_FORM_CUSTOM_FREQUENCY = (interval: number = 1) => [
},
];

export const ISO_WEEKDAYS = [1, 2, 3, 4, 5, 6, 7];

export const WEEKDAY_OPTIONS = ISO_WEEKDAYS.map((n) => ({
id: String(n),
label: moment().isoWeekday(n).format('ddd'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
*/

import { Moment } from 'moment';
import { ISO_WEEKDAYS, ISO_WEEKDAYS_TO_RRULE } from '../constants';
import { ISO_WEEKDAYS } from '../../../../common';
import { ISO_WEEKDAYS_TO_RRULE } from '../constants';

export const getInitialByWeekday = (initialStateByweekday: string[], date: Moment | null) => {
const dayOfWeek = date ? date.isoWeekday() : 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2434,6 +2434,168 @@ describe('createGetSummarizedAlertsFn', () => {
});
});

it('creates function that correctly returns lifecycle alerts using alerts filter with no days selected', async () => {
ruleDataClientMock.getReader().search.mockResolvedValueOnce({
hits: {
total: {
value: 1,
},
hits: [
{
_id: '1',
_index: '.alerts-default-000001',
_source: {
[TIMESTAMP]: '2020-01-01T12:00:00.000Z',
[ALERT_RULE_EXECUTION_UUID]: 'abc',
[ALERT_RULE_UUID]: 'rule-id',
[EVENT_ACTION]: 'open',
[ALERT_INSTANCE_ID]: 'TEST_ALERT_3',
[ALERT_UUID]: 'uuid1',
},
},
],
},
} as any);
ruleDataClientMock.getReader().search.mockResolvedValueOnce({
hits: {
total: {
value: 0,
},
hits: [],
},
} as any);
ruleDataClientMock.getReader().search.mockResolvedValueOnce({
hits: {
total: {
value: 0,
},
hits: [],
},
} as any);
const getSummarizedAlertsFn = createGetSummarizedAlertsFn({
ruleDataClient: ruleDataClientMock,
useNamespace: false,
isLifecycleAlert: true,
})();

await getSummarizedAlertsFn({
executionUuid: 'abc',
ruleId: 'rule-id',
spaceId: 'space-id',
excludedAlertInstanceIds: [],
alertsFilter: {
query: {
kql: 'kibana.alert.rule.name:test',
dsl: '{"bool":{"minimum_should_match":1,"should":[{"match":{"kibana.alert.rule.name":"test"}}]}}',
filters: [],
},
timeframe: {
days: [],
hours: { start: '08:00', end: '17:00' },
timezone: 'UTC',
},
},
});

expect(ruleDataClientMock.getReader).toHaveBeenCalledWith();
expect(ruleDataClientMock.getReader().search).toHaveBeenCalledTimes(3);
expect(ruleDataClientMock.getReader().search).toHaveBeenNthCalledWith(1, {
body: {
size: 100,
track_total_hits: true,
query: {
bool: {
filter: [
{
term: {
[ALERT_RULE_EXECUTION_UUID]: 'abc',
},
},
{
term: {
[ALERT_RULE_UUID]: 'rule-id',
},
},
{
bool: {
must_not: {
exists: {
field: ALERT_MAINTENANCE_WINDOW_IDS,
},
},
},
},
{
term: {
[EVENT_ACTION]: 'open',
},
},
{
bool: {
minimum_should_match: 1,
should: [
{
match: {
'kibana.alert.rule.name': 'test',
},
},
],
},
},
{
script: {
script: {
params: {
datetimeField: '@timestamp',
days: [1, 2, 3, 4, 5, 6, 7],
timezone: 'UTC',
},
source:
'params.days.contains(doc[params.datetimeField].value.withZoneSameInstant(ZoneId.of(params.timezone)).dayOfWeek.getValue())',
},
},
},
{
script: {
script: {
params: {
datetimeField: '@timestamp',
end: '17:00',
start: '08:00',
timezone: 'UTC',
},
source: `
def alertsDateTime = doc[params.datetimeField].value.withZoneSameInstant(ZoneId.of(params.timezone));
def alertsTime = LocalTime.of(alertsDateTime.getHour(), alertsDateTime.getMinute());
def start = LocalTime.parse(params.start);
def end = LocalTime.parse(params.end);
if (end.isBefore(start) || end.equals(start)){ // overnight
def dayEnd = LocalTime.parse("23:59:59");
def dayStart = LocalTime.parse("00:00:00");
if ((alertsTime.isAfter(start) && alertsTime.isBefore(dayEnd)) || (alertsTime.isAfter(dayStart) && alertsTime.isBefore(end))) {
return true;
} else {
return false;
}
} else {
if (alertsTime.isAfter(start) && alertsTime.isBefore(end)) {
return true;
} else {
return false;
}
}
`,
},
},
},
],
},
},
},
});
});

it('throws error if search throws error', async () => {
ruleDataClientMock.getReader().search.mockImplementation(() => {
throw new Error('search error');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ import {
QueryDslQueryContainer,
SearchTotalHits,
} from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { AlertsFilter } from '@kbn/alerting-plugin/common';
import { AlertsFilter, ISO_WEEKDAYS } from '@kbn/alerting-plugin/common';
import { AlertHit, SummarizedAlerts } from '@kbn/alerting-plugin/server/types';
import { ParsedTechnicalFields } from '../../common';
import { ParsedExperimentalFields } from '../../common/parse_experimental_fields';
import { IRuleDataClient, IRuleDataReader } from '../rule_data_client';

const MAX_ALERT_DOCS_TO_RETURN = 100;

export type AlertDocument = Partial<ParsedTechnicalFields & ParsedExperimentalFields>;

interface CreateGetSummarizedAlertsFnOpts {
Expand Down Expand Up @@ -586,7 +587,10 @@ const generateAlertsFilterDSL = (alertsFilter: AlertsFilter): QueryDslQueryConta
source:
'params.days.contains(doc[params.datetimeField].value.withZoneSameInstant(ZoneId.of(params.timezone)).dayOfWeek.getValue())',
params: {
days: alertsFilter.timeframe.days,
days:
alertsFilter.timeframe.days.length === 0
? ISO_WEEKDAYS
: alertsFilter.timeframe.days,
timezone: alertsFilter.timeframe.timezone,
datetimeField: TIMESTAMP,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ import {
EuiComboBox,
} from '@elastic/eui';
import deepEqual from 'fast-deep-equal';
import { AlertsFilterTimeframe, IsoWeekday } from '@kbn/alerting-plugin/common';
import { I18N_WEEKDAY_OPTIONS_DDD, ISO_WEEKDAYS } from '../../../common/constants';
import { AlertsFilterTimeframe, ISO_WEEKDAYS, IsoWeekday } from '@kbn/alerting-plugin/common';
import { I18N_WEEKDAY_OPTIONS_DDD } from '../../../common/constants';

interface ActionAlertsFilterTimeframeProps {
state?: AlertsFilterTimeframe;
Expand Down Expand Up @@ -49,11 +49,11 @@ const useDefaultTimezone = () => {
const useTimeframe = (initialTimeframe?: AlertsFilterTimeframe) => {
const timezone = useDefaultTimezone();
const DEFAULT_TIMEFRAME = {
days: ISO_WEEKDAYS,
days: [],
timezone,
hours: {
start: '00:00',
end: '23:59',
end: '00:00',
},
};
return useState<AlertsFilterTimeframe>(initialTimeframe || DEFAULT_TIMEFRAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import { i18n } from '@kbn/i18n';
import { invert, mapValues } from 'lodash';
import { RRuleFrequency } from '../../../../../../types';

export { ISO_WEEKDAYS, I18N_WEEKDAY_OPTIONS } from '../../../../../../common/constants';
export { I18N_WEEKDAY_OPTIONS } from '../../../../../../common/constants';
export { ISO_WEEKDAYS } from '@kbn/alerting-plugin/common';

export const RECURRENCE_END_OPTIONS = [
{ id: 'never', label: 'Never' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
import { i18n } from '@kbn/i18n';
import moment, { Moment } from 'moment';

import { ISO_WEEKDAYS } from '@kbn/alerting-plugin/common';
import { RecurrenceSchedule, RRuleFrequency } from '../../../../../../types';
import { i18nMonthDayDate } from '../../../../../lib/i18n_month_day_date';
import { ISO_WEEKDAYS, ISO_WEEKDAYS_TO_RRULE, RRULE_WEEKDAYS_TO_ISO_WEEKDAYS } from './constants';
import { ISO_WEEKDAYS_TO_RRULE, RRULE_WEEKDAYS_TO_ISO_WEEKDAYS } from './constants';
import { i18nFreqSummary, i18nNthWeekdayShort } from './translations';

export interface CustomFrequencyState {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import { IsoWeekday } from '@kbn/alerting-plugin/common';
import { ISO_WEEKDAYS } from '@kbn/alerting-plugin/common';
import moment from 'moment';

export const ISO_WEEKDAYS: IsoWeekday[] = [1, 2, 3, 4, 5, 6, 7];

export const I18N_WEEKDAY_OPTIONS = ISO_WEEKDAYS.map((n) => ({
id: String(n),
label: moment().isoWeekday(n).format('dd'),
Expand Down

0 comments on commit d157389

Please sign in to comment.