Repository navigation
Conversation
|
Hi Prometheus team and @tonyganch @tonyganch, first of all, thank you for working on this and opening this PR. I would like to ask the Prometheus team (@TheMeier and @SoloJacobs) about their plans regarding this PR and feature. Customizable embed colors are still missing in Alertmanager's Discord integration today, even though this is a very common and needed capability (especially for visually distinguishing alert severities like If the Prometheus team is on board with moving forward with this feature, I think the current implementation seems no longer compatible with Thanks! |
| {{ end }} | ||
| {{ define "discord.default.color" }} | ||
| {{ if gt (len .Alerts.Firing) 0 }} | ||
| 10038562 |
There was a problem hiding this comment.
Can we have a comment here what color it is, please?
There was a problem hiding this comment.
It's dark red (#992D22 / 0x992D22), which was the original colorRed constant in discord.go.
Will definitely add a clarifying comment like {{/* Red: #992D22 */}} during the rebase.
|
I think we can merge this if it is rebased and merge conflicts are resolved. |
Thanks for the feedback @siavashs I'll give @tonyganch a couple of days to see if he's available to rebase and address @ultrotter's comment. If he doesn't have the time or doesn't respond, I'll take it over and open a fresh, rebased PR against current |
|
@arsalanyavari Could you also describe how you intend to use this feature? Templating is always a bit more fragile than other approaches. Currently, we (the maintainers) couldn't come up with a better approach, and we are happy to land this PR if templating is the only way. But perhaps there is something with better ergonomics? For this we need to understand the use-case though. |
Thanks for the question @SoloJacobs! The main use case is visual triage by severity. Right now, every firing alert in Discord has a hardcoded red frame, so an informational notification (like a routine RAID check or backup notice) looks just as alarming as a critical outage. We'd like to be able to tell critical (red), warning (yellow), and info (grey/blue) apart at a glance to reduce alert fatigue. Templating makes sense here because different teams use different labels (some use For ergonomics, the main annoyance right now is having to calculate and write raw decimal integers like Does that sound like a reasonable direction? |
|
Hi @SoloJacobs (and @ultrotter), |
I've upgraded Discord notifier to accept
colorin template. It's the color of the embed frame, and right now there are only three options: red, green and grey. By passing the color via template, we allow developers to make better integrations. For example, one can choose different colors based on severity, or project, or any other logic that is useful to a team.https://discord.com/developers/docs/resources/channel#embed-object-embed-structure
colormust be of type integer, but in reality Discord accepts untrimmed strings as well, as long as they contain a 10-base integer.stringinstead ofint, and it has its own pros and cons, I'm hoping someone will advise me on the best approach. I haven't found a super easy way to get int out of template without some sort of string-to-int conversion. And with parsing, we need to know if its 10-base int inside a string or0x. And it seems like doing parsing on our side just adds more places where something might go wrong, and it doesn't feel worth it. Discord API works okay with parsing on their end as long as colors are 10-base. It's a simpler and safer approach in my opinion, so I did that.This is my first PR to alertmanager, so if there are any other requirements for the PR, i'll be happy to update it.
Also, I will be happy to add support for the whole
embedstruct based on current Discord API if this change is welcome, I believe this will give more freedom to developers to setup their integrations.Screenshot of the alert with custom color, and thumbnail (thumbnail is not part of this PR):
