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: Support values in notification templates #56457

Merged
merged 6 commits into from Oct 10, 2022

Conversation

grobinson-grafana
Copy link
Contributor

@grobinson-grafana grobinson-grafana commented Oct 6, 2022

What this PR does / why we need it:

We have received a lot of feedback regarding the ValueString in alert notifications. Perhaps one of the most frequent complaints about ValueString is that it is difficult to read because it contains a lot of information, and the information is shown as a JSON-like string. Users have often asked how it can be templated and the answer is that it can't.

Until now users have been able to add custom annotations to their alert rules which contains values via the $values variable added in previous versions of Grafana. However, these custom annotations must be added for each of the user's alert rule, instead of once in a template that all of their alerts can be notified via.

This commit adds then the much requested feature to support values in notification templates. Users can then create a single template that prints the annotations, labels and values of their alerts in a format of their choice!

Here is an example of the default template with an instant vector:

**Firing**

Value: A=1
Labels:
 - alertname = Test
 - grafana_folder = General Alerting
Annotations:
Source: http://localhost:3000/alerting/grafana/wXZdwwVVz/view
Silence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DTest&matcher=grafana_folder%3DGeneral+Alerting

Here is an example of the default template with a reduce expression:

**Firing**

Value: B=1
Labels:
 - alertname = Test
 - grafana_folder = General Alerting
Annotations:
Source: http://localhost:3000/alerting/grafana/wXZdwwVVz/view
Silence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DTest&matcher=grafana_folder%3DGeneral+Alerting

Here is an example of the default template with a reduce and math expression:

**Firing**

Value: B=1, C=1
Labels:
 - alertname = Test
 - grafana_folder = General Alerting
Annotations:
Source: http://localhost:3000/alerting/grafana/wXZdwwVVz/view
Silence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DTest&matcher=grafana_folder%3DGeneral+Alerting

Last is an example of the default template with a non-reduced time series:

**Firing**

Value: [no value]
Labels:
 - alertname = DatasourceError
 - grafana_folder = General Alerting
 - rulename = Test
Annotations:
Source: http://localhost:3000/alerting/grafana/wXZdwwVVz/view
Silence: http://localhost:3000/alerting/silence/new?alertmanager=grafana&matcher=alertname%3DDatasourceError&matcher=grafana_folder%3DGeneral+Alerting&matcher=rulename%3DTest

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@grafanabot
Copy link
Contributor

@grobinson-grafana grobinson-grafana added the no-backport Skip backport of PR label Oct 6, 2022
@grafanabot
Copy link
Contributor

@grafanabot
Copy link
Contributor

@grobinson-grafana grobinson-grafana force-pushed the grobinson/values-in-notification-templates branch from 75f3cbe to 5ab45f2 Compare October 6, 2022 20:14
@grobinson-grafana grobinson-grafana marked this pull request as ready for review October 6, 2022 21:04
@grobinson-grafana grobinson-grafana requested a review from a team as a code owner October 6, 2022 21:04
@grobinson-grafana grobinson-grafana modified the milestone: 9.3.0 Oct 6, 2022
@grobinson-grafana grobinson-grafana added no-backport Skip backport of PR add to changelog and removed no-backport Skip backport of PR labels Oct 6, 2022
@grobinson-grafana grobinson-grafana force-pushed the grobinson/values-in-notification-templates branch 2 times, most recently from 30259f2 to 05931fa Compare October 7, 2022 11:29
@gillesdemey gillesdemey self-requested a review October 7, 2022 12:40
Copy link
Member

@gillesdemey gillesdemey left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

Copy link
Contributor

@yuri-tceretian yuri-tceretian left a comment

Choose a reason for hiding this comment

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

Can you please update the docs in UI
image

here

{
name: 'ValueString',
type: 'string',
notes: 'String that contains the labels and value of each reduced expression in the alert.',
},

@grafanabot
Copy link
Contributor

@jtheory
Copy link
Contributor

jtheory commented Oct 10, 2022

Awesome, thank you; this'll be hugely helpful 🏆

It looks to me like this doesn't risk breaking any existing alerting rule / templates -- the only difference it would make is that anyone who's currently dumping the entire ValueString into a notification will get a differently formatted result, right?

The main question I have is about updating docs & providing examples there, plus (ideally) how we should update the UI to help users 1) learn this is now possible and 2) help them do it correctly
(UI updates aren't a blocker for this PR; I'm not sure if basic doc updates are a blocker or not)

@grobinson-grafana
Copy link
Contributor Author

It looks to me like this doesn't risk breaking any existing alerting rule / templates -- the only difference it would make is that anyone who's currently dumping the entire ValueString into a notification will get a differently formatted result, right?

All users who are using ValueString in their custom templates will see no change as I suspect we are neither changing nor removing ValueString until at least Grafana 10.

However, all users who are using the default template, or even just default.message in their custom templates, will see this change. Any users who were using ValueString can update their templates to use __text_values_list and also see the change.

I will work with @brendamuir to also document this in the Whats New page. I would like to get it into 9.2.0 if there will be a beta2, otherwise I think we should wait for 9.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

8 participants