[Investigations] - Add unified components to eql tab #180972

michaelolo24 · 2024-04-16T18:52:24Z

Summary

This PR introduces the unified table and field list to the correlations tab as seen in the snapshots below.

As of this PR, items that are not working

Table row height controls
Expandable flyout integration
Leading cell actions (pinning, notes, row actions, analyzer, session view)

Changes in this PR:

Sequence Highlighting:

Building block highlighting:

To test:

Add xpack.securitySolution.enableExperimental: [unifiedComponentsInTimelineEnabled] to your kibana.dev.yml
Generate test data (endpoint data is fine)
Go to the correlations tab and enter this query to see default events/alerts that should have no highlighting
any where true
Enter this query to see a generic sequence

sequence
[any where true]
[any where true]

You can also do something like

sequence
[any where host.name=={HOST NAME VALUE HERE}]
[any where true]

You can also create a correlation rule using any of the above queries to generate building block alerts, and then query those alerts in the correlations tab as well

michaelolo24 · 2024-04-16T18:53:24Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx

@@ -106,15 +131,26 @@ export const EqlTabContentComponent: React.FC<Props> = ({
    !isEmpty(end) &&
    !isBlankTimeline;

+  const defaultColumns = useMemo(


A lot of this logic is going to end up being shared between all the tabs so should probably be abstracted...but may not be necessary when the old table is removed

true, is there a value on lifting this ( till line 145 ) up in Tabs component as mentioned above?

Yea, I want to do that for almost all of the tabs. The only real difference at this point between them is the query being run..

Did a lot of deduping and cleaning up here: e37faca - Let me know if there's anything else you see

This looks great. Will check it.

elasticmachine · 2024-04-17T17:16:05Z

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

janmonschke · 2024-04-17T17:58:45Z

@michaelolo24 I noticed that removing a field from the table feels slow:

Screen.Recording.2024-04-17.at.19.41.06.mov

In comparison, this is how it feels like in Discover:

Screen.Recording.2024-04-17.at.19.56.56.mov

michaelolo24 · 2024-04-17T18:00:16Z

@michaelolo24 I noticed that removing a field from the table feels slow:

Screen.Recording.2024-04-17.at.19.41.06.mov
In comparison, this is how it feels like in Discover:

Screen.Recording.2024-04-17.at.19.56.56.mov

Thanks! Yea, I noticed that as well. I think it has something to do with the row rendering, but maybe I'm just making that up. I noticed the same with the query tab with the row renderers, and I believe @logeekal is taking a look. Once this is merged, I'll be circling back to some of the performance, clean up items

michaelolo24 · 2024-04-17T18:00:21Z

/ci

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx

...elines/components/timeline/unified_components/data_table/use_get_event_type_row_classname.ts

PhilippeOberti · 2024-04-17T20:07:29Z

When clicking on the expand button on a row, the flyout isn't opening (works fine without the unifiedComponentsInTimelineEnabled feature flag). At first it seemed related to this issue but in this PR the flyout just plain does not open, and in the issue it opens but behind timeline...

Screen.Recording.2024-04-17.at.3.04.29.PM.mov

PhilippeOberti · 2024-04-17T20:12:49Z

I found a behavior that's most likely rarely going to happen but still annoying. I'm not sure we really have control over this though...:

click on a field in the left sidebar
click Visualize
click cancel

Expected behavior: we would stay in the same spot we were on

Actual behavior: the view goes back to the Query tab

Screen.Recording.2024-04-17.at.3.09.56.PM.mov

PhilippeOberti · 2024-04-17T20:17:35Z

Changing the value in the custom Cell row height doesn't seem to be doing anything

Screen.Recording.2024-04-17.at.3.15.53.PM.mov

Could be related, but @logeekal it seems that the behavior is similar in the Query tab now. Wasn't this working a few days/weeks ago? Could the addition to the row renderer have broken this?

PhilippeOberti · 2024-04-17T20:46:58Z

Is the sorting supposed to work? I'm clicking in a bunch of places and nothing seems to be happening...

Screen.Recording.2024-04-17.at.3.45.51.PM.mov

logeekal · 2024-04-18T07:36:17Z

I noticed that removing a field from the table feels slow:

@janmonschke : As @michaelolo24 also pointed out, it is because of combination of customBodyRender for rowRenders, cellActions , plus some extra updates to redux on our side. I am trying to see how best we can reduce this time.

Will soon raise a PR.

logeekal

Thanks for making these changes. Seeing your PR, i remebered, I forgot about building block styling. thanks for getting these done.

Additionally, I see that flyout is not opening in Correlation tab only.

eql_code_review.mp4

logeekal · 2024-04-18T08:42:15Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx

+import { UnifiedTimelineBody } from '../../body/unified_timeline_body';
+import { getColumnHeaders } from '../../body/column_headers/helpers';
+
+const memoizedGetColumnHeaders: (


Since, all tabs are rendered together. Do you think it makes sense to lift this up in tabs component?

All tabs such as query, EQL anyway take columns as one of the prop.

logeekal · 2024-04-18T08:43:29Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx

@@ -106,15 +131,26 @@ export const EqlTabContentComponent: React.FC<Props> = ({
    !isEmpty(end) &&
    !isBlankTimeline;

+  const defaultColumns = useMemo(


true, is there a value on lifting this ( till line 145 ) up in Tabs component as mentioned above?

...melines/components/timeline/unified_components/data_table/custom_timeline_data_grid_body.tsx

michaelolo24 · 2024-04-18T15:31:17Z

@PhilippeOberti

When clicking on the expand button on a row, the flyout isn't opening (works fine without the unifiedComponentsInTimelineEnabled feature flag). At first it seemed related to this issue but in this PR the flyout just plain does not open, and in the issue it opens but behind timeline...

The query tab property was hardcoded, updated it here

Is the sorting supposed to work? I'm clicking in a bunch of places and nothing seems to be happening...

No, sorting is not supposed to work for the EQL tab as it would mess up the sequence listing for an EQL query. I forgot to check whether the NO_SORT would disable the actual buttons themselves, but it didn't, needed to plumb this through:

For:

I found a behavior that's most likely rarely going to happen but still annoying. I'm not sure we really have control over this though...:

Changing the value in the custom Cell row height doesn't seem to be doing anything

I added them to our clean up tasks here

michaelolo24 · 2024-04-18T15:35:12Z

Thanks for making these changes. Seeing your PR, i remebered, I forgot about building block styling. thanks for getting these done.

Additionally, I see that flyout is not opening in Correlation tab only.

@logeekal see my response to @PhilippeOberti about the same 😄

...k/plugins/security_solution/public/timelines/components/timeline/tabs/query/header/index.tsx

michaelolo24 · 2024-04-18T15:45:26Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/query/index.tsx

-        header={header}
-        columns={augumentedColumnHeaders}
+        header={
+          <QueryTabHeader


Couldn't decide whether I wanted this behind a memoized const and just pass those two values instead of duplicating them. I can go either way...

+1 for having it in useMemo. Lot of things going on in timeline and that might recreate header everytime.

Which values are you talking about?

PhilippeOberti · 2024-04-18T18:07:00Z

@michaelolo24 only item that I see missing right now is the fact that despite the fact that have the expandableTimelineFlyoutEnabled feature flag in my kibana.yml file, I do not see the expandable flyout open when clicking on a row. The old flyout shows up...

PhilippeOberti

left a few small comments about clean up and adding some unit tests maybe? Biggest thing left I think is my comment related to the expandable flyout not showing up even with the expandableTimelineFlyoutEnabled enabled

...k/plugins/security_solution/public/timelines/components/timeline/tabs/query/header/index.tsx

...s/security_solution/public/timelines/components/timeline/tabs/shared/use_timeline_columns.ts

PhilippeOberti · 2024-04-18T18:20:19Z

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/header.tsx

wouldn't we want to unit test this new component?

yep, adding! 👍🏾

michaelolo24 · 2024-04-18T19:16:37Z

@michaelolo24 only item that I see missing right now is the fact that despite the fact that have the expandableTimelineFlyoutEnabled feature flag in my kibana.yml file, I do not see the expandable flyout open when clicking on a row. The old flyout shows up...

Yea, the logic for that wasn't here anymore, I added it in this PR: 8fb5199

Going to work on adding tests, but just wanted to get this there so you can take a look!

PhilippeOberti · 2024-04-18T19:54:27Z

@michaelolo24 only item that I see missing right now is the fact that despite the fact that have the expandableTimelineFlyoutEnabled feature flag in my kibana.yml file, I do not see the expandable flyout open when clicking on a row. The old flyout shows up...

Yea, the logic for that wasn't here anymore, I added it in this PR: 8fb5199

Going to work on adding tests, but just wanted to get this their so you can take a look!

verified! Thanks for adding this. Soon we'll have the onClose logic working

logeekal · 2024-04-23T05:26:12Z

Changing the value in the custom Cell row height doesn't seem to be doing anything

@PhilippeOberti , this behaviour seems to be related to renderCustomBody. I will take this one up.

...s/security_solution/public/timelines/components/timeline/tabs/shared/use_timeline_columns.ts

logeekal · 2024-04-23T06:28:20Z

only item that I see missing right now is the fact that despite the fact that have the expandableTimelineFlyoutEnabled feature flag in my kibana.yml file, I do not see the expandable flyout open when clicking on a row. The old flyout shows up...

Yea, the logic for that wasn't here anymore, I added it in this PR: 8fb5199

Thank @michaelolo24 for adding changes for new expandable flyout.

Actually after my discussion with @PhilippeOberti , I did the related changes but I kept it aside as a patch as my original PR was already big.

Now that you have already added the code for expandable flyout, I am attaching my patch which also includes handling flyout closure ( new hook : x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.tsx ) . You can either choose to include it or we can also do that it follow up PR because scope of your PR is increasing :D. I guess new expandable flyout should have been separated PR anyways.

It resolves the problem when user closed the flyout, the expand-event icon does not change back.

Here is the Patch for Expandable flyout

Note: I am happy to add it as well in case you are busy.

michaelolo24 · 2024-04-24T13:18:20Z

only item that I see missing right now is the fact that despite the fact that have the expandableTimelineFlyoutEnabled feature flag in my kibana.yml file, I do not see the expandable flyout open when clicking on a row. The old flyout shows up...

Yea, the logic for that wasn't here anymore, I added it in this PR: 8fb5199

Thank @michaelolo24 for adding changes for new expandable flyout.

Actually after my discussion with @PhilippeOberti , I did the related changes but I kept it aside as a patch as my original PR was already big.

Now that you have already added the code for expandable flyout, I am attaching my patch which also includes handling flyout closure ( new hook : x-pack/plugins/security_solution/public/timelines/components/timeline/unified_components/hooks/use_unified_timeline_expandable_flyout.tsx ) . You can either choose to include it or we can also do that it follow up PR because scope of your PR is increasing :D. I guess new expandable flyout should have been separated PR anyways.

It resolves the problem when user closed the flyout, the expand-event icon does not change back.

Here is the Patch for Expandable flyout

Note: I am happy to add it as well in case you are busy.

@logeekal and @PhilippeOberti let's make the introduction of the new expandable flyout it's own PR, potentially after Kevin merges his PR for the row actions. I think we should use the default detail expand action the unified data table provides over the one that is in our leadingControlColumns, but that decision will affect this implementation. I'm going to revert the flyout changes in my PR that I mentioned here. @logeekal would you mind opening a PR with your patch for testing purposes? We can take a look at @PhilippeOberti 's changes there as well once it's merged 😄

logeekal · 2024-04-24T13:58:26Z

I think we should use the default detail expand action the unified data table provides over the one that is in our leadingControlColumns,

Agreed. 💯

would you mind opening a PR with your patch for testing purposes?

Will do.

…-fix'

kibana-ci · 2024-04-24T15:36:13Z

💚 Build Succeeded

Buildkite Build
Commit: 5e306c7

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	5451	5454	+3

Async chunks

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

id	before	after	diff
`securitySolution`	14.6MB	14.7MB	+67.6KB

History

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

logeekal

Desk Testing went great 🚀 . Only one small issue as mentioned below. Looking through code now.

logeekal · 2024-04-25T12:11:09Z

...plugins/security_solution/public/timelines/components/timeline/unified_components/styles.tsx

+  .udtTimeline .euiDataGridRow:has(.nonRawEvent) {
+    .euiDataGridRowCell--firstColumn,
+    .euiDataGridRowCell--lastColumn {
+      ${({ theme }) => `border-left: 4px solid ${theme.eui.euiColorWarning};`}


I think because of this both building block and non-buildingblock events are getting highlighted by a left yellow border in both query and correlation tabs. Is this expected?

Nope, just alerts, not regular events. Regular events have the gray border (that's also how it currently is if you check on main)

logeekal

Code looks clean ✨ ✨ . Approving but I have small comments/questions below.

logeekal · 2024-04-25T12:39:35Z

...s/security_solution/public/timelines/components/timeline/tabs/shared/use_timeline_columns.ts

+      defaultColumns,
+      getTimelineQueryFieldsFromColumns,
+      leadingControlColumns,
+      localColumns,


I am curious what is local columns here?

Columns that are passed in to the hook, which are in turn passed to the consuming components. If I remember correctly, it's primarily to allow for external customization of the columns

logeekal · 2024-04-25T12:52:50Z

...curity_solution/public/timelines/components/timeline/unified_components/data_table/index.tsx

@@ -149,27 +151,27 @@ export const TimelineDataTableComponent: React.FC<DataTableProps> = memo(
        dispatch(
          timelineActions.toggleDetailPanel({
            ...updatedExpandedDetail,
-            tabType: TimelineTabs.query,
+            tabType: activeTab,


Sorry for this screw up 😅 🙏

Lol, all good. 😄

logeekal · 2024-04-25T12:54:50Z

...elines/components/timeline/unified_components/data_table/use_get_event_type_row_classname.ts

+        ? isEvenEqlSequence(ecsData)
+          ? 'eqlSequence'
+          : 'eqlNonSequence'
+        : 'nonRawEvent',


For my knowledge, what is diff between nonRawEvent and rawEvent.

rawEvent is a generic event, whereas nonRawEvent is a catch all for a modified event, which the only one I know of right now is an alert/signal

PhilippeOberti · 2024-04-25T15:50:38Z

@PhilippeOberti

When clicking on the expand button on a row, the flyout isn't opening (works fine without the unifiedComponentsInTimelineEnabled feature flag). At first it seemed related to this issue but in this PR the flyout just plain does not open, and in the issue it opens but behind timeline...

The query tab property was hardcoded, updated it here

@michaelolo24 This is still broken for me. If you don't want to fix it in this PR let's add it to the list in your list of cleanup tasks (link to my original comment for reference)

PhilippeOberti

LGTM! It seems that we're tracking everything that still need to be fixed and we'll do follow up PRs.

michaelolo24 commented Apr 16, 2024

View reviewed changes

michaelolo24 assigned michaelolo24 and unassigned michaelolo24 Apr 16, 2024

michaelolo24 added the Team:Threat Hunting:Investigations Security Solution Investigations Team label Apr 16, 2024

michaelolo24 added this to the 8.15 milestone Apr 16, 2024

michaelolo24 added the release_note:skip Skip the PR/issue when compiling release notes label Apr 16, 2024

michaelolo24 force-pushed the add-unified-components-to-eql-tab branch from 688ec93 to 5f4cf36 Compare April 17, 2024 17:03

michaelolo24 added the v8.15.0 label Apr 17, 2024

michaelolo24 marked this pull request as ready for review April 17, 2024 17:16

michaelolo24 requested a review from a team as a code owner April 17, 2024 17:16

janmonschke reviewed Apr 17, 2024

View reviewed changes

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx Outdated Show resolved Hide resolved

janmonschke reviewed Apr 17, 2024

View reviewed changes

x-pack/plugins/security_solution/public/timelines/components/timeline/tabs/eql/index.tsx Outdated Show resolved Hide resolved

janmonschke reviewed Apr 17, 2024

View reviewed changes

...elines/components/timeline/unified_components/data_table/use_get_event_type_row_classname.ts Outdated Show resolved Hide resolved

logeekal requested changes Apr 18, 2024

View reviewed changes

michaelolo24 force-pushed the add-unified-components-to-eql-tab branch 2 times, most recently from 3dadc05 to e37faca Compare April 18, 2024 15:22

michaelolo24 added the Feature:Timeline Security Solution Timeline feature label Apr 18, 2024

michaelolo24 commented Apr 18, 2024

View reviewed changes

...k/plugins/security_solution/public/timelines/components/timeline/tabs/query/header/index.tsx Show resolved Hide resolved

michaelolo24 commented Apr 18, 2024

View reviewed changes

PhilippeOberti reviewed Apr 18, 2024

View reviewed changes

michaelolo24 force-pushed the add-unified-components-to-eql-tab branch from 0aa5bb5 to 8fb5199 Compare April 18, 2024 19:14

logeekal reviewed Apr 23, 2024

View reviewed changes

...s/security_solution/public/timelines/components/timeline/tabs/shared/use_timeline_columns.ts Show resolved Hide resolved

michaelolo24 and others added 10 commits April 24, 2024 10:14

enable unified components in correlations tab

b729225

add tests and rename hook

a578759

styling pr feedback

b6c482b

pr feedback: dedupe code, disable sort buttons, fix flyout

ecbc446

remove unused imports

e2380dc

fix file name

18cbe95

get expandable flyout to show up

cef3720

remove console log

fdae624

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

afb05f3

…-fix'

tests and revert flyout

5e306c7

michaelolo24 force-pushed the add-unified-components-to-eql-tab branch from aded1b5 to 5e306c7 Compare April 24, 2024 14:15

logeekal reviewed Apr 25, 2024

View reviewed changes

logeekal approved these changes Apr 25, 2024

View reviewed changes

PhilippeOberti approved these changes Apr 25, 2024

View reviewed changes

michaelolo24 merged commit d13d89e into elastic:main Apr 25, 2024
35 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Apr 25, 2024

[Investigations] - Add unified components to eql tab #180972

[Investigations] - Add unified components to eql tab #180972

Conversation

michaelolo24 commented Apr 16, 2024 • edited

Summary

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 Apr 18, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Apr 17, 2024

janmonschke commented Apr 17, 2024

michaelolo24 commented Apr 17, 2024 • edited

michaelolo24 commented Apr 17, 2024

PhilippeOberti commented Apr 17, 2024

PhilippeOberti commented Apr 17, 2024

PhilippeOberti commented Apr 17, 2024 • edited

PhilippeOberti commented Apr 17, 2024

logeekal commented Apr 18, 2024

logeekal left a comment • edited

Choose a reason for hiding this comment

logeekal Apr 18, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 commented Apr 18, 2024 • edited by PhilippeOberti

michaelolo24 commented Apr 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhilippeOberti commented Apr 18, 2024 • edited

PhilippeOberti left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelolo24 commented Apr 18, 2024 • edited

PhilippeOberti commented Apr 18, 2024

logeekal commented Apr 23, 2024

logeekal commented Apr 23, 2024 • edited

michaelolo24 commented Apr 24, 2024

logeekal commented Apr 24, 2024

kibana-ci commented Apr 24, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

logeekal left a comment

Choose a reason for hiding this comment

logeekal Apr 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

logeekal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhilippeOberti commented Apr 25, 2024

PhilippeOberti left a comment

Choose a reason for hiding this comment

michaelolo24 commented Apr 16, 2024 •

edited

michaelolo24 Apr 18, 2024 •

edited

michaelolo24 commented Apr 17, 2024 •

edited

PhilippeOberti commented Apr 17, 2024 •

edited

logeekal left a comment •

edited

logeekal Apr 18, 2024 •

edited

michaelolo24 commented Apr 18, 2024 •

edited by PhilippeOberti

PhilippeOberti commented Apr 18, 2024 •

edited

michaelolo24 commented Apr 18, 2024 •

edited

logeekal commented Apr 23, 2024 •

edited

logeekal Apr 25, 2024 •

edited