notify/discord: Create Discord integration#2948
Conversation
metalmatze
force-pushed
the
discord
branch
2 times, most recently
from
9787bd9 to
d52e1ce
Compare
|
This integration was heavily inspired by the work done in https://github.com/benjojo/alertmanager-discord |
|
Ah ha! Sorry for not getting around to the original, but thanks for doing this anyway! |
No worries. This is how it works in open source, I guess. |
gotjosh
left a comment
gotjosh
left a comment
There was a problem hiding this comment.
LGTM - I think the template can look better (e.g. better formatting when having multiple alerts and adding a link to the silences) but let's see what kind of feedback we get after people start using it.
I do have a request for the Avatar URL and optional override of username - can you please take a look at my comment?
Once, we finish the discussion around avatar and username I'll be happy to approve and merge.
There was a problem hiding this comment.
I know this might be a bit more work, but I'd love it if we can add two new options:
- Avatar URL
avatar_url - Username
username
(they belong on the main webhook object and not the embedded, see: https://discord.com/developers/docs/resources/webhook#execute-webhook)
For the sake of transparency, Grafana supports both of these options in its discord notifier and I'd love it if they could be uniform. Being conscious that we're all short on time, I'm happy to address my own comments on top of this PR in case you don't have the time to execute them.
There was a problem hiding this comment.
One thing about alertmanager-discord is that the username is for some use cases is set to "firing/resolved" because on push notifications, embeds don't show up as easily, so it's easier to work with at a glance
There was a problem hiding this comment.
I don't really have a strong opinion on this. Would the two of you be fine with taking this into an issue to follow up on and then decide with how to move forward outside this PR?
Overall the messages in Discord could look way better indeed. However, most of the notifications use the same default template for messages. Hence, I opted to use the same one. It's always possible to write custom templates and use them with alertmanager. |
Signed-off-by: Matthias Loibl <mail@matthiasloibl.com>
|
Are we waiting on something? Honestly don't remember. |
simonpasquier
left a comment
simonpasquier
left a comment
There was a problem hiding this comment.
Late to the party but lgtm. I suppose that we can address @gotjosh requests in a later PR?
Even if the current PR is limited, it's a good first step and nothing forbids adding new config options after it merges.
|
Does someone want to merge this? I can do that myself, but don't feel like that's a best practice. :) |
|
Hey! I'd like to hear from @gotjosh before merging. But since he's busy these days, I propose to merge it next week in case of no answer from him. |
|
Given it's been a lot more days, I'll go ahead and merge this. Let me know if there's something we should follow this up with! |
|
@metalmatze Hi, I just noticed that the docs for this are still missing at https://prometheus.io/docs/alerting/latest/configuration/ (source: https://github.com/prometheus/alertmanager/blob/main/docs/configuration.md). Could you add those as well? |
|
Hey, thanks for the ping. I bookmarked this issue in the hopes to remind myself in my free time to make the contribution. |
|
@metalmatze I can take a stab at it today if that's ok with you. If it remains undocumented, it's basically like the feature doesn't exist, so I think it's good to do it now :) |
|
PR out at #3201 |
7 participants
For our https://demo.parca.dev instance we want to alert to be send to Discord.
Therefore I've worked on integrating Discord directly into Alertmanager.
The integration doesn't need any external dependencies and uses JSON to send the payloads to Discord via webhooks to a specific channel.
This integration is already in use for our alerting for a week now and has been working great so far.