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

[RAM] Implement global alerts page #175143

Merged
merged 48 commits into from
Feb 13, 2024

Conversation

umbopepato
Copy link
Member

@umbopepato umbopepato commented Jan 18, 2024

Summary

Adds a global alerts page to the Stack management section to allow users to see alerts across different solutions/apps through a unified UX.

Screenshot 2024-01-22 at 17 34 46 image

Spec

Tasks

Deferred work items

Checklist

@umbopepato umbopepato force-pushed the 173647-global-alerts-page branch from 516a822 to bc455f8 Compare January 22, 2024 16:42
@umbopepato
Copy link
Member Author

/ci

@umbopepato
Copy link
Member Author

/ci

@umbopepato
Copy link
Member Author

/ci

@umbopepato umbopepato added Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) release_note:feature Makes this part of the condensed release notes v8.13.0 labels Feb 2, 2024
@umbopepato
Copy link
Member Author

/ci

@umbopepato
Copy link
Member Author

/ci

@umbopepato
Copy link
Member Author

/ci

Copy link
Contributor

@Zacqary Zacqary left a comment

Choose a reason for hiding this comment

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

LGTM!

@umbopepato umbopepato force-pushed the 173647-global-alerts-page branch from dab4336 to a75223b Compare February 12, 2024 14:33
Copy link
Contributor

@dplumlee dplumlee left a comment

Choose a reason for hiding this comment

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

alerts_as_data_rbac.ts changes lgtm

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

As I mention in my comment below, this looks great but I would like to move the logic for the panels creation to the consumers and then pass the panel items to the unified search. In that way we make the component more generic!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanx a lot for the contribution. It seems as a very nice enhancement but it is very specific for the alerts stack page so I would like to make this more generic in unified search. I suggest:

  • change the quickFilters to additionalQueryBarMenuPanels
  • create the panels in the consumer (stack alerts)
  • pass them to unfied search new prop additionalQueryBarMenuPanels
  • Display them in the query bar menu

In that way other consumers can use the same logic to display panel items in than menu without the need to integrating this inside unified search

Let me know if you have any questions!

Copy link
Member Author

@umbopepato umbopepato Feb 13, 2024

Choose a reason for hiding this comment

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

Thanks for your review @stratoula! 😊
In that case, how would you like me to address the issue of specifying top-level items as well as nested panels? Would a nested structure like the one I used for quick filters (flattened at QueryBarMenuPanels-level) be ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes I think that this

...quickFiltersContextMenuData.panels

should come from the consumers of unified search. So after the proposed changes it should look

... additionalQueryBarMenuPanels

Copy link
Member Author

@umbopepato umbopepato Feb 13, 2024

Choose a reason for hiding this comment

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

What I mean is that quickFilters accepts a nested structure where items like

{ name: '', filter: {} }

render top-level menu items like these:

image

whereas items like

{ groupName: '', items: [...] }

render menu items that link to a panel with sub-items (see Status):

image

The EuiContextMenu API expects a flattened structure thought, so I guess the options are either accepting a nested structure like the one described above, then flattened at QueryBarMenuPanels-level, something like:

additionalQueryBarMenuItems={[
  { name: '' }, // Top-level
  { title/groupName: '', items: [{ name: '' }] }, // Nested in subpanel
]}

or let the consumer handle the association between panels and items:

additionalQueryBarMenuItems={[
  { name: '' }, // Top-level
  { name: '', panel: 1 }, // Link to panel
]}
additionalQueryBarMenuPanels={[
  { id: 1, title: '', items: [...] },
  { id: 2, title: '', items: [...] },
]}

// or alternatively

additionalQueryBarMenuItems={{
  items: [...],
  panels: [...],
}}

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry If I was not very clear. I want this additionalQueryBarMenuItemsData to be built outside Unified search.

So let's go with this

additionalQueryBarMenuItems={{
  items: [...],
  panels: [...],
}}

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Looks great, except from the broken story! Thanx for addressing my feedback! I am approving to not block you anymore but let's merge after the storybook fix! Great job!

@@ -711,4 +711,34 @@ storiesOf('SearchBar', module)
submitButtonStyle: 'full',
renderQueryInputAppend: () => <EuiButton onClick={() => {}}>Append</EuiButton>,
})
)
.add('with quick filters', () =>
Copy link
Contributor

Choose a reason for hiding this comment

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

This story is broken now! Can we fix it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed, thanks for the heads up! 👍

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
triggersActionsUi 662 689 +27

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
triggersActionsUi 571 570 -1

Async chunks

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

id before after diff
alerting 89.1KB 89.2KB +24.0B
apm 3.2MB 3.2MB +24.0B
cases 457.0KB 457.0KB +24.0B
discover 578.2KB 578.3KB +24.0B
infra 1.4MB 1.4MB +24.0B
ml 3.7MB 3.7MB +24.0B
securitySolution 11.4MB 11.4MB +72.0B
triggersActionsUi 1.4MB 1.5MB +67.0KB
unifiedSearch 223.9KB 224.3KB +424.0B
total +67.7KB

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
unifiedSearch 23 24 +1

Page load bundle

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

id before after diff
observability 102.9KB 103.0KB +24.0B
triggersActionsUi 106.3KB 108.1KB +1.7KB
unifiedSearch 35.8KB 35.9KB +58.0B
total +1.8KB
Unknown metric groups

API count

id before after diff
triggersActionsUi 597 596 -1

async chunk count

id before after diff
triggersActionsUi 64 65 +1

ESLint disabled line counts

id before after diff
triggersActionsUi 129 127 -2

References to deprecated APIs

id before after diff
triggersActionsUi 50 53 +3

Total ESLint disabled count

id before after diff
triggersActionsUi 135 133 -2

History

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

@umbopepato umbopepato merged commit 7424285 into elastic:main Feb 13, 2024
22 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Feb 13, 2024
@umbopepato umbopepato added release_note:skip Skip the PR/issue when compiling release notes and removed release_note:feature Makes this part of the condensed release notes labels Feb 13, 2024
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this pull request Feb 15, 2024
Adds a global alerts page to the Stack management section to allow users
to browse alerts across different solutions/apps through a unified UX.

Refs elastic#166709
Closes elastic#173647
Closes elastic#173650
Closes elastic#173648
fkanout pushed a commit to fkanout/kibana that referenced this pull request Mar 4, 2024
Adds a global alerts page to the Stack management section to allow users
to browse alerts across different solutions/apps through a unified UX.

Refs elastic#166709
Closes elastic#173647
Closes elastic#173650
Closes elastic#173648
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants