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.

Sign up for GitHub

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

Jump to bottom

[SecuritySolution] Replace risk scores donut charts with Lens #148755

Merged
merged 13 commits into from
Jan 17, 2023
Merged

[SecuritySolution] Replace risk scores donut charts with Lens #148755

merged 13 commits into from
Jan 17, 2023

Conversation

angorayc
Copy link
Contributor

@angorayc angorayc commented Jan 11, 2023 •
edited
Loading

Summary

Behind feature flag chartEmbeddablesEnabled

  • Replace host and user risk scores donut charts with Lens

Known issues

  1. legend is temporary disabled as Lens does not render the legend if its value is zero. - [Security Solution] Custom dashboard UI prerequisite #136409 - It doesn't show the legend item if its value is zero #149681
  2. No title in the donut by default when open in Lens. - [Lens] An option to display title / subtitle for charts #149675
  3. [SecuritySolution] Page crashes after editing the filter applied from a Lens Embeddable. #148710
  4. Colour palette does not match with the setting of Host / User risk score classification - as it uses one of the color palette settings in Lens. - [Lens] Keep the filters order in the partition chart legend #149034 [Lens] Support categorical coloring by name #101942
  5. https://github.com/elastic/security-team/issues/5817

Before
Screenshot 2023-01-27 at 18 16 06

After
Screenshot 2023-01-27 at 18 14 22

Checklist

Delete any items that are not applicable to this PR.

@angorayc angorayc changed the title replace donut charts for risk scores [SecuritySolution] Replace risk scores donut charts with Lens Jan 12, 2023
@angorayc angorayc mentioned this pull request Jan 12, 2023
Closed
41 tasks
@angorayc angorayc added Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore v8.7.0 backport:skip This commit does not require backporting labels Jan 12, 2023
@angorayc angorayc marked this pull request as ready for review January 13, 2023 11:57
@angorayc angorayc requested review from a team as code owners January 13, 2023 11:57
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@angorayc angorayc added the release_note:skip Skip the PR/issue when compiling release notes label Jan 13, 2023
@angorayc angorayc added the ci:cloud-deploy Create or update a Cloud deployment label Jan 13, 2023
machadoum
machadoum reviewed Jan 13, 2023
View reviewed changes
...ecurity_solution/public/common/components/visualization_actions/visualization_embeddable.tsx

useEffect(() => {
dispatch(
inputsActions.setQuery({
Copy link
Member

Choose a reason for hiding this comment

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

Question: Could we use a utility function like useInspectButton or useInspectQuery here?
Note: I have to delete one of them. 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I'm not using useInspectQuery is because I'd like to avoid useGlobalTime in this component. As it deletes all the queries in useGlobalTime when component unmount, I don't want to affect other components.

The reason I'm not using useInspectButton is because session.current.start() has to be wrapped in useEffect, so I think it makes more sense to leave the logic of session in this component.

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jan 16, 2023 •
edited
Loading

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 3478 3482 +4

Async chunks

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

id before after diff
securitySolution 12.7MB 12.7MB +8.4KB

History

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

machadoum
machadoum approved these changes Jan 17, 2023
View reviewed changes
Copy link
Member

@machadoum machadoum left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the great work! 👏

@angorayc angorayc merged commit 3ccd8fc into elastic:main Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@machadoum machadoum machadoum approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Explore Team:Threat Hunting Security Solution Threat Hunting Team v8.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@angorayc @elasticmachine @kibana-ci @machadoum @kibanamachine