Enhance Event Fields Browser performance

[oatkiller]

Summary

This PR enhances the performance of the Event Details 'Table' tab, especially the filtering interaction. This affects users with a Data View that has many fields. Note: the Security app creates a Data View that has one field for each mapping in a list of indices.

Before

before_event_fields_browser_enhancements_trimmed.mov

Chrome dev tools performance profile from before the fix

After

after_event_fields_browser_enhancements.mov

Chrome dev tools performance profile from after the fix

Reproducing the issue

1. Use the Elastic Security app to select a Data View that has many fields. If you don't have a Data View with many fields, reach out to me and I'll provide you with details on how to get one.
2. Have an alert. If you don't have one, try creating a simple rule that alerts on any event with @timestamp present in any form, then index a single document that has an @timestamp value.
3. View the details of the Alert
4. View the 'Table' tab. Notice that when clicking it, the app becomes unresponsive briefly.
5. Try filtering the list of fields. This process is slow. Especially if you type in a filter that limits the number of fields to a small number, and then erase that filter.

Context: What is the code doing?

The EventFieldsBrowser component shows a row for each field. For each row, the component decides which actions a user can take. These actions are represented by small icons. For example, one action allows users to filter by the field with the given value. Another action lets users see the 'Top N' similar events. This action is only available on fields that are 'aggregatable'. Therefore, the component only shows this action for applicable fields.

Areas to Enhance

In order to determine whether a given row should have the 'Top N' action, the component does a calculation for each row, up front. This algorithm has a runtime complexity of O(n*m) where n is the number of fields in the data view and m is the number of rows/fields shown in the table. This could be enhanced by doing the calculation lazily, only when the user accesses the overflow menu that shows the Top N action. However that fix will be non-trivial, so we'll postpone it for now.

The calculation to determine if each row is 'aggregatable' is also done on every render. This is because it is done using useMemo, but one value in the dependencies array changes on each render. This value is called getAddToTimelineButton. It is a function which is bound, via .bind, to the timeline store on each render. This results in a different function reference being passed to the dependencies array on each render, thereby forcing the calculation to happen on each render. This PR addresses the issue by lazily binding this function to the store, and then saving that reference. When the store is changed, the cached reference is removed.

Once the first enhancement was made, a second issue was discovered. The table rows component instances were not being reconciled correctly by React. The EuiInMemoryTableComponent will set a React key property for rows, but only if the itemId property is passed. If this property is not passed to the table, then each row will use its positional index as a key. This will prevent React from correctly reconciling components. This means that a component may be re-rendered with props that previously belong to a sibling component. This can cause memos to be invalidated unnecessarily. This was corrected by passing the itemId property. We should consider requesting enhanced documentation about the importance of this property. Also, it may be useful to add a console warning when this property is missing (similar to how React shows a console warning when key properties are missing in certain situations.)

What's left

The component should be enhanced so that the calculation to determine which fields can show the 'Top N' action is more efficient. The 'Top N' action is, in practice, only visible from an overflow menu. This means that the calculation could theoretically take place lazily, only when the user accesses the overflow menu. Instead, the component decides whether each field is 'aggregatable' before rendering. It could also be possible to improve the algorithm that determines which fields are aggregatable. Currently this is done by a simple search across all fields in the DataView.

Also, I would like to spend some time explaining the methods I used to find these issues.

Checklist

• Add more comments to the timelines plugin code explaining the need for the lazily calculated reference to the getAddToTimelineButton fn. This design is a stop gap fix.
• Test this with a more realistic alert. The alert I suggest using is not representative of a realistic alert.
• Unit or functional tests were updated or added to match the most common scenarios

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
This does not fix all known performance issues that affect the Event Fields Browser	100%	Low	This shouldn't have a negative impact on performance or behavior. Any improvement is good. These changes should go a long way for users with large data views.

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[janmonschke]

Thanks for this improvement and the great PR description 👍

[semd]

LGTM

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 0364001
• Storybooks Preview
• Cloud Deployment
• Webpack Bundle Analyzer

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
securitySolution	4.8MB	4.8MB	+15.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
timelines	286.6KB	286.7KB	+121.0B

History

• 💚 Build #37169 succeeded 4c89a3a
• 💔 Build #37148 failed 37119d5
• 💔 Build #37146 failed be3d1756719e0dc4a017af8bdb98cfdce000c83e
• 💔 Build #37144 failed 99556e9430d834195799db9380a4cf653237091e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[oatkiller]

💚 All backports created successfully

Status	Branch	Result
✅	8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation