[Security Solution] [Trigger Actions] Alert Table Refactoring #149128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
logeekal
force-pushed
the
alert-table-columns-actions
branch
from
92455c5 to
13b439b
Compare
logeekal
force-pushed
the
alert-table-columns-actions
branch
from
cc9b213 to
740cc18
Compare
MadameSheema
approved these changes
Feb 21, 2023
|
Thanks @cnasikas, I did notice it and thought that this PR is old and I was checking our code might have fixed it. But I will skip it for now.. Thanks. |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Any counts in public APIs
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
|
I think this may have broken building block highlighting? wtf.mov |
@stephmilovic , created bug for the same : [Security Solution] New Alert table may have broken building block highlighting. |
1 task
michaelolo24
pushed a commit
that referenced
this pull request
May 8, 2023
… from timestamp instead of @timestamp field (#156884) ## Summary In Kibana 8.8 we've done a huge refactor of the alert table (see [PR](#149128)). The new table's trigger actions are no longer some of the Security Solution server side methods that were adding a `timestamp` field to the response (see [here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)). That `timestamp` field was basically a duplication of the `@timestamp` field (done via [this helper function](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12)) Running the stack locally, you would see that clicking on the `Investigate in timeline` icon button or ANY alerts in the alerts table (pretty new or months/years old) would bring the timeline flyout with always the `to` value being the `current date`, and the `from` value being the `current date - kibana.alert.rule.from`. The `timetamp` field [here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158) does not exists, so we always fall back to `new Date()` (unless you're looking at an alert generated by a threshold rule, a new terms rule or a suppressed alert). This PR fixes the issue by retrieving the `@timestamp` field instead of the unwanted `timestamp` field. More work will be done in the future to actually entirely remove and cleanup the `timestamp` field (see [this WIP ticket](#156879)). ### Checklist - [ ] [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 Before (recorded on May 4th and we're looking at an alert generated on May 3rd) https://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov After https://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov Closes #126077
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
May 8, 2023
… from timestamp instead of @timestamp field (elastic#156884) ## Summary In Kibana 8.8 we've done a huge refactor of the alert table (see [PR](elastic#149128)). The new table's trigger actions are no longer some of the Security Solution server side methods that were adding a `timestamp` field to the response (see [here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)). That `timestamp` field was basically a duplication of the `@timestamp` field (done via [this helper function](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12)) Running the stack locally, you would see that clicking on the `Investigate in timeline` icon button or ANY alerts in the alerts table (pretty new or months/years old) would bring the timeline flyout with always the `to` value being the `current date`, and the `from` value being the `current date - kibana.alert.rule.from`. The `timetamp` field [here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158) does not exists, so we always fall back to `new Date()` (unless you're looking at an alert generated by a threshold rule, a new terms rule or a suppressed alert). This PR fixes the issue by retrieving the `@timestamp` field instead of the unwanted `timestamp` field. More work will be done in the future to actually entirely remove and cleanup the `timestamp` field (see [this WIP ticket](elastic#156879)). ### Checklist - [ ] [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 Before (recorded on May 4th and we're looking at an alert generated on May 3rd) https://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov After https://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov Closes elastic#126077 (cherry picked from commit 114c98d)
kibanamachine
referenced
this pull request
May 9, 2023
…pulled from timestamp instead of @timestamp field (#156884) (#157114) # Backport This will backport the following commits from `main` to `8.8`: - [[Security Solution] fix from-to values investigate in timeline pulled from timestamp instead of @timestamp field (#156884)](#156884) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Philippe Oberti","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-05-08T23:44:14Z","message":"[Security Solution] fix from-to values investigate in timeline pulled from timestamp instead of @timestamp field (#156884)\n\n## Summary\r\n\r\nIn Kibana 8.8 we've done a huge refactor of the alert table (see\r\n[PR](https://github.com/elastic/kibana/pull/149128)).\r\nThe new table's trigger actions are no longer some of the Security\r\nSolution server side methods that were adding a `timestamp` field to the\r\nresponse (see\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)).\r\nThat `timestamp` field was basically a duplication of the `@timestamp`\r\nfield (done via [this helper\r\nfunction](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12))\r\n\r\nRunning the stack locally, you would see that clicking on the\r\n`Investigate in timeline` icon button or ANY alerts in the alerts table\r\n(pretty new or months/years old) would bring the timeline flyout with\r\nalways the `to` value being the `current date`, and the `from` value\r\nbeing the `current date - kibana.alert.rule.from`.\r\nThe `timetamp` field\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158)\r\ndoes not exists, so we always fall back to `new Date()` (unless you're\r\nlooking at an alert generated by a threshold rule, a new terms rule or a\r\nsuppressed alert).\r\n\r\nThis PR fixes the issue by retrieving the `@timestamp` field instead of\r\nthe unwanted `timestamp` field.\r\n\r\nMore work will be done in the future to actually entirely remove and\r\ncleanup the `timestamp` field (see [this WIP\r\nticket](https://github.com/elastic/kibana/issues/156879)).\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\nBefore (recorded on May 4th and we're looking at an alert generated on\r\nMay 3rd)\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov\r\n\r\nAfter\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov\r\n\r\nCloses https://github.com/elastic/kibana/issues/126077","sha":"114c98d3b5df4252f500ea762879524540b2c50a","branchLabelMapping":{"^v8.9.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Threat Hunting","Team: SecuritySolution","Team:Threat Hunting:Investigations","v8.8.0","v8.9.0"],"number":156884,"url":"https://github.com/elastic/kibana/pull/156884","mergeCommit":{"message":"[Security Solution] fix from-to values investigate in timeline pulled from timestamp instead of @timestamp field (#156884)\n\n## Summary\r\n\r\nIn Kibana 8.8 we've done a huge refactor of the alert table (see\r\n[PR](https://github.com/elastic/kibana/pull/149128)).\r\nThe new table's trigger actions are no longer some of the Security\r\nSolution server side methods that were adding a `timestamp` field to the\r\nresponse (see\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)).\r\nThat `timestamp` field was basically a duplication of the `@timestamp`\r\nfield (done via [this helper\r\nfunction](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12))\r\n\r\nRunning the stack locally, you would see that clicking on the\r\n`Investigate in timeline` icon button or ANY alerts in the alerts table\r\n(pretty new or months/years old) would bring the timeline flyout with\r\nalways the `to` value being the `current date`, and the `from` value\r\nbeing the `current date - kibana.alert.rule.from`.\r\nThe `timetamp` field\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158)\r\ndoes not exists, so we always fall back to `new Date()` (unless you're\r\nlooking at an alert generated by a threshold rule, a new terms rule or a\r\nsuppressed alert).\r\n\r\nThis PR fixes the issue by retrieving the `@timestamp` field instead of\r\nthe unwanted `timestamp` field.\r\n\r\nMore work will be done in the future to actually entirely remove and\r\ncleanup the `timestamp` field (see [this WIP\r\nticket](https://github.com/elastic/kibana/issues/156879)).\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\nBefore (recorded on May 4th and we're looking at an alert generated on\r\nMay 3rd)\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov\r\n\r\nAfter\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov\r\n\r\nCloses https://github.com/elastic/kibana/issues/126077","sha":"114c98d3b5df4252f500ea762879524540b2c50a"}},"sourceBranch":"main","suggestedTargetBranches":["8.8"],"targetPullRequestStates":[{"branch":"8.8","label":"v8.8.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.9.0","labelRegex":"^v8.9.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/156884","number":156884,"mergeCommit":{"message":"[Security Solution] fix from-to values investigate in timeline pulled from timestamp instead of @timestamp field (#156884)\n\n## Summary\r\n\r\nIn Kibana 8.8 we've done a huge refactor of the alert table (see\r\n[PR](https://github.com/elastic/kibana/pull/149128)).\r\nThe new table's trigger actions are no longer some of the Security\r\nSolution server side methods that were adding a `timestamp` field to the\r\nresponse (see\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/format_timeline_data.ts#L28)).\r\nThat `timestamp` field was basically a duplication of the `@timestamp`\r\nfield (done via [this helper\r\nfunction](https://github.com/elastic/kibana/blob/main/x-pack/plugins/timelines/server/search_strategy/timeline/factory/helpers/get_timestamp.ts#L12))\r\n\r\nRunning the stack locally, you would see that clicking on the\r\n`Investigate in timeline` icon button or ANY alerts in the alerts table\r\n(pretty new or months/years old) would bring the timeline flyout with\r\nalways the `to` value being the `current date`, and the `from` value\r\nbeing the `current date - kibana.alert.rule.from`.\r\nThe `timetamp` field\r\n[here](https://github.com/elastic/kibana/blob/main/x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx#L158)\r\ndoes not exists, so we always fall back to `new Date()` (unless you're\r\nlooking at an alert generated by a threshold rule, a new terms rule or a\r\nsuppressed alert).\r\n\r\nThis PR fixes the issue by retrieving the `@timestamp` field instead of\r\nthe unwanted `timestamp` field.\r\n\r\nMore work will be done in the future to actually entirely remove and\r\ncleanup the `timestamp` field (see [this WIP\r\nticket](https://github.com/elastic/kibana/issues/156879)).\r\n\r\n### Checklist\r\n\r\n- [ ] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\nBefore (recorded on May 4th and we're looking at an alert generated on\r\nMay 3rd)\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509848-a5b0a363-c9c5-4d80-a139-84e3df3a1bd6.mov\r\n\r\nAfter\r\n\r\n\r\nhttps://user-images.githubusercontent.com/17276605/236509884-74805cef-ccf2-4b09-a174-2fcb6b75d4bb.mov\r\n\r\nCloses https://github.com/elastic/kibana/issues/126077","sha":"114c98d3b5df4252f500ea762879524540b2c50a"}}]}] BACKPORT--> Co-authored-by: Philippe Oberti <[email protected]>
|
Another related issue: #159276 I have no idea what oldAlertsData or ecsAlertsData even are, nor what the difference is or how long they need to be supported and what if any the migration plan is, when to use one vs the other, kinda goes beyond a (non-existent) tech debt ticket as mentioned here: #149128 (comment) . Probably should have released this behind a feature flag defaulted to true or something |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR replaces the existing
Alert Tableused in Security solution & cases with that oftriggers-actions-ui.Ideally, this PR does make any changes to the functionality of the product and from user perspective. Nothing should Change.
Things to test and changes
@elastic/actionable-observability
triggers-actions-uialert table.@elastic/response-ops
security-solutionneeded some parameters/values to achieve the parity in the functionality as compared to the current alert Table.