[Security Solution] Alert page charts new layout and chart collapse #150242
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
christineweng
added
backport:skip
christineweng
changed the title
|
Pinging @elastic/security-threat-hunting (Team:Threat Hunting) |
|
Pinging @elastic/security-solution (Team: SecuritySolution) |
|
Testing this PR by cloning and it is working very well. However, I have some comments regarding UI.
Screen.Recording.2023-02-06.at.11.33.53.mov
|
logeekal
reviewed
Feb 6, 2023
...security_solution/public/detections/components/alerts_kpis/alerts_count_panel/index.test.tsx
Show resolved
Hide resolved
.../security_solution/public/detections/components/alerts_kpis/alerts_by_type_panel/helpers.tsx
Outdated
Show resolved
Hide resolved
.../security_solution/public/detections/components/alerts_kpis/alerts_histogram_panel/index.tsx
Show resolved
Hide resolved
...rity_solution/public/detections/components/alerts_kpis/alerts_progress_bar_panel/helpers.tsx
Show resolved
Hide resolved
...rity_solution/public/detections/components/alerts_kpis/alerts_summary_charts_panel/index.tsx
Show resolved
Hide resolved
.../security_solution/public/detections/components/alerts_kpis/severity_level_panel/helpers.tsx
Outdated
Show resolved
Hide resolved
x-pack/plugins/security_solution/public/detections/pages/detection_engine/detection_engine.tsx
Show resolved
Hide resolved
christineweng
force-pushed
the
8.7_crosstab
branch
from
228135a
to
4626aeb
Compare
|
@logeekal ui fixes
Screen.Recording.2023-02-06.at.4.18.22.PM.mov
|
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
History
To update your PR or re-run it, just comment with: |
benakansara
pushed a commit
to benakansara/kibana
that referenced
this pull request
Feb 7, 2023
…lastic#150242) ## Summary This PR is part 3 of elastic#149173 and elastic#146938 that add additional KPI visualizations to the Alerts page. #### Capabilities added Charts menu: changed from a drop down selection to tabs format, with wording that better describe the usage of each charts Chart collapse: when the toggle is collapsed, instead of showing the same menu options, a summary of the KPIs are shown. Feature flag: `alertsPageChartsEnabled` is set to true by default #### Changes from previous PR Before this PR, each chart (trend, tree map etc.) keeps its own state of toggle status. This is no longer suitable because the new layout does not show options when collapsed. This PR also moves the toggle status to be at the chart panel's level, and be passed down to each chart component. One exception is the histogram (trend analysis), it is currently being used in alerts detail page and overview dashboard, hence it needs to keep track of toggle state on its own. #### When charts are expanded  #### When collapsed and has data  #### When collapsed with no data  ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [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 - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers)
christineweng
added a commit
that referenced
this pull request
Feb 8, 2023
…150504) This PR addresses the following: #### Bug fix #150278 described a discrepancy between total alert count in alert by type chart and everywhere else on alerts page. This is due to `event.type` being a multi-select, if an alert has 3 event types (i.e. creation, info, denied), it is counted 3 times on alert by type graph. This logic is now updated to categorize an alert once - if `denied` event type exists, such event count => `Prevention` - total alert count - prevention count => `Detection`. #### UI improvements - Top alerts chart no longer shows `Other` when number of grouping is less than 10 per #150242 (comment)  - Changed `EmptyDonutChart`'s background based on dark/light mode Before -> After  - Loading spinner for donut chart was not showing, it is now fixed  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Feb 8, 2023
…lastic#150504) This PR addresses the following: #### Bug fix elastic#150278 described a discrepancy between total alert count in alert by type chart and everywhere else on alerts page. This is due to `event.type` being a multi-select, if an alert has 3 event types (i.e. creation, info, denied), it is counted 3 times on alert by type graph. This logic is now updated to categorize an alert once - if `denied` event type exists, such event count => `Prevention` - total alert count - prevention count => `Detection`. #### UI improvements - Top alerts chart no longer shows `Other` when number of grouping is less than 10 per elastic#150242 (comment)  - Changed `EmptyDonutChart`'s background based on dark/light mode Before -> After  - Loading spinner for donut chart was not showing, it is now fixed  --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit 2846b8c)
kibanamachine
added a commit
that referenced
this pull request
Feb 9, 2023
…ments (#150504) (#150649) # Backport This will backport the following commits from `main` to `8.7`: - [[Security Solution][Bug] Alerts type discrepancy and ui improvements (#150504)](#150504) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"christineweng","email":"18648970+christineweng@users.noreply.github.com"},"sourceCommit":{"committedDate":"2023-02-08T22:40:49Z","message":"[Security Solution][Bug] Alerts type discrepancy and ui improvements (#150504)\n\nThis PR addresses the following:\r\n\r\n#### Bug fix\r\nhttps://github.com//issues/150278 described a discrepancy\r\nbetween total alert count in alert by type chart and everywhere else on\r\nalerts page. This is due to `event.type` being a multi-select, if an\r\nalert has 3 event types (i.e. creation, info, denied), it is counted 3\r\ntimes on alert by type graph. This logic is now updated to categorize an\r\nalert once\r\n- if `denied` event type exists, such event count => `Prevention`\r\n- total alert count - prevention count => `Detection`.\r\n\r\n#### UI improvements\r\n- Top alerts chart no longer shows `Other` when number of grouping is\r\nless than 10 per\r\nhttps://github.com//pull/150242#issuecomment-1419628829\r\n\r\n\r\n- Changed `EmptyDonutChart`'s background based on dark/light mode \r\nBefore -> After\r\n\r\n\r\n- Loading spinner for donut chart was not showing, it is now fixed\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"2846b8c27cf7da5a9e5c8152177376fdb8d2cffe","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Threat Hunting","Team: SecuritySolution","Team:Threat Hunting:Investigations","v8.7.0","v8.8.0"],"number":150504,"url":"#150504 Solution][Bug] Alerts type discrepancy and ui improvements (#150504)\n\nThis PR addresses the following:\r\n\r\n#### Bug fix\r\nhttps://github.com//issues/150278 described a discrepancy\r\nbetween total alert count in alert by type chart and everywhere else on\r\nalerts page. This is due to `event.type` being a multi-select, if an\r\nalert has 3 event types (i.e. creation, info, denied), it is counted 3\r\ntimes on alert by type graph. This logic is now updated to categorize an\r\nalert once\r\n- if `denied` event type exists, such event count => `Prevention`\r\n- total alert count - prevention count => `Detection`.\r\n\r\n#### UI improvements\r\n- Top alerts chart no longer shows `Other` when number of grouping is\r\nless than 10 per\r\nhttps://github.com//pull/150242#issuecomment-1419628829\r\n\r\n\r\n- Changed `EmptyDonutChart`'s background based on dark/light mode \r\nBefore -> After\r\n\r\n\r\n- Loading spinner for donut chart was not showing, it is now fixed\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"2846b8c27cf7da5a9e5c8152177376fdb8d2cffe"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"#150504 Solution][Bug] Alerts type discrepancy and ui improvements (#150504)\n\nThis PR addresses the following:\r\n\r\n#### Bug fix\r\nhttps://github.com//issues/150278 described a discrepancy\r\nbetween total alert count in alert by type chart and everywhere else on\r\nalerts page. This is due to `event.type` being a multi-select, if an\r\nalert has 3 event types (i.e. creation, info, denied), it is counted 3\r\ntimes on alert by type graph. This logic is now updated to categorize an\r\nalert once\r\n- if `denied` event type exists, such event count => `Prevention`\r\n- total alert count - prevention count => `Detection`.\r\n\r\n#### UI improvements\r\n- Top alerts chart no longer shows `Other` when number of grouping is\r\nless than 10 per\r\nhttps://github.com//pull/150242#issuecomment-1419628829\r\n\r\n\r\n- Changed `EmptyDonutChart`'s background based on dark/light mode \r\nBefore -> After\r\n\r\n\r\n- Loading spinner for donut chart was not showing, it is now fixed\r\n\r\n\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>","sha":"2846b8c27cf7da5a9e5c8152177376fdb8d2cffe"}}]}] BACKPORT--> Co-authored-by: christineweng <18648970+christineweng@users.noreply.github.com>
\r\n-){kind=link}
\r\n-){kind=link}
\r\n\r\n---------\r\n\r\nCo-authored-by){kind=link}
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 is part 3 of #149173 and #146938 that add additional KPI visualizations to the Alerts page.
Capabilities added
Charts menu: changed from a drop down selection to tabs format, with wording that better describe the usage of each charts
Chart collapse: when the toggle is collapsed, instead of showing the same menu options, a summary of the KPIs are shown.
Feature flag:
alertsPageChartsEnabledis set to true by defaultChanges from previous PR
Before this PR, each chart (trend, tree map etc.) keeps its own state of toggle status. This is no longer suitable because the new layout does not show options when collapsed. This PR also moves the toggle status to be at the chart panel's level, and be passed down to each chart component.
One exception is the histogram (trend analysis), it is currently being used in alerts detail page and overview dashboard, hence it needs to keep track of toggle state on its own.
When charts are expanded
When collapsed and has data
When collapsed with no data
Checklist
Delete any items that are not applicable to this PR.