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

Alerting: Extend server API type AlertType with property actionVariables: string[] #58529

Closed
YulNaumenko opened this issue Feb 25, 2020 · 6 comments · Fixed by #59756
Closed
Assignees
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@YulNaumenko
Copy link
Contributor

Each Alert Type should has a possibility to provide a set of application specific variables, that could be added to the action message in UI.

@YulNaumenko YulNaumenko added the Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) label Feb 25, 2020
@elasticmachine
Copy link
Contributor

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

@pmuellr
Copy link
Member

pmuellr commented Feb 28, 2020

Ya, we need something like this. We currently call them context variables, here are the ones from the index threshold:

export interface ActionContext extends BaseActionContext {
// a short generic message which may be used in an action message
subject: string;
// a longer generic message which may be used in an action message
message: string;
}
export interface BaseActionContext {
// the alert name
name: string;
// the spaceId of the alert
spaceId: string;
// the namespace of the alert (spaceId === (namespace || 'default')
namespace?: string;
// the alert tags
tags?: string[];
// the aggType used in the alert
// the value of the aggField, if used, otherwise 'all documents'
group: string;
// the date the alert was run as an ISO date
date: string;
// the value that met the threshold
value: number;
}

The alerting framework itself adds some additional top-level properties available, not under context:

export function transformActionParams({
alertId,
alertInstanceId,
context,
params,
state,
}: TransformActionParamsOptions): AlertActionParams {

One issue is that these is the typing. You can basically think of the complete set of variables that we provide being a Record<string, any>, where you can have objects, arrays, numbers, Dates, etc in the leaf property values.

I'm not sure the best way to represent this type data. I'm not sure the final leaf property value types matter that much - we can stringify them if needed. Arrays seem to be the biggest problem. If you imagine we didn't allow arrays anywhere in the tree of variables, then each alert type could represent it's variables as a list of flattened keys, or perhaps with the object structure, and the property values being a "type" of the data.

Eg, from the index threshold alert type, here's a flat list of properties (none are nested):

'subject message name spaceId namespace tags group date value'.split(' ')

Here's a "structure" version:

subject: 'string',
message: 'string',
name: 'string',
spaceId: 'string',
namespace?: 'string',
tags?: 'string[]',
group: 'string',
date: 'string',
value: 'number',

Note that tags is already problematic, as it's an array. Also, the following instance threshold context vars really should be added to all alerts, in the alerting lib: spaceId, namespace, tags. The state variable already provided by the alert lib is also a little problematic, as the state is just a JSON-able object that the alertType can use for whatever it wants. We'd need to provide a similar sort of "schema" like I proposed for the context, for that particular structure.

@YulNaumenko YulNaumenko mentioned this issue Mar 3, 2020
3 tasks
@pmuellr
Copy link
Member

pmuellr commented Mar 4, 2020

I'm currently thinking that alertTypes should probably define an array of strings of context variable names. The values for these, when expanded, would always be strings. So, tags will have to be pre-expanded to tags.join(", ") or similar, by the alert, before sending as a context variable value.

In the future, instead of array of strings, an element could be an object instead of a string, where the object would define the name, and some kind of typing information that an editor could make use of: is it an array? is it a number? etc.

Seems like this is a nice 90% solution, and fairly easy to implement, and keep up to date, perhaps expandable in the future.

@mikecote
Copy link
Contributor

mikecote commented Mar 4, 2020

In the future, instead of array of strings, an element could be an object instead of a string, where the object would define the name, and some kind of typing information that an editor could make use of: is it an array? is it a number? etc.

If ever we really want to support nested objects / arrays. I wonder if we could add support for something like foo.bar or even foo[0].bar with the usage of _.get(...). It would support nested objects and arrays while keeping it an array of strings in the definition.

Though maybe we want to put pretty names on those so they can also be localized.

@pmuellr
Copy link
Member

pmuellr commented Mar 4, 2020

I just did a little mustache test, rendering an array by it's name:

$ node
> mustache = require('mustache')
{ ... }
> mustache.render('hi')
'hi'
> mustache.render('hi {{{name}}}', {name: 'bob'})
'hi bob'
> mustache.render('hi {{{name}}}', {name: ['bob']})
'hi bob'
> mustache.render('hi {{{name}}}', {name: ['bob', 'jim']})
'hi bob,jim'
> mustache.render('{{#name}}{{.}}, {{/name}}', {name: ['bob', 'jim']})
'bob, jim, '

That last one is an example of "looping" through an array with mustache sections. So that works as well, but I don't think we need to provide UI help for it, as it could get quite elaborate.

I think that may be good enough for now, to deal with arrays. Just make them available as a variable with their name, let mustache do the join on them.

@pmuellr
Copy link
Member

pmuellr commented Mar 4, 2020

Though maybe we want to put pretty names on those so they can also be localized.

I'm not sure we want the "names" of the context variables localized, we should allow customers to enter any reasonable mustache-able template, should be the same across localization. However, it probably makes a ton of sense to provide a short description for each variable, that we would localize, that could be used as a tooltip or otherwise.

@pmuellr pmuellr self-assigned this Mar 9, 2020
pmuellr added a commit to pmuellr/kibana that referenced this issue Mar 10, 2020
This is a pre-cursor to elastic#58529

I realized a bit ago that we weren't making quite enough info available
in the action parameter templating that happens when alerts schedule
actions to execute.  Missing were alert name, tags, and spaceId.

For the index threshold alert, I had added them to it's context, but
then every other action would have to do the same if they also
wanted those values.

So I added these as additional top-level variables that can be
used in templates, along with the alert id, alert instance id,
context, and state.  The other bits in RawAlert didn't seem
that interesting, to be used as an action parameter.
pmuellr added a commit that referenced this issue Mar 10, 2020
…59718)

This is a pre-cursor to #58529

I realized a bit ago that we weren't making quite enough info available
in the action parameter templating that happens when alerts schedule
actions to execute.  Missing were alert name, tags, and spaceId.

For the index threshold alert, I had added them to it's context, but
then every other action would have to do the same if they also
wanted those values.

So I added these as additional top-level variables that can be
used in templates, along with the alert id, alert instance id,
context, and state.  The other bits in RawAlert didn't seem
that interesting, to be used as an action parameter.
pmuellr added a commit to pmuellr/kibana that referenced this issue Mar 10, 2020
…lastic#59718)

This is a pre-cursor to elastic#58529

I realized a bit ago that we weren't making quite enough info available
in the action parameter templating that happens when alerts schedule
actions to execute.  Missing were alert name, tags, and spaceId.

For the index threshold alert, I had added them to it's context, but
then every other action would have to do the same if they also
wanted those values.

So I added these as additional top-level variables that can be
used in templates, along with the alert id, alert instance id,
context, and state.  The other bits in RawAlert didn't seem
that interesting, to be used as an action parameter.
pmuellr added a commit that referenced this issue Mar 10, 2020
…59718) (#59829)

This is a pre-cursor to #58529

I realized a bit ago that we weren't making quite enough info available
in the action parameter templating that happens when alerts schedule
actions to execute.  Missing were alert name, tags, and spaceId.

For the index threshold alert, I had added them to it's context, but
then every other action would have to do the same if they also
wanted those values.

So I added these as additional top-level variables that can be
used in templates, along with the alert id, alert instance id,
context, and state.  The other bits in RawAlert didn't seem
that interesting, to be used as an action parameter.
pmuellr added a commit to pmuellr/kibana that referenced this issue Mar 12, 2020
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.
pmuellr added a commit that referenced this issue Mar 13, 2020
…les (#59756)

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.
pmuellr added a commit to pmuellr/kibana that referenced this issue Mar 13, 2020
…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.
pmuellr added a commit that referenced this issue Mar 13, 2020
…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.
@kobelb kobelb added the needs-team Issues missing a team label label Jan 31, 2022
@botelastic botelastic bot removed the needs-team Issues missing a team label label Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants