[Alerting] extend Alert Type with names/descriptions of action variables #59756

pmuellr · 2020-03-10T13:56:44Z

resolves #58529

This PR extends alertType with an actionVariables property, which
should be an object with optionial properties context and state.
These properties should be typed as optional Record<string, string>
values. The keys are the names of the relevant action variables,
and the values are the localized descriptions of the variables.

This new property is optional, for the moment.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios, except as noted below.

TODO

add ui test that the actionVariables get rendered to the web page as expected - issue [Alerting] add ui test that the actionVariables get rendered to the web page as expected #60083
open issue to resolve built-in alert action variables spec'd in both server and ui, can we put these in /common? - issue [Alerting] move always-available action variables to alerting/common #60085

For maintainers

This was checked for breaking API changes and was labeled appropriately

pmuellr · 2020-03-10T17:33:06Z

I didn't realize we already had a property set up for the action variables!

kibana/x-pack/plugins/triggers_actions_ui/public/types.ts

Lines 74 to 80 in 8bc051a

    
           export interface AlertType { 
        
             id: string; 
        
             name: string; 
        
             actionGroups: ActionGroup[]; 
        
             actionVariables: string[]; 
        
             defaultActionGroupId: ActionGroup['id']; 
        
           }

So, that will need to be changed, and I haven't looked, but I suspect there is already some code that will display them, so I'll fix that up too. We'll need to figure out if we want to display a tooltip for the localized description of them, or display them in some other way.

pmuellr · 2020-03-10T17:42:07Z

There are also a number of "always provided" action variables, by the alerting framework itself, like alertName, alertId, etc. I didn't provide a way to get these from the server, as they are basically static. I thought we'd just hard-code them in the UI code for now, if we think we need them from the server, we can add that later.

pmuellr · 2020-03-10T18:22:50Z

Yee Haw!

pmuellr · 2020-03-10T19:19:05Z

commit #728e8f69eb7e22a3b0875a2b81d20e1abadb3ec4 now displays the variables in the UI context menu

YulNaumenko

LGTM, I have a suggestion regarding empty variables list to be optional as on the server. I think it should be nice do not show it in this case.

YulNaumenko · 2020-03-11T18:06:51Z

x-pack/plugins/triggers_actions_ui/public/types.ts

 export interface AlertType {
  id: string;
  name: string;
  actionGroups: ActionGroup[];
-  actionVariables: string[];
+  actionVariables: AlertTypeActionVariables;


I think it makes sense to have it as optional and do not show the empty dropdown list in the case it is undefined

messageVariables which use actionVariables in UI is already optional for the ActionParamsProps as well, but some UI fixes still should be done to not display it. Maybe make sense to have a separate issue for this

The action variables list will never be empty by the time the context menu displays, as there are 5-6 "always available" variables that come from the alert library; eg, alertName.

I've also reshaped these since you reviewed, as the shape was radically different than actionGroups (which is an array of id / name object), so I changed the actionVariables to match. This also means we won't have prototype pollution issues (even if not our fault), as we won't be using unknown properties on objects.

resolves elastic#58529 This PR extends alertType with an `actionVariables` property, which should be an object with optionial properties `context` and `state`. These properties should be typed as optional `Record<string, string>` values. The keys are the names of the relevant action variables, and the values are the localized descriptions of the variables.

elasticmachine · 2020-03-12T19:23:31Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

mikecote

Changes LGTM! Had one question in hopes of removing the double places to maintain the default list of action variables but you have more context than I do.

~~I will also open an enhancement issue to use the user friendly description attributes defined within the alert action param templating.~~

Edit: As discussed, we'll wait and see before thinking too far ahead.

mikecote · 2020-03-12T20:53:44Z

x-pack/plugins/triggers_actions_ui/public/application/lib/action_variables.ts

+// this list should be the same as in:
+//   x-pack/plugins/alerting/server/task_runner/transform_action_params.ts


I recall previously the mention of having two files to maintain. I wonder if there's a way we could move this function into a common folder and do something like _.pick({}, ...) within x-pack/plugins/alerting/server/task_runner/transform_action_params.ts based on the accumulation of name properties.

Ah ... the common folder ... yes, that might well work.

For whatever reason I was thinking that we'd need a new server API to retrieve these.

I'll open a new issue to resolve this.

pmuellr · 2020-03-12T22:09:50Z

@elasticmachine merge upstream

pmuellr · 2020-03-13T00:10:28Z

@elasticmachine merge upstream

…les (elastic#59756) resolves elastic#58529 This PR extends alertType with an `actionVariables` property, which describes the properties of the context object passed when scheduling actions, and the current state. These property descriptions are used by the web ui for the alert create and edit forms, to allow the properties to be added to action parameters as mustache template variables.

…les (#59756) (#60082) resolves #58529 This PR extends alertType with an `actionVariables` property, which describes the properties of the context object passed when scheduling actions, and the current state. These property descriptions are used by the web ui for the alert create and edit forms, to allow the properties to be added to action parameters as mustache template variables.

kibanamachine · 2020-03-13T05:40:13Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 08027da

History

💚 Build #32988 succeeded 08027da
💔 Build #32963 failed 94494c7
💚 Build #32858 succeeded a8e2afa
💚 Build #32652 succeeded 42d9b19
💔 Build #32626 failed 07d8325

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

* master: [Alerting] extend Alert Type with names/descriptions of action variables (elastic#59756) [Ingest] Fix data source creation and double system data source (elastic#60069) Add button to view full service map (elastic#59394) unskip tests for code coverage (elastic#59725) [Ingest] Add Fleet & EPM features (elastic#59376) [Logs UI] Show navigation bar while loading source configurati… (elastic#59997) [Endpoint] ERT-82 Alerts search bar (elastic#59702) Aggregate queue types being used by Beats (elastic#59850) skip flaky suite (elastic#59541) Convert Timeline to TypeScript (elastic#59966) Make context.core required argument to context providers (elastic#59996)

pmuellr mentioned this pull request Mar 10, 2020

[Alerting] add more alert properties to action parameter templating #59718

Merged

2 tasks

pmuellr force-pushed the alerting/action-vars-in-type branch 2 times, most recently from 356604c to d6c8457 Compare March 10, 2020 21:17

YulNaumenko self-requested a review March 11, 2020 16:48

YulNaumenko approved these changes Mar 11, 2020

View reviewed changes

pmuellr force-pushed the alerting/action-vars-in-type branch from ec74b52 to 42d9b19 Compare March 12, 2020 02:50

fix up some existing tests, add some new ones

a8e2afa

pmuellr marked this pull request as ready for review March 12, 2020 19:22

pmuellr requested a review from a team as a code owner March 12, 2020 19:22

pmuellr added Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Mar 12, 2020

pmuellr added v7.7.0 release_note:skip Skip the PR/issue when compiling release notes v8.0.0 labels Mar 12, 2020

pmuellr mentioned this pull request Mar 12, 2020

enhancement for alertType metrics.alert.threshold context variables #60060

Closed

mikecote self-requested a review March 12, 2020 20:44

mikecote approved these changes Mar 12, 2020

View reviewed changes

Merge branch 'master' into alerting/action-vars-in-type

94494c7

Merge branch 'master' into alerting/action-vars-in-type

08027da

pmuellr merged commit ce1836b into elastic:master Mar 13, 2020

pmuellr mentioned this pull request Mar 13, 2020

[7.x] [Alerting] extend Alert Type with names/descriptions of action variables (#59756) #60082

Merged

This was referenced Mar 13, 2020

[Alerting] add ui test that the actionVariables get rendered to the web page as expected #60083

Closed

[Alerting] move always-available action variables to alerting/common #60085

Open

pmuellr added the backported label Mar 13, 2020

mikecote added release_note:enhancement and removed release_note:skip Skip the PR/issue when compiling release notes labels Apr 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Alerting] extend Alert Type with names/descriptions of action variables #59756

[Alerting] extend Alert Type with names/descriptions of action variables #59756

pmuellr commented Mar 10, 2020 •

edited

pmuellr commented Mar 10, 2020

pmuellr commented Mar 10, 2020

pmuellr commented Mar 10, 2020

pmuellr commented Mar 10, 2020

YulNaumenko left a comment •

edited

YulNaumenko Mar 11, 2020

YulNaumenko Mar 11, 2020 •

edited

pmuellr Mar 11, 2020

elasticmachine commented Mar 12, 2020

mikecote left a comment •

edited

mikecote Mar 12, 2020

pmuellr Mar 12, 2020

pmuellr commented Mar 12, 2020

pmuellr commented Mar 13, 2020

kibanamachine commented Mar 13, 2020

		// this list should be the same as in:
		// x-pack/plugins/alerting/server/task_runner/transform_action_params.ts

[Alerting] extend Alert Type with names/descriptions of action variables #59756

[Alerting] extend Alert Type with names/descriptions of action variables #59756

Conversation

pmuellr commented Mar 10, 2020 • edited

Checklist

TODO

For maintainers

pmuellr commented Mar 10, 2020

pmuellr commented Mar 10, 2020

pmuellr commented Mar 10, 2020

pmuellr commented Mar 10, 2020

YulNaumenko left a comment • edited

Choose a reason for hiding this comment

YulNaumenko Mar 11, 2020

Choose a reason for hiding this comment

YulNaumenko Mar 11, 2020 • edited

Choose a reason for hiding this comment

pmuellr Mar 11, 2020

Choose a reason for hiding this comment

elasticmachine commented Mar 12, 2020

mikecote left a comment • edited

Choose a reason for hiding this comment

mikecote Mar 12, 2020

Choose a reason for hiding this comment

pmuellr Mar 12, 2020

Choose a reason for hiding this comment

pmuellr commented Mar 12, 2020

pmuellr commented Mar 13, 2020

kibanamachine commented Mar 13, 2020

💚 Build Succeeded

History

pmuellr commented Mar 10, 2020 •

edited

YulNaumenko left a comment •

edited

YulNaumenko Mar 11, 2020 •

edited

mikecote left a comment •

edited