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

Add discord message fields #3821

Closed

Conversation

yaskinny
Copy link

This PR adds some fields to Discord message like Content which is already mentioned in issue #3667.

Being able to set Content is really important to have since at this moment, there is no way to tag users in alerts. Default Discord
template embeds alerts and in Discord, ATM, tagging someone in embedded fields, does not tag them. It will just include user/role name without sending notification like when you get tagged in normal message.

Discord code already had content field but it's not used in the code nor config.

I added other fields like avatar url and username.

Below content are example configs and result:
Alertmanager receiver config:

receivers:
  - name: 'discord'
    discord_configs:
    - webhook_url: 'https://discord.com/api/webhooks/1153054938155450368/kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkEwc3KouoUWE4'
      content: '{{ template "my.content" . }}'
      avatar_url: 'https://www.tbstat.com/wp/uploads/2023/05/Fvz9hOIXwAEaIR8-669x675.jpeg'
      username: GOFW

Template:

{{- define "my.content" }}
tags: {{- range .Alerts }}
{{- if .Annotations.tags }}
{{- reReplaceAll "," " " .Annotations.tags }}
{{- end }}
{{- end }}
{{- end }}

Prometheus alert:

---
groups:
- name: watchdogs
  rules:
  - alert: WatchDog-1
    expr: vector(1)
    labels:
      severity: info
    annotations:
      summary: "WatchDog1"
      description: "FAKE"
      tags: '<@5132333333333336177>,<@5132333333333336177>'
        
  - alert: WatchDog-2
    expr: vector(2)
    labels:
      severity: critical
    annotations:
      summary: "WatchDog2"
      description: "FAKE"
      tags: '<@5132333333333336177>,<@5132333333333336177>'

Screenshot from 2024-04-26 20-04-24

@grobinson-grafana
Copy link
Contributor

@yaskinny can you signoff your commits? That will fix the DCO check 🙂

yaskinny added 2 commits May 23, 2024 12:05
Add Discord webhook content, avatar url and username fields

Signed-off-by: yaser <yaserkalali.work@gmail.com>
Add Username

Signed-off-by: yaser <yaserkalali.work@gmail.com>
@yaskinny yaskinny force-pushed the discord/improve-message-body branch from 3178f11 to 85d44df Compare May 23, 2024 08:36
@yaskinny
Copy link
Author

@grobinson-grafana

hey, sorry for the delay. i was busy.

I signed off my commits and pushed them again.

let me know if i need to do anything else.

Best Regards,
Yaser Kalali

@yaskinny
Copy link
Author

also it might be beneficial to edit the default discord template in a way that older config version(without username/content) works without any issue after updating to newer alertmanager version.

like making username/content optional in the template. since at this point, users need to rewrite their own template if they want to use username/content fields. with updating default template, it will has better UX since by default it will be included in the alerts.

@grobinson-grafana let me know if you want me to work on that or not.

@grobinson-grafana
Copy link
Contributor

also it might be beneficial to edit the default discord template in a way that older config version(without username/content) works without any issue after updating to newer alertmanager version.

like making username/content optional in the template. since at this point, users need to rewrite their own template if they want to use username/content fields. with updating default template, it will has better UX since by default it will be included in the alerts.

I'm not sure I understand. What are you suggesting?

@yaskinny
Copy link
Author

@grobinson-grafana

I mean the default Discord template does not include the content field.

{{ define "discord.default.title" }}{{ template "__subject" . }}{{ end }}
{{ define "discord.default.message" }}
{{ if gt (len .Alerts.Firing) 0 }}
Alerts Firing:
{{ template "__text_alert_list" .Alerts.Firing }}
{{ end }}
{{ if gt (len .Alerts.Resolved) 0 }}
Alerts Resolved:
{{ template "__text_alert_list" .Alerts.Resolved }}
{{ end }}
{{ end }}

It would be better if the default template used the content field to allow anyone who uses the content option to utilize it without having to write their own template from scratch. However, since older versions of Alertmanager do not include the content field, we need to make it optional (use a condition in the template to check if content is set before using it). By making it optional, users won't face errors after updating to the new Alertmanager version.

@grobinson-grafana
Copy link
Contributor

@grobinson-grafana

I mean the default Discord template does not include the content field.

{{ define "discord.default.title" }}{{ template "__subject" . }}{{ end }}
{{ define "discord.default.message" }}
{{ if gt (len .Alerts.Firing) 0 }}
Alerts Firing:
{{ template "__text_alert_list" .Alerts.Firing }}
{{ end }}
{{ if gt (len .Alerts.Resolved) 0 }}
Alerts Resolved:
{{ template "__text_alert_list" .Alerts.Resolved }}
{{ end }}
{{ end }}

It would be better if the default template used the content field to allow anyone who uses the content option to utilize it without having to write their own template from scratch. However, since older versions of Alertmanager do not include the content field, we need to make it optional (use a condition in the template to check if content is set before using it). By making it optional, users won't face errors after updating to the new Alertmanager version.

Is the suggestion to also replace the embeds with content? If not, what would the default template for content be and what would happen to the embeds?

@grobinson-grafana
Copy link
Contributor

You might also want to check out #3761 which extends the use of embeds.

@mogoll92
Copy link
Contributor

You might also want to check out #3761 which extends the use of embeds.

I think he meant don't touch embedded at all just to make content field configurable, atm we have it just in webhook struct but does use nowhere.

type webhook struct {
	Content string         `json:"content"`
	Embeds  []webhookEmbed `json:"embeds"`
}

It would be really useful as for some custom solutions you would like to mention users over user_id or role_id and make sure they got notification. In current implementation of discord embedded everything that is tagged inside embedded won't trigger discord notification.

featheredtoast added a commit to featheredtoast/alertmanager that referenced this pull request Oct 23, 2024
Allow for custom username and avatar URLs to be set in discord notifications.

Add `username` and `avatar_url` to discord configuration, default empty string.

Re-implement prometheus#3821

Signed-off-by: Jeff Wong <awole20@gmail.com>
featheredtoast added a commit to featheredtoast/alertmanager that referenced this pull request Oct 23, 2024
Allow for custom username and avatar URLs to be set in discord notifications.

Add `username` and `avatar_url` to discord configuration, default empty string.

Re-implement prometheus#3821

Signed-off-by: Jeff Wong <awole20@gmail.com>
gotjosh added a commit that referenced this pull request Oct 24, 2024
* Feat(discord):

Allow for custom username and avatar URLs to be set in discord notifications.

Add `username` and `avatar_url` to discord configuration, default empty string.

Re-implement #3821

Signed-off-by: Jeff Wong <awole20@gmail.com>

* Test the new fields

Signed-off-by: gotjosh <josue.abreu@gmail.com>

* These are not templeatable strings

Signed-off-by: gotjosh <josue.abreu@gmail.com>

---------

Signed-off-by: Jeff Wong <awole20@gmail.com>
Signed-off-by: gotjosh <josue.abreu@gmail.com>
Co-authored-by: gotjosh <josue.abreu@gmail.com>
@gotjosh
Copy link
Member

gotjosh commented Oct 24, 2024

Duplicate of #4007 and #4081

@gotjosh gotjosh closed this Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants