Skip to content

Commit

Permalink
[8.x] [Security Solution] Security Alert Table Performance Optimizati…
Browse files Browse the repository at this point in the history
…ons (#192827) (#195935)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Security Alert Table Performance Optimizations
(#192827)](#192827)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Jatin
Kathuria","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-11T14:29:58Z","message":"[Security
Solution] Security Alert Table Performance Optimizations (#192827)\n\n##
Summary\r\n\r\nThere are few performance issues with Security's alert
table today which\r\nthis PR aims to fix.\r\n\r\n1. Virtualization was
not working in Grid View and Event Rendered View.\r\nConstraining the
height of `EuiDataGrid` makes virtualization work.\r\n2. It was taking
lot of time when switching between Event Rendered View\r\nand Grid
view.\r\n\r\nBelow we compare some performance metric in both above
scenarios.\r\n\r\n## Solution\r\n\r\n- Solution was to enable
virtualization but bounding the height of the\r\ntable.\r\n- If number
of rows < 10, then height is `auto` else it maxed at `600px`\r\n\r\n##
Grid View - 100 Rows - Changing Query\r\n\r\nIf you compare times below
for rendering of Alert Table Components, we\r\nsee that improvement is
of 2.5-5 times.\r\n\r\nNumber of re-renders are also fewer.\r\n\r\n###
Before\r\n\r\n\r\n![grafik](https://github.com/user-attachments/assets/c2e14b24-750b-4c7c-981f-d7fde85cfeff)\r\n\r\n\r\n###
After\r\n\r\n\r\n![grafik](https://github.com/user-attachments/assets/8eab15e2-4e93-41c3-88fe-bdadb367785d)\r\n\r\n\r\n##
Grid View - 100 Rows - Changing view to Event Rendered View\r\n\r\nEvent
Rendered View is where these optimization actually shine. If
you\r\nobserve the rendering times, performance gain is upto 10 times
for all\r\nrenders. Number of re-renders are also reduced.\r\n\r\n###
Before\r\n\r\n![grafik](https://github.com/user-attachments/assets/5cba7d86-6b71-496a-9004-8ce5b3599f4e)\r\n\r\n\r\n###
After\r\n\r\n![grafik](https://github.com/user-attachments/assets/db9d07fa-2afe-4f3d-8261-cc16d40165bc)\r\n\r\n\r\n##
Grid View - Change page size from 20 rows to 100 rows\r\n\r\nHere also
we achieve almost 15 times improvement in the render times of\r\nthe
table.\r\n\r\n###
Before\r\n\r\n![grafik](https://github.com/user-attachments/assets/a8a3467a-7774-4d02-ab4c-80433aa99660)\r\n\r\n
\r\n###
After\r\n\r\n![grafik](https://github.com/user-attachments/assets/1c9a8645-7fbd-49f2-b72b-83d8688decbc)\r\n\r\n\r\n##
Desk Testing Guide\r\n\r\nBelow scenarios need to tested to make sure
Alert Table is working fine\r\nin both Grid View and Event Rendered
View\r\n\r\n1. Alert Page\r\n - Analyzer is working fine\r\n - Session
Viewer is working fine\r\n - Alert Table when grouping is enabled.\r\n3.
Rule Details Page\r\n - Analyzer is working fine\r\n - Session Viewer is
working fine\r\n - Alert Table when grouping is enabled.\r\n5. Full
Screen.\r\n - Analyzer is working fine\r\n - Session Viewer is working
fine\r\n7. Number of records < 10\r\n - should resize the table to fit
the content.\r\n8. Number of records > 10\r\n - should have fixed height
and no-virtualization enabled.\r\n9. Change view from Event Rendered
View\r\n10. Increase rows from default of `20` to
`100`","sha":"a84d62dce6f386ef114ef25a99d2b7e0c56719c2","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:Threat
Hunting:Investigations","backport:prev-minor"],"title":"[Security
Solution] Security Alert Table Performance
Optimizations","number":192827,"url":"https://github.com/elastic/kibana/pull/192827","mergeCommit":{"message":"[Security
Solution] Security Alert Table Performance Optimizations (#192827)\n\n##
Summary\r\n\r\nThere are few performance issues with Security's alert
table today which\r\nthis PR aims to fix.\r\n\r\n1. Virtualization was
not working in Grid View and Event Rendered View.\r\nConstraining the
height of `EuiDataGrid` makes virtualization work.\r\n2. It was taking
lot of time when switching between Event Rendered View\r\nand Grid
view.\r\n\r\nBelow we compare some performance metric in both above
scenarios.\r\n\r\n## Solution\r\n\r\n- Solution was to enable
virtualization but bounding the height of the\r\ntable.\r\n- If number
of rows < 10, then height is `auto` else it maxed at `600px`\r\n\r\n##
Grid View - 100 Rows - Changing Query\r\n\r\nIf you compare times below
for rendering of Alert Table Components, we\r\nsee that improvement is
of 2.5-5 times.\r\n\r\nNumber of re-renders are also fewer.\r\n\r\n###
Before\r\n\r\n\r\n![grafik](https://github.com/user-attachments/assets/c2e14b24-750b-4c7c-981f-d7fde85cfeff)\r\n\r\n\r\n###
After\r\n\r\n\r\n![grafik](https://github.com/user-attachments/assets/8eab15e2-4e93-41c3-88fe-bdadb367785d)\r\n\r\n\r\n##
Grid View - 100 Rows - Changing view to Event Rendered View\r\n\r\nEvent
Rendered View is where these optimization actually shine. If
you\r\nobserve the rendering times, performance gain is upto 10 times
for all\r\nrenders. Number of re-renders are also reduced.\r\n\r\n###
Before\r\n\r\n![grafik](https://github.com/user-attachments/assets/5cba7d86-6b71-496a-9004-8ce5b3599f4e)\r\n\r\n\r\n###
After\r\n\r\n![grafik](https://github.com/user-attachments/assets/db9d07fa-2afe-4f3d-8261-cc16d40165bc)\r\n\r\n\r\n##
Grid View - Change page size from 20 rows to 100 rows\r\n\r\nHere also
we achieve almost 15 times improvement in the render times of\r\nthe
table.\r\n\r\n###
Before\r\n\r\n![grafik](https://github.com/user-attachments/assets/a8a3467a-7774-4d02-ab4c-80433aa99660)\r\n\r\n
\r\n###
After\r\n\r\n![grafik](https://github.com/user-attachments/assets/1c9a8645-7fbd-49f2-b72b-83d8688decbc)\r\n\r\n\r\n##
Desk Testing Guide\r\n\r\nBelow scenarios need to tested to make sure
Alert Table is working fine\r\nin both Grid View and Event Rendered
View\r\n\r\n1. Alert Page\r\n - Analyzer is working fine\r\n - Session
Viewer is working fine\r\n - Alert Table when grouping is enabled.\r\n3.
Rule Details Page\r\n - Analyzer is working fine\r\n - Session Viewer is
working fine\r\n - Alert Table when grouping is enabled.\r\n5. Full
Screen.\r\n - Analyzer is working fine\r\n - Session Viewer is working
fine\r\n7. Number of records < 10\r\n - should resize the table to fit
the content.\r\n8. Number of records > 10\r\n - should have fixed height
and no-virtualization enabled.\r\n9. Change view from Event Rendered
View\r\n10. Increase rows from default of `20` to
`100`","sha":"a84d62dce6f386ef114ef25a99d2b7e0c56719c2"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192827","number":192827,"mergeCommit":{"message":"[Security
Solution] Security Alert Table Performance Optimizations (#192827)\n\n##
Summary\r\n\r\nThere are few performance issues with Security's alert
table today which\r\nthis PR aims to fix.\r\n\r\n1. Virtualization was
not working in Grid View and Event Rendered View.\r\nConstraining the
height of `EuiDataGrid` makes virtualization work.\r\n2. It was taking
lot of time when switching between Event Rendered View\r\nand Grid
view.\r\n\r\nBelow we compare some performance metric in both above
scenarios.\r\n\r\n## Solution\r\n\r\n- Solution was to enable
virtualization but bounding the height of the\r\ntable.\r\n- If number
of rows < 10, then height is `auto` else it maxed at `600px`\r\n\r\n##
Grid View - 100 Rows - Changing Query\r\n\r\nIf you compare times below
for rendering of Alert Table Components, we\r\nsee that improvement is
of 2.5-5 times.\r\n\r\nNumber of re-renders are also fewer.\r\n\r\n###
Before\r\n\r\n\r\n![grafik](https://github.com/user-attachments/assets/c2e14b24-750b-4c7c-981f-d7fde85cfeff)\r\n\r\n\r\n###
After\r\n\r\n\r\n![grafik](https://github.com/user-attachments/assets/8eab15e2-4e93-41c3-88fe-bdadb367785d)\r\n\r\n\r\n##
Grid View - 100 Rows - Changing view to Event Rendered View\r\n\r\nEvent
Rendered View is where these optimization actually shine. If
you\r\nobserve the rendering times, performance gain is upto 10 times
for all\r\nrenders. Number of re-renders are also reduced.\r\n\r\n###
Before\r\n\r\n![grafik](https://github.com/user-attachments/assets/5cba7d86-6b71-496a-9004-8ce5b3599f4e)\r\n\r\n\r\n###
After\r\n\r\n![grafik](https://github.com/user-attachments/assets/db9d07fa-2afe-4f3d-8261-cc16d40165bc)\r\n\r\n\r\n##
Grid View - Change page size from 20 rows to 100 rows\r\n\r\nHere also
we achieve almost 15 times improvement in the render times of\r\nthe
table.\r\n\r\n###
Before\r\n\r\n![grafik](https://github.com/user-attachments/assets/a8a3467a-7774-4d02-ab4c-80433aa99660)\r\n\r\n
\r\n###
After\r\n\r\n![grafik](https://github.com/user-attachments/assets/1c9a8645-7fbd-49f2-b72b-83d8688decbc)\r\n\r\n\r\n##
Desk Testing Guide\r\n\r\nBelow scenarios need to tested to make sure
Alert Table is working fine\r\nin both Grid View and Event Rendered
View\r\n\r\n1. Alert Page\r\n - Analyzer is working fine\r\n - Session
Viewer is working fine\r\n - Alert Table when grouping is enabled.\r\n3.
Rule Details Page\r\n - Analyzer is working fine\r\n - Session Viewer is
working fine\r\n - Alert Table when grouping is enabled.\r\n5. Full
Screen.\r\n - Analyzer is working fine\r\n - Session Viewer is working
fine\r\n7. Number of records < 10\r\n - should resize the table to fit
the content.\r\n8. Number of records > 10\r\n - should have fixed height
and no-virtualization enabled.\r\n9. Change view from Event Rendered
View\r\n10. Increase rows from default of `20` to
`100`","sha":"a84d62dce6f386ef114ef25a99d2b7e0c56719c2"}}]}] BACKPORT-->

Co-authored-by: Jatin Kathuria <[email protected]>
  • Loading branch information
kibanamachine and logeekal authored Oct 11, 2024
1 parent 77e8ad4 commit 5ebef44
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ import { useFetchNotes } from '../../../notes/hooks/use_fetch_notes';

const { updateIsLoading, updateTotalCount } = dataTableActions;

const DEFAULT_DATA_GRID_HEIGHT = 600;

const ALERT_TABLE_FEATURE_ID: AlertsTableStateProps['featureIds'] = ['siem'];

// Highlight rows with building block alerts
const shouldHighlightRow = (alert: Alert) => !!alert[ALERT_BUILDING_BLOCK_TYPE];

Expand Down Expand Up @@ -158,6 +162,7 @@ export const AlertsTableComponent: FC<DetectionEngineAlertTableProps> = ({
sessionViewConfig,
viewMode: tableView = eventsDefaultModel.viewMode,
columns,
totalCount: count,
} = getAlertsDefaultModel(license),
} = useShallowEqualSelector((state: State) => eventsViewerSelector(state, tableId));

Expand Down Expand Up @@ -269,13 +274,20 @@ export const AlertsTableComponent: FC<DetectionEngineAlertTableProps> = ({

const { onLoad } = useFetchNotes();

const toolbarVisibility = useMemo(() => {
return {
showColumnSelector: !isEventRenderedView,
showSortSelector: !isEventRenderedView,
};
}, [isEventRenderedView]);

const alertStateProps: AlertsTableStateProps = useMemo(
() => ({
alertsTableConfigurationRegistry: triggersActionsUi.alertsTableConfigurationRegistry,
configurationId: configId,
// stores separate configuration based on the view of the table
id: `detection-engine-alert-table-${configId}-${tableView}`,
featureIds: ['siem'],
featureIds: ALERT_TABLE_FEATURE_ID,
query: finalBoolQuery,
gridStyle,
shouldHighlightRow,
Expand All @@ -286,11 +298,12 @@ export const AlertsTableComponent: FC<DetectionEngineAlertTableProps> = ({
cellContext,
onLoaded: onLoad,
runtimeMappings: sourcererDataView?.runtimeFieldMap as RunTimeMappings,
toolbarVisibility: {
showColumnSelector: !isEventRenderedView,
showSortSelector: !isEventRenderedView,
},
dynamicRowHeight: isEventRenderedView,
toolbarVisibility,
// if records are too less, we don't want table to be of fixed height.
// it should shrink to the content height.
// Height setting enables/disables virtualization depending on fixed/undefined height values respectively.
height: count >= 20 ? `${DEFAULT_DATA_GRID_HEIGHT}px` : undefined,
initialPageSize: 50,
}),
[
triggersActionsUi.alertsTableConfigurationRegistry,
Expand All @@ -305,7 +318,8 @@ export const AlertsTableComponent: FC<DetectionEngineAlertTableProps> = ({
cellContext,
onLoad,
sourcererDataView?.runtimeFieldMap,
isEventRenderedView,
toolbarVisibility,
count,
]
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import React from 'react';

import type { EcsSecurityExtension as Ecs } from '@kbn/securitysolution-ecs';
import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
import type { RowRenderer } from '../../../../../../../common/types';
import type { RowRendererId } from '../../../../../../../common/api/timeline';

Expand All @@ -33,9 +34,13 @@ export const combineRenderers = ({
isDraggable: boolean;
scopeId: string;
}) => (
<>
{a.isInstance(data) && a.renderRow({ contextId, data, isDraggable, scopeId })}
{b.isInstance(data) && b.renderRow({ contextId, data, isDraggable, scopeId })}
</>
<EuiFlexGroup direction="column" gutterSize="m">
{a.isInstance(data) && (
<EuiFlexItem> {a.renderRow({ contextId, data, isDraggable, scopeId })}</EuiFlexItem>
)}
{b.isInstance(data) && (
<EuiFlexItem>{b.renderRow({ contextId, data, isDraggable, scopeId })}</EuiFlexItem>
)}
</EuiFlexGroup>
),
});
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,7 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = memo((props: Aler
toolbarVisibility: toolbarVisibilityProp,
shouldHighlightRow,
fieldFormats,
height,
} = props;

const dataGridRef = useRef<EuiDataGridRefProps>(null);
Expand Down Expand Up @@ -723,6 +724,10 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = memo((props: Aler
</Suspense>
{alertsCount > 0 && (
<EuiDataGrid
// As per EUI docs, it is not recommended to switch between undefined and defined height.
// If user changes height, it is better to unmount and mount the component.
// Ref: https://eui.elastic.co/#/tabular-content/data-grid#virtualization
key={height ? 'fixedHeight' : 'autoHeight'}
aria-label="Alerts table"
data-test-subj="alertsTable"
columns={columnsWithCellActions}
Expand All @@ -741,6 +746,7 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = memo((props: Aler
ref={dataGridRef}
renderCustomGridBody={dynamicRowHeight ? renderCustomGridBody : undefined}
renderCellPopover={handleRenderCellPopover}
height={height}
/>
)}
</section>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ const AlertsTableStateWithQueryProvider = memo(
dynamicRowHeight,
lastReloadRequestTime,
emptyStateHeight,
height,
}: AlertsTableStateProps) => {
const {
data,
Expand Down Expand Up @@ -339,6 +340,7 @@ const AlertsTableStateWithQueryProvider = memo(
data,
...queryParams,
});

const {
alerts = [],
oldAlertsData = [],
Expand Down Expand Up @@ -513,6 +515,7 @@ const AlertsTableStateWithQueryProvider = memo(
onSortChange,
onPageChange,
fieldFormats,
height,
}),
[
alertsTableConfiguration,
Expand Down Expand Up @@ -552,6 +555,7 @@ const AlertsTableStateWithQueryProvider = memo(
onSortChange,
onPageChange,
fieldFormats,
height,
]
);

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/triggers_actions_ui/public/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ export type AlertsTableProps = {
onPageChange: (pagination: RuleRegistrySearchRequestPagination) => void;
renderCellPopover?: ReturnType<GetRenderCellPopover>;
fieldFormats: FieldFormatsStart;
} & Partial<Pick<EuiDataGridProps, 'gridStyle' | 'rowHeightsOptions'>>;
} & Partial<Pick<EuiDataGridProps, 'gridStyle' | 'rowHeightsOptions' | 'height'>>;

export type SetFlyoutAlert = (alertId: string) => void;

Expand Down

0 comments on commit 5ebef44

Please sign in to comment.