Skip to content

Commit

Permalink
[8.18] [Performance][Security Solution][1/4] - Field Browser Performa…
Browse files Browse the repository at this point in the history
…nce (elastic#212469) (elastic#213023)

# Backport

This will backport the following commits from `main` to `8.18`:
- [[Performance][Security Solution][1/4] - Field Browser Performance
(elastic#212469)](elastic#212469)

<!--- Backport version: 9.6.6 -->

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

<!--BACKPORT [{"author":{"name":"Michael
Olorunnisola","email":"michael.olorunnisola@elastic.co"},"sourceCommit":{"committedDate":"2025-03-04T01:22:25Z","message":"[Performance][Security
Solution][1/4] - Field Browser Performance (elastic#212469)\n\n## Summary\nPart
1 of https://github.com/elastic/kibana/pull/212173\n\n### Testing\nFor
setup see testing section
here:\nhttps://github.com/elastic/pull/212173#issue-2870522020\n\n**Areas
to test:**\n- Alert Table (Alerts, Rule Detail, Rule Preview pages)\n-
Security solution field browser component\n- Flyout table tab.\n\n###
Background\n\nWhen investigating the performance of the security
solution application,\none of the issues that was observed was locking
of the page and field\nbrowser component when the total number of fields
returned were\nsignificantly high.\n\nThis led to cell values not
rendering in the alert table, and the field\nbrowser in all the security
solution pages causing the page to crash.\nThe relevant images can be
seen at the bottom of this description\n\nIn short: The
`push(...fields)` is non-performant at scale, and at a\nsignificant
enough scale (Testing was done with 500k mapped fields),\nfails to run
due to excessive arguments provided to the `push` method.\nIn this PR
improvements are made in the `browserFields` transformations\nthat are
done for the field browser component, expandable flyout table\ntab, and
alert/rule tables via `CellValue` component.\n\nThis work was done to
get immediate improvements in the security\nsolution UI, but a longer
term consideration will be whether or not the\n`browserFields` is even
necessary anymore as a concept based on what is\navailable via the
`fields` api. We will revisit once our Sourcerer\nrefactoring work is
done.\n\n<img width=\"1728\" alt=\"Screenshot 2025-02-26 at 10 15
29 AM\"\nsrc=\"https://github.com/user-attachments/assets/a25f577f-f758-415e-9c93-5452eadb8020\"\n/>\n\n<img
width=\"1445\" alt=\"Screenshot 2025-02-26 at 10 18
36 AM\"\nsrc=\"https://github.com/user-attachments/assets/d70970d3-991a-47ba-b617-5862d18101b6\"\n/>\n\n<img
width=\"1469\" alt=\"Screenshot 2025-02-26 at 10 19
48 AM\"\nsrc=\"https://github.com/user-attachments/assets/1767aa9b-66ab-46be-bc1a-5311630c2765\"\n/>\n\n\n![image](https://github.com/user-attachments/assets/5d746b21-fa9b-425b-826a-cc7abd444f21)\n\n\n![image](https://github.com/user-attachments/assets/4dff2378-d61b-4770-b46b-41cb37d6ead4)\n\n\n###
After the fix\n(Done on [this
branch](https://github.com/elastic/kibana/pull/212173)\nthat has the
other changes as
well)\n\n\nhttps://github.com/user-attachments/assets/da992296-4eb8-49d4-96ca-b0a19a00f1f0\n\n\n###
Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers
should verify this PR satisfies this list as well.\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750e156c26078015025551c4f10d299ba269fa35","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat
Hunting","Team:Threat
Hunting:Investigations","backport:version","v8.18.0","v9.1.0","v8.17.3"],"title":"[Performance][Security
Solution][1/4] - Field Browser
Performance","number":212469,"url":"https://github.com/elastic/kibana/pull/212469","mergeCommit":{"message":"[Performance][Security
Solution][1/4] - Field Browser Performance (elastic#212469)\n\n## Summary\nPart
1 of https://github.com/elastic/kibana/pull/212173\n\n### Testing\nFor
setup see testing section
here:\nhttps://github.com/elastic/pull/212173#issue-2870522020\n\n**Areas
to test:**\n- Alert Table (Alerts, Rule Detail, Rule Preview pages)\n-
Security solution field browser component\n- Flyout table tab.\n\n###
Background\n\nWhen investigating the performance of the security
solution application,\none of the issues that was observed was locking
of the page and field\nbrowser component when the total number of fields
returned were\nsignificantly high.\n\nThis led to cell values not
rendering in the alert table, and the field\nbrowser in all the security
solution pages causing the page to crash.\nThe relevant images can be
seen at the bottom of this description\n\nIn short: The
`push(...fields)` is non-performant at scale, and at a\nsignificant
enough scale (Testing was done with 500k mapped fields),\nfails to run
due to excessive arguments provided to the `push` method.\nIn this PR
improvements are made in the `browserFields` transformations\nthat are
done for the field browser component, expandable flyout table\ntab, and
alert/rule tables via `CellValue` component.\n\nThis work was done to
get immediate improvements in the security\nsolution UI, but a longer
term consideration will be whether or not the\n`browserFields` is even
necessary anymore as a concept based on what is\navailable via the
`fields` api. We will revisit once our Sourcerer\nrefactoring work is
done.\n\n<img width=\"1728\" alt=\"Screenshot 2025-02-26 at 10 15
29 AM\"\nsrc=\"https://github.com/user-attachments/assets/a25f577f-f758-415e-9c93-5452eadb8020\"\n/>\n\n<img
width=\"1445\" alt=\"Screenshot 2025-02-26 at 10 18
36 AM\"\nsrc=\"https://github.com/user-attachments/assets/d70970d3-991a-47ba-b617-5862d18101b6\"\n/>\n\n<img
width=\"1469\" alt=\"Screenshot 2025-02-26 at 10 19
48 AM\"\nsrc=\"https://github.com/user-attachments/assets/1767aa9b-66ab-46be-bc1a-5311630c2765\"\n/>\n\n\n![image](https://github.com/user-attachments/assets/5d746b21-fa9b-425b-826a-cc7abd444f21)\n\n\n![image](https://github.com/user-attachments/assets/4dff2378-d61b-4770-b46b-41cb37d6ead4)\n\n\n###
After the fix\n(Done on [this
branch](https://github.com/elastic/kibana/pull/212173)\nthat has the
other changes as
well)\n\n\nhttps://github.com/user-attachments/assets/da992296-4eb8-49d4-96ca-b0a19a00f1f0\n\n\n###
Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers
should verify this PR satisfies this list as well.\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750e156c26078015025551c4f10d299ba269fa35"}},"sourceBranch":"main","suggestedTargetBranches":["9.0","8.18","8.17"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/212469","number":212469,"mergeCommit":{"message":"[Performance][Security
Solution][1/4] - Field Browser Performance (elastic#212469)\n\n## Summary\nPart
1 of https://github.com/elastic/kibana/pull/212173\n\n### Testing\nFor
setup see testing section
here:\nhttps://github.com/elastic/pull/212173#issue-2870522020\n\n**Areas
to test:**\n- Alert Table (Alerts, Rule Detail, Rule Preview pages)\n-
Security solution field browser component\n- Flyout table tab.\n\n###
Background\n\nWhen investigating the performance of the security
solution application,\none of the issues that was observed was locking
of the page and field\nbrowser component when the total number of fields
returned were\nsignificantly high.\n\nThis led to cell values not
rendering in the alert table, and the field\nbrowser in all the security
solution pages causing the page to crash.\nThe relevant images can be
seen at the bottom of this description\n\nIn short: The
`push(...fields)` is non-performant at scale, and at a\nsignificant
enough scale (Testing was done with 500k mapped fields),\nfails to run
due to excessive arguments provided to the `push` method.\nIn this PR
improvements are made in the `browserFields` transformations\nthat are
done for the field browser component, expandable flyout table\ntab, and
alert/rule tables via `CellValue` component.\n\nThis work was done to
get immediate improvements in the security\nsolution UI, but a longer
term consideration will be whether or not the\n`browserFields` is even
necessary anymore as a concept based on what is\navailable via the
`fields` api. We will revisit once our Sourcerer\nrefactoring work is
done.\n\n<img width=\"1728\" alt=\"Screenshot 2025-02-26 at 10 15
29 AM\"\nsrc=\"https://github.com/user-attachments/assets/a25f577f-f758-415e-9c93-5452eadb8020\"\n/>\n\n<img
width=\"1445\" alt=\"Screenshot 2025-02-26 at 10 18
36 AM\"\nsrc=\"https://github.com/user-attachments/assets/d70970d3-991a-47ba-b617-5862d18101b6\"\n/>\n\n<img
width=\"1469\" alt=\"Screenshot 2025-02-26 at 10 19
48 AM\"\nsrc=\"https://github.com/user-attachments/assets/1767aa9b-66ab-46be-bc1a-5311630c2765\"\n/>\n\n\n![image](https://github.com/user-attachments/assets/5d746b21-fa9b-425b-826a-cc7abd444f21)\n\n\n![image](https://github.com/user-attachments/assets/4dff2378-d61b-4770-b46b-41cb37d6ead4)\n\n\n###
After the fix\n(Done on [this
branch](https://github.com/elastic/kibana/pull/212173)\nthat has the
other changes as
well)\n\n\nhttps://github.com/user-attachments/assets/da992296-4eb8-49d4-96ca-b0a19a00f1f0\n\n\n###
Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers
should verify this PR satisfies this list as well.\n\n- [x] [Unit or
functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere
updated or added to match the most common
scenarios","sha":"750e156c26078015025551c4f10d299ba269fa35"}},{"branch":"8.17","label":"v8.17.3","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
  • Loading branch information
michaelolo24 authored Mar 4, 2025
1 parent 7735d47 commit 4ec4ee0
Show file tree
Hide file tree
Showing 5 changed files with 211 additions and 168 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ import {
EuiBadge,
EuiScreenReaderOnly,
} from '@elastic/eui';
import { uniqBy } from 'lodash/fp';
import { BrowserFields } from '@kbn/rule-registry-plugin/common';
import type { BrowserFields } from '@kbn/rule-registry-plugin/common';
import { EcsFlat } from '@elastic/ecs';
import { EcsMetadata } from '@kbn/alerts-as-data-utils/src/field_maps/types';

Expand Down Expand Up @@ -64,29 +63,41 @@ export const getFieldItemsData = ({
selectedCategoryIds.length > 0 ? selectedCategoryIds : Object.keys(browserFields);
const selectedFieldIds = new Set(columnIds);

const fieldItems = uniqBy(
'name',
categoryIds.reduce<BrowserFieldItem[]>((fieldItemsAcc, categoryId) => {
const getFieldItems = () => {
const fieldItemsAcc: BrowserFieldItem[] = [];
const fieldsSeen: Set<string> = new Set();

/**
* Both categoryIds and the fields in browserFields can be significantly large. Technically speaking,
* there is no upper cap on how many fields a customer can have. We are using a for loop here to avoid
* the performance issues that can arise from using map/filter/reduce.
*/
for (let i = 0; i < categoryIds.length; i += 1) {
const categoryId = categoryIds[i];
const categoryBrowserFields = Object.values(browserFields[categoryId]?.fields ?? {});
if (categoryBrowserFields.length > 0) {
fieldItemsAcc.push(
...categoryBrowserFields.map(({ name = '', ...field }) => {
return {
name,
type: field.type,
description: getDescription(name, EcsFlat as Record<string, EcsMetadata>),
example: field.example?.toString(),
category: getCategory(name),
selected: selectedFieldIds.has(name),
isRuntime: !!field.runtimeField,
};
})
);
for (let j = 0; j < categoryBrowserFields.length; j += 1) {
const field = categoryBrowserFields[j];
const name = field.name !== undefined ? field.name : '';
if (fieldsSeen.has(name)) continue;
fieldsSeen.add(name);
const categoryFieldItem = {
name,
type: field.type,
description: getDescription(name, EcsFlat as Record<string, EcsMetadata>),
example: field.example?.toString(),
category: getCategory(name),
selected: selectedFieldIds.has(name),
isRuntime: !!field.runtimeField,
};
fieldItemsAcc.push(categoryFieldItem);
}
}
return fieldItemsAcc;
}, [])
);
return { fieldItems };
}
return fieldItemsAcc;
};

return { fieldItems: getFieldItems() };
};

const getDefaultFieldTableColumns = ({ highlight }: { highlight: string }): FieldTableColumns => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,151 +5,19 @@
* 2.0.
*/

import type { IndexFieldSearch } from './use_data_view';
import { useDataView } from './use_data_view';
import { mocksSource } from './mock';
import { mockGlobalState, TestProviders } from '../../mock';
import { act, renderHook } from '@testing-library/react';
import { useKibana } from '../../lib/kibana';

const mockDispatch = jest.fn();
jest.mock('react-redux', () => {
const original = jest.requireActual('react-redux');

return {
...original,
useDispatch: () => mockDispatch,
};
});
jest.mock('../../lib/kibana');
jest.mock('../../lib/apm/use_track_http_request');
import { mockBrowserFields, mockIndexFields, mockIndexFieldsByName } from './mock';
import * as indexUtils from '.';

describe('source/index.tsx', () => {
describe('useDataView hook', () => {
const mockSearchResponse = {
...mocksSource,
indicesExist: ['auditbeat-*', mockGlobalState.sourcerer.signalIndexName],
isRestore: false,
rawResponse: {},
runtimeMappings: {},
};

beforeEach(() => {
jest.clearAllMocks();
const mock = {
subscribe: ({ next }: { next: Function }) => next(mockSearchResponse),
unsubscribe: jest.fn(),
};

(useKibana as jest.Mock).mockReturnValue({
services: {
data: {
dataViews: {
...useKibana().services.data.dataViews,
get: async (dataViewId: string, displayErrors?: boolean, refreshFields = false) => {
const dataViewMock = {
id: dataViewId,
matchedIndices: refreshFields
? ['hello', 'world', 'refreshed']
: ['hello', 'world'],
fields: mocksSource.indexFields,
getIndexPattern: () =>
refreshFields ? 'hello*,world*,refreshed*' : 'hello*,world*',
getRuntimeMappings: () => ({
myfield: {
type: 'keyword',
},
}),
};
return Promise.resolve({
toSpec: () => dataViewMock,
...dataViewMock,
});
},
getFieldsForWildcard: async () => Promise.resolve(),
getExistingIndices: async (indices: string[]) => Promise.resolve(indices),
},
search: {
search: jest.fn().mockReturnValue({
subscribe: ({ next }: { next: Function }) => {
next(mockSearchResponse);
return mock;
},
unsubscribe: jest.fn(),
}),
},
},
},
});
});
it('sets field data for data view', async () => {
const { result } = renderHook(() => useDataView(), {
wrapper: TestProviders,
});

await act(async () => {
await result.current.indexFieldsSearch({ dataViewId: 'neato' });
});
expect(mockDispatch.mock.calls[0][0]).toEqual({
type: 'x-pack/security_solution/local/sourcerer/SET_DATA_VIEW_LOADING',
payload: { id: 'neato', loading: true },
});
const { type: sourceType, payload } = mockDispatch.mock.calls[1][0];
expect(sourceType).toEqual('x-pack/security_solution/local/sourcerer/SET_DATA_VIEW');
expect(payload.id).toEqual('neato');
});

it('should reuse the result for dataView info when cleanCache not passed', async () => {
let indexFieldsSearch: IndexFieldSearch;

const { result } = renderHook(() => useDataView(), {
wrapper: TestProviders,
});

await act(async () => {
indexFieldsSearch = result.current.indexFieldsSearch;
});

await indexFieldsSearch!({ dataViewId: 'neato' });
const {
payload: { browserFields, indexFields },
} = mockDispatch.mock.calls[1][0];

mockDispatch.mockClear();

await indexFieldsSearch!({ dataViewId: 'neato' });
const {
payload: { browserFields: newBrowserFields, indexFields: newIndexFields },
} = mockDispatch.mock.calls[1][0];

expect(browserFields).toBe(newBrowserFields);
expect(indexFields).toBe(newIndexFields);
describe('getAllBrowserFields', () => {
it('should return the expected browser fields list', () => {
expect(indexUtils.getAllBrowserFields(mockBrowserFields)).toEqual(mockIndexFields);
});
});

it('should not reuse the result for dataView info when cleanCache passed', async () => {
let indexFieldsSearch: IndexFieldSearch;
const { result } = renderHook(() => useDataView(), {
wrapper: TestProviders,
});

await act(async () => {
indexFieldsSearch = result.current.indexFieldsSearch;
});

await indexFieldsSearch!({ dataViewId: 'neato' });
const {
payload: { patternList },
} = mockDispatch.mock.calls[1][0];

mockDispatch.mockClear();

await indexFieldsSearch!({ dataViewId: 'neato', cleanCache: true });
const {
payload: { patternList: newPatternList },
} = mockDispatch.mock.calls[1][0];
expect(patternList).not.toBe(newPatternList);
expect(patternList).not.toContain('refreshed*');
expect(newPatternList).toContain('refreshed*');
describe('getAllFieldsByName', () => {
it('should return the expected browser fields list', () => {
expect(indexUtils.getAllFieldsByName(mockBrowserFields)).toEqual(mockIndexFieldsByName);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ import type { ENDPOINT_FIELDS_SEARCH_STRATEGY } from '../../../../common/endpoin
export type { BrowserFields };

export function getAllBrowserFields(browserFields: BrowserFields): Array<Partial<FieldSpec>> {
const result: Array<Partial<FieldSpec>> = [];
let result: Array<Partial<FieldSpec>> = [];
for (const namespace of Object.values(browserFields)) {
if (namespace.fields) {
result.push(...Object.values(namespace.fields));
const namespaceFields = Object.values(namespace.fields);
result = result.concat(namespaceFields);
}
}
return result;
Expand All @@ -36,9 +37,11 @@ export function getAllBrowserFields(browserFields: BrowserFields): Array<Partial
* @param browserFields
* @returns
*/
export const getAllFieldsByName = (
browserFields: BrowserFields
): { [fieldName: string]: Partial<FieldSpec> } => keyBy('name', getAllBrowserFields(browserFields));
export const getAllFieldsByName = memoizeOne(
(browserFields: BrowserFields): { [fieldName: string]: Partial<FieldSpec> } =>
keyBy('name', getAllBrowserFields(browserFields)),
(newArgs, lastArgs) => newArgs[0] === lastArgs[0]
);

export const getIndexFields = memoizeOne(
(title: string, fields: IIndexPatternFieldList): DataViewBase =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type { MappingRuntimeFieldType } from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { flatten } from 'lodash';
import type { FieldSpec } from '@kbn/data-views-plugin/common';
import type { BrowserFields } from '../../../../common/search_strategy/index_fields';

export const mocksSource = {
Expand Down Expand Up @@ -387,6 +388,13 @@ export const mockIndexFields = flatten(
Object.values(mockBrowserFields).map((fieldItem) => Object.values(fieldItem.fields ?? {}))
);

export const mockIndexFieldsByName = mockIndexFields.reduce((acc, indexFieldObj) => {
if (indexFieldObj.name) {
acc[indexFieldObj.name] = indexFieldObj;
}
return acc;
}, {} as { [fieldName: string]: Partial<FieldSpec> });

const runTimeType: MappingRuntimeFieldType = 'keyword' as const;

export const mockRuntimeMappings = {
Expand Down
Loading

0 comments on commit 4ec4ee0

Please sign in to comment.