Skip to content

Commit

Permalink
[8.2] [Security Solution][Detections] Rule Execution Log Feedback and…
Browse files Browse the repository at this point in the history
… Fixes Part Deux (#130072) (#131574)

* [Security Solution][Detections] Rule Execution Log Feedback and Fixes Part Deux (#130072)

## Summary

Addresses feedback and fixes identified in #126215 & #129003

##### Feedback addressed includes:
* Adds toast for restoring global query state after performing `view alerts for execution` action
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/164511565-b77d3dc8-a8b5-4927-a947-54966a58c74f.gif" />
</p>

* Updates global SuperDatePicker to daterange of execution (+/- day) for `view alerts for execution` action (and clear all other filters)
  * See above gif
* Remove redundant `RuleExecutionStatusType` (#129003 (comment))
* Persist table state (DatePicker/StatusFilter/SortField/SortOrder/Pagination) when navigating to other tabs on the same page
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/164512498-59416601-d967-4a27-b0cc-0715cc0662c0.gif" />
</p>

* Fix duration hours bug (`7 hours (25033167ms)` as `06:417:13:000`)
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/164511478-bf0bb6d8-d8b7-4c86-8fbd-b60090f00555.png" />
</p>

* Support `disabled rule` platform error (#126215 (comment))
  * Updated `getAggregateExecutionEvents` to fallback to platform status from `event.outcome` if `security_status` is empty, and also falls back to `error.message` is `security_message` is empty. This also now queries for corresponding `event.outcome` if filter is provided so that platform-only events can still be displayed when filtering.
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/164510056-1e0bce86-8360-4d46-b591-2041457e3244.png" />
</p>

* Verify StatusFilter issue #126215 (comment)
  * Unable to reproduce, I believe the query updates around first querying for status may've fixed this?
* Provide helpful defaults for `to`/`from` and support datemath strings again (#129003 (comment))
  * Created enhancement for this here: #131095
* Adds UI Unit tests for RuleExecutionLog Table
* Finalize API Integration tests for gap remediation events
  * Test methods developed for injecting arbitrary execution events while still working with event-log RBAC. See last [API integration test](https://github.com/elastic/kibana/blob/22cc0c8dbd2a1300675caf4c6d471d211ed44858/x-pack/test/detection_engine_api_integration/security_and_spaces/tests/get_rule_execution_events.ts#L121-L166) for technique. This can further be used to inject many execution events and expand tests around pagination, sorting, filters, etc.
* Fixes `gap_duration`'s of `1-499`ms showing up as `-` instead of `0`
* Fixes restore filters action to restore either absolute or relative datepicker as it originally was
* Resolves #130946
  * Adds `min-height` to tab container
  * Removes scroll-pane from ExceptionsViewer to match Alerts/Execution Log
---

##### Remaining follow-ups:

None! 🎉

### Checklist

Delete any items that are not applicable to this PR.

- [X] 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/main/packages/kbn-i18n/README.md)
- [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
- [X] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [X] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))

(cherry picked from commit 683463e)

# Conflicts:
#	x-pack/plugins/security_solution/cypress/tasks/alerts.ts
#	x-pack/plugins/security_solution/public/detections/pages/detection_engine/rules/details/execution_log_table/execution_log_columns.tsx
#	x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/event_log_reader.ts
#	x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/get_execution_event_aggregation/index.test.ts
#	x-pack/plugins/security_solution/server/lib/detection_engine/rule_execution_log/event_log/get_execution_event_aggregation/index.ts
#	x-pack/test/detection_engine_api_integration/utils/index_event_log_execution_events.ts

* Fixing import
  • Loading branch information
spong authored May 5, 2022
1 parent 8d469d4 commit ba3dc27
Show file tree
Hide file tree
Showing 43 changed files with 1,329 additions and 642 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ export enum RuleExecutionStatus {

export const ruleExecutionStatus = enumeration('RuleExecutionStatus', RuleExecutionStatus);

export type RuleExecutionStatusType = t.TypeOf<typeof ruleExecutionStatus>;

export const ruleExecutionStatusOrder = PositiveInteger;
export type RuleExecutionStatusOrder = t.TypeOf<typeof ruleExecutionStatusOrder>;

Expand Down Expand Up @@ -130,7 +128,7 @@ export const aggregateRuleExecutionEvent = t.type({
timed_out: t.boolean,
indexing_duration_ms: t.number,
search_duration_ms: t.number,
gap_duration_ms: t.number,
gap_duration_s: t.number,
security_status: t.string,
security_message: t.string,
});
Expand All @@ -140,7 +138,7 @@ export type AggregateRuleExecutionEvent = t.TypeOf<typeof aggregateRuleExecution
export const executionLogTableSortColumns = t.keyof({
timestamp: IsoDateString,
duration_ms: t.number,
gap_duration_ms: t.number,
gap_duration_s: t.number,
indexing_duration_ms: t.number,
search_duration_ms: t.number,
schedule_delay_ms: t.number,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,30 @@ import {
ExecutionLogTableSortColumns,
executionLogTableSortColumns,
ruleExecutionStatus,
RuleExecutionStatusType,
RuleExecutionStatus,
} from '../common';

/**
* Types the DefaultStatusFiltersStringArray as:
* - If undefined, then a default array will be set
* - If an array is sent in, then the array will be validated to ensure all elements are a ruleExecutionStatus
* - If an array is sent in, then the array will be validated to ensure all elements are a ruleExecutionStatus (or that the array is empty)
*/
export const DefaultStatusFiltersStringArray = new t.Type<
RuleExecutionStatusType[],
RuleExecutionStatusType[],
RuleExecutionStatus[],
RuleExecutionStatus[],
unknown
>(
'DefaultStatusFiltersStringArray',
t.array(ruleExecutionStatus).is,
(input, context): Either<t.Errors, RuleExecutionStatusType[]> => {
(input, context): Either<t.Errors, RuleExecutionStatus[]> => {
if (input == null) {
return t.success([]);
} else if (typeof input === 'string') {
return t.array(ruleExecutionStatus).validate(input.split(','), context);
if (input === '') {
return t.success([]);
} else {
return t.array(ruleExecutionStatus).validate(input.split(','), context);
}
} else {
return t.array(ruleExecutionStatus).validate(input, context);
}
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/cypress/tasks/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ export const closeAlerts = () => {
};

export const expandFirstAlertActions = () => {
cy.get(TIMELINE_CONTEXT_MENU_BTN).should('be.visible');
cy.get(TIMELINE_CONTEXT_MENU_BTN).find('svg').should('have.class', 'euiIcon-isLoaded');
cy.get(TIMELINE_CONTEXT_MENU_BTN).first().click({ force: true });
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ const MyFlexItem = styled(EuiFlexItem)`
}
`;

const MyExceptionsContainer = styled(EuiFlexGroup)`
height: 600px;
overflow: hidden;
`;

const MyExceptionItemContainer = styled(EuiFlexGroup)`
margin: ${({ theme }) => `0 ${theme.eui.euiSize} ${theme.eui.euiSize} 0`};
`;
Expand Down Expand Up @@ -55,7 +50,7 @@ const ExceptionsViewerItemsComponent: React.FC<ExceptionsViewerItemsProps> = ({
onEditExceptionItem,
disableActions,
}): JSX.Element => (
<MyExceptionsContainer direction="column" className="eui-yScrollWithShadows">
<EuiFlexGroup direction="column" className="eui-yScrollWithShadows">
{showEmpty || showNoResults || isInitLoading ? (
<EuiFlexItem grow={1}>
<EuiEmptyPrompt
Expand Down Expand Up @@ -107,7 +102,7 @@ const ExceptionsViewerItemsComponent: React.FC<ExceptionsViewerItemsProps> = ({
</MyExceptionItemContainer>
</EuiFlexItem>
)}
</MyExceptionsContainer>
</EuiFlexGroup>
);

ExceptionsViewerItemsComponent.displayName = 'ExceptionsViewerItemsComponent';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ describe('VisualizationActions', () => {
addError: jest.fn(),
addSuccess: jest.fn(),
addWarning: jest.fn(),
remove: jest.fn(),
},
},
http: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ jest.mock('../../../../common/lib/kibana', () => ({
addError: jest.fn(),
addSuccess: jest.fn(),
addWarning: jest.fn(),
remove: jest.fn(),
}),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ jest.mock('../../lib/kibana', () => ({
addError: jest.fn(),
addSuccess: jest.fn(),
addWarning: mockAddWarning,
remove: jest.fn(),
}),
useKibana: () => ({
services: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const createAppToastsMock = (): jest.Mocked<UseAppToasts> => ({
addError: jest.fn(),
addSuccess: jest.fn(),
addWarning: jest.fn(),
remove: jest.fn(),
api: {
get$: jest.fn(),
add: jest.fn(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,18 @@ describe('useAppToasts', () => {
let addErrorMock: jest.Mock;
let addSuccessMock: jest.Mock;
let addWarningMock: jest.Mock;
let removeMock: jest.Mock;

beforeEach(() => {
addErrorMock = jest.fn();
addSuccessMock = jest.fn();
addWarningMock = jest.fn();
removeMock = jest.fn();
(useToasts as jest.Mock).mockImplementation(() => ({
addError: addErrorMock,
addSuccess: addSuccessMock,
addWarning: addWarningMock,
remove: removeMock,
}));
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { IEsError, isEsError } from '../../../../../../src/plugins/data/public';
import { ErrorToastOptions, ToastsStart, Toast } from '../../../../../../src/core/public';
import { useToasts } from '../lib/kibana';

export type UseAppToasts = Pick<ToastsStart, 'addSuccess' | 'addWarning'> & {
export type UseAppToasts = Pick<ToastsStart, 'addSuccess' | 'addWarning' | 'remove'> & {
api: ToastsStart;
addError: (error: unknown, options: ErrorToastOptions) => Toast;
};
Expand All @@ -36,6 +36,7 @@ export const useAppToasts = (): UseAppToasts => {
const addError = useRef(toasts.addError.bind(toasts)).current;
const addSuccess = useRef(toasts.addSuccess.bind(toasts)).current;
const addWarning = useRef(toasts.addWarning.bind(toasts)).current;
const remove = useRef(toasts.remove.bind(toasts)).current;

const _addError = useCallback(
(error: unknown, options: ErrorToastOptions) => {
Expand All @@ -46,8 +47,8 @@ export const useAppToasts = (): UseAppToasts => {
);

return useMemo(
() => ({ api: toasts, addError: _addError, addSuccess, addWarning }),
[_addError, addSuccess, addWarning, toasts]
() => ({ api: toasts, addError: _addError, addSuccess, addWarning, remove }),
[_addError, addSuccess, addWarning, remove, toasts]
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ export const createStartServicesMock = (
theme: {
theme$: themeServiceMock.createTheme$(),
},
timelines: {
getLastUpdated: jest.fn(),
getFieldBrowser: jest.fn(),
},
} as unknown as StartServices;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ jest.mock('../../../../common/lib/kibana/kibana_react', () => {
addWarning: jest.fn(),
addError: jest.fn(),
addSuccess: jest.fn(),
remove: jest.fn(),
},
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export const fetchRuleExecutionEvents = async ({
duration_ms: 3866,
es_search_duration_ms: 1236,
execution_uuid: '88d15095-7937-462c-8f21-9763e1387cad',
gap_duration_ms: 0,
gap_duration_s: 0,
indexing_duration_ms: 95,
message:
"rule executed: siem.queryRule:fb1fc150-a292-11ec-a2cf-c1b28b0392b0: 'Lots of Execution Events'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ describe('Detections Rules API', () => {
start: '2001-01-01T17:00:00.000Z',
end: '2001-01-02T17:00:00.000Z',
queryText: '',
statusFilters: '',
statusFilters: [],
signal: abortCtrl.signal,
});

Expand Down Expand Up @@ -659,7 +659,7 @@ describe('Detections Rules API', () => {
start: 'now-30',
end: 'now',
queryText: '',
statusFilters: '',
statusFilters: [],
signal: abortCtrl.signal,
});
expect(response).toEqual(responseMock);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { SortOrder } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { camelCase } from 'lodash';
import dateMath from '@kbn/datemath';
import { HttpStart } from 'src/core/public';
Expand All @@ -18,7 +19,11 @@ import {
DETECTION_ENGINE_RULES_PREVIEW,
detectionEngineRuleExecutionEventsUrl,
} from '../../../../../common/constants';
import { BulkAction } from '../../../../../common/detection_engine/schemas/common';
import {
AggregateRuleExecutionEvent,
BulkAction,
RuleExecutionStatus,
} from '../../../../../common/detection_engine/schemas/common';
import {
FullResponseSchema,
PreviewResponse,
Expand Down Expand Up @@ -320,11 +325,11 @@ export const exportRules = async ({
* @param start Start daterange either in UTC ISO8601 or as datemath string (e.g. `2021-12-29T02:44:41.653Z` or `now-30`)
* @param end End daterange either in UTC ISO8601 or as datemath string (e.g. `2021-12-29T02:44:41.653Z` or `now/w`)
* @param queryText search string in querystring format (e.g. `event.duration > 1000 OR kibana.alert.rule.execution.metrics.execution_gap_duration_s > 100`)
* @param statusFilters comma separated string of `statusFilters` (e.g. `succeeded,failed,partial failure`)
* @param statusFilters RuleExecutionStatus[] array of `statusFilters` (e.g. `succeeded,failed,partial failure`)
* @param page current page to fetch
* @param perPage number of results to fetch per page
* @param sortField field to sort by
* @param sortOrder what order to sort by (e.g. `asc` or `desc`)
* @param sortField keyof AggregateRuleExecutionEvent field to sort by
* @param sortOrder SortOrder what order to sort by (e.g. `asc` or `desc`)
* @param signal AbortSignal Optional signal for cancelling the request
*
* @throws An error if response is not OK
Expand All @@ -345,11 +350,11 @@ export const fetchRuleExecutionEvents = async ({
start: string;
end: string;
queryText?: string;
statusFilters?: string;
statusFilters?: RuleExecutionStatus[];
page?: number;
perPage?: number;
sortField?: string;
sortOrder?: string;
sortField?: keyof AggregateRuleExecutionEvent;
sortOrder?: SortOrder;
signal?: AbortSignal;
}): Promise<GetAggregateRuleExecutionEventsResponse> => {
const url = detectionEngineRuleExecutionEventsUrl(ruleId);
Expand All @@ -361,7 +366,7 @@ export const fetchRuleExecutionEvents = async ({
start: startDate?.utc().toISOString(),
end: endDate?.utc().toISOString(),
query_text: queryText,
status_filters: statusFilters,
status_filters: statusFilters?.sort()?.join(','),
page,
per_page: perPage,
sort_field: sortField,
Expand Down
Loading

0 comments on commit ba3dc27

Please sign in to comment.