Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Security Solution][Serverless] - Improve security solution performance #194241

Merged
merged 3 commits into from
Sep 27, 2024
Merged

[Security Solution][Serverless] - Improve security solution performance #194241

merged 3 commits into from
Sep 27, 2024

Conversation

michaelolo24
Copy link
Contributor

@michaelolo24 michaelolo24 commented Sep 27, 2024 •
edited
Loading

Summary

The goal of this PR is to improve the default performance of many of our security solution views.

  1. Upon scale testing, it was observed that the default events histogram aggregation was a source of application slowness, so to improve the performance of the default security experience, we've made the default breakdown for the events histogram No Breakdown similar to what is seen in the default discover histogram experience.

  2. After looking through some telemetry, it was observed that the field list query run in the background for timeline can also take a significant amount of time based on the user's field count, so we now only run that query after timeline has been opened.

Demos

1. By default the events visualizations on the overview and explore events pages will not have an aggregation. The user will have to manually select the breakdown they desire: d354d27

Screen.Recording.2024-09-26.at.3.30.00.PM.mov

2. Timeline fields list will only load after the first interaction with timeline: ad55726

Before:

Screen.Recording.2024-09-26.at.9.52.49.PM.mov

After:

Screen.Recording.2024-09-26.at.10.25.18.PM.mov

@elastic elastic deleted a comment from kibana-ci Sep 27, 2024
@michaelolo24 michaelolo24 marked this pull request as ready for review September 27, 2024 14:00
@michaelolo24 michaelolo24 requested review from a team as code owners September 27, 2024 14:00
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting-explore (Team:Threat Hunting:Explore)

angorayc
angorayc approved these changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@angorayc angorayc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the enhancement, tested locally, LGTM

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #6 / FilterPopover renders button label correctly
  • [job] [logs] Jest Tests #12 / Timeline rendering should trim kqlQueryExpression

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 20.5MB 20.5MB +711.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
securitySolution 542 543 +1

Total ESLint disabled count

id before after diff
securitySolution 626 627 +1

History

  • 💔 Build #237670 failed 033a8f803ca03a7f90593f906057eae3414df031

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

@michaelolo24 michaelolo24 changed the title [Security Solution][Serverless] - Improve security solution serverless performance [Security Solution][Serverless] - Improve security solution performance Sep 27, 2024
@michaelolo24 michaelolo24 enabled auto-merge (squash) September 27, 2024 16:18
janmonschke
janmonschke approved these changes Sep 27, 2024
View reviewed changes
Copy link
Contributor

@janmonschke janmonschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these improvements and making sure to comment the purpose of the flags in the code.

One thing I noticed in the recording of the "after" demo of the new timeline behaviour, is that the request is still executed three (?) times. Or is it just the console.log statement firing three times?

If it's really the request executing three times, I think it's worth fixing that as well, given the request can be quite expensive. Wdyt?

@michaelolo24 michaelolo24 merged commit e45d97b into elastic:main Sep 27, 2024
42 checks passed
@michaelolo24
Copy link
Contributor Author

Thanks for the review @janmonschke ! To answer your question, the Query tab, Correlation tab, and Pinned events tab all load the fields list which is why the call is made 3 times. After the first request, every subsequent request should hit the cache, but they resolve pretty closely to each other currently.

@logeekal
Copy link
Contributor

I guess not a huge effect but does it make sense to not load all the tabs until they are first opened so that request is fired only once?

@michaelolo24
Copy link
Contributor Author

I guess not a huge effect but does it make sense to not load all the tabs until they are first opened so that request is fired only once?

Yea, I can make that change as a follow up. I don't see any reasons why not to. I would just prevent all tabs besides the query tab from loading.

@logeekal
Copy link
Contributor

logeekal commented Sep 27, 2024 •
edited
Loading

Thanks.

I have desk tested this and it is working great. Although, when Row renderers are switched ON, it takes a lot of time for timeline to open up and then it waits for fields to be fetched so first click from the user takes a lot of time.

But i guess that is row renderer problem which should be fixed by this PR : #193316
See if you get time to test it.

@michaelolo24 michaelolo24 added the backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) label Oct 1, 2024
@michaelolo24 michaelolo24 mentioned this pull request Oct 1, 2024
Merged
@michaelolo24
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
✅ 8.x

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

Questions ?

Please refer to the Backport tool documentation

michaelolo24 added a commit to michaelolo24/kibana that referenced this pull request Oct 1, 2024
@michaelolo24
[Security Solution][Serverless] - Improve security solution performan…
fe33c66 
…ce (elastic#194241)

## Summary

The goal of this PR is to improve the default performance of many of our
security solution views.

1. Upon scale testing, it was observed that the default events histogram
aggregation was a source of application slowness, so to improve the
performance of the default security experience, we've made the default
breakdown for the events histogram `No Breakdown` similar to what is
seen in the default discover histogram experience.

2. After looking through some telemetry, it was observed that the field
list query run in the background for timeline can also take a
significant amount of time based on the user's field count, so we now
only run that query after timeline has been opened.

### Demos
#### 1. By default the events visualizations on the overview and explore
events pages will not have an aggregation. The user will have to
manually select the breakdown they desire:
elastic@d354d27

https://github.com/user-attachments/assets/a6d6987b-73fc-4735-9c37-973917c2fa2d

#### 2. Timeline fields list will only load after the first interaction
with timeline:
elastic@ad55726

**Before:**

https://github.com/user-attachments/assets/0ad2e903-ac15-4daa-925b-da8ad05e80dd

**After:**

https://github.com/user-attachments/assets/27d5d3d5-02c8-49b5-b699-239ebc36b16c
(cherry picked from commit e45d97b)
michaelolo24 added a commit that referenced this pull request Oct 1, 2024
@michaelolo24
[8.x] [Security Solution][Serverless] - Improve security solution per…
ca251ff 
…formance (#194241) (#194588)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution][Serverless] - Improve security solution
performance (#194241)](#194241)

<!--- Backport version: 8.9.8 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Michael
Olorunnisola","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-27T18:45:45Z","message":"[Security
Solution][Serverless] - Improve security solution performance
(#194241)\n\n## Summary\r\n\r\nThe goal of this PR is to improve the
default performance of many of our\r\nsecurity solution views.\r\n\r\n1.
Upon scale testing, it was observed that the default events
histogram\r\naggregation was a source of application slowness, so to
improve the\r\nperformance of the default security experience, we've
made the default\r\nbreakdown for the events histogram `No Breakdown`
similar to what is\r\nseen in the default discover histogram
experience.\r\n\r\n2. After looking through some telemetry, it was
observed that the field\r\nlist query run in the background for timeline
can also take a\r\nsignificant amount of time based on the user's field
count, so we now\r\nonly run that query after timeline has been
opened.\r\n\r\n### Demos\r\n#### 1. By default the events visualizations
on the overview and explore\r\nevents pages will not have an
aggregation. The user will have to\r\nmanually select the breakdown they
desire:\r\nhttps://github.com/elastic/kibana/commit/d354d27962ebbd6d5fda19e912ec344ffe8a6c75\r\n\r\n\r\nhttps://github.com/user-attachments/assets/a6d6987b-73fc-4735-9c37-973917c2fa2d\r\n\r\n\r\n####
2. Timeline fields list will only load after the first
interaction\r\nwith
timeline:\r\nhttps://github.com/elastic/kibana/commit/ad557260d8f9c5dd0810a5a6aa51e5de0430000f\r\n\r\n**Before:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0ad2e903-ac15-4daa-925b-da8ad05e80dd\r\n\r\n\r\n**After:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/27d5d3d5-02c8-49b5-b699-239ebc36b16c","sha":"e45d97b26c6d0e0798d620ad0b097cad9009c179","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","backport:skip","v9.0.0","Team:Threat
Hunting:Investigations","Team:Threat
Hunting:Explore","backport:prev-minor","v8.16.0"],"number":194241,"url":"https://github.com/elastic/kibana/pull/194241","mergeCommit":{"message":"[Security
Solution][Serverless] - Improve security solution performance
(#194241)\n\n## Summary\r\n\r\nThe goal of this PR is to improve the
default performance of many of our\r\nsecurity solution views.\r\n\r\n1.
Upon scale testing, it was observed that the default events
histogram\r\naggregation was a source of application slowness, so to
improve the\r\nperformance of the default security experience, we've
made the default\r\nbreakdown for the events histogram `No Breakdown`
similar to what is\r\nseen in the default discover histogram
experience.\r\n\r\n2. After looking through some telemetry, it was
observed that the field\r\nlist query run in the background for timeline
can also take a\r\nsignificant amount of time based on the user's field
count, so we now\r\nonly run that query after timeline has been
opened.\r\n\r\n### Demos\r\n#### 1. By default the events visualizations
on the overview and explore\r\nevents pages will not have an
aggregation. The user will have to\r\nmanually select the breakdown they
desire:\r\nhttps://github.com/elastic/kibana/commit/d354d27962ebbd6d5fda19e912ec344ffe8a6c75\r\n\r\n\r\nhttps://github.com/user-attachments/assets/a6d6987b-73fc-4735-9c37-973917c2fa2d\r\n\r\n\r\n####
2. Timeline fields list will only load after the first
interaction\r\nwith
timeline:\r\nhttps://github.com/elastic/kibana/commit/ad557260d8f9c5dd0810a5a6aa51e5de0430000f\r\n\r\n**Before:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0ad2e903-ac15-4daa-925b-da8ad05e80dd\r\n\r\n\r\n**After:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/27d5d3d5-02c8-49b5-b699-239ebc36b16c","sha":"e45d97b26c6d0e0798d620ad0b097cad9009c179"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194241","number":194241,"mergeCommit":{"message":"[Security
Solution][Serverless] - Improve security solution performance
(#194241)\n\n## Summary\r\n\r\nThe goal of this PR is to improve the
default performance of many of our\r\nsecurity solution views.\r\n\r\n1.
Upon scale testing, it was observed that the default events
histogram\r\naggregation was a source of application slowness, so to
improve the\r\nperformance of the default security experience, we've
made the default\r\nbreakdown for the events histogram `No Breakdown`
similar to what is\r\nseen in the default discover histogram
experience.\r\n\r\n2. After looking through some telemetry, it was
observed that the field\r\nlist query run in the background for timeline
can also take a\r\nsignificant amount of time based on the user's field
count, so we now\r\nonly run that query after timeline has been
opened.\r\n\r\n### Demos\r\n#### 1. By default the events visualizations
on the overview and explore\r\nevents pages will not have an
aggregation. The user will have to\r\nmanually select the breakdown they
desire:\r\nhttps://github.com/elastic/kibana/commit/d354d27962ebbd6d5fda19e912ec344ffe8a6c75\r\n\r\n\r\nhttps://github.com/user-attachments/assets/a6d6987b-73fc-4735-9c37-973917c2fa2d\r\n\r\n\r\n####
2. Timeline fields list will only load after the first
interaction\r\nwith
timeline:\r\nhttps://github.com/elastic/kibana/commit/ad557260d8f9c5dd0810a5a6aa51e5de0430000f\r\n\r\n**Before:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/0ad2e903-ac15-4daa-925b-da8ad05e80dd\r\n\r\n\r\n**After:**\r\n\r\n\r\nhttps://github.com/user-attachments/assets/27d5d3d5-02c8-49b5-b699-239ebc36b16c","sha":"e45d97b26c6d0e0798d620ad0b097cad9009c179"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@angorayc angorayc angorayc approved these changes

@janmonschke janmonschke janmonschke approved these changes

Assignees
No one assigned
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) backport:skip This commit does not require backporting release_note:enhancement Team:Threat Hunting:Explore Team:Threat Hunting:Investigations Security Solution Investigations Team v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@michaelolo24 @elasticmachine @kibana-ci @logeekal @janmonschke @angorayc @kibanamachine