-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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.
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
[Security Solution] Use search strategy error in timeline #125178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's awesome! LGTM
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
Pinging @elastic/security-solution (Team: SecuritySolution) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making these changes Steph!
💚 Build SucceededMetrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…125423) (cherry picked from commit 4fe96b7) Co-authored-by: Steph Milovic <[email protected]>
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
5 similar comments
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync. |
…125422) (cherry picked from commit 4fe96b7) Co-authored-by: Steph Milovic <[email protected]>
Summary
Implements better error handling to fix #122990
The search strategy error handler does some extra work on errors that we are missing out on by using our own error toast. In timeline, I switched it to use the better error handling so we can get this error on a bad runtime field:
This is how it used to look:
We should probably do this replace all of our
addError
within search strategy calls withdata.search.showError
To test: On alerts page, add runtime field for
emit(doc["process.name"].value)
this will likely show the error. If it does not, sort by the new runtime field and this will show the error.