Skip to content

Commit

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

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Performance][Security Solution][1/4] - Field Browser Performance
(#212469)](#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 (#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//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 (#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//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 (#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//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 72460c6 commit 543bf80
Show file tree
Hide file tree
Showing 5 changed files with 211 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,160 +5,19 @@
* 2.0.
*/

import type { PropsWithChildren } from 'react';
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-hooks';
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 () => {
await act(async () => {
const { waitForNextUpdate, result } = renderHook<
PropsWithChildren<{}>,
{ indexFieldsSearch: IndexFieldSearch }
>(() => useDataView(), {
wrapper: TestProviders,
});
await waitForNextUpdate();
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;
await act(async () => {
const { waitForNextUpdate, result } = renderHook<
PropsWithChildren<{}>,
{ indexFieldsSearch: IndexFieldSearch }
>(() => useDataView(), {
wrapper: TestProviders,
});
await waitForNextUpdate();
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;
await act(async () => {
const { waitForNextUpdate, result } = renderHook<
PropsWithChildren<{}>,
{ indexFieldsSearch: IndexFieldSearch }
>(() => useDataView(), {
wrapper: TestProviders,
});
await waitForNextUpdate();
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 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');

describe('useDataView', () => {
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);
});

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*');
});
});
Loading

0 comments on commit 543bf80

Please sign in to comment.