new argument to alert the channel on critical events #43
Conversation
Signed-off-by: sabbene <[email protected]>
Signed-off-by: sabbene <[email protected]>
sabbene
force-pushed
the
alert-on-critical
branch
from
ed45181 to
28cef10
Compare
There was a problem hiding this comment.
Thank you for the PR! I didn't test it, but I think I've identified some changes that should be made.
-
we should use the official golangci-lint action, not the one made by matoous, which has been deprecated
-
the environment variable should be called SLACK_ALERT_ON_CRITICAL, not SENSU_SLACK_ALERT_CRITICAL, just for the sake of consistency with the other variables.
Thanks!
.github/workflows/lint.yml
Outdated
| @@ -13,4 +13,4 @@ jobs: | |||
| - name: Checkout code | |||
| uses: actions/checkout@v2 | |||
| - name: Run golangci-lint | |||
| uses: actions-contrib/golangci-lint@v1 | |||
| uses: matoous/golangci-lint-action@v1.23.3 | |||
There was a problem hiding this comment.
It seems that the official action is to be found here https://github.com/golangci/golangci-lint-action
matoous/golangci-lint-action appears to be deprecated
There was a problem hiding this comment.
Sure. I have updated this in the latest push. Thanks!
README.md
Outdated
| @@ -53,6 +54,7 @@ Flags: | |||
| |--username |SLACK_USERNAME | | |||
| |--icon-url |SLACK_ICON_URL | | |||
| |--description-template |SLACK_DESCRIPTION_TEMPLATE | | |||
| |--alert-on-critical |SENSU_SLACK_ALERT_CRITICAL | | |||
There was a problem hiding this comment.
To be consistent with the naming of the other environment variables, it appears that this variable should be named "SLACK_ALERT_ON_CRITICAL".
There was a problem hiding this comment.
I have updated this in the latest push. Thanks!
main.go
Outdated
| @@ -89,6 +93,15 @@ var ( | |||
| Usage: "The Slack notification output template, in Golang text/template format", | |||
| Value: &config.slackDescriptionTemplate, | |||
| }, | |||
| { | |||
| Path: alertCritical, | |||
| Env: "SLACK_ALERT_CRITICAL", | |||
There was a problem hiding this comment.
Should be SLACK_ALERT_ON_CRITICAL as mentioned above.
There was a problem hiding this comment.
I have updated this in the latest push. Thanks!
main.go
Outdated
| @@ -111,6 +124,9 @@ func checkArgs(_ *corev2.Event) error { | |||
| if icon := os.Getenv("SENSU_SLACK_ICON_URL"); icon != "" && config.slackIconURL == defaultIconURL { | |||
| config.slackIconURL = icon | |||
| } | |||
| if alert := os.Getenv("SENSU_SLACK_ALERT_CRITICAL"); alertCritical != "" && !config.slackAlertCritical { | |||
There was a problem hiding this comment.
The manual environment variable lookups are technical debt and this code should probably be deleted at some point. No need to add this manual lookup, environment variables are handled transparently by the plugin SDK.
There was a problem hiding this comment.
Sure I have removed this in the latest push. Thanks!
| @@ -164,7 +180,11 @@ func messageStatus(event *corev2.Event) string { | |||
| case 0: | |||
| return "Resolved" | |||
| case 2: | |||
| return "Critical" | |||
| if config.slackAlertCritical { | |||
| return "<!channel> Critical" | |||
There was a problem hiding this comment.
Perhaps I'm missing something, but shouldn't this be @channel Critical? Or does this somehow get translated into an @?
There was a problem hiding this comment.
This somehow gets translated to @channel
https://slack.com/help/articles/202009646-Notify-a-channel-or-workspace#u64channel
Signed-off-by: sabbene [email protected]
Hello,
This pull request is to introduce a new feature. I have added the argument --alert-on-critical to sensu-slack-handler. The purpose of this new argument is to give people the option to have @channel added to the alert sent to slack.
Currently we are appending @channel to the output of our checks to get slack notifications for critical events, but this looks silly for other alerting mechanisms. For example getting an email with @channel in the body.
Here is a screen shot of how the alerts look with the new argument:
I also set up the github actions for this repo in my fork, and noticed that the Lint action was no longer working. I updated the 'Run golangci-lint' step to use the latest release of the tool (v1.23.3).
Please let me know if there are any questions or concerns.