[Security Solution] useListsConfig performs many repeated /api/lists/index calls #169268
Comments
umbopepato
added
enhancement
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
michaelolo24
added
bug
|
Pinging @elastic/security-defend-workflows (Team:Defend Workflows) |
|
Hey @kevinlog and @ashokaditya - I'm not sure if the defend workflows team is the correct team for this, but based on @kqualters-elastic 's investigation. It may be worth caching the lists call or lifting it out of the |
|
Apologies if I'm missing context here, but wondering why the logic inside the take actions components gets called on render. Shouldn't that logic only be invoked when a user clicks on the |
|
@yctercero Keen observation with the logic. I notice that currently the menu items that should be shown within the I'm going to have a closer look at this and decouple some logic as you suggest and make a fix. |
|
Even if it worked like that, as a user opens the menu for each component, we are making the same exact request, that's not using any data from the document associated with the row in the request itself, as each menu is opened. The problem of repeated, redundant requests still exists. Also think that this was done to play nicely with eui, as this button ultimately is passed along to EuiDataGrid via the leadingControlColumns prop, and updating just the single row control for a row does not seem possible with the current component api. Up until a few weeks ago, everything has worked just fine, something was changed in the overly complicated nested hooks based architecture that drives the menu. |
|
Thanks for looking into it @kqualters-elastic. The changes that caused this went in last month with this PR /pull/165613. |
Closes #168470 ## Summary Adds a `dynamicRowHeight` option to the alerts table that replaces the virtualized table body with a custom non-virtualized one to improve the performance of variable row heights (even though the rows weren't recycled the auto height was causing a lot of unnecessary re-renderings). When enabled, this option reduces the amount of re-renders from [~500](#168470 (comment)) to ~180 when expanding the table from 10 to 100 visible rows:  When combined with a possible solution to #169268 , the number of renders drastically reduces to around 30:  ### Checklist Delete any items that are not applicable to this PR. - [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 - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Xavier Mouligneau <[email protected]>
|
PR /pull/171412 |
…k to check access to`.lists-*` (elastic#171412) ## Summary Instead of using `useListsConfig` this PR uses `useListPrivileges` to verify access to `.lists-*` index pattern. follow up of elastic/pull/165613 related elastic/pull/170671 (closed in favour of this) fixes elastic/issues/169268 ### Checklist - [ ] [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 --------- Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit 8c3322e)
…k to check access to`.lists-*` (elastic#171412) ## Summary Instead of using `useListsConfig` this PR uses `useListPrivileges` to verify access to `.lists-*` index pattern. follow up of elastic/pull/165613 related elastic/pull/170671 (closed in favour of this) fixes elastic/issues/169268 ### Checklist - [ ] [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 --------- Co-authored-by: Kibana Machine <[email protected]>
…to check access to`.lists-*` for endpoint exceptions access (elastic#171412) (elastic#171794) # Backport This will backport the following commits from `main` to `8.11`: - [[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)](elastic#171412) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Ash","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-11-22T19:10:50Z","message":"[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)\n\n## Summary\r\n\r\nInstead of using `useListsConfig` this PR uses `useListPrivileges` to\r\nverify access to `.lists-*` index pattern.\r\n\r\nfollow up of elastic/pull/165613\r\nrelated elastic/pull/170671 (closed in favour of this)\r\nfixes elastic/issues/169268\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"8c3322ed44ccfbc4e91e0e9ef31f77b79c549cb8","branchLabelMapping":{"^v8.12.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","OLM Sprint","v8.11.0","v8.12.0","v8.11.1"],"number":171412,"url":"https://github.com/elastic/kibana/pull/171412","mergeCommit":{"message":"[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)\n\n## Summary\r\n\r\nInstead of using `useListsConfig` this PR uses `useListPrivileges` to\r\nverify access to `.lists-*` index pattern.\r\n\r\nfollow up of elastic/pull/165613\r\nrelated elastic/pull/170671 (closed in favour of this)\r\nfixes elastic/issues/169268\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"8c3322ed44ccfbc4e91e0e9ef31f77b79c549cb8"}},"sourceBranch":"main","suggestedTargetBranches":["8.11"],"targetPullRequestStates":[{"branch":"8.11","label":"v8.11.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.12.0","labelRegex":"^v8.12.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/171412","number":171412,"mergeCommit":{"message":"[Serverless][Security Solution][Endpoint] Remove use of hooks to check access to`.lists-*` for endpoint exceptions access (elastic#171412)\n\n## Summary\r\n\r\nInstead of using `useListsConfig` this PR uses `useListPrivileges` to\r\nverify access to `.lists-*` index pattern.\r\n\r\nfollow up of elastic/pull/165613\r\nrelated elastic/pull/170671 (closed in favour of this)\r\nfixes elastic/issues/169268\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine <[email protected]>","sha":"8c3322ed44ccfbc4e91e0e9ef31f77b79c549cb8"}}]}] BACKPORT--> Co-authored-by: Ash <[email protected]>
The various usages of the
useListsConfighook acrossDetectionEnginePagecause a lot of repeatedGET /arp/api/lists/indexhttp requests (by means ofuseListsIndex). This causes a lot of unnecessary re-renders in the alerts table, triggered by each row action cell:As part of our ongoing work to improve the alerts table's performance we've tried to share the lists configuration response through a context, reducing the number of renders from ~500 to ~30.
The text was updated successfully, but these errors were encountered: