Skip to content

Commit

Permalink
[Security Solution] Security Alert Table Performance Optimizations (#…
Browse files Browse the repository at this point in the history
…192827)

## Summary

There are few performance issues with Security's alert table today which
this PR aims to fix.

1. Virtualization was not working in Grid View and Event Rendered View.
Constraining the height of `EuiDataGrid` makes virtualization work.
2. It was taking lot of time when switching between Event Rendered View
and Grid view.

Below we compare some performance metric in both above scenarios.

## Solution

- Solution was to enable virtualization but bounding the height of the
table.
- If number of rows < 10, then height is `auto` else it maxed at `600px`

## Grid View - 100 Rows - Changing Query

If you compare times below for rendering of Alert Table Components, we
see that improvement is of 2.5-5 times.

Number of re-renders are also fewer.

### Before


![grafik](https://github.com/user-attachments/assets/c2e14b24-750b-4c7c-981f-d7fde85cfeff)


### After


![grafik](https://github.com/user-attachments/assets/8eab15e2-4e93-41c3-88fe-bdadb367785d)


## Grid View - 100 Rows - Changing view to Event Rendered View

Event Rendered View is where these optimization actually shine. If you
observe the rendering times, performance gain is upto 10 times for all
renders. Number of re-renders are also reduced.

### Before

![grafik](https://github.com/user-attachments/assets/5cba7d86-6b71-496a-9004-8ce5b3599f4e)


### After

![grafik](https://github.com/user-attachments/assets/db9d07fa-2afe-4f3d-8261-cc16d40165bc)


## Grid View - Change page size from 20 rows to 100 rows

Here also we achieve almost 15 times improvement in the render times of
the table.

### Before

![grafik](https://github.com/user-attachments/assets/a8a3467a-7774-4d02-ab4c-80433aa99660)

 
### After

![grafik](https://github.com/user-attachments/assets/1c9a8645-7fbd-49f2-b72b-83d8688decbc)


## Desk Testing Guide

Below scenarios need to tested to make sure Alert Table is working fine
in both Grid View and Event Rendered View

1. Alert Page
   - Analyzer is working fine
   - Session Viewer is working fine
   - Alert Table when grouping is enabled.
3. Rule Details Page
   - Analyzer is working fine
   - Session Viewer is working fine
   - Alert Table when grouping is enabled.
5. Full Screen.
   - Analyzer is working fine
   - Session Viewer is working fine
7. Number of records < 10
     - should resize the table to fit the content.
8. Number of records > 10
     - should have fixed height and no-virtualization enabled.
9. Change view from Event Rendered View
10. Increase rows from default of `20` to `100`
  • Loading branch information
logeekal authored Oct 11, 2024
1 parent c1c70c7 commit a84d62d
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 a84d62d

Please sign in to comment.