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.

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][Alert details] - improving session view experience in expandable flyout #200270

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PhilippeOberti
Copy link
Contributor

@PhilippeOberti PhilippeOberti commented Nov 14, 2024

Summary

This PR started the move of the analyzer and session view components from the table to the flyout. Shortly after we added an advanced settings (via this PR) to allow users to switch back and forth between the old table view and the flyout view.

This current PR focuses on the session view component and enhances its user experience, when rendered in the expandable flyout.

No changes should be made for the user in the table as well as the other usages of the session view component (like for example the Kubernetes dashboard).

Old UI (in table)

Screen.Recording.2024-12-16.at.4.56.50.PM.mov

####. New UI (in flyout)

Screen.Recording.2024-12-16.at.4.55.37.PM.mov

As can seen in the video above, when the session view component is opened in the expandable flyout, we show the tree view and the detailed panel separated. This allow for better use of the horizontal space, especially visible on a wide monitor. This is also combined with the fact that the flyout is resizable (and can take the whole screen) and the preview panel is also resizable, to provide more space to the detailed panel.

Note: the session view full screen functionality is lost, but this is by design. As mentioned above, the user can resize the flyout's width to take the full screen, and the flyout's vertical space is already near full height.

Code decisions

To guarantee as much as possible that the usage of the Session View component in the table or in the other places (like the Kubernetes dashboard) were not impacted by this PR, only additive changes were made. All these changes are also protected behind if conditions, that should only be run when the correct props are being passed in.
Some components (like the content of each of the tabs of the detailed panels - Process, Metadata and Alerts) as well as a hook, are exposed outisde of the session_view plugin, to be reused in the expandable flyout directly.

Code changes were kept to a bare minimum in the session_view plugin!

What to test

  • functionality of the Session View component should be exactly the same when used in the table as when used in the flyout:
    • clicking on a row in the tree should update the detailed panel accordingly
    • jumping to a process from the detailed panel should correctly update the tree
    • viewing the details of an alert should work
    • the
  • the UI will be mostly the same, with some small tweaks:
    • viewing an alert details now opens a preview panel instead of the flyout. The user can go back to the previous panel by clicking on the Back button in the top-left corner
    • the alerts tab does not show the number of alerts as it previously was. We might be able to get this to work later, but after discussing with Product this is an acceptable solution as the feature is still behind an Advanced Settings
    • the Open details has been replaced by a expand icon button, to be more consistent with the rest of the UI in the flyout

Notes:

  • there is a small update in the analyzer graph to the icon used in the open detail button. We're now using the expand icon to be consistent with the Session View component (which already has another eye icon)

How to test

  • turn on the securitySolution:enableVisualizationsInFlyout Advanced Settings
    Screenshot 2024-12-16 at 5 05 05 PM
  • generate alerts with data for session view (yarn test:generate -n http://elastic:changeme@localhost:9200 -k http://elastic:changeme@localhost:5601)

@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-session-view branch 3 times, most recently from 81477ae to 4069ba9 Compare November 25, 2024 23:24
@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-session-view branch 4 times, most recently from b7ad4a4 to 171a9ac Compare December 6, 2024 22:38
@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-session-view branch 4 times, most recently from 5107c10 to 06f6971 Compare December 17, 2024 00:05
@PhilippeOberti PhilippeOberti marked this pull request as ready for review December 17, 2024 00:07
@PhilippeOberti PhilippeOberti requested review from a team as code owners December 17, 2024 00:07
@PhilippeOberti PhilippeOberti added release_note:skip Skip the PR/issue when compiling release notes v9.0.0 Team:Threat Hunting:Investigations Security Solution Investigations Team backport:version Backport to applied version labels v8.18.0 labels Dec 17, 2024
@elasticmachine
Copy link
Contributor

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

@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-session-view branch from 06f6971 to 5efd87f Compare December 17, 2024 05:36
scopeId,
jumpToEntityId,
jumpToCursor,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed there is no preview banner here (not saying the Preview analyzer details makes sense...) but we should align both to have the same UI, wdyt?

image
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

aaaah yup I missed that, on it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, actually I wonder if in this case it makes sense... We're not really previewing any data, we're just using the preview as a navigation...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so now I wonder if we should remove it from the analyzer graph instead... 🤔

@PhilippeOberti PhilippeOberti force-pushed the expandable-flyout-session-view branch from 5efd87f to 2d2b681 Compare December 17, 2024 22:15
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6434 6443 +9

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
sessionView 134 141 +7

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.8MB 14.8MB +15.9KB
sessionView 392.1KB 355.1KB -36.9KB
total -21.0KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
sessionView 8.4KB 48.7KB +40.3KB
Unknown metric groups

API count

id before after diff
sessionView 134 144 +10

History

@opauloh
Copy link
Contributor

opauloh commented Dec 19, 2024

I noticed there's an issue with the search when paginating the search results, where the highlighted search doesn't follow the pagination and it freezes the Session View details preview

Screen.Recording.2024-12-18.at.8.27.14.PM.mov

On the non-preview version, paginating on the search should open the details on the details panel:

Screen.Recording.2024-12-18.at.8.33.30.PM.mov

I will try to investigate the issue, but maybe could we consider moving search logic to the context?

@PhilippeOberti
Copy link
Contributor Author

I noticed there's an issue with the search when paginating the search results, where the highlighted search doesn't follow the pagination and it freezes the Session View details preview

Screen.Recording.2024-12-18.at.8.27.14.PM.mov
On the non-preview version, paginating on the search should open the details on the details panel:

Screen.Recording.2024-12-18.at.8.33.30.PM.mov
I will try to investigate the issue, but maybe could we consider moving search logic to the context?

Ha! Thanks for finding this I actually did not test the search functionality. I will be taking a look at it tomorrow!

@opauloh
Copy link
Contributor

opauloh commented Dec 19, 2024

@christineweng I guess that was introduced earlier, it seems it's trying to fetch .ds-logs-endpoint.events.process-default index when it should be querying from alerts indice

image

@PhilippeOberti
Copy link
Contributor Author

@christineweng I guess that was introduced earlier, it seems it's trying to fetch .ds-logs-endpoint.events.process-default index when it should be querying from alerts indice

interesting, I thought I had fixed that one, I will double check tomorrow to make sure I'm using the correct indices when showing the detailed panel for the session view component and the correct ones when showing the alert details panel!

@opauloh
Copy link
Contributor

opauloh commented Dec 19, 2024

Minor but when toggling the TTY view could we hide the Details panel button on the preview flyout mode?

image

@PhilippeOberti
Copy link
Contributor Author

Minor but when toggling the TTY view could we hide the Details panel button on the preview flyout mode?

should be possible yes (I think), I'll take a look tomorrow as well!

{DETAIL_PANEL}
</EuiButton>
{openDetailsInExpandableFlyout ? (
<EuiButtonIcon onClick={toggleDetailPanelInFlyout} iconType="expand" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we maybe remove the button on the preview flyout mode? since it can be closed by clicking on the close button on the flyout (what doesn't have on the details panel), and it can be expanded by clicking on the process.

I think the expand icon might lead users to think it's a fullscreen option, also it doesn't do anything when the preview is already expanded.

Screen.Recording.2024-12-18.at.9.25.08.PM.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can, but this was added to be consistent with the analyzer graph component, and was a request by the UIUX team. I'll check with Product and UIUX if they have a preference. I'll get back to you on it!

@opauloh
Copy link
Contributor

opauloh commented Dec 19, 2024

@PhilippeOberti other than the freezing issue commented here, feel free to address the other issues / nits as follow-up issues if required

Thanks for the detailed documentation and precise comments 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting:Investigations Security Solution Investigations Team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants