feat(slack): add support for top-level text field in Slack notificati… #4867
Conversation
…ons (prometheus#3071) Signed-off-by: mihir-dixit2k27 <[email protected]>
mihir-dixit2k27
force-pushed
the
feat-slack-text-field
branch
from
7f1af86 to
9669a53
Compare
Signed-off-by: mihir-dixit2k27 <[email protected]>
mihir-dixit2k27
force-pushed
the
feat-slack-text-field
branch
9 times, most recently
from
9f1f2ab to
95dc178
Compare
Signed-off-by: mihir-dixit2k27 <[email protected]>
mihir-dixit2k27
force-pushed
the
feat-slack-text-field
branch
from
95dc178 to
92a9b2d
Compare
go.sum
Outdated
| github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= | ||
| github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= | ||
| github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= | ||
| github.com/go-kit/log v0.1.0 h1:DGJh0Sm43HbOeYDNnVZFl8BvcYVvjD5bqYJvp0REbwQ= |
There was a problem hiding this comment.
Are these changes to go.mod unrelated?
There was a problem hiding this comment.
You are absolutely correct—those changes were unintended updates caused by my local environment. I have reverted go.mod and go.sum to match main, so this PR now remains focused purely on the new feature
| TitleLink string `yaml:"title_link,omitempty" json:"title_link,omitempty"` | ||
| Pretext string `yaml:"pretext,omitempty" json:"pretext,omitempty"` | ||
| Text string `yaml:"text,omitempty" json:"text,omitempty"` | ||
| Message string `yaml:"message,omitempty" json:"message,omitempty"` |
There was a problem hiding this comment.
This needs to be added to configuration.md. Also having Title, Pretext, Text, and then Message is slightly confusing, and people may wonder what's going on, especially as in the code you then populate request.Text with the contents of Message...
There was a problem hiding this comment.
I have added the missing entry to configuration.md as requested.
Regarding the naming: I used Message in the Go struct (mapped to text in JSON) because the field name Text is already reserved for the text inside an attachment. I wanted to avoid collision or confusion between the two. The updated documentation now clarifies that this specific field populates the top-level text field in the payload.
notify/slack/slack_test.go
Outdated
| u, _ := url.Parse(server.URL) | ||
| conf := &config.SlackConfig{ | ||
| APIURL: &config.SecretURL{URL: u}, | ||
| Message: "My Top Level Message", |
There was a problem hiding this comment.
What happens when you set more of those configs? Also can we attach a screenshot of how the slack messages are without and with this new field?
There was a problem hiding this comment.
I have updated the unit test TestSlackMessageField to specifically verify this scenario. The test now configures both the new Message field and existing fields (like Title and Attachments) simultaneously.
It asserts that the resulting JSON payload correctly contains the top-level text field and the attachments array side-by-side. Since I am working in a headless environment, I cannot easily generate a UI screenshot, but the updated test confirms that the payload structure is valid and preserves all existing fields.
mihir-dixit2k27
force-pushed
the
feat-slack-text-field
branch
from
7512848 to
e3a2f93
Compare
Signed-off-by: Mihir Dixit <[email protected]>
mihir-dixit2k27
force-pushed
the
feat-slack-text-field
branch
from
8ed3af5 to
7822d5b
Compare
ultrotter
left a comment
ultrotter
left a comment
There was a problem hiding this comment.
Besides the go.mod changes in the diff, LGTM. Can you remove that please?
Signed-off-by: Mihir Dixit <[email protected]>
mihir-dixit2k27
force-pushed
the
feat-slack-text-field
branch
from
c9c940c to
d62cd5d
Compare
Spaceman1701
left a comment
Spaceman1701
left a comment
There was a problem hiding this comment.
What does the resulting slack message look like when both text and message are set?
docs/configuration.md
Outdated
| [ icon_url: <tmpl_string> ] | ||
| [ link_names: <boolean> | default = false ] | ||
| # The text content of the Slack message. | ||
| # If set, this is sent as the top-level 'text' field in the Slack payload. |
There was a problem hiding this comment.
would you mind adding a link to the slack docs here? Most people will not know the difference between the attachment's text and the message's text.
There was a problem hiding this comment.
@Spaceman1701 Thanks for the review!
Documentation: I have just pushed an update to configuration.md that adds the link to the Slack API reference as requested.
Behavior: Regarding your question on what happens when both are set:
The new message field populates the top-level text field in the JSON payload (the main notification body).
The existing fields continue to populate the attachments array.
Slack renders both simultaneously: the top-level text appears first, followed by the color-coded attachment block. The unit test TestSlackMessageField verifies that the payload structure correctly includes both side-by-side.
Ready for another look!
Add link to Slack API documentation in configuration.md to clarify the difference between top-level text and attachment text. Signed-off-by: Mihir Dixit <[email protected]> Signed-off-by: mihir-dixit2k27 <[email protected]>
3 participants
Closes #3071
Changes:
messagefield toSlackConfig.textfield in the Slack JSON payload.Reasoning:
This allows users to send simple text messages compatible with Slack Workflow Webhooks, which cannot parse attachments.