Conversation
SuperQ
left a comment
SuperQ
left a comment
There was a problem hiding this comment.
Needs a rebase to pickup the slog migration changes.
Signed-off-by: UndeadDemidov <45305584+UndeadDemidov@users.noreply.github.com>
Signed-off-by: UndeadDemidov <45305584+UndeadDemidov@users.noreply.github.com>
Signed-off-by: UndeadDemidov <45305584+UndeadDemidov@users.noreply.github.com>
|
@SuperQ done with migrating to |
|
Hi @SuperQ, it would be helpful, if we can use this feature. |
|
Taking a look! 👀 |
|
Hey @grobinson-grafana - could this be merged or is there anything else blocking it? I would really appreciate this feature |
|
+1 on this being a great addition. Would love to utilize it :) Thanks for the implementation 🙇🏾 |
|
Hello ! Thanks everyone for the great work you do and @UndeadDemidov for writing this PR ! I see that alertmanager 0.29.0 first release candidate is out. Is it possible to add this PR in this 0.29.0 release ? It seems like nothing is blocking (reviewer @SuperQ approved, all checks have passed, no conflicts with base branch). It would be really nice to be able to use alertmanager cleanly with Mattermost without hacks. |
|
+1 waiting on this too |
|
+1 |
1 similar comment
|
+1 |
Signed-off-by: Ben Kochie <superq@gmail.com>
Signed-off-by: Ben Kochie <superq@gmail.com>
| [ http_config: <http_config> | default = global.http_config ] | ||
| ``` | ||
|
|
||
| #### `<attachment_config>` |
There was a problem hiding this comment.
Not related directly to this PR but I think in general we have potential for anchor link conflicts with generic names like this or <link_config> for PagerDuty, etc.
There was a problem hiding this comment.
Good catch. I don't want to block this PR on this. But we should go through the docs for this kind of problem.
Co-authored-by: Siavash Safi <git@hosted.run> Signed-off-by: Ben Kochie <superq@gmail.com>
| // request is the request for sending a Mattermost notification. | ||
| // https://developers.mattermost.com/integrate/webhooks/incoming/#parameters | ||
| type request struct { |
There was a problem hiding this comment.
nitpick: use payload instead of request:
| // request is the request for sending a Mattermost notification. | |
| // https://developers.mattermost.com/integrate/webhooks/incoming/#parameters | |
| type request struct { | |
| // payload is the payload sent to mattermost's incoming webhook API. | |
| // https://developers.mattermost.com/integrate/webhooks/incoming/#parameters | |
| type payload struct { |
| if n.conf.WebhookURL != nil { | ||
| url = n.conf.WebhookURL.String() | ||
| } else { | ||
| content, err := os.ReadFile(n.conf.WebhookURLFile) |
There was a problem hiding this comment.
I did not check yet, but do other notify integrations behave the same?
Basically configs overriding each other, if not then the behaviour should be documented and maybe log a warning message?
| return false, err | ||
| } | ||
|
|
||
| resp, err := n.postJSONFunc(ctx, n.client, url, &buf) |
There was a problem hiding this comment.
Including a timeout here might be a good idea. Example
There was a problem hiding this comment.
Is there maybe a way to use a default/global timeout when notify.PostJSON/notfiy.PostText is used. eg. by setting a Deadline it the context does not already have a Deadline.
13 participants
#1512
Here is Mattermost notifier. It supports Mattermost Webhooks according specs.
https://developers.mattermost.com/integrate/webhooks/incoming/
Tests and lints are passed.