[Cloud Security] Alerts Preview Contextual Flyout #197102

animehart · 2024-10-21T16:44:32Z

Summary

This PR is for Alerts preview component in Contextual Flyout

animehart · 2024-11-06T19:53:22Z

/ci

elasticmachine · 2024-11-08T10:57:09Z

Pinging @elastic/kibana-cloud-security-posture (Team:Cloud Security)

x-pack/packages/kbn-cloud-security-posture/common/utils/helpers.ts

...ns/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.test.tsx

maxcold · 2024-11-08T13:29:09Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

@@ -0,0 +1,125 @@
+/*


I think we shouldn't be the owners of this component, it has nothing to do with Cloud Security Posture. I understand that this is the outcome of this unclear ownership, but I would try to find a more suitable place for the alerts component. @PhilippeOberti wdyt about the code ownership? right now it's a bit like "who wrote the code owns it", but I don't think it is a good idea longer term

yeah it's a bit tricky... While I agree that the logic isn't related to Cloud Security, if the component is only used within another component owned by Cloud Security it's difficult for other teams to know how to properly test it (where it is, how to generate data to have things show correctly...).
A few times I've had situation where I didn't even know that one of our components was used by another team and didn't test things when I made changes to it...

Also, if that component is only used in one place, it's hard to justify having us own it. What if we need to make changes to it? We would still need a way to have you guys review it to make sure we don't brake your functionality?

on the other side, we are the ones that are supposed to know how this thing works, so I totally get your point of us owning it. I just feel like if we want to do this, we then need to have a good look at it before it's being merged to make sure we understand what it does, we should have a say on how it's being written and place it in a folder that makes sense

yeah, let's discuss it in the sync as proposed. Right now we have the situation that we've built misconfig, vuln and alert previews on the entity flyout and you folks built the same but on document flyout. For me the ideal situation would be to split the ownership by the business domain: we own everything related to cloud security (misconfiguration and vulnerability) and alerts should be owned by the team owning the business domain of alerts. Which team is the owner of the alerts logic in the Security Org is unclear to me tbh. But if we continue to own based on who wrote the code, we have the risk of things being broken (no necessarily technically but rather in business sense) without the owners of the domain logic realising that

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

x-pack/plugins/security_solution/public/cloud_security_posture/components/entity_insight.tsx

maxcold

most of the logic lgtm, some additional comments to make the code cleaner and more error prone

...ns/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.test.tsx

maxcold · 2024-11-12T12:28:48Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

@@ -0,0 +1,125 @@
+/*


yeah, let's discuss it in the sync as proposed. Right now we have the situation that we've built misconfig, vuln and alert previews on the entity flyout and you folks built the same but on document flyout. For me the ideal situation would be to split the ownership by the business domain: we own everything related to cloud security (misconfiguration and vulnerability) and alerts should be owned by the team owning the business domain of alerts. Which team is the owner of the alerts logic in the Security Org is unclear to me tbh. But if we continue to own based on who wrote the code, we have the risk of things being broken (no necessarily technically but rather in business sense) without the owners of the domain logic realising that

x-pack/plugins/security_solution/public/cloud_security_posture/components/entity_insight.tsx

maxcold · 2024-11-13T10:54:44Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

+
+  const severityMap = new Map<string, number>();
+
+  (['open', 'acknowledged'] as AlertsByStatus[]).forEach((status) => {


you don't need this piece of logic here as you filter out closed in the entity insight. So the component can be free of knowing about different statuses

maxcold · 2024-11-13T10:57:26Z

x-pack/plugins/security_solution/public/cloud_security_posture/components/entity_insight.tsx

+  });
+
+  const filteredAlertsData: ParsedAlertsData = alertsData
+    ? Object.fromEntries(Object.entries(alertsData).filter(([key]) => key !== 'closed'))


let's use FILTER_CLOSED from x-pack/plugins/security_solution/common/types/index.ts

maxcold · 2024-11-13T10:59:33Z

...ns/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.test.tsx

+      </TestProviders>
+    );
+
+    // there should be 3 element with this id because we have 3 different severities in this test


nit: let's change the test case name to reflect that instead of a comment

seanrathier

Look good! Just a few minor comments.

seanrathier · 2024-11-13T14:42:32Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

+  alertsTotal,
+  euiTheme,
+}: {
+  alertsTotal: string | number;


Can this truly be a string? We can use only a number instead of a union.

seanrathier · 2024-11-13T14:44:51Z

...plugins/security_solution/public/cloud_security_posture/components/alerts/alerts_preview.tsx

+            css={css`
+              font-weight: ${euiTheme.font.weight.semiBold};
+            `}


Suggested change

css={css`

font-weight: ${euiTheme.font.weight.semiBold};

`}

css={{

fontWeight: euiTheme.font.weight.semiBold,

}}

>

elasticmachine · 2024-11-13T18:08:28Z

💚 Build Succeeded

Buildkite Build
Commit: cdf9a17

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	6197	6198	+1

Async chunks

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

id	before	after	diff
`securitySolution`	13.4MB	13.4MB	+2.5KB

History

💔 Build #250593 failed 24bfb3d
💚 Build #249777 succeeded c32edb4
💛 Build #249499 was flaky d49d5ab
💚 Build #249011 succeeded 25e54ba

kibanamachine · 2024-11-13T18:10:30Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11823224736

## Summary <img width="1447" alt="Screenshot 2024-10-21 at 10 38 49 PM" src="https://github.com/user-attachments/assets/e7d011dd-8245-4bf8-ad96-a4fd634e82c1"> This PR is for Alerts preview component in Contextual Flyout (Host Name) (cherry picked from commit 675b54b)

kibanamachine · 2024-11-13T18:17:00Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.x`: - [[Cloud Security] Alerts Preview for Host Name (#197102)](#197102)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Rickyanto Ang <[email protected]>

## Summary <img width="1447" alt="Screenshot 2024-10-21 at 10 38 49 PM" src="https://github.com/user-attachments/assets/e7d011dd-8245-4bf8-ad96-a4fd634e82c1"> This PR is for Alerts preview component in Contextual Flyout (Host Name)

…ment details flyout (#200268) ## Summary This PR made some changes to the alert count API in document details flyout. Closed alerts are now removed when showing total count and distributions. Data fetching logic is updated to match the one used in host flyout (#197102). Notable changes: - Closed alerts are now excluded - Number of alerts in alerts flyout should match the ones in host flyout - Clicking the number will open timeline with the specific entity and `NOT kibana.alert.workflow_status: closed` filters - If a host/user does not have active alerts (all closed), no distribution bar is shown https://github.com/user-attachments/assets/3a1d6e36-527e-4b62-816b-e1f4de7314ca ### Checklist - [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

…ment details flyout (elastic#200268) ## Summary This PR made some changes to the alert count API in document details flyout. Closed alerts are now removed when showing total count and distributions. Data fetching logic is updated to match the one used in host flyout (elastic#197102). Notable changes: - Closed alerts are now excluded - Number of alerts in alerts flyout should match the ones in host flyout - Clicking the number will open timeline with the specific entity and `NOT kibana.alert.workflow_status: closed` filters - If a host/user does not have active alerts (all closed), no distribution bar is shown https://github.com/user-attachments/assets/3a1d6e36-527e-4b62-816b-e1f4de7314ca ### Checklist - [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 (cherry picked from commit a37a02e)

seanrathier · 2024-12-03T20:58:44Z

@animehart How can I test this?

…ment details flyout (elastic#200268) ## Summary This PR made some changes to the alert count API in document details flyout. Closed alerts are now removed when showing total count and distributions. Data fetching logic is updated to match the one used in host flyout (elastic#197102). Notable changes: - Closed alerts are now excluded - Number of alerts in alerts flyout should match the ones in host flyout - Clicking the number will open timeline with the specific entity and `NOT kibana.alert.workflow_status: closed` filters - If a host/user does not have active alerts (all closed), no distribution bar is shown https://github.com/user-attachments/assets/3a1d6e36-527e-4b62-816b-e1f4de7314ca ### Checklist - [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

animehart added 5 commits October 21, 2024 09:43

initial Push

329020e

reversed array for dist bar

344a65f

Merge branch 'main' into contextual-flyout-alerts-preview-host

34fffd2

added test + updated logic to show entity insight component

c457662

updated logic to only exlude closed alerts

25e54ba

animehart marked this pull request as ready for review November 6, 2024 22:14

animehart requested a review from a team as a code owner November 6, 2024 22:14

updated hooks used

d49d5ab

animehart added v8.17.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v9.0.0 Team:Cloud Security Cloud Security team related labels Nov 8, 2024

animehart added the release_note:skip Skip the PR/issue when compiling release notes label Nov 8, 2024

maxcold requested changes Nov 8, 2024

View reviewed changes

animehart added 3 commits November 8, 2024 13:35

reverting logic to previous for preview component

136b04c

fix capital letter issue

3362998

fix vuln tooltips

c32edb4

animehart requested a review from maxcold November 11, 2024 23:13

maxcold reviewed Nov 12, 2024

View reviewed changes

animehart added 2 commits November 12, 2024 11:46

Merge branch 'main' into contextual-flyout-alerts-preview-host

24bfb3d

pr comments

b8bc727

animehart requested a review from maxcold November 12, 2024 22:23

maxcold reviewed Nov 13, 2024

View reviewed changes

maxcold approved these changes Nov 13, 2024

View reviewed changes

seanrathier reviewed Nov 13, 2024

View reviewed changes

pr comments

a945603

clean up

cdf9a17

animehart merged commit 675b54b into elastic:main Nov 13, 2024
43 checks passed

kibanamachine mentioned this pull request Nov 13, 2024

[8.x] [Cloud Security] Alerts Preview for Host Name (#197102) #200062

Merged

animehart changed the title ~~[Cloud Security] Alerts Preview for Host Name~~ [Cloud Security] Alerts Preview Contextual Flyout Nov 13, 2024

christineweng mentioned this pull request Nov 14, 2024

[Security Solution] Update alert kpi to exclude closed alerts in document details flyout #200268

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cloud Security] Alerts Preview Contextual Flyout #197102

[Cloud Security] Alerts Preview Contextual Flyout #197102

animehart commented Oct 21, 2024 •

edited

Loading

animehart commented Nov 6, 2024

elasticmachine commented Nov 8, 2024

maxcold Nov 8, 2024

PhilippeOberti Nov 8, 2024

PhilippeOberti Nov 8, 2024

maxcold Nov 12, 2024

maxcold left a comment

maxcold Nov 12, 2024

maxcold Nov 13, 2024 •

edited

Loading

maxcold Nov 13, 2024

maxcold Nov 13, 2024

seanrathier left a comment

seanrathier Nov 13, 2024

seanrathier Nov 13, 2024

elasticmachine commented Nov 13, 2024

kibanamachine commented Nov 13, 2024

kibanamachine commented Nov 13, 2024

seanrathier commented Dec 3, 2024


		const severityMap = new Map<string, number>();

		(['open', 'acknowledged'] as AlertsByStatus[]).forEach((status) => {

[Cloud Security] Alerts Preview Contextual Flyout #197102

[Cloud Security] Alerts Preview Contextual Flyout #197102

Conversation

animehart commented Oct 21, 2024 • edited Loading

Summary

animehart commented Nov 6, 2024

elasticmachine commented Nov 8, 2024

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

maxcold left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxcold Nov 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanrathier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 13, 2024

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

kibanamachine commented Nov 13, 2024

kibanamachine commented Nov 13, 2024

💚 All backports created successfully

Questions ?

seanrathier commented Dec 3, 2024

animehart commented Oct 21, 2024 •

edited

Loading

maxcold Nov 13, 2024 •

edited

Loading