Skip to content

Commit

Permalink
[RAM] Improve alerts table actions column performance (#178632)
Browse files Browse the repository at this point in the history
## Summary

Ensures the default `leadingControlColumns` is referentially stable in
`AlertsTable` and stabilizes the declarations of
`renderCustomActionsRow` in o11y's alerts table configurations (this
aspect will likely be improved in the alerts table configuration
refactor). This results in the `AlertActions` component being called
over three times less frequently for each alert.

Before:
<img width="1250" alt="image"
src="https://github.com/elastic/kibana/assets/18363145/247522a9-d92d-48f6-a272-97bf00687778">


After:
<img width="1241" alt="image"
src="https://github.com/elastic/kibana/assets/18363145/8e87f3cd-90e1-4e1f-95f7-151d5acbc4fa">

Refs #178547

---------

Co-authored-by: Xavier Mouligneau <xavier.mouligneau@elastic.co>
  • Loading branch information
umbopepato and XavierM authored Apr 5, 2024
1 parent 29fcdda commit f93faea
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 98 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,37 +23,40 @@ import { getColumns } from '../common/get_columns';
export const getAlertsPageTableConfiguration = (
observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry,
config: ConfigSchema
): AlertsTableConfigurationRegistry => ({
id: observabilityFeatureId,
cases: { featureId: casesFeatureId, owner: [observabilityFeatureId] },
columns: getColumns({ showRuleName: true }),
getRenderCellValue: ({ setFlyoutAlert }) =>
getRenderCellValue({
observabilityRuleTypeRegistry,
setFlyoutAlert,
}),
sort: [
{
[ALERT_START]: {
order: 'desc' as SortOrder,
): AlertsTableConfigurationRegistry => {
const renderCustomActionsRow = (props: RenderCustomActionsRowArgs) => {
return (
<AlertActions
{...props}
config={config}
observabilityRuleTypeRegistry={observabilityRuleTypeRegistry}
/>
);
};
return {
id: observabilityFeatureId,
cases: { featureId: casesFeatureId, owner: [observabilityFeatureId] },
columns: getColumns({ showRuleName: true }),
getRenderCellValue: ({ setFlyoutAlert }) =>
getRenderCellValue({
observabilityRuleTypeRegistry,
setFlyoutAlert,
}),
sort: [
{
[ALERT_START]: {
order: 'desc' as SortOrder,
},
},
],
useActionsColumn: () => ({
renderCustomActionsRow,
}),
useInternalFlyout: () => {
const { header, body, footer } = useGetAlertFlyoutComponents(observabilityRuleTypeRegistry);
return { header, body, footer };
},
],
useActionsColumn: () => ({
renderCustomActionsRow: (props: RenderCustomActionsRowArgs) => {
return (
<AlertActions
{...props}
config={config}
observabilityRuleTypeRegistry={observabilityRuleTypeRegistry}
/>
);
},
}),
useInternalFlyout: () => {
const { header, body, footer } = useGetAlertFlyoutComponents(observabilityRuleTypeRegistry);
return { header, body, footer };
},
ruleTypeIds: observabilityRuleTypeRegistry.list(),
showInspectButton: true,
});
ruleTypeIds: observabilityRuleTypeRegistry.list(),
showInspectButton: true,
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,36 +24,39 @@ import { getColumns } from '../common/get_columns';
export const getRuleDetailsTableConfiguration = (
observabilityRuleTypeRegistry: ObservabilityRuleTypeRegistry,
config: ConfigSchema
): AlertsTableConfigurationRegistry => ({
id: RULE_DETAILS_ALERTS_TABLE_CONFIG_ID,
cases: { featureId: casesFeatureId, owner: [observabilityFeatureId] },
columns: getColumns(),
getRenderCellValue: ({ setFlyoutAlert }) =>
getRenderCellValue({
observabilityRuleTypeRegistry,
setFlyoutAlert,
}),
sort: [
{
[ALERT_START]: {
order: 'desc' as SortOrder,
): AlertsTableConfigurationRegistry => {
const renderCustomActionsRow = (props: RenderCustomActionsRowArgs) => {
return (
<AlertActions
{...props}
config={config}
observabilityRuleTypeRegistry={observabilityRuleTypeRegistry}
/>
);
};
return {
id: RULE_DETAILS_ALERTS_TABLE_CONFIG_ID,
cases: { featureId: casesFeatureId, owner: [observabilityFeatureId] },
columns: getColumns(),
getRenderCellValue: ({ setFlyoutAlert }) =>
getRenderCellValue({
observabilityRuleTypeRegistry,
setFlyoutAlert,
}),
sort: [
{
[ALERT_START]: {
order: 'desc' as SortOrder,
},
},
],
useActionsColumn: () => ({
renderCustomActionsRow,
}),
useInternalFlyout: () => {
const { header, body, footer } = useGetAlertFlyoutComponents(observabilityRuleTypeRegistry);
return { header, body, footer };
},
],
useActionsColumn: () => ({
renderCustomActionsRow: (props: RenderCustomActionsRowArgs) => {
return (
<AlertActions
{...props}
config={config}
observabilityRuleTypeRegistry={observabilityRuleTypeRegistry}
/>
);
},
}),
useInternalFlyout: () => {
const { header, body, footer } = useGetAlertFlyoutComponents(observabilityRuleTypeRegistry);
return { header, body, footer };
},
ruleTypeIds: observabilityRuleTypeRegistry.list(),
});
ruleTypeIds: observabilityRuleTypeRegistry.list(),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ export function AlertActions({
const observabilityAlert = parseObservabilityAlert(alert);

useEffect(() => {
const alertLink = observabilityAlert.link as unknown as string;
if (!observabilityAlert.hasBasePath) {
const alertLink = observabilityAlert.link;
if (!observabilityAlert.hasBasePath && prepend) {
setViewInAppUrl(prepend(alertLink ?? ''));
} else {
setViewInAppUrl(alertLink);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ import { SystemCellId } from './types';
import { SystemCellFactory, systemCells } from './cells';
import { triggersActionsUiQueriesKeys } from '../../hooks/constants';
import { AlertsTableQueryContext } from './contexts/alerts_table_context';

const AlertsFlyout = lazy(() => import('./alerts_flyout'));

const DefaultGridStyle: EuiDataGridStyle = {
border: 'none',
header: 'underline',
Expand Down Expand Up @@ -213,10 +213,13 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab

const { sortingColumns, onSort } = useSorting(onSortChange, visibleColumns, sortingFields);

const { renderCustomActionsRow, actionsColumnWidth, getSetIsActionLoadingCallback } =
useActionsColumn({
options: props.alertsTableConfiguration.useActionsColumn,
});
const {
renderCustomActionsRow: CustomActionsRow,
actionsColumnWidth,
getSetIsActionLoadingCallback,
} = useActionsColumn({
options: props.alertsTableConfiguration.useActionsColumn,
});
const casesConfig = props.alertsTableConfiguration.cases;
const renderCellContext = props.alertsTableConfiguration.useFetchPageContext?.({
alerts,
Expand Down Expand Up @@ -299,7 +302,7 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab
fieldBrowserOptions,
getInspectQuery,
showInspectButton,
toolbarVisiblityProp: props.toolbarVisibility,
toolbarVisibilityProp: props.toolbarVisibility,
});
}, [
bulkActionsState,
Expand All @@ -325,7 +328,7 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab
const leadingControlColumns = useMemo(() => {
let controlColumns = [...props.leadingControlColumns];

if (renderCustomActionsRow) {
if (CustomActionsRow) {
controlColumns = [
{
id: 'expandColumn',
Expand All @@ -348,18 +351,18 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab

return (
<EuiFlexGroup gutterSize="none" responsive={false} alignItems="center">
{renderCustomActionsRow({
alert: alerts[visibleRowIndex],
ecsAlert: ecsAlertsData[visibleRowIndex],
nonEcsData: oldAlertsData[visibleRowIndex],
rowIndex: visibleRowIndex,
setFlyoutAlert: handleFlyoutAlert,
id: props.id,
cveProps,
setIsActionLoading: getSetIsActionLoadingCallback(visibleRowIndex),
refresh,
clearSelection,
})}
<CustomActionsRow
id={props.id}
alert={alerts[visibleRowIndex]}
ecsAlert={ecsAlertsData[visibleRowIndex]}
nonEcsData={oldAlertsData[visibleRowIndex]}
rowIndex={visibleRowIndex}
setFlyoutAlert={handleFlyoutAlert}
setIsActionLoading={getSetIsActionLoadingCallback(visibleRowIndex)}
cveProps={cveProps}
refresh={refresh}
clearSelection={clearSelection}
/>
</EuiFlexGroup>
);
},
Expand All @@ -374,19 +377,19 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab

return controlColumns;
}, [
props.leadingControlColumns,
props.id,
CustomActionsRow,
isBulkActionsColumnActive,
actionsColumnWidth,
ecsAlertsData,
alerts,
oldAlertsData,
ecsAlertsData,
getBulkActionsLeadingControlColumn,
handleFlyoutAlert,
isBulkActionsColumnActive,
props.id,
props.leadingControlColumns,
renderCustomActionsRow,
getSetIsActionLoadingCallback,
refresh,
clearSelection,
getBulkActionsLeadingControlColumn,
]);

useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
EuiButton,
EuiCode,
EuiCopy,
EuiDataGridControlColumn,
} from '@elastic/eui';
import type { MappingRuntimeFields } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import {
Expand Down Expand Up @@ -176,14 +177,16 @@ const AlertsTableState = (props: AlertsTableStateProps) => {
);
};

const DEFAULT_LEADING_CONTROL_COLUMNS: EuiDataGridControlColumn[] = [];

const AlertsTableStateWithQueryProvider = ({
alertsTableConfigurationRegistry,
configurationId,
id,
featureIds,
query,
pageSize,
leadingControlColumns,
leadingControlColumns = DEFAULT_LEADING_CONTROL_COLUMNS,
rowHeightsOptions,
renderCellValue,
renderCellPopover,
Expand Down Expand Up @@ -436,7 +439,7 @@ const AlertsTableStateWithQueryProvider = ({
pageSize: pagination.pageSize,
pageSizeOptions: [10, 20, 50, 100],
id,
leadingControlColumns: leadingControlColumns ?? [],
leadingControlColumns,
showAlertStatusWithFlapping,
trailingControlColumns: [],
useFetchAlertsData,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export const getToolbarVisibility = ({
fieldBrowserOptions,
getInspectQuery,
showInspectButton,
toolbarVisiblityProp,
toolbarVisibilityProp,
}: {
bulkActions: BulkActionsPanelConfig[];
alertsCount: number;
Expand All @@ -138,7 +138,7 @@ export const getToolbarVisibility = ({
fieldBrowserOptions?: FieldBrowserOptions;
getInspectQuery: GetInspectQuery;
showInspectButton: boolean;
toolbarVisiblityProp?: EuiDataGridToolBarVisibilityOptions;
toolbarVisibilityProp?: EuiDataGridToolBarVisibilityOptions;
}): EuiDataGridToolBarVisibilityOptions => {
const selectedRowsCount = rowSelection.size;
const defaultVisibility = getDefaultVisibility({
Expand All @@ -159,7 +159,7 @@ export const getToolbarVisibility = ({
if (isBulkActionsActive)
return {
...defaultVisibility,
...(toolbarVisiblityProp ?? {}),
...(toolbarVisibilityProp ?? {}),
};

const options = {
Expand All @@ -185,7 +185,7 @@ export const getToolbarVisibility = ({
),
},
},
...(toolbarVisiblityProp ?? {}),
...(toolbarVisibilityProp ?? {}),
};

return options;
Expand Down
4 changes: 2 additions & 2 deletions x-pack/test/security_solution_cypress/cypress/tasks/alerts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,8 @@ export const openAlertsFieldBrowser = () => {
export const selectNumberOfAlerts = (numberOfAlerts: number) => {
for (let i = 0; i < numberOfAlerts; i++) {
waitForAlerts();
cy.get(ALERT_CHECKBOX).eq(i).as('checkbox').click({ force: true });
cy.get('@checkbox').should('have.attr', 'checked');
cy.get(ALERT_CHECKBOX).eq(i).as('checkbox').check();
cy.get('@checkbox').should('be.checked');
}
};

Expand Down

0 comments on commit f93faea

Please sign in to comment.