Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solution][Detections] Adds Rule Execution Log table #124198

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d8ddc0b
Adds Rule Execution Log table
spong Feb 1, 2022
c336868
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 1, 2022
2187823
Fixes tests and cleanup
spong Feb 1, 2022
9de216d
Adds support for maxEvents and cleans up types around execution event
spong Feb 1, 2022
729b272
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 1, 2022
defe453
Adds totalHits metric to remaining executors
spong Feb 1, 2022
1fec617
Fixes test
spong Feb 1, 2022
bd96ce4
Unused import
spong Feb 1, 2022
de5ec8d
Fixed translations
spong Feb 1, 2022
a675d98
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 1, 2022
5a6d237
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 3, 2022
eb66e1a
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 3, 2022
90c1ef6
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 5, 2022
5a736ab
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 7, 2022
cdb317b
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 7, 2022
825b103
Removes going-to-run event
spong Feb 7, 2022
b5e50a5
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 8, 2022
404b2bb
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 9, 2022
0ae6f0d
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 10, 2022
b492e77
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 11, 2022
d703131
Merge branch 'main' of github.com:elastic/kibana into rule-execution-…
spong Feb 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion x-pack/plugins/event_log/generated/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,12 @@
"number_of_triggered_actions": {
"type": "long"
},
Comment on lines 292 to 294
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saw this was just added in #123567, shall we add it to the table as well, or is it getting too congested and better to wait till we allow expanding rows with additional data?

"total_alerts": {
"type": "long"
},
"total_hits": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"hits" isn't a general alerting thing, so I think we'll need a comment somewhere describing what it is - perhaps in the mappings.js file?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If "hits" isn't a general alerting thing, I don't think it should be part of the event-log. How do the "hits" differ from the "alerts"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see these two issues for details #120678 #120668, but gist here is total_hits is the total number of candidate alerts found during a specific execution, and total_alerts is the total number of those candidate alerts that were actually created during that execution. The totals between candidates and created can differ as a result of reaching the configured max_signals threshold, or if duplicate alerts (alerts previously detected) were identified, but never written (and I believe a case with exceptions as well, but would need to verify).

I totally agree about naming (perhaps total_candidate_alerts better fits?), but that aside, is there no representation of these concepts in general alerting? Is there no circuit breaker for the number alerts that can be written per execution, or management of duplicates where discerning between these two values is of use to the general alerting user (like for the security solution user outlined in the issues above)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, interesting. We'll have an issue open soon-ish (from some previous research) about having a maximum number of actions and alerts (separate values) per rule execution. And we would want to track the total number as being different if we cap the number via the maximums.

So it isn't a thing now, but probably will be. So ... maybe just some word-smithing on this. Candidate doesn't seem quite right. Something like "total" as the number the rule produced, and "triggered" instead of "candidate", so "totalAlerts" and "triggeredAlerts"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If "hits" isn't a general alerting thing, I don't think it should be part of the event-log

We already have a few metrics stored in the event log which are not a general alerting thing (strictly speaking) but which make sense from the Security Solution and AAD perspective. These are total_indexing_duration_ms, total_search_duration_ms and execution_gap_duration_s. Imho the new metrics have the same relation to the overall logic of Detection Engine and AAD.

I agree that additional documentation and/or better naming would help though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation, @spong, that makes a lot of sense to me.

What about total_alerts_created and total_alerts_detected?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about total_alerts_created and total_alerts_detected?

Those two sound good to me @kobelb! 🙂 I'll wait a moment to see if anyone else has additional input and will then make the change.

In addition to adding documentation within event-log as @pmuellr recommended, are there any other supporting docs we should be updating with these fields as well? I know there's the AAD schema spreadsheet, but I don't believe the event-log is schema represented in there, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to adding documentation within event-log as @pmuellr recommended, are there any other supporting docs we should be updating with these fields as well? I know there's the AAD schema spreadsheet, but I don't believe the event-log is schema represented in there, no?

I'm not aware of any docs about the fields in the event-log or any docs that recommend that users search these indices. We've treated them as a traditional hidden index where users might find it, but they can't rely on the names of fields.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pulled this feature out to a dedicated PR (#126210), will ping reviewers when ready for review.

"type": "long"
},
"total_indexing_duration_ms": {
"type": "long"
},
Expand Down Expand Up @@ -347,4 +353,4 @@
}
}
}
}
}
2 changes: 2 additions & 0 deletions x-pack/plugins/event_log/generated/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,8 @@ export const EventSchema = schema.maybe(
metrics: schema.maybe(
schema.object({
number_of_triggered_actions: ecsNumber(),
total_alerts: ecsNumber(),
total_hits: ecsNumber(),
total_indexing_duration_ms: ecsNumber(),
total_search_duration_ms: ecsNumber(),
execution_gap_duration_s: ecsNumber(),
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/event_log/scripts/mappings.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ exports.EcsCustomPropertyMappings = {
number_of_triggered_actions: {
type: 'long',
},
total_alerts: {
type: 'long',
},
total_hits: {
type: 'long',
},
total_indexing_duration_ms: {
type: 'long',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ export enum RuleExecutionStatus {
}

export const ruleExecutionStatus = enumeration('RuleExecutionStatus', RuleExecutionStatus);
export type RuleExecutionStatuslol = t.TypeOf<typeof ruleExecutionStatus>;

export const ruleExecutionStatusOrder = PositiveInteger;
export type RuleExecutionStatusOrder = t.TypeOf<typeof ruleExecutionStatusOrder>;
Expand All @@ -70,10 +71,15 @@ export const ruleExecutionStatusOrderByStatus: Record<
// -------------------------------------------------------------------------------------------------
// Rule execution metrics

export const countMetric = PositiveInteger;
export type CountMetric = t.TypeOf<typeof countMetric>;

export const durationMetric = PositiveInteger;
export type DurationMetric = t.TypeOf<typeof durationMetric>;

export const ruleExecutionMetrics = t.partial({
total_alerts: countMetric,
total_hits: countMetric,
total_search_duration_ms: durationMetric,
total_indexing_duration_ms: durationMetric,
execution_gap_duration_s: durationMetric,
Expand Down Expand Up @@ -106,3 +112,48 @@ export const ruleExecutionEvent = t.type({
});

export type RuleExecutionEvent = t.TypeOf<typeof ruleExecutionEvent>;

// -------------------------------------------------------------------------------------------------
// Aggregate Rule execution events

const metrics = t.type({
total_alerts: t.number,
total_hits: t.number,
total_indexing_duration_ms: t.number,
total_search_duration_ms: t.number,
});

const execution = t.type({
metrics,
status: t.string,
});

const rule = t.type({
execution,
});

const alert = t.type({
rule,
});

const event = t.type({
duration: t.number,
});

const task = t.type({
schedule_delay: t.number,
});

const kibana = t.type({
task,
alert,
});

export const aggregateRuleExecutionEvent = t.type({
kibana,
event,
message: t.string,
'@timestamp': IsoDateString,
});

export type AggregateRuleExecutionEvent = t.TypeOf<typeof aggregateRuleExecutionEvent>;
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ export const GetRuleExecutionEventsRequestParams = t.exact(
})
);

export const GetRuleExecutionEventsQueryParams = t.exact(
t.type({
start: t.string,
end: t.string,
filters: t.union([t.string, t.undefined]),
})
);

export type GetRuleExecutionEventsRequestParams = t.TypeOf<
typeof GetRuleExecutionEventsRequestParams
>;
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import * as t from 'io-ts';
import { ruleExecutionEvent } from '../common';
import { aggregateRuleExecutionEvent, ruleExecutionEvent } from '../common';

export const GetRuleExecutionEventsResponse = t.exact(
t.type({
Expand All @@ -15,3 +15,14 @@ export const GetRuleExecutionEventsResponse = t.exact(
);

export type GetRuleExecutionEventsResponse = t.TypeOf<typeof GetRuleExecutionEventsResponse>;

export const GetAggregateRuleExecutionEventsResponse = t.exact(
t.type({
events: t.array(aggregateRuleExecutionEvent),
maxEvents: t.number,
})
);

export type GetAggregateRuleExecutionEventsResponse = t.TypeOf<
typeof GetAggregateRuleExecutionEventsResponse
>;
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
* 2.0.
*/

import { RuleExecutionStatus } from '../../../../../../common/detection_engine/schemas/common';
import {
GetRuleExecutionEventsResponse,
GetAggregateRuleExecutionEventsResponse,
RulesSchema,
} from '../../../../../../common/detection_engine/schemas/response';

Expand Down Expand Up @@ -62,19 +61,46 @@ export const fetchRules = async (_: FetchRulesProps): Promise<FetchRulesResponse

export const fetchRuleExecutionEvents = async ({
ruleId,
start,
end,
filters,
signal,
}: {
ruleId: string;
start: string;
end: string;
filters?: string;
signal?: AbortSignal;
}): Promise<GetRuleExecutionEventsResponse> => {
}): Promise<GetAggregateRuleExecutionEventsResponse> => {
return Promise.resolve({
events: [
{
date: '2021-12-29T10:42:59.996Z',
status: RuleExecutionStatus.succeeded,
message: 'Rule executed successfully',
kibana: {
task: {
schedule_delay: 13980000000,
},
alert: {
rule: {
execution: {
metrics: {
total_alerts: 0,
total_hits: 0,
total_indexing_duration_ms: 0,
total_search_duration_ms: 9,
},
status: 'succeeded',
},
},
},
},
event: {
duration: 2065000000,
},
message: 'succeeded',
'@timestamp': '2022-02-01T05:51:27.143Z',
},
],
maxEvents: 1,
});
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -678,19 +678,36 @@ describe('Detections Rules API', () => {
});

test('calls API with correct parameters', async () => {
await fetchRuleExecutionEvents({ ruleId: '42', signal: abortCtrl.signal });
await fetchRuleExecutionEvents({
ruleId: '42',
start: 'now-30',
end: 'now',
filters: '',
signal: abortCtrl.signal,
});

expect(fetchMock).toHaveBeenCalledWith(
'/internal/detection_engine/rules/42/execution/events',
{
method: 'GET',
query: {
end: 'now',
filters: '',
start: 'now-30',
},
signal: abortCtrl.signal,
}
);
});

test('returns API response as is', async () => {
const response = await fetchRuleExecutionEvents({ ruleId: '42', signal: abortCtrl.signal });
const response = await fetchRuleExecutionEvents({
ruleId: '42',
start: 'now-30',
end: 'now',
filters: '',
signal: abortCtrl.signal,
});
expect(response).toEqual(responseMock);
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {
} from '../../../../../common/detection_engine/schemas/request';
import {
RulesSchema,
GetRuleExecutionEventsResponse,
GetAggregateRuleExecutionEventsResponse,
} from '../../../../../common/detection_engine/schemas/response';

import {
Expand Down Expand Up @@ -371,20 +371,30 @@ export const exportRules = async ({
* Fetch rule execution events (e.g. status changes) from Event Log.
*
* @param ruleId string Saved Object ID of the rule (`rule.id`, not static `rule.rule_id`)
* @param start string Start daterange either in UTC ISO8601 or as datemath string (e.g. `2021-12-29T02:44:41.653Z` or `now-30`)
* @param end string End daterange either in UTC ISO8601 or as datemath string (e.g. `2021-12-29T02:44:41.653Z` or `now/w`)
* @param filters string Filters to apply to the search in querystring format (e.g. `event.duration > 1000 OR kibana.alert.rule.execution.metrics.total_hits > 100`)
* @param signal AbortSignal Optional signal for cancelling the request
*
* @throws An error if response is not OK
*/
export const fetchRuleExecutionEvents = async ({
ruleId,
start,
end,
filters,
signal,
}: {
ruleId: string;
start: string;
end: string;
filters?: string;
signal?: AbortSignal;
}): Promise<GetRuleExecutionEventsResponse> => {
}): Promise<GetAggregateRuleExecutionEventsResponse> => {
const url = detectionEngineRuleExecutionEventsUrl(ruleId);
return KibanaServices.get().http.fetch<GetRuleExecutionEventsResponse>(url, {
return KibanaServices.get().http.fetch<GetAggregateRuleExecutionEventsResponse>(url, {
method: 'GET',
query: { start, end, filters },
signal,
});
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ export const RULE_AND_TIMELINE_FETCH_FAILURE = i18n.translate(
}
);

export const RULE_EXECUTION_FETCH_FAILURE = i18n.translate(
'xpack.securitySolution.containers.detectionEngine.ruleExecutionLogFailureDescription',
{
defaultMessage: 'Failed to fetch Rule Execution Events',
}
);

export const RULE_ADD_FAILURE = i18n.translate(
'xpack.securitySolution.containers.detectionEngine.addRuleFailDescription',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@ export type RulesSortingFields =
| 'enabled'
| 'execution_summary.last_execution.date'
| 'execution_summary.last_execution.metrics.execution_gap_duration_s'
| 'execution_summary.last_execution.metrics.total_alerts'
| 'execution_summary.last_execution.metrics.total_hits'
| 'execution_summary.last_execution.metrics.total_indexing_duration_ms'
| 'execution_summary.last_execution.metrics.total_search_duration_ms'
| 'execution_summary.last_execution.status'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import React from 'react';
import { QueryClient, QueryClientProvider } from 'react-query';
import { renderHook, cleanup } from '@testing-library/react-hooks';

import { RuleExecutionStatus } from '../../../../../common/detection_engine/schemas/common';
import { useRuleExecutionEvents } from './use_rule_execution_events';

import * as api from './api';
Expand Down Expand Up @@ -45,9 +44,18 @@ describe('useRuleExecutionEvents', () => {
};

const render = () =>
renderHook(() => useRuleExecutionEvents(SOME_RULE_ID), {
wrapper: createReactQueryWrapper(),
});
renderHook(
() =>
useRuleExecutionEvents({
ruleId: SOME_RULE_ID,
start: 'now-30',
end: 'now',
filters: '',
}),
{
wrapper: createReactQueryWrapper(),
}
);

it('calls the API via fetchRuleExecutionEvents', async () => {
const fetchRuleExecutionEvents = jest.spyOn(api, 'fetchRuleExecutionEvents');
Expand Down Expand Up @@ -77,13 +85,36 @@ describe('useRuleExecutionEvents', () => {
expect(result.current.isLoading).toEqual(false);
expect(result.current.isSuccess).toEqual(true);
expect(result.current.isError).toEqual(false);
expect(result.current.data).toEqual([
{
date: '2021-12-29T10:42:59.996Z',
status: RuleExecutionStatus.succeeded,
message: 'Rule executed successfully',
},
]);
expect(result.current.data).toEqual({
events: [
{
kibana: {
task: {
schedule_delay: 13980000000,
},
alert: {
rule: {
execution: {
metrics: {
total_alerts: 0,
total_hits: 0,
total_indexing_duration_ms: 0,
total_search_duration_ms: 9,
},
status: 'succeeded',
},
},
},
},
event: {
duration: 2065000000,
},
message: 'succeeded',
'@timestamp': '2022-02-01T05:51:27.143Z',
},
],
maxEvents: 1,
});
});

it('handles exceptions from the API', async () => {
Expand Down
Loading