[Performance][Security Solution][2/4] - Timeline Performance #212478

michaelolo24 · 2025-02-26T08:43:26Z

Summary

Testing

For setup see testing section here: #212173 (comment)

Areas/How to test:

For the following pages, test there are no fields api requests in the inspector network tab when visiting from another page. IF YOU REFRESH on any of these pages, you will see these requests as they are called by the Query Search Bar and the useInitSourcerer call
- Cases Page
- Dashboard Page
- Timelines Page
Timeline
- All Tabs
  - Does it show the loading screen on first interaction?
  - Does the fields api fire on first interaction with the tab
  - When you navigate back to those tabs, do they not re-render?
All other pages hosting timeline
Do you feel like the performance is generally better?

Background

When investigating the performance of the security solution application, one of the issues that was observed was queries to the fields api on pages that had no reason making that request (such as Cases, or the Dashboards list view). This was due to the background background loaded tabs of timeline loading the relevant dataView necessary for their search functionality. When the fields request is significantly large this can have a massive impact on the experience of users on pages that should be relatively responsive.

To fix this a few changes were made.

First the withDataView HOC was removed as it was only used in 2 components that shared a parent - child relationship, and the child UnifiedTimeline was only used in the parent. The hook that HOC calls was not caching the dataView being created, so dataView.create was being called up to 6 times unnecessarily. Now it is only called once in each tab.
A new wrapper OnDemandRenderer (open to different naming 😅) was created that will not render any of the nested tabs until they are opened. Once they are opened, they stay in memory, to avoid re-calling expensive api's every time a user switches tabs.
Note: There is currently a known issue where navigating between various routes in security solution causes the whole application to unmount and re-mount. Which means every page change will lead to timeline needing to be re-loaded when the tab is opened. This is being resolved in a separate effort.
Additional checks were added to the useTimelineEvents hook to limit additional re-renders caused by unnecessary reference changes when the underlying values never actually change

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

Unit or functional tests were updated or added to match the most common scenarios

Identify risks

elasticmachine · 2025-02-26T16:17:55Z

Pinging @elastic/security-threat-hunting-investigations (Team:Threat Hunting:Investigations)

logeekal

Done code review as of now. Will post the results of Desk Testing soon.

logeekal · 2025-02-27T17:34:02Z

x-pack/solutions/security/plugins/security_solution/public/timelines/containers/index.tsx

-        loadNextBatch,
-        refreshedAt: 0,
-      });
+      setTimelineResponse(defaultTimelineResponse);


logeekal · 2025-02-27T17:36:16Z

x-pack/solutions/security/plugins/security_solution/public/timelines/containers/index.tsx

@@ -529,6 +522,8 @@ export const useTimelineEventsHandler = ({
  return [loading, finalTimelineLineResponse, timelineSearchHandler];
 };

+const defaultEvents: TimelineItem[][] = [];


might result in error somewhere since it is a 2D array and previously eventsPerPage[0] was [] and with this change it is undefined

It's only accessed on line 597, which would only be doing empty work to flatten a nested empty array. Happy to revert it, but not sure if it's necessary

...y/plugins/security_solution/public/timelines/components/timeline/tabs/on_demand_renderer.tsx

logeekal · 2025-03-06T12:33:51Z

...y/plugins/security_solution/public/timelines/components/timeline/tabs/on_demand_renderer.tsx

+
+OnDemandRenderer.displayName = 'OnDemandRenderer';
+
+export const OnDemandSuspenseFallback = () => (


Since this component is not in itself related to React.suspense, I would suggest renaming it to TimelineTabFallback

...y/plugins/security_solution/public/timelines/components/timeline/tabs/on_demand_renderer.tsx

...gins/security_solution/public/timelines/components/timeline/tabs/on_demand_renderer.test.tsx

logeekal · 2025-03-06T12:54:37Z

...y/plugins/security_solution/public/timelines/components/timeline/tabs/query/events_count.tsx

+    timerangeKind,
+  });
+
+  useEffect(() => {


Do you think we need to remove similar useEffect in query and eql tab so there it does not override the effect of this useEffect 🤷

Potentially, but it should be aligned with the query tab as they should be running the same query string. I don't expect this change to be necessary

logeekal · 2025-03-06T13:00:07Z

...olutions/security/plugins/security_solution/public/timelines/components/bottom_bar/index.tsx

@@ -65,7 +65,7 @@ export const TimelineBottomBar = React.memo<TimelineBottomBarProps>(
          </EuiFlexItem>
          {!show && ( // this is a hack because TimelineEventsCountBadge is using react-reverse-portal so the component which is used in multiple places cannot be visible in multiple places at the same time
            <EuiFlexItem grow={false} data-test-subj="timeline-event-count-badge">
-              <TimelineEventsCountBadge />
+              <TimelineQueryTabEventsCount timelineId={timelineId} />


Are you planning to remove that OutPortal InPortal stuff as well in this PR?

No, I wasn't planning on making that change in this PR

logeekal

Desk Testing looks good. Below are couple of observations:

Timeline Search Strategy firing twice

When there is a timeline bottom bar, timelineSearchStrategy is firing twice to get the count of events, everytime. See demo below:

timelineSearchStrategy_firing_twice.mp4

App Perf with 500K total fields

I tried app performance with 500K total fields with :

one document having all 500K fields populated
around 10K docs with 25K fields populated

the whole app feels slow specially because useInitSourcerer takes around 10s to finish and get all the fields when visiting Alerts Page. a

Below URL took 12s to resolve and that essential blocks whole Alerts page. And app also seems slow. Not sure why.

http://localhost:5601/internal/data_views/fields?pattern=.alerts-security.alerts-default%2Capm-*-transaction*%2Cauditbeat-*%2Cendgame-*%2Cfilebeat-*%2Clogs-*%2Cpacketbeat-*%2Ctest*%2Ctraces-apm*%2Cwinlogbeat-*%2C-*elastic-cloud-logs-*&meta_fields=_source&meta_fields=_id&meta_fields=_index&meta_fields=_score&meta_fields=_ignored&allow_no_index=true&apiVersion=1

Screen.Recording.2025-03-07.at.12.31.40.mov

logeekal · 2025-03-07T12:46:44Z

One more feedback. it looks like this PR introduces a wierd space in timeline as highlighted below. for comparison, i have posted a screenshot from Release-sec instance.

michaelolo24 · 2025-03-07T20:23:59Z

@logeekal - Can you retest, I was unable to replicate the duplicate queries...and the gap is from the borealis changes as that is also currently on main, but not 8.18

elasticmachine · 2025-03-10T14:07:00Z

💛 Build succeeded, but was flaky

Failed CI Steps

Detection Engine - Security Solution Cypress Tests #5

Test Failures

[job] [logs] Detection Engine - Security Solution Cypress Tests #5 / Custom query rules Custom detection rules creation with data views Creates and enables a new rule Creates and enables a new rule

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	7062	7064	+2

Async chunks

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

id	before	after	diff
`securitySolution`	8.9MB	8.8MB	-13.2KB

Page load bundle

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

id	before	after	diff
`securitySolution`	87.5KB	87.5KB	-9.0B

History

💛 Build #281813 was flaky d1c290b1d285f514e1ac244268420b3bf16602ef
💚 Build #281587 succeeded e3c8e54bd0c0b1cead00bd56e5e4129916864ab8
💔 Build #281555 failed 7536190dd72c3809274348e835919415b42379ca
💔 Build #281399 failed c3585261c3e9fb16a9c345d7c245f0f6c23a0a39

logeekal

Can you retest, I was unable to replicate the duplicate queries...and the gap is from the borealis changes as that is also currently on main, but not 8.18

@michaelolo24 , I retested and can confirm that now only single query is firing. I also noticed that gap is only in light theme and not in borealis 🤦 .

Thanks for making these changes.

kibanamachine · 2025-03-11T16:57:03Z

Starting backport for target branches: 8.17, 8.x

https://github.com/elastic/kibana/actions/runs/13793767412

…#212478) ## Summary Part 2 of elastic#212173 ### Testing For setup see testing section here: elastic#212173 (comment) **Areas/How to test:** - For the following pages, test there are no `fields` api requests in the inspector network tab when visiting from another page. IF YOU REFRESH on any of these pages, you will see these requests as they are called by the Query Search Bar and the `useInitSourcerer` call - Cases Page - Dashboard Page - Timelines Page - Timeline - All Tabs - Does it show the loading screen on first interaction? - Does the `fields` api fire on first interaction with the tab - When you navigate back to those tabs, do they not re-render? - All other pages hosting timeline - Do you feel like the performance is generally better? ### Background When investigating the performance of the security solution application, one of the issues that was observed was queries to the `fields` api on pages that had no reason making that request (such as Cases, or the Dashboards list view). This was due to the background background loaded tabs of timeline loading the relevant `dataView` necessary for their search functionality. When the fields request is significantly large this can have a massive impact on the experience of users on pages that should be relatively responsive. To fix this a few changes were made. 1. First the `withDataView` HOC was removed as it was only used in 2 components that shared a parent - child relationship, and the child `UnifiedTimeline` was only used in the parent. The hook that HOC calls was not caching the dataView being created, so `dataView.create` was being called up to 6 times unnecessarily. Now it is only called once in each tab. 2. A new wrapper `OnDemandRenderer` (open to different naming 😅) was created that will not render any of the nested tabs until they are opened. Once they are opened, they stay in memory, to avoid re-calling expensive api's every time a user switches tabs. _Note_: There is currently a known issue where navigating between various routes in security solution causes the whole application to unmount and re-mount. Which means every page change will lead to timeline needing to be re-loaded when the tab is opened. This is being resolved in a separate effort. 3. Additional checks were added to the `useTimelineEvents` hook to limit additional re-renders caused by unnecessary reference changes when the underlying values never actually change ### 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 ### Identify risks (cherry picked from commit 2d8f3c1)

kibanamachine · 2025-03-11T17:09:59Z

💔 Some backports could not be created

Status	Branch	Result
❌	8.17	Backport failed because of merge conflicts
✅	8.x

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 212478

Questions ?

Please refer to the Backport tool documentation

…#212478) ## Summary Part 2 of elastic#212173 ### Testing For setup see testing section here: elastic#212173 (comment) **Areas/How to test:** - For the following pages, test there are no `fields` api requests in the inspector network tab when visiting from another page. IF YOU REFRESH on any of these pages, you will see these requests as they are called by the Query Search Bar and the `useInitSourcerer` call - Cases Page - Dashboard Page - Timelines Page - Timeline - All Tabs - Does it show the loading screen on first interaction? - Does the `fields` api fire on first interaction with the tab - When you navigate back to those tabs, do they not re-render? - All other pages hosting timeline - Do you feel like the performance is generally better? ### Background When investigating the performance of the security solution application, one of the issues that was observed was queries to the `fields` api on pages that had no reason making that request (such as Cases, or the Dashboards list view). This was due to the background background loaded tabs of timeline loading the relevant `dataView` necessary for their search functionality. When the fields request is significantly large this can have a massive impact on the experience of users on pages that should be relatively responsive. To fix this a few changes were made. 1. First the `withDataView` HOC was removed as it was only used in 2 components that shared a parent - child relationship, and the child `UnifiedTimeline` was only used in the parent. The hook that HOC calls was not caching the dataView being created, so `dataView.create` was being called up to 6 times unnecessarily. Now it is only called once in each tab. 2. A new wrapper `OnDemandRenderer` (open to different naming 😅) was created that will not render any of the nested tabs until they are opened. Once they are opened, they stay in memory, to avoid re-calling expensive api's every time a user switches tabs. _Note_: There is currently a known issue where navigating between various routes in security solution causes the whole application to unmount and re-mount. Which means every page change will lead to timeline needing to be re-loaded when the tab is opened. This is being resolved in a separate effort. 3. Additional checks were added to the `useTimelineEvents` hook to limit additional re-renders caused by unnecessary reference changes when the underlying values never actually change ### 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 ### Identify risks (cherry picked from commit 2d8f3c1) # Conflicts: # x-pack/plugins/security_solution/public/timelines/components/timeline/context.ts # x-pack/plugins/security_solution/public/timelines/components/timeline/index.tsx # x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/index.tsx # x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/lazy_timeline_tab_renderer.test.tsx # x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/lazy_timeline_tab_renderer.tsx # x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/events_count.tsx # x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/index.tsx

michaelolo24 · 2025-03-11T18:23:19Z

💚 All backports created successfully

Status	Branch	Result
✅	8.17

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