Extend Discord Notifier #3761

LucaDev · 2024-03-12T07:36:04Z

This PR extends the Discord notifier to improve the readability of alerts.
It changes 5 key elements:

A configureable icon as bot Logo (I'm currently using the GitHub Avatar. Is that alright? Is there a better way for doing this?)
A configurable name for the bot
Alert-ID in the footer
Start & end times in the footer (Even if the message is sent with a delay, the correct time will be displayed)
It uses Discords fields-feature to display labels
It sends one message per alert

It also fixes UI builds on apple silicon by forcing the linux/amd86 platform for the elm container

Before:

gotjosh · 2024-03-12T10:03:55Z

This looks nice! Thank you very much for your contribution -- can you please show me an example of the after with multiple labels?

LucaDev · 2024-03-12T10:18:07Z

This looks nice! Thank you very much for your contribution -- can you please show me an example of the after with multiple labels?

Sure! Desktop:

I'm also planing to change the title to not include the labels anymore

LucaDev · 2024-03-12T11:01:42Z

Small update:

LucaDev · 2024-03-14T15:31:35Z

I'd love to get some first feedback @gotjosh

LucaDev · 2024-04-26T19:03:01Z

Hey. I‘d love to get some feedback on this.

LucaDev · 2024-04-26T19:06:06Z

I‘ll correct a few minor things this weekend. Afterwards it should be ready to be merged

grobinson-grafana · 2024-05-12T21:53:13Z

The visual changes look nice! You'll need to remove the one message per alert change though as it interferes with alert grouping behavior. For example, if a user wants one message per alert then grouping can be disabled in the route, like so:

route:
    receiver: test
    group_by: ['...']

You can find it documented here.

LucaDev · 2024-05-23T15:57:45Z

Good catch! Thank you!

The visual changes look nice! You'll need to remove the one message per alert change though as it interferes with alert grouping behavior. For example, if a user wants one message per alert then grouping can be disabled in the route, like so:
route:
    receiver: test
    group_by: ['...']
You can find it documented here.

Could you please have a look at the latest changes? Grouping is now allowed by only sending one message per alert - containing as much 'embeds' (those alert boxes you're seeing) as allowed per Discord API limits. Is it okay like that?

I still have to clean the code up and make it a bit more readable before this is ready to be merged tho. (Argh.. and signing off the past commits. I always forget to do it.. I will add a signed-off git message template now 😄)

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

grobinson-grafana · 2024-05-29T16:09:29Z

config/notifiers.go

+	Title                string `yaml:"title,omitempty" json:"title,omitempty"`
+	Message              string `yaml:"message,omitempty" json:"message,omitempty"`
+	TitleURL             string `yaml:"title_url,omitempty" json:"title_url,omitempty"`
+	SkipFields           bool   `yaml:"skip_fields,omitempty" json:"skip_fields,omitempty"`


I think we should remove this as I haven't seen users asking for this in the past. It's much easier to add it later if we need. It's much harder to remove something once it's been added.

I kinda agree. On the other hand... #3821 wants to add them. So apparently users are asking for them. What do you think? I'm okay with both options. I'll let you guys - as the actual code owners - decide what direction to choose here. It would certainly reduce the huge amount of validation / truncation I had to add 😄 On the other hand - customizability is always a good thing I guess. @gotjosh what do you think?

grobinson-grafana · 2024-05-29T16:11:08Z

notify/discord/discord.go

+	// 2000 characters per message is the limit (not counting embeds).
+	maxMessageContentLength = 2000
+	// 10 embeds per Message is the maximum.
+	maxEmbedsPerMessage = 10


A concern I have here is that it's not uncommon for users to have large groups (more than 10 alerts). If Discord is limited to 10 embeds then this might make notifications less useful for those users as truncation will become a regular occurrence for them. What do you think @gotjosh?

grobinson-grafana · 2024-05-29T16:15:39Z

notify/discord/discord.go

 	data := notify.GetTemplateData(ctx, n.tmpl, as, n.logger)
 	tmpl := notify.TmplText(n.tmpl, data, &err)
 	if err != nil {
 		return false, err
 	}

-	title, truncated := notify.TruncateInRunes(tmpl(n.conf.Title), maxTitleLenRunes)
+	author, truncated := notify.TruncateInRunes(tmpl(n.conf.BotUsername), maxEmbedAuthorNameLenRunes)


Suggested change

author, truncated := notify.TruncateInRunes(tmpl(n.conf.BotUsername), maxEmbedAuthorNameLenRunes)

username, truncated := notify.TruncateInRunes(tmpl(n.conf.BotUsername), maxEmbedAuthorNameLenRunes)

grobinson-grafana · 2024-05-29T16:15:48Z

notify/discord/discord.go

-	if alerts.Status() == model.AlertFiring {
-		color = colorRed
+	w := webhook{
+		Username:  author,


Suggested change

Username: author,

Username: username,

grobinson-grafana · 2024-05-29T16:17:28Z

notify/discord/discord.go

+
+	alerts := types.Alerts(as...)
+
+	for alertIndex, alert := range alerts {


Can use idiomatic variable name i here:

Suggested change

for alertIndex, alert := range alerts {

for i, alert := range alerts {

grobinson-grafana · 2024-05-29T16:18:18Z

notify/discord/discord.go

+			break
+		}
+
+		title, truncated := notify.TruncateInRunes(tmpl(n.conf.Title), maxTitleLenRunes)


The title gets repeated for each embed which is kind of weird. For example: [FIRING:2] but each embed contains just one alert.

You're right. I'm gonna refactor this - using the embed title for displaying the alert summary (CommonAnnotations.summary) and adding the total alert count to the message above the embeds.

grobinson-grafana · 2024-05-29T16:23:08Z

I noticed that when the maximum number of embeds is exceeded, just the first embed is sent? Is that expected? Looking at the code I would expect 10 embeds to be sent with the truncation message rather than just 1 embed?

LucaDev force-pushed the main branch from 227592e to 064d99d Compare March 12, 2024 09:04

LucaDev marked this pull request as ready for review April 26, 2024 19:03

LucaDev force-pushed the main branch from a82bcd0 to 6277afd Compare May 23, 2024 15:38

LucaDev added 10 commits May 23, 2024 18:05

enhance discord integration

096ea99

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

fix UI build on apple silicon

e6cfc4f

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

extend discord notifier config options

00cc5d3

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

add new discord docs, fix skip_fields

29d6e56

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

add URL & templating support, improve validation

b16aa08

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

fix discord webhook sending

a41c6d6

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

allow alert grouping

4c72f1e

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

Enforce the limit of 10 embeds per message

c5f7187

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

minor code cleanup

f2bd09e

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

Fixed embed limit enforcement

20c9aea

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

LucaDev force-pushed the main branch from 656d2cf to 20c9aea Compare May 23, 2024 16:15

LucaDev added 3 commits May 23, 2024 18:23

fix formatting

48ba07a

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

Make sure all templating errors are handled correctly

5eab8d9

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

fixed some more formatting errors

4a6a4be

Signed-off-by: Luca Kröger <l.kroeger01@gmail.com>

grobinson-grafana reviewed May 29, 2024

View reviewed changes

grobinson-grafana mentioned this pull request May 30, 2024

Add discord message fields #3821

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extend Discord Notifier #3761

Extend Discord Notifier #3761

LucaDev commented Mar 12, 2024 •

edited

gotjosh commented Mar 12, 2024

LucaDev commented Mar 12, 2024

LucaDev commented Mar 12, 2024

LucaDev commented Mar 14, 2024

LucaDev commented Apr 26, 2024

LucaDev commented Apr 26, 2024

grobinson-grafana commented May 12, 2024

LucaDev commented May 23, 2024

grobinson-grafana May 29, 2024

LucaDev May 30, 2024 •

edited

grobinson-grafana May 29, 2024

grobinson-grafana May 29, 2024

grobinson-grafana May 29, 2024

grobinson-grafana May 29, 2024

grobinson-grafana May 29, 2024

LucaDev May 30, 2024

grobinson-grafana commented May 29, 2024

	author, truncated := notify.TruncateInRunes(tmpl(n.conf.BotUsername), maxEmbedAuthorNameLenRunes)
	username, truncated := notify.TruncateInRunes(tmpl(n.conf.BotUsername), maxEmbedAuthorNameLenRunes)


		alerts := types.Alerts(as...)

		for alertIndex, alert := range alerts {

	for alertIndex, alert := range alerts {
	for i, alert := range alerts {

Extend Discord Notifier #3761

Are you sure you want to change the base?

Extend Discord Notifier #3761

Conversation

LucaDev commented Mar 12, 2024 • edited

gotjosh commented Mar 12, 2024

LucaDev commented Mar 12, 2024

LucaDev commented Mar 12, 2024

LucaDev commented Mar 14, 2024

LucaDev commented Apr 26, 2024

LucaDev commented Apr 26, 2024

grobinson-grafana commented May 12, 2024

LucaDev commented May 23, 2024

grobinson-grafana May 29, 2024

Choose a reason for hiding this comment

LucaDev May 30, 2024 • edited

Choose a reason for hiding this comment

grobinson-grafana May 29, 2024

Choose a reason for hiding this comment

grobinson-grafana May 29, 2024

Choose a reason for hiding this comment

grobinson-grafana May 29, 2024

Choose a reason for hiding this comment

grobinson-grafana May 29, 2024

Choose a reason for hiding this comment

grobinson-grafana May 29, 2024

Choose a reason for hiding this comment

LucaDev May 30, 2024

Choose a reason for hiding this comment

grobinson-grafana commented May 29, 2024

LucaDev commented Mar 12, 2024 •

edited

LucaDev May 30, 2024 •

edited