-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Performance][Security Solution][1/4] - Field Browser Performance (#2…
…12469) ## Summary Part 1 of #212173 ### Testing For setup see testing section here: #212173 (comment) **Areas to test:** - Alert Table (Alerts, Rule Detail, Rule Preview pages) - Security solution field browser component - Flyout table tab. ### Background When investigating the performance of the security solution application, one of the issues that was observed was locking of the page and field browser component when the total number of fields returned were significantly high. This led to cell values not rendering in the alert table, and the field browser in all the security solution pages causing the page to crash. The relevant images can be seen at the bottom of this description In short: The `push(...fields)` is non-performant at scale, and at a significant enough scale (Testing was done with 500k mapped fields), fails to run due to excessive arguments provided to the `push` method. In this PR improvements are made in the `browserFields` transformations that are done for the field browser component, expandable flyout table tab, and alert/rule tables via `CellValue` component. This work was done to get immediate improvements in the security solution UI, but a longer term consideration will be whether or not the `browserFields` is even necessary anymore as a concept based on what is available via the `fields` api. We will revisit once our Sourcerer refactoring work is done. <img width="1728" alt="Screenshot 2025-02-26 at 10 15 29 AM" src="https://github.com/user-attachments/assets/a25f577f-f758-415e-9c93-5452eadb8020" /> <img width="1445" alt="Screenshot 2025-02-26 at 10 18 36 AM" src="https://github.com/user-attachments/assets/d70970d3-991a-47ba-b617-5862d18101b6" /> <img width="1469" alt="Screenshot 2025-02-26 at 10 19 48 AM" src="https://github.com/user-attachments/assets/1767aa9b-66ab-46be-bc1a-5311630c2765" />   ### After the fix (Done on [this branch](#212173) that has the other changes as well) https://github.com/user-attachments/assets/da992296-4eb8-49d4-96ca-b0a19a00f1f0 ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 750e156) # Conflicts: # x-pack/plugins/security_solution/public/common/containers/source/index.test.tsx # x-pack/plugins/security_solution/public/common/containers/source/use_data_view.test.tsx # x-pack/plugins/triggers_actions_ui/public/application/sections/field_browser/components/field_items/field_items.tsx
- Loading branch information
1 parent
263cbcb
commit 9bf60ff
Showing
5 changed files
with
212 additions
and
177 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
154 changes: 154 additions & 0 deletions
154
x-pack/plugins/security_solution/public/common/containers/source/use_data_view.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| /* | ||
| * 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 { renderHook } from '@testing-library/react-hooks/dom'; | ||
| import { act } 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*'); | ||
| }); | ||
| }); |
Oops, something went wrong.