Skip to content

[8.18] [Security Solution] Fix timeline dynamic batching (#204034) | [ Security Solution ] Fix Refetch logic with new timeline batching (#205893)#210066

Merged
logeekal merged 4 commits intoelastic:8.18from
logeekal:backport/8.18/pr-204034_pr-205893
Feb 8, 2025
Merged

Conversation

@logeekal
Copy link
Copy Markdown
Contributor

@logeekal logeekal commented Feb 6, 2025

Backport

This will backport the following commits from main to 8.18:

Questions ?

Please refer to the Backport tool documentation

\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\n---------\n\nCo-authored-by: Philippe Oberti \nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>"}}]},{"author":{"name":"Jatin Kathuria","email":"jatin.kathuria@elastic.co"},"sourceCommit":{"committedDate":"2025-02-05T21:12:38Z","message":"[ Security Solution ] Fix Refetch logic with new timeline batching (#205893)\n\n## Summary\n\nPR : https://github.com//pull/204034 fixed some issues\nwith timeline batching. It was not able to fix one of the issue with\n`Refetch` logic which exists in `main` ( resulting in a flaky test ) and\ncausing some tests to fail in `8.16`, `8.17` and `8.x`.\n\n## Issue Description\n\nThere are 2 issues with below video:\n\n1. When user updates a status of an alert, the `Refetch` only happens on\nthe first `batch`. This behaviour is flaky currently. Even if the user\nis on nth batch, table will fetch 0th batch and reset the user's page\nback to 1.\n\n\n\nhttps://github.com/user-attachments/assets/eaf88a82-0e9b-4743-8b2d-60fd327a2443\n \n\n\n3. When user clicks `Refresh` manually, then also only first (0th)\n`batch` is fetched, which should have rather fetched all the present\nbatches.\n\n\n\n\nhttps://github.com/user-attachments/assets/8d578ce3-4f24-4e70-bc3a-ed6ba99167a0\n\n\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"54b4fac705c231b52396d70906f3259f9b129a3b","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["backport","release_note:skip","v9.0.0","Team:Threat Hunting:Investigations","v9.1.0"],"title":"[ Security Solution ] Fix Refetch logic with new timeline batching","number":205893,"url":"https://github.com//pull/205893","mergeCommit":{"message":"[ Security Solution ] Fix Refetch logic with new timeline batching (#205893)\n\n## Summary\n\nPR : https://github.com//pull/204034 fixed some issues\nwith timeline batching. It was not able to fix one of the issue with\n`Refetch` logic which exists in `main` ( resulting in a flaky test ) and\ncausing some tests to fail in `8.16`, `8.17` and `8.x`.\n\n## Issue Description\n\nThere are 2 issues with below video:\n\n1. When user updates a status of an alert, the `Refetch` only happens on\nthe first `batch`. This behaviour is flaky currently. Even if the user\nis on nth batch, table will fetch 0th batch and reset the user's page\nback to 1.\n\n\n\nhttps://github.com/user-attachments/assets/eaf88a82-0e9b-4743-8b2d-60fd327a2443\n \n\n\n3. When user clicks `Refresh` manually, then also only first (0th)\n`batch` is fetched, which should have rather fetched all the present\nbatches.\n\n\n\n\nhttps://github.com/user-attachments/assets/8d578ce3-4f24-4e70-bc3a-ed6ba99167a0\n\n\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"54b4fac705c231b52396d70906f3259f9b129a3b"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com//pull/209916","number":209916,"state":"MERGED","mergeCommit":{"sha":"aeaff4957aded708fecfa0e57f16b3cba5f77485","message":"[9.0] [ Security Solution ] Fix Refetch logic with new timeline batching (#205893) (#209916)\n\n# Backport\n\nThis will backport the following commits from `main` to `9.0`:\n- [[ Security Solution ] Fix Refetch logic with new timeline batching\n(#205893)](https://github.com//pull/205893)\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\nCo-authored-by: Jatin Kathuria "}},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com//pull/205893","number":205893,"mergeCommit":{"message":"[ Security Solution ] Fix Refetch logic with new timeline batching (#205893)\n\n## Summary\n\nPR : https://github.com//pull/204034 fixed some issues\nwith timeline batching. It was not able to fix one of the issue with\n`Refetch` logic which exists in `main` ( resulting in a flaky test ) and\ncausing some tests to fail in `8.16`, `8.17` and `8.x`.\n\n## Issue Description\n\nThere are 2 issues with below video:\n\n1. When user updates a status of an alert, the `Refetch` only happens on\nthe first `batch`. This behaviour is flaky currently. Even if the user\nis on nth batch, table will fetch 0th batch and reset the user's page\nback to 1.\n\n\n\nhttps://github.com/user-attachments/assets/eaf88a82-0e9b-4743-8b2d-60fd327a2443\n \n\n\n3. When user clicks `Refresh` manually, then also only first (0th)\n`batch` is fetched, which should have rather fetched all the present\nbatches.\n\n\n\n\nhttps://github.com/user-attachments/assets/8d578ce3-4f24-4e70-bc3a-ed6ba99167a0\n\n\n\n### Checklist\n\nCheck the PR satisfies following conditions. \n\nReviewers should verify this PR satisfies this list as well.\n\n\n- [x] [Unit or functional\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\nwere updated or added to match the most common scenarios","sha":"54b4fac705c231b52396d70906f3259f9b129a3b"}},{"url":"https://github.com//pull/205676","number":205676,"branch":"8.17","state":"OPEN"},{"url":"https://github.com//pull/205702","number":205702,"branch":"8.x","state":"MERGED","mergeCommit":{"sha":"e480de112ae6bc121c4415e2e1726dd551805672","message":"[8.x] [Security Solution] Fix timeline dynamic batching (#204034) (#205702)\n\n# Backport\n\nThis will backport the following commits from `main` to `8.x`:\n- [[Security Solution] Fix timeline dynamic batching\n(#204034)](https://github.com//pull/204034)\n - https://github.com//pull/205893\n\n\n\n### Questions ?\nPlease refer to the [Backport tool\ndocumentation](https://github.com/sqren/backport)\n\n\n\n---------\n\nCo-authored-by: Philippe Oberti \nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>"}},{"url":"https://github.com//pull/205674","number":205674,"branch":"8.16","state":"OPEN"}]}] BACKPORT-->

## Summary

Handles :

### Issue with Batches
- elastic#201405
- Timeline had a bug where if users fetched multiple batches and then if
user adds a new column, the value of this new columns will only be
fetched for the latest batch and not old batches.
- This PR fixes that ✅ by cumulatively fetching the data for old batches
till current batch `iff a new column has been added`.
- For example, if user has already fetched the 3rd batch, data for
1st,2nd and 3rd will be fetched together when a column has been added,
otherwise, data will be fetched incrementally.

### Issue with Elastic search limit

- Elastic search has a limit of 10K hits at max but we throw error at
10K which should be allowed.
    - Error should be thrown at anything `>10K`. 10001 for example.
    - ✅  This PR fixes that just for timeline by allowing 10K hits.

### Removal of obsolete code

Below files related to old Timeline code are removed as well:
-
x-pack/plugins/security_solution/public/timelines/components/timeline/footer/index.test.tsx
-
x-pack/plugins/security_solution/public/timelines/components/timeline/footer/index.tsx

---------

Co-authored-by: Philippe Oberti <philippe.oberti@elastic.co>
(cherry picked from commit 088169f)

# Conflicts:
#	x-pack/solutions/security/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.tsx
#	x-pack/solutions/security/plugins/security_solution/public/timelines/containers/index.test.tsx
…lastic#205893)

## Summary

PR : elastic#204034 fixed some issues
with timeline batching. It was not able to fix one of the issue with
`Refetch` logic which exists in `main` ( resulting in a flaky test ) and
causing some tests to fail in `8.16`, `8.17` and `8.x`.

## Issue Description

There are 2 issues with below video:

1. When user updates a status of an alert, the `Refetch` only happens on
the first `batch`. This behaviour is flaky currently. Even if the user
is on nth batch, table will fetch 0th batch and reset the user's page
back to 1.

https://github.com/user-attachments/assets/eaf88a82-0e9b-4743-8b2d-60fd327a2443

3. When user clicks `Refresh` manually, then also only first (0th)
`batch` is fetched, which should have rather fetched all the present
batches.

https://github.com/user-attachments/assets/8d578ce3-4f24-4e70-bc3a-ed6ba99167a0

### 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 54b4fac)
@logeekal logeekal merged commit 45f6001 into elastic:8.18 Feb 8, 2025
@elasticmachine
Copy link
Copy Markdown
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #12 / AddObservable opens the modal when clicked

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 18.8MB 18.8MB +1.6KB

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants