Skip to content

Commit 641c670

Browse files
[SEIM][Detection Engine] Time gap detection and logging
## Summary This adds utilities and logging of time gap detection. Gaps happen whenever rules begin to fall behind their interval. This isn't a perfect works for all inputs and if it detects unexpected input that is not of an interval format (but could be valid date time math) it will just return null and ignore it. This also fixes a bug with interval where we were using the object instead of the primitive since alerting team changed their structure. For testing, fire up any rule and shutdown Kibana for more than 6 minutes and then when restarting you should see the warning message. ### Checklist Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR. ~~- [ ] This was checked for cross-browser compatibility, [including a check against IE11](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility)~~ ~~- [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/master/packages/kbn-i18n/README.md)~~ ~~- [ ] [Documentation](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#writing-documentation) was added for features that require explanation or tutorials~~ - [x] [Unit or functional tests](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#cross-browser-compatibility) were updated or added to match the most common scenarios ~~- [ ] This was checked for [keyboard-only and screenreader accessibility](https://developer.mozilla.org/en-US/docs/Learn/Tools_and_testing/Cross_browser_testing/Accessibility#Accessibility_testing_checklist)~~ ### For maintainers ~~- [ ] This was checked for breaking API changes and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)~~ - [x] This includes a feature addition or change that requires a release note and was [labeled appropriately](https://github.com/elastic/kibana/blob/master/CONTRIBUTING.md#release-notes-process)
1 parent ebd2c21 commit 641c670

File tree

3 files changed

+341
-5
lines changed

3 files changed

+341
-5
lines changed

x-pack/legacy/plugins/siem/server/lib/detection_engine/signals/signal_rule_alert_type.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import { schema } from '@kbn/config-schema';
88
import { Logger } from 'src/core/server';
9+
import moment from 'moment';
910
import {
1011
SIGNALS_ID,
1112
DEFAULT_MAX_SIGNALS,
@@ -17,6 +18,7 @@ import { getInputIndex } from './get_input_output_index';
1718
import { searchAfterAndBulkCreate } from './search_after_bulk_create';
1819
import { getFilter } from './get_filter';
1920
import { SignalRuleAlertTypeDefinition } from './types';
21+
import { getGapBetweenRuns } from './utils';
2022

2123
export const signalRulesAlertType = ({
2224
logger,
@@ -57,7 +59,8 @@ export const signalRulesAlertType = ({
5759
version: schema.number({ defaultValue: 1 }),
5860
}),
5961
},
60-
async executor({ alertId, services, params }) {
62+
// fun fact: previousStartedAt is not actually a Date but a String of a date
63+
async executor({ previousStartedAt, alertId, services, params }) {
6164
const {
6265
from,
6366
ruleId,
@@ -70,17 +73,26 @@ export const signalRulesAlertType = ({
7073
to,
7174
type,
7275
} = params;
73-
7476
// TODO: Remove this hard extraction of name once this is fixed: https://github.com/elastic/kibana/issues/50522
7577
const savedObject = await services.savedObjectsClient.get('alert', alertId);
7678
const name: string = savedObject.attributes.name;
7779
const tags: string[] = savedObject.attributes.tags;
7880

7981
const createdBy: string = savedObject.attributes.createdBy;
8082
const updatedBy: string = savedObject.attributes.updatedBy;
81-
const interval: string = savedObject.attributes.interval;
83+
const interval: string = savedObject.attributes.schedule.interval;
8284
const enabled: boolean = savedObject.attributes.enabled;
83-
85+
const gap = getGapBetweenRuns({
86+
previousStartedAt: previousStartedAt != null ? moment(previousStartedAt) : null, // TODO: Remove this once previousStartedAt is no longer a string
87+
interval,
88+
from,
89+
to,
90+
});
91+
if (gap != null && gap.asMilliseconds() > 0) {
92+
logger.warn(
93+
`Signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}" has a time gap of ${gap.humanize()} (${gap.asMilliseconds()}ms), and could be missing signals within that time. Consider increasing your look behind time or adding more Kibana instances.`
94+
);
95+
}
8496
// set searchAfter page size to be the lesser of default page size or maxSignals.
8597
const searchAfterSize =
8698
DEFAULT_SEARCH_AFTER_PAGE_SIZE <= params.maxSignals
@@ -155,7 +167,7 @@ export const signalRulesAlertType = ({
155167
// TODO: Error handling and writing of errors into a signal that has error
156168
// handling/conditions
157169
logger.error(
158-
`Error from signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}"`
170+
`Error from signal rule name: "${name}", id: "${alertId}", rule_id: "${ruleId}" message: ${err.message}`
159171
);
160172
}
161173
},
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
7+
import moment from 'moment';
8+
9+
import { generateId, parseInterval, getDriftTolerance, getGapBetweenRuns } from './utils';
10+
11+
describe('utils', () => {
12+
let nowDate = moment('2020-01-01T00:00:00.000Z');
13+
14+
beforeEach(() => {
15+
nowDate = moment('2020-01-01T00:00:00.000Z');
16+
});
17+
18+
describe('generateId', () => {
19+
test('it generates expected output', () => {
20+
const id = generateId('index-123', 'doc-123', 'version-123', 'rule-123');
21+
expect(id).toEqual('10622e7d06c9e38a532e71fc90e3426c1100001fb617aec8cb974075da52db06');
22+
});
23+
24+
test('expected output is a hex', () => {
25+
const id = generateId('index-123', 'doc-123', 'version-123', 'rule-123');
26+
expect(id).toMatch(/[a-f0-9]+/);
27+
});
28+
});
29+
30+
describe('getIntervalMilliseconds', () => {
31+
test('it returns a duration when given one that is valid', () => {
32+
const duration = parseInterval('5m');
33+
expect(duration).not.toBeNull();
34+
expect(duration?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds());
35+
});
36+
37+
test('it returns null given an invalid duration', () => {
38+
const duration = parseInterval('junk');
39+
expect(duration).toBeNull();
40+
});
41+
});
42+
43+
describe('getDriftToleranceMilliseconds', () => {
44+
test('it returns a drift tolerance in milliseconds of 1 minute when from overlaps to by 1 minute and the interval is 5 minutes', () => {
45+
const drift = getDriftTolerance({
46+
from: 'now-6m',
47+
to: 'now',
48+
interval: moment.duration(5, 'minutes'),
49+
});
50+
expect(drift).not.toBeNull();
51+
expect(drift?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds());
52+
});
53+
54+
test('it returns a drift tolerance of 0 when from equals the interval', () => {
55+
const drift = getDriftTolerance({
56+
from: 'now-5m',
57+
to: 'now',
58+
interval: moment.duration(5, 'minutes'),
59+
});
60+
expect(drift?.asMilliseconds()).toEqual(0);
61+
});
62+
63+
test('it returns a drift tolerance of 5 minutes when from is 10 minutes but the interval is 5 minutes', () => {
64+
const drift = getDriftTolerance({
65+
from: 'now-10m',
66+
to: 'now',
67+
interval: moment.duration(5, 'minutes'),
68+
});
69+
expect(drift).not.toBeNull();
70+
expect(drift?.asMilliseconds()).toEqual(moment.duration(5, 'minutes').asMilliseconds());
71+
});
72+
73+
test('it returns a drift tolerance of 10 minutes when from is 10 minutes ago and the interval is 0', () => {
74+
const drift = getDriftTolerance({
75+
from: 'now-10m',
76+
to: 'now',
77+
interval: moment.duration(0, 'milliseconds'),
78+
});
79+
expect(drift).not.toBeNull();
80+
expect(drift?.asMilliseconds()).toEqual(moment.duration(10, 'minutes').asMilliseconds());
81+
});
82+
83+
test('returns null if the "to" is not "now" since we have limited support for date math', () => {
84+
const drift = getDriftTolerance({
85+
from: 'now-6m',
86+
to: 'invalid', // if not set to "now" this function returns null
87+
interval: moment.duration(1000, 'milliseconds'),
88+
});
89+
expect(drift).toBeNull();
90+
});
91+
92+
test('returns null if the "from" does not start with "now-" since we have limited support for date math', () => {
93+
const drift = getDriftTolerance({
94+
from: 'valid', // if not set to "now-x" where x is an interval such as 6m
95+
to: 'now',
96+
interval: moment.duration(1000, 'milliseconds'),
97+
});
98+
expect(drift).toBeNull();
99+
});
100+
101+
test('returns null if the "from" starts with "now-" but has a string instead of an integer', () => {
102+
const drift = getDriftTolerance({
103+
from: 'now-dfdf', // if not set to "now-x" where x is an interval such as 6m
104+
to: 'now',
105+
interval: moment.duration(1000, 'milliseconds'),
106+
});
107+
expect(drift).toBeNull();
108+
});
109+
});
110+
111+
describe('getGapBetweenRuns', () => {
112+
test('it returns a gap of 0 when from and interval match each other and the previous started was from the previous interval time', () => {
113+
const gap = getGapBetweenRuns({
114+
previousStartedAt: nowDate.clone().subtract(5, 'minutes'),
115+
interval: '5m',
116+
from: 'now-5m',
117+
to: 'now',
118+
now: nowDate.clone(),
119+
});
120+
expect(gap).not.toBeNull();
121+
expect(gap?.asMilliseconds()).toEqual(0);
122+
});
123+
124+
test('it returns a negative gap of 1 minute when from overlaps to by 1 minute and the previousStartedAt was 5 minutes ago', () => {
125+
const gap = getGapBetweenRuns({
126+
previousStartedAt: nowDate.clone().subtract(5, 'minutes'),
127+
interval: '5m',
128+
from: 'now-6m',
129+
to: 'now',
130+
now: nowDate.clone(),
131+
});
132+
expect(gap).not.toBeNull();
133+
expect(gap?.asMilliseconds()).toEqual(moment.duration(-1, 'minute').asMilliseconds());
134+
});
135+
136+
test('it returns a negative gap of 5 minutes when from overlaps to by 1 minute and the previousStartedAt was 5 minutes ago', () => {
137+
const gap = getGapBetweenRuns({
138+
previousStartedAt: nowDate.clone().subtract(5, 'minutes'),
139+
interval: '5m',
140+
from: 'now-10m',
141+
to: 'now',
142+
now: nowDate.clone(),
143+
});
144+
expect(gap).not.toBeNull();
145+
expect(gap?.asMilliseconds()).toEqual(moment.duration(-5, 'minute').asMilliseconds());
146+
});
147+
148+
test('it returns a negative gap of 1 minute when from overlaps to by 1 minute and the previousStartedAt was 10 minutes ago and so was the interval', () => {
149+
const gap = getGapBetweenRuns({
150+
previousStartedAt: nowDate.clone().subtract(10, 'minutes'),
151+
interval: '10m',
152+
from: 'now-11m',
153+
to: 'now',
154+
now: nowDate.clone(),
155+
});
156+
expect(gap).not.toBeNull();
157+
expect(gap?.asMilliseconds()).toEqual(moment.duration(-1, 'minute').asMilliseconds());
158+
});
159+
160+
test('it returns a gap of only -30 seconds when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is 30 seconds more', () => {
161+
const gap = getGapBetweenRuns({
162+
previousStartedAt: nowDate
163+
.clone()
164+
.subtract(5, 'minutes')
165+
.subtract(30, 'seconds'),
166+
interval: '5m',
167+
from: 'now-6m',
168+
to: 'now',
169+
now: nowDate.clone(),
170+
});
171+
expect(gap).not.toBeNull();
172+
expect(gap?.asMilliseconds()).toEqual(moment.duration(-30, 'seconds').asMilliseconds());
173+
});
174+
175+
test('it returns an exact 0 gap when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is one minute late', () => {
176+
const gap = getGapBetweenRuns({
177+
previousStartedAt: nowDate.clone().subtract(6, 'minutes'),
178+
interval: '5m',
179+
from: 'now-6m',
180+
to: 'now',
181+
now: nowDate.clone(),
182+
});
183+
expect(gap).not.toBeNull();
184+
expect(gap?.asMilliseconds()).toEqual(moment.duration(0, 'minute').asMilliseconds());
185+
});
186+
187+
test('it returns a gap of 30 seconds when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is one minute and 30 seconds late', () => {
188+
const gap = getGapBetweenRuns({
189+
previousStartedAt: nowDate
190+
.clone()
191+
.subtract(6, 'minutes')
192+
.subtract(30, 'seconds'),
193+
interval: '5m',
194+
from: 'now-6m',
195+
to: 'now',
196+
now: nowDate.clone(),
197+
});
198+
expect(gap).not.toBeNull();
199+
expect(gap?.asMilliseconds()).toEqual(moment.duration(30, 'seconds').asMilliseconds());
200+
});
201+
202+
test('it returns a gap of 1 minute when the from overlaps with now by 1 minute, the interval is 5 minutes but the previous started is two minutes late', () => {
203+
const gap = getGapBetweenRuns({
204+
previousStartedAt: nowDate.clone().subtract(7, 'minutes'),
205+
interval: '5m',
206+
from: 'now-6m',
207+
to: 'now',
208+
now: nowDate.clone(),
209+
});
210+
expect(gap?.asMilliseconds()).not.toBeNull();
211+
expect(gap?.asMilliseconds()).toEqual(moment.duration(1, 'minute').asMilliseconds());
212+
});
213+
214+
test('it returns null if given a previousStartedAt of null', () => {
215+
const gap = getGapBetweenRuns({
216+
previousStartedAt: null,
217+
interval: '5m',
218+
from: 'now-5m',
219+
to: 'now',
220+
now: nowDate.clone(),
221+
});
222+
expect(gap).toBeNull();
223+
});
224+
225+
test('it returns null if the interval is an invalid string such as "invalid"', () => {
226+
const gap = getGapBetweenRuns({
227+
previousStartedAt: nowDate.clone(),
228+
interval: 'invalid', // if not set to "x" where x is an interval such as 6m
229+
from: 'now-5m',
230+
to: 'now',
231+
now: nowDate.clone(),
232+
});
233+
expect(gap).toBeNull();
234+
});
235+
236+
test('it returns null if from is an invalid string such as "invalid"', () => {
237+
const gap = getGapBetweenRuns({
238+
previousStartedAt: nowDate.clone(),
239+
interval: '5m',
240+
from: 'invalid', // if not set to "now-x" where x is an interval such as 6m
241+
to: 'now',
242+
now: nowDate.clone(),
243+
});
244+
expect(gap).toBeNull();
245+
});
246+
247+
test('it returns null if to is an invalid string such as "invalid"', () => {
248+
const gap = getGapBetweenRuns({
249+
previousStartedAt: nowDate.clone(),
250+
interval: '5m',
251+
from: 'now-5m',
252+
to: 'invalid', // if not set to "now" this function returns null
253+
now: nowDate.clone(),
254+
});
255+
expect(gap).toBeNull();
256+
});
257+
});
258+
});

0 commit comments

Comments
 (0)