[SecuritySolution] Replace donuts with Lens #148519

angorayc · 2023-01-09T09:28:09Z

Summary

This pr is a part of #147261: Replace donut charts with Lens:
This is behind feature flag chartEmbeddablesEnabled

Items to verify:

If the queries and filters from the global search box are applied.
If the correct alert index is applied by different spaced.
Visualization actions working correctly.

Detection and Response dashboard:

Host details:

User details:

Network details:

Known issues:

Not showing legend for alerts donut charts at the moment: There is a logic in Lens that it doesn't show the legend item if its value is zero. - It doesn't show the legend item if its value is zero #149681
JS warnings triggered by incorrect state changed. Fixed by [Security Solution] Resolve JS warnings triggered by incorrect state changed #148552
No label in the donut by default when open in Lens - Lens doesn't support displaying a label in the middle of the donut chart by default, so it is currently available in Security Solution. - [Lens] An option to display title / subtitle for charts #149675
Applying filters or extra action while clicking the donut is not available at the moment. - [Lens][Embeddable] Provide a way to override default action on trigger #149027
Page crashes after editing the filter applied from a Lens Embeddable - [SecuritySolution] Page crashes after editing the filter applied from a Lens Embeddable. #148710

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2023-01-09T19:42:24Z

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

elasticmachine · 2023-01-09T19:42:25Z

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

jamster10 · 2023-01-10T05:18:17Z

...solution/public/overview/components/detection_response/alerts_by_status/alerts_by_status.tsx

+                          label={STATUS_OPEN}
+                          title={<ChartLabel count={openCount} />}
+                          totalCount={openCount}
+                          isChartEmbeddablesEnabled={isChartEmbeddablesEnabled}


if isChartEmbeddablesEnabled is true though, this component wouldnt render as As AlertDonutEmbeddable would :P. Same for the instances below it. I think you can remove the prop.

OK, I have this removed in this PR: https://github.com/elastic/kibana/pull/147261/files

jamster10

Hey @angorayc this looks great. I went through the code as well as tested the build and it all looks good.

Some questions:
It looks like youre trying to swap out all usage of DonutCharts in favour of lens embeddables, leaving the only usage of DonutChart on the Alerts page and Risk score. Is that because of the lack of legend?

Also, am i right in assuming the default behaviour for lens embeddable clicks is to add a filter. Is that something we can plug in to? Curious because it has ramifications for something I was working on- routing to the Alert page on click from the donut charts on the Explore pages.

Finally, it looks like the bug Steph fixed might impact [useRefetchByRestartingSession](https://github.com/elastic/kibana/blob/ee27615e644cf391b7b04ea44f1d3fc88bf6031c/x-pack/plugins/security_solution/public/explore/containers/use_refetch_by_session.tsx) I dont think in an adverse way though.

…fix'

angorayc · 2023-01-10T09:23:55Z

Hey @angorayc this looks great. I went through the code as well as tested the build and it all looks good.

Some questions: It looks like youre trying to swap out all usage of DonutCharts in favour of lens embeddables, leaving the only usage of DonutChart on the Alerts page and Risk score. Is that because of the lack of legend?

Because this PR has lots of files changed already, so I'd like to replace them in a follow up PR.

Also, am i right in assuming the default behaviour for lens embeddable clicks is to add a filter. Is that something we can plug in to? Curious because it has ramifications for something I was working on- routing to the Alert page on click from the donut charts on the Explore pages.

Yup, should be able to do that. I'll put it on the list.

Finally, it looks like the bug Steph fixed might impact [useRefetchByRestartingSession](https://github.com/elastic/kibana/blob/ee27615e644cf391b7b04ea44f1d3fc88bf6031c/x-pack/plugins/security_solution/public/explore/containers/use_refetch_by_session.tsx) I dont think in an adverse way though.

Thanks for the heads up, I'm looking to that with Steph.

kibana-ci · 2023-01-10T10:48:13Z

💚 Build Succeeded

Buildkite Build
Commit: 7d03079

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	3462	3466	+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	+39.1KB

History

💔 Build #98947 failed 7867014
💛 Build #98887 was flaky 7479091
💔 Build #98822 failed 1ce2b7d
💔 Build #98767 failed 3b999cc
💔 Build #98713 failed 8a84964

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

cc @angorayc

dasansol92

Only checked linter changes on x-pack/plugins/security_solution/public/management/cypress/tsconfig.json

dasansol92 · 2023-01-10T11:09:01Z

x-pack/plugins/security_solution/public/management/cypress/tsconfig.json

@@ -22,6 +22,8 @@
      "path": "../../../tsconfig.json",
      "force": true
    },
-    "@kbn/security-plugin"
+    "@kbn/security-plugin",


Approving linter changes from management folder

…changed (#148552) ## Summary Found these js warning after replacing charts with Lens in #148519: <img width="1671" alt="Screenshot 2023-01-09 at 15 31 17" src="https://user-images.githubusercontent.com/6295984/211345750-8c4e67ee-bf96-49d2-8bb2-0f71e5f9bcd2.png"> Wrap `search.session.start()` with useEffect to avoid incorrect state changed. ### Checklist Delete any items that are not applicable to this PR. - [x] [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 Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>

## Summary This pr is a part of elastic#147261: Replace donut charts with Lens: This is behind feature flag `chartEmbeddablesEnabled ` Items to verify: 1. If the queries and filters from the global search box are applied. 2. If the correct alert index is applied by different spaced. 3. Visualization actions working correctly. Detection and Response dashboard: <img width="1662" alt="Screenshot 2023-01-09 at 09 21 56" src="https://user-images.githubusercontent.com/6295984/211275765-8177e9bd-3623-4bb2-b1d9-8a3044d523a0.png"> Host details: <img width="1666" alt="Screenshot 2023-01-09 at 09 21 38" src="https://user-images.githubusercontent.com/6295984/211275770-be95353f-4d1b-410a-b7bf-b232692af1ab.png"> User details: <img width="1662" alt="Screenshot 2023-01-09 at 09 21 19" src="https://user-images.githubusercontent.com/6295984/211275773-dd0bcaaf-58e6-404b-b010-d1c464cbd101.png"> Network details: <img width="1663" alt="Screenshot 2023-01-09 at 09 20 47" src="https://user-images.githubusercontent.com/6295984/211275775-0fd39ac3-e977-44bd-bd40-304463dce613.png"> Known issues: 1. Not showing legend for alerts donut charts at the moment: There is a logic in Lens that it doesn't show the legend item if its value is zero. 4. JS warnings triggered by incorrect state changed. Fixed by elastic#148552 5. No label in the donut by default when open in Lens - Lens doesn't support displaying a label in the middle of the donut chart by default, so it is currently available in Security Solution. 6. Applying filters or extra action while clicking the donut is not available atm ### Checklist Delete any items that are not applicable to this PR. - [x] [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 Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>

…changed (elastic#148552) ## Summary Found these js warning after replacing charts with Lens in elastic#148519: <img width="1671" alt="Screenshot 2023-01-09 at 15 31 17" src="https://user-images.githubusercontent.com/6295984/211345750-8c4e67ee-bf96-49d2-8bb2-0f71e5f9bcd2.png"> Wrap `search.session.start()` with useEffect to avoid incorrect state changed. ### Checklist Delete any items that are not applicable to this PR. - [x] [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 Co-authored-by: Steph Milovic <stephanie.milovic@elastic.co>

replace donuts with Lens

6bde2f0

angorayc added release_note:skip Skip the PR/issue when compiling release notes 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 labels Jan 9, 2023

angorayc added 3 commits January 9, 2023 12:13

unit tests

99e07e3

fix import

8a84964

fix unit tests

3b999cc

angorayc mentioned this pull request Jan 9, 2023

[Security Solution] Resolve JS warnings triggered by incorrect state changed #148552

Merged

1 task

angorayc and others added 2 commits January 9, 2023 15:39

Merge branch 'main' of github.com:elastic/kibana into lens-donut

1ce2b7d

Merge branch 'main' into lens-donut

7479091

angorayc marked this pull request as ready for review January 9, 2023 19:42

angorayc requested review from a team as code owners January 9, 2023 19:42

Merge branch 'main' into lens-donut

7867014

jamster10 assigned angorayc Jan 9, 2023

jamster10 reviewed Jan 10, 2023

View reviewed changes

jamster10 approved these changes Jan 10, 2023

View reviewed changes

angorayc and others added 2 commits January 10, 2023 09:04

Merge branch 'main' into lens-donut

82ddf7e

[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…

7d03079

…fix'

kibanamachine requested a review from a team as a code owner January 10, 2023 09:10

angorayc mentioned this pull request Jan 10, 2023

[Security Solution] Custom dashboard UI prerequisite #136409

Closed

41 tasks

dasansol92 approved these changes Jan 10, 2023

View reviewed changes

angorayc merged commit 09e10b8 into elastic:main Jan 10, 2023

kibanamachine added the backport:skip This commit does not require backporting label Jan 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SecuritySolution] Replace donuts with Lens #148519

[SecuritySolution] Replace donuts with Lens #148519

angorayc commented Jan 9, 2023 •

edited

Loading

elasticmachine commented Jan 9, 2023

elasticmachine commented Jan 9, 2023

jamster10 Jan 10, 2023

angorayc Jan 10, 2023

jamster10 left a comment

angorayc commented Jan 10, 2023

kibana-ci commented Jan 10, 2023

dasansol92 left a comment

dasansol92 Jan 10, 2023

[SecuritySolution] Replace donuts with Lens #148519

[SecuritySolution] Replace donuts with Lens #148519

Conversation

angorayc commented Jan 9, 2023 • edited Loading

Summary

Checklist

elasticmachine commented Jan 9, 2023

elasticmachine commented Jan 9, 2023

jamster10 Jan 10, 2023

Choose a reason for hiding this comment

angorayc Jan 10, 2023

Choose a reason for hiding this comment

jamster10 left a comment

Choose a reason for hiding this comment

angorayc commented Jan 10, 2023

kibana-ci commented Jan 10, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

dasansol92 left a comment

Choose a reason for hiding this comment

dasansol92 Jan 10, 2023

Choose a reason for hiding this comment

angorayc commented Jan 9, 2023 •

edited

Loading