-
-
Notifications
You must be signed in to change notification settings - Fork 592
feat(alerting): Add message-content parameter for Discord pings #1335
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
Conversation
|
I've been thinking about this for a couple minutes, does calling this My concern here is that for most people, they may run into this issue (discord's embeds not pinging users), which means that when it does actually matter (i.e. there's an outage and the alert went out), they won't get the notification. Sure, adding For that reason, I think perhaps the right approach is to abstract the complexity of needing to specify What do you think? Does it make sense? |
I've seen it called prefix-message before in other projects, but I'm by no means married to that term. No one accused me of being creative with names LOL. I definitely see your point though, it's a little annoying having to add a separate field just for pings and not being pinged in the description. For me personally, I would rather just add the ping message than have the default description just copy-pasted multiple times over with the small modification of adding the ping that I want. It might also not be the most elegant approach to extract pings. I'm personally more partial to keeping this approach and documenting it very well to make sure the user knows that pings don't work in the description section and they're expected to use prefix-message or whatever we end up calling it. Also just having the option to add an out-of-embed message, whatever it may be, might be useful for parsing or bot integrations. I'm fine with having the pings extracted out of the embeds but it wouldn't be my preference. |
|
I'm not super against it either, but I'm not a fan of prefix-message, however nitpicky that might sound. Maybe we should just name it I thought just extracting the pings/mentions would make sense, because there's probably many alerting providers (e.g. slack, teams, etc) that have a similar behavior (no pings in embed), so taking the ping extraction approach might save us having to add new fields to multiple providers |
Lol we can just call it I'm not sure how other providers like Teams would work, but if we were to extract pings, that logic would probably differ between providers so we'd likely have to modify all the providers anyways to extract pings. For example for discord we can use Again not totally against it but not my preference, if you'd prefer I try something like that let me know and I can give it a shot |
|
message-content sounds good to me! Just make sure to update the README to mention why this is needed (because pings work elsewhere) |
Done! Glad we didn't have to get creative 😂 |
|
@aaldebs99 Thank you for the contribution! |
Thank you for the great app! |
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [ghcr.io/twin/gatus](https://github.com/TwiN/gatus) | minor | `v5.26.0` -> `v5.27.0` | --- ### Release Notes <details> <summary>TwiN/gatus (ghcr.io/twin/gatus)</summary> ### [`v5.27.0`](https://github.com/TwiN/gatus/releases/tag/v5.27.0) [Compare Source](TwiN/gatus@v5.26.0...v5.27.0) #### What's Changed - feat(alerting): Add message-content parameter for Discord pings by [@​aaldebs99](https://github.com/aaldebs99) in [#​1335](TwiN/gatus#1335) - feat(ui): Make tooltips toggleable by [@​perfectra1n](https://github.com/perfectra1n) in [#​1236](TwiN/gatus#1236) - fix(alerting): remove discontinued jetbrains space alerting provider by [@​michael-baraboo](https://github.com/michael-baraboo) in [#​1329](TwiN/gatus#1329) - fix(ui): Handle refresh properly on SuiteDetails.vue by [@​TwiN](https://github.com/TwiN) in [#​1324](TwiN/gatus#1324) - fix(key): Support `(`, `)`, `+` and `&` as name/group by [@​TwiN](https://github.com/TwiN) in [#​1340](TwiN/gatus#1340) #### New Contributors - [@​aaldebs99](https://github.com/aaldebs99) made their first contribution in [#​1335](TwiN/gatus#1335) **Full Changelog**: <TwiN/gatus@v5.26.0...v5.27.0> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xMzUuNCIsInVwZGF0ZWRJblZlciI6IjQxLjEzNS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJpbWFnZSJdfQ==--> Reviewed-on: https://gitea.alexlebens.dev/alexlebens/infrastructure/pulls/1792 Co-authored-by: Renovate Bot <[email protected]> Co-committed-by: Renovate Bot <[email protected]>

Summary
Relevant issue: #1091
This PR allows the addition of a "prefix-message" to discord alerts. Discord doesn't allow pinging users/roles within embeds so the prefix-message aims to bring that functionality by sending a
Contentmessage alongside the embed that allows user/roles ping.Checklist
README.md, if applicable.