-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add Slack additional "fields" to notifications #1135
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good. could you fix the merge conflicts?
notify/impl.go
Outdated
var fields = make([]slackAttachmentField, numFields) | ||
for k, v := range n.conf.Fields { | ||
fields[k] = slackAttachmentField{ | ||
v["title"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add tmplText()
around both title
and value
, so the users can optionally define a template
notify/impl.go
Outdated
@@ -669,8 +669,8 @@ func (n *Slack) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { | |||
var fields = make([]slackAttachmentField, numFields) | |||
for k, v := range n.conf.Fields { | |||
fields[k] = slackAttachmentField{ | |||
v["title"], | |||
v["value"], | |||
tmplText(v["title"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stuartnelson3 is this what you meant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, exactly
Closes #731 |
This capability isn't documented anywhere on the Prometheus website, despite the latest release being from 13 days ago. see: https://prometheus.io/docs/alerting/configuration/#%3Cslack_config%3E Am I looking in the wrong place, or should the documentation be updated? |
@JeanMertz i think the doc just needs to be updated. i'll see if i can contrib that |
@JeanMertz @rbtr yeah the docs need to be updated |
Had a go at implementing #731