Extend discord messages with content

[mogoll92]

These changes are aimed to support discord content parameter besides with embedded message and allow to overcome limitation when you need to ping someone or group of people. (Ping over embedded messages doesn't trigger discord notifications)

Alertmanager already has struct that allows to send content.

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

These changes make content optional and allow to send any text that doesn't break discord limits with embedded message.
To enable this user should apply the following changes:

receivers:
  - name: nothing
  - name: discord-success
    discord_configs:
      - webhook_url: ""
        content: '{{ template "discord.content" . }}'
        title: '{{ template "discord.title" . }}'
        message: '{{ template "discord.message" . }}'

and override text in custom template like the following.

{{ define "discord.content" }}
Hey you! <@6532081833221312329>
{{ end }}

Here is the result.

[grobinson-grafana]

Hello! 👋 I think this might be a duplicate of #3821?

[mogoll92]

Hello! 👋 I think this might be a duplicate of #3821?

Yeah, I saw that PR but looks like author is not interested in pushing it forward and I really don't see value in making that additional changes with avatar and username. The only feature that makes sense there is content that is really needed in cases there you need to ping ppl outside.

[grobinson-grafana]

Sounds good to me! Before I review, it looks like you have somehow brought in a number of additional commits that aren't relevant to your PR. For example, commits fbcb49b and c07eec8. Can you fix your PR to remove those additional commits? I think just we need commits ae07f25 and 5875042.

[grobinson-grafana]

Hi, looks like you added even more commits?

[mogoll92]

Hi, looks like you added even more commits?

yeah, a bit struggled on replacing with needed set of commits. I will ping you here once I'm done.

[mogoll92]

@grobinson-grafana check if nothing needs to add, please.

[featheredtoast]

Is this still being looked at? I am interested in the avatar/username fields, and it'd be nice for this work to be in place before chipping away at adding other fields.

[grobinson-grafana]

You want to update DefaultDiscordConfig on Line 46 of config/notifiers.go so this template is used.

[grobinson-grafana]

I've tested it and looks good to me! I've left a comment of something small that's missing. Please also add some documentation for this in docs/configuration.md.

Something that I'm still not 100% satisfied with is the intuitiveness of title, message and content. It seems like users will be confused when to use title and message and when to use content.

Since the use case here is mentioning users, and nothing else, it might make more sense to do the following instead:

receivers:
  - name: discord
    discord_configs:
      - webhook_url: https://discord.com/api/webhooks/abc
        title: '{{ template "discord.title" . }}'
        message: '{{ template "discord.message" . }}'
        mentions:
          - 6532081833221312329

This will avoid the confusion and be more intuitive to users while still solving the intended use case.

[mogoll92]

I've tested it and looks good to me! I've left a comment of something small that's missing. Please also add some documentation for this in docs/configuration.md.

Something that I'm still not 100% satisfied with is the intuitiveness of title, message and content. It seems like users will be confused when to use title and message and when to use content.

Since the use case here is mentioning users, and nothing else, it might make more sense to do the following instead:

receivers:
  - name: discord
    discord_configs:
      - webhook_url: https://discord.com/api/webhooks/abc
        title: '{{ template "discord.title" . }}'
        message: '{{ template "discord.message" . }}'
        mentions:
          - 6532081833221312329

This will avoid the confusion and be more intuitive to users while still solving the intended use case.

It will confuse even more, as there is no mentions entity in discord, mentions is some kind of feature that you can use in context, but it isn't the same. Overall in discord the embedded message it's a part of the message and can't exist separately. If you would like to have mirror between alertmanager discord configuration and discord's entities I would recommend the following structure.

receivers:
  - name: discord
    discord_configs:
      - webhook_url: https://discord.com/api/webhooks/abc
        context: '{{ template "discord.content" . }}'
        embedded:
            title: '{{ template "discord.title" . }}'
            message: '{{ template "discord.message" . }}'

In this case it represent exactly what it is. Also this structure could be extended with other fields that discord supports.

[grobinson-grafana]

If you would like to have mirror between alertmanager discord configuration and discord's entities I would recommend the following structure.

The problem we have is that renaming/moving/deleting existing fields creates a lot of complex migration issues for other software that builds on top of Alertmanager, like Cortex and Mimir. We can add new fields, but making breaking changes to existing ones is hard.

Could you please update the documentation in docs/configuration.md for this new field? We can then merge it and add it to the 0.28 release.

[gotjosh]

LGTM

I'll add the documentation in a follow up PR.

[gotjosh]

Hey @mogoll92 I'll give you about ~30 minutes or so to see if you can get it sorted and update the documentation. Otherwise, I'll proceed with #4080 unless you feel oppose to it.

[mogoll92]

Hey @mogoll92 I'll give you about ~30 minutes or so to see if you can get it sorted and update the documentation. Otherwise, I'll proceed with #4080 unless you feel oppose to it.

if it's okay to you feel free to proceed with it, no objections at all.

[gotjosh]

Can you rebase and update the vfs_asserts?

[gotjosh]

Thank you very much for your contribution!

[mogoll92]

Thank you very much for your contribution!

you are welcome, first of all I need those changes so I'm interested in it. Thought it never will be happened since so much time passed with no response :)