Alerting: Add actions extension point to alert instances table view #77900

sd2k · 2023-11-08T22:29:34Z

What is this feature?

This PR adds a new extension point to each row of the alert instances table, which is shown in both the 'Rule list' and the 'Rule details' pages. This allows plugins to add actions to a dropdown menu in the rightmost column of the table. Those actions are passed the alert instance so can use it for contextual handling. This is similar to the dropdown menu in the 'Explore' page (when you click 'Add') or in the panel menu dropdown (under 'Extensions').

See https://github.com/grafana/machine-learning/pull/3461 for an video example of how this can be used (e.g. by Grafana Sift here).

Why do we need this feature?

So far if plugins have wanted to add functionality to the alerting page it's been O(N) in the number of UI features added - e.g. adding the Grafana Incident button required work on the alerting page that was specific to that plugin. By adding a plugin extension point, any plugin can easily inject useful functionality in a non-intrusive way without needing further development on the alerting side.

Who is this feature for?

Plugin developers who would like to hook into the alerting page (specifically the Sift team, right now), and users who can then interact with alert instances in more varied ways.

Which issue(s) does this PR fix?:

Fixes https://github.com/grafana/machine-learning/issues/3458.

Special notes for your reviewer:

Things to consider:

UX: is it immediately obvious that the icon is a clickable button? what could we replace it with, if not?
- I chose to omit a heading and keep the column nice and narrow so it was as unintrusive as possible, but it's not massively discoverable now 🤔
I've updated existing tests but haven't added any specifically for this, happy to do so but might need some guidance.
The PluginExtensionAlertInstanceContext should probably be exported as part of @grafana/data (similar to PluginExtensionPanelContext), but it contains private types i.e. (Alert). Is there a subset of this Alert interface we could add to @grafana/data?

Screenshots:

unexpanded:

expanded with one action:

Please check that:

It works as expected from a user's perspective.
If this is a pre-GA feature, it is behind a feature toggle.
The docs are updated, and if this is a notable improvement, it's added to our What's New doc.

This PR adds a new [extension point][] to each row of the alert instances table. This allows plugins to add actions to a dropdown menu in the rightmost column of the table. Those actions are passed the alert instance so can use it for contextual handling. See grafana/machine-learning#3461 for an example of how this can be used (e.g. by Grafana Sift here).

The rule will be very useful for users to be able to access things like the name and query string.

sunker

I think this looks great, but would like someone with more experience from working with extensions to have a look too. @mckn @leventebalogh can you review?

Regarding the icon button. This doesn't feel totally intuitive to me. Think there's an ellipsis icon in grafana ui. Can you try that instead?

sd2k · 2023-11-10T10:39:49Z

Sure! Here's how it looks with ellipsis-v:

sunker · 2023-11-10T10:55:40Z

Sure! Here's how it looks with ellipsis-v:

Think that looks much better, thank you!

sympatheticmoose · 2023-11-10T13:09:38Z

In other extension points we're indicating which app has contributed the extension, maybe a pattern to follow here too?

gotjosh · 2023-11-14T08:25:06Z

A tiny nit that perhaps the title should include "table view" - I got terrified for a second as I thought this was referring to the alert instance database table.

gillesdemey

Code changes look good, thanks @sd2k! Couple of small nits and some changes on the UI / interaction model.

Tagging @dhalachliyski here to drop some thoughts on that :)

gillesdemey · 2023-11-14T15:34:41Z

public/app/features/alerting/unified/components/extensions/AlertInstanceExtensionPoint.tsx

+  return (
+    <>
+      <Dropdown placement="bottom-start" overlay={menu}>
+        <IconButton name="ellipsis-v" aria-label="Actions" disabled={extensions.length === 0} variant="secondary" />


I believe we can improve this part of the UI; there's generally two patterns we tend to use in Alerting.

We don't show the dropdown at all if no actions can be taken, a disabled button with no explanation is a tad confusing :)

We always show the dropdown, with all of the actions but disable the actions the user has no permissions for are cannot take for some other reason.

If 2 we try to wrap the item with a tooltip explaining why we've disabled the action if we know the reason for it (insufficient permissions, x is not installed / configured, etc)

Makes sense! Happy to add either of those. 1. would be easier for sure, otherwise we'll need to customise the AlertExtensionPointMenu (or possibly upstream some improvements to the ToolbarExtensionPointMenu we're hijacking).

I'll wait for @dhalachliyski's feedback before changing it.

If modifications need to happen to support the second pattern let's go with the first one to unblock this PR – that should be good from a UX perspective :)

public/app/plugins/panel/alertlist/unified-alerting/GroupedView.tsx

leventebalogh

LGTM (mostly from a plugin-extensions perspective) 👍

gillesdemey · 2023-11-15T20:04:47Z

Code changes aside, does this warrant adding to the changelog / what's new if public plugins can inject actions? Do we need to update docs in this PR?

sd2k · 2023-11-15T22:03:00Z

Code changes aside, does this warrant adding to the changelog / what's new if public plugins can inject actions? Do we need to update docs in this PR?

I guess that's an alerting decision, but from my perspective it might be nice to call out in the changelog that users may find some new actions in their alert instances lists, depending on the plugins they have installed. I'm not sure how that should be documented from a user's POV though, since we don't know what other plugins will extend it.

From a dev POV, maybe the plugin extension docs should be updated to reference the new extension point, but I'll leave that up to the plugin extensions folks to decide!

Instead of showing a disabled dropdown menu (which can be confusing), just don't show the menu at all if no extensions are registered for the alert instance extension point.

sympatheticmoose · 2023-11-16T11:44:03Z

I'm not sure how that should be documented from a user's POV though, since we don't know what other plugins will extend it.

It could be a fairly generic statement such as "Application plugins can contribute extensions which allow you to take actions based on a given alert, such as running a Sift investigation to help uncover the route cause of the alert" (I'm not sure on an appropriate location within the alerting docs)

From a dev POV, maybe the plugin extension docs should be updated to reference the new extension point, but I'll leave that up to the plugin extensions folks to decide!

Would welcome a PR to add it to the list here https://grafana.com/developers/plugin-tools/ui-extensions/register-an-extension#available-extension-points-within-grafana 🙏

If/when grafana/grafana#77900 is merged there will be a new extension point which plugin developers can register against. This PR adds the ID of the extension point to the `register-an-extension` docs. Keeping as draft for now since the upstream PR isn't merged yet.

sd2k · 2023-11-20T07:38:48Z

Thanks for all the reviews folks. @gillesdemey I'll leave the final decision to merge up to you - happy to contribute some docs if you'd like some, but I'm not 100% sure where they live!

@sympatheticmoose I've got a draft PR in place for the plugin tools docs 👍

gillesdemey

This is mostly for our own plugins for now – so I think it's fine to merge without docs update :)

LGTM! 🚀

…77900) This PR adds a new [extension point][] to each row of the alert instances table. This allows plugins to add actions to a dropdown menu in the rightmost column of the table. Those actions are passed the alert instance so can use it for contextual handling. See grafana/machine-learning#3461 for an example of how this can be used (e.g. by Grafana Sift here).

This PR adds an [extension point] to the Grafana plugin, allowing other plugins to add menu items to the dropdown independently of this plugin. It also moves the 'Declare incident' button into the 'Actions' menu representing the extension point, and displays it conditionally; the menu item is shown if all of the following are true: 1. the Grafana Incident plugin is installed (using the `PluginBridge` as before) 2. the alert group has a valid Grafana Incident URL 3. the alert group hasn't already been linked to an Incident 4. the Grafana Incident plugin hasn't _also_ configured an identical extension link to show in the dropdown. Ideally the Grafana Incident plugin would soon configure an extension using this extension point, at which point we can remove the custom 'Declare incident' button (and the `PluginBridge`) from the OnCall app. --- The main use case I have for this is to be able to run a Sift investigation from an OnCall alert group, similar to how it can be done from a Grafana alert instance as of [this PR][pr]. I'll try and get the Grafana ML and Incident plugins running in the same Grafana instance as this to test it out more thoroughly! Part of grafana/machine-learning#3558. [extension point]: https://grafana.com/developers/plugin-tools/ui-extensions/create-an-extension-point [pr]: grafana/grafana#77900

# What this PR does This PR adds an [extension point] to the Grafana plugin, allowing other plugins to add menu items to the dropdown independently of this plugin. It also moves the 'Declare incident' button into the 'Actions' menu representing the extension point, and displays it conditionally; the menu item is shown if all of the following are true: 1. the Grafana Incident plugin is installed (using the `PluginBridge` as before) 2. the alert group has a valid Grafana Incident URL 3. the alert group hasn't already been linked to an Incident 4. the Grafana Incident plugin hasn't _also_ configured an identical extension link to show in the dropdown. Ideally the Grafana Incident plugin would soon configure an extension using this extension point, at which point we can remove the custom 'Declare incident' button (and the `PluginBridge`) from the OnCall app. --- The main use case I have for this is to be able to run a Sift investigation from an OnCall alert group, similar to how it can be done from a Grafana alert instance as of [this PR][pr]. I'll try and get the Grafana ML and Incident plugins running in the same Grafana instance as this to test it out more thoroughly! Note: this is heavily inspired by, and in some case stolen from, the [Explore toolbar extension point in Grafana](https://github.com/grafana/grafana/blob/main/public/app/features/explore/extensions/ToolbarExtensionPoint.tsx). ## Which issue(s) this PR fixes Part of grafana/machine-learning#3558. ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) [extension point]: https://grafana.com/developers/plugin-tools/ui-extensions/create-an-extension-point [pr]: grafana/grafana#77900

grafana-delivery-bot bot added this to the 10.3.x milestone Nov 8, 2023

grafana-pr-automation bot added the area/frontend label Nov 8, 2023

sd2k force-pushed the alert-instance-action-extension-point branch from a6f90ef to 68f5fa9 Compare November 9, 2023 07:50

sd2k force-pushed the alert-instance-action-extension-point branch from 68f5fa9 to 5df9f51 Compare November 9, 2023 07:58

sd2k marked this pull request as ready for review November 9, 2023 08:18

sd2k requested review from a team as code owners November 9, 2023 08:18

sd2k requested review from gillesdemey, VikaCep, konrad147, soniaAguilarPeiron, tskarhed, L-M-K-B and sunker and removed request for a team November 9, 2023 08:18

Remove unnecessary second plugin context type

869f79b

sd2k force-pushed the alert-instance-action-extension-point branch from e197881 to 869f79b Compare November 9, 2023 09:34

sd2k added 2 commits November 9, 2023 09:34

Provide alert rule in PluginExtensionAlertInstanceContext

3a0c0fe

The rule will be very useful for users to be able to access things like the name and query string.

Remove unused children from IconButton

431407a

sd2k added the no-backport Skip backport of PR label Nov 9, 2023

Pass rule to AlertInstances in UngroupedView

127a599

sunker reviewed Nov 10, 2023

View reviewed changes

Use ellipsis instead of bars icon

71a2452

sd2k changed the title ~~Alerting: add actions extension point to alert instances table~~ Alerting: add actions extension point to alert instances table view Nov 14, 2023

gillesdemey reviewed Nov 14, 2023

View reviewed changes

Remove now-unused GroupedRules type

e75d608

leventebalogh approved these changes Nov 15, 2023

View reviewed changes

Don't show extension dropdown if no extensions are registered

9c8a562

Instead of showing a disabled dropdown menu (which can be confusing), just don't show the menu at all if no extensions are registered for the alert instance extension point.

sd2k added the add to changelog label Nov 16, 2023

sd2k mentioned this pull request Nov 16, 2023

Mention new alerting extension point PR in docs grafana/plugin-tools#545

Merged

sd2k changed the title ~~Alerting: add actions extension point to alert instances table view~~ Alerting: Add actions extension point to alert instances table view Nov 20, 2023

gillesdemey approved these changes Nov 20, 2023

View reviewed changes

gillesdemey merged commit 35b1e49 into main Nov 20, 2023
24 checks passed

gillesdemey deleted the alert-instance-action-extension-point branch November 20, 2023 16:43

gillesdemey added the area/alerting Grafana Alerting label Nov 20, 2023

sd2k mentioned this pull request Nov 23, 2023

Add alert group actions extension point to plugin grafana/oncall#3416

Merged

3 tasks

aangelisc modified the milestones: 10.3.x, 10.2.3 Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alerting: Add actions extension point to alert instances table view #77900

Alerting: Add actions extension point to alert instances table view #77900

sd2k commented Nov 8, 2023 •

edited

sunker left a comment

sd2k commented Nov 10, 2023

sunker commented Nov 10, 2023

sympatheticmoose commented Nov 10, 2023

gotjosh commented Nov 14, 2023

gillesdemey left a comment

gillesdemey Nov 14, 2023

sd2k Nov 14, 2023

gillesdemey Nov 15, 2023

leventebalogh left a comment

gillesdemey commented Nov 15, 2023

sd2k commented Nov 15, 2023

sympatheticmoose commented Nov 16, 2023

sd2k commented Nov 20, 2023

gillesdemey left a comment

Alerting: Add actions extension point to alert instances table view #77900

Alerting: Add actions extension point to alert instances table view #77900

Conversation

sd2k commented Nov 8, 2023 • edited

sunker left a comment

Choose a reason for hiding this comment

sd2k commented Nov 10, 2023

sunker commented Nov 10, 2023

sympatheticmoose commented Nov 10, 2023

gotjosh commented Nov 14, 2023

gillesdemey left a comment

Choose a reason for hiding this comment

gillesdemey Nov 14, 2023

Choose a reason for hiding this comment

sd2k Nov 14, 2023

Choose a reason for hiding this comment

gillesdemey Nov 15, 2023

Choose a reason for hiding this comment

leventebalogh left a comment

Choose a reason for hiding this comment

gillesdemey commented Nov 15, 2023

sd2k commented Nov 15, 2023

sympatheticmoose commented Nov 16, 2023

sd2k commented Nov 20, 2023

gillesdemey left a comment

Choose a reason for hiding this comment

sd2k commented Nov 8, 2023 •

edited