Add templated text support for webhook receivers #2184
Conversation
|
Thank you for your pull request. There is no consensus to change the current webhook body. You can append your input to #701. |
|
@roidelapluie I actually read that (note I referenced #1496 too!), this is not about integrating with tools that are not dedicated Prometheus alertmanager webhook receivers but about allowing webhook receivers to integrate closer with the rest of the Prometheus ecosystem. |
|
If you want to improve integration that's better approached at the config management level, not by adding more knobs that need configuration. |
|
@brian-brazil I'd agree in most cases, but I don't really see this about config management. For example right now because the PagerDuty integration supports templating built-in to alertmanager it's not possible to send the same formatted copy to another service that is only supported by webhooks, unless I reimplement that on the receiving side. (Or rather than implementing the native webhooks I just implement one of the other integrations that supports templating, which makes me sad when commercial products have better support...) |
|
What you're basically proposing is #701, but far more limited and I'd expect that over time we'd reinvent jsonnet. The place for this logic is inside the webhooks, not the Alertmanager configuration file. |
|
On 15 Feb 14:27, David Leadbeater wrote:
@brian-brazil I'd agree in most cases, but I don't really see this about config management.
For example right now because the PagerDuty integration supports templating built-in to alertmanager it's not possible to send the same formatted copy to another service that is only supported by webhooks, unless I reimplement that on the receiving side. (Or rather than implementing the native webhooks I just implement one of the other integrations that supports templating, which makes me sad when commercial products have better support...)
I would not recommend you to re-implement PD or another notifier. If
that 3rd party changes their API or go bankrupt, they *could* be
changed or removed in the alertmanager.
At this stage the webhook content is fixed. If we allow to have it
changed, it is opening a new range of feature requests like:
- custom headers
- custom handling of http error codes
- non-json payloads
- grpc support
and probably many other examples. I hope you can see that even it if
looks simple at first sight, it is not at the end.
|
I 100% agree, and I don't really intend to do it, but I'm pointing out the decision to not allow templating means this method (webhooks) is not on par with the other methods.
Custom headers are in my mind a totally different beast, any custom HTTP support is not something that lines up with webhooks where the expected use is delivery to a server designed for use only with alertmanager. How do we change the mind that the webhook content is fixed? (is it?, I only see notification methods being mentioned which makes sense) I really think even if #701 was implemented I'd still want this -- the point is to share go text templates with all possible notification methods, rather than write custom JSON. I'm happy to write a design doc. |
|
On 15 Feb 14:52, David Leadbeater wrote:
> I would not recommend you to re-implement PD or another notifier. If
> that 3rd party changes their API or go bankrupt, they *could* be
> changed or removed in the alertmanager.
I 100% agree, and I don't really intend to do it, but I'm pointing out the decision to not allow templating means this method (webhooks) is not on par with the other methods.
> At this stage the webhook content is fixed. If we allow to have it
> changed, it is opening a new range of feature requests like:
[snip list of things]
Custom headers are in my mind a totally different beast, any custom HTTP support is not something that lines up with webhooks where the expected use is delivery to a server designed for use only with alertmanager.
The point is that if we allow that flexibility, the webhooks will be used with
services not designed to work with alertmanager and there will be a demand
for other features like http headers etc.
If your endpoint is designed to used with alert manager then I think too
that it is a config management problem: it appears that you want to
avoid configuring templates on the custom receivers.
…--
(o- Julien Pivotto
//\ Open-Source Consultant
V_/_ Inuits - https://www.inuits.eu
|
I don't quite see how an extra field inside the existing structure is enabling that level of flexibility.
This also presumes a lot about an environment, in some cases/environments it may make sense to write a custom receiver in something other than Go, which won't have text/template, so how do I configure a template on something like that? |
|
On 15 Feb 15:13, David Leadbeater wrote:
> The point is that if we allow that flexibility, the webhooks will be used with
> services not designed to work with alertmanager and there will be a demand
> for other features like http headers etc.
I don't quite see how an extra field inside the existing structure is enabling that level of flexibility.
Adding an extra field is not going to help a lot with many use cases. I
was speaking about the general idea of changing the webhook.
> If your endpoint is designed to used with alert manager then I think too
> that it is a config management problem: it appears that you want to
> avoid configuring templates on the custom receivers.
This also presumes a lot about an environment, in some cases/environments it may make sense to write a custom receiver in something other than Go, which won't have text/template, so how do I configure a template on something like that?
python has jinja2
perl has templating modules
c++ has inja
java has freemarker
So not everything has to be go.
|
|
Sure, but alertmanager only supports Go's text/template, if this is a config management problem then it's just a case of sharing templates... except that isn't portable across template language, so it's not (just) a config management problem. |
|
I was thinking about something like this to help with templates for the GitLab integration. Rather than be a separate set of data, I was thinking about allowing the standard |
|
@SuperQ the problem I see with that is it complicates the data model further, the notification is potentially about multiple alerts. Do the templated fields get applied to each alert before CommonAnnotations are calculated? Which would imply the input to these templates is different to other notification methods. Having this as one top-level map means the template has control of how multiple alerts are templated and the templates execute in the same way as other methods. |
|
@dgl Good point, I hadn't thought about exactly how to apply it. |
|
@SuperQ I do like the concept of them being annotations though, I didn't really like the "templated" name I gave these, maybe calling them something like "templated_annotations" would help to make it clearer what they are and their limited scope. i.e. they aren't a way to implement a generic webhook which maybe would remove some opposition to them. |
This can be used to avoid webhook receivers potentially having to
reimplement the templating already done in alertmanager.
An example configuration using this would look something like:
receivers:
- name: webhook
webhook_configs:
- url: http://localhost:1111/alert
templates:
summary: "{{ .CommonLabels.severity }}: {{ .CommonLabels.alertname }}"
The webhook will then receive something like:
{...,"templated":{"summary":"page: Test"}}
'templated' is marked as "omitempty" to avoid adding this for receivers
who don't configure it, therefore keeping this fully backward
compatible.
Signed-off-by: David Leadbeater <dgl@dgl.cx>
|
I've updated this PR with some prompting from @SuperQ, I believe this is a good middle ground for making it possible to share templates between integrations, without needing to add all the integrations to alertmanager itself. (Still happy to bikeshed the name, I kept them as |
Currently a system implementing alertmanager webhook reception may have to also
reimplement templating, for example:
https://github.com/messagebird/sachet#message-templating
In addition to needing to reimplement the templating this results in the
templating configuration being split across several applications.
Right now it's possible to achieve some of this by implementing e.g. a
PagerDuty webhook rather than the generic webhook, but that's not ideal. This
seems like a nice compromise that keeps the webhook format simple (i.e. not
making it generic like #1496).
An example configuration using this would look something like:
The webhook will then receive something like:
{...,"templated":{"summary":"page: Test"}}'templated' is marked as "omitempty" to avoid adding this for receivers
who don't configure it, therefore keeping this fully backward
compatible.
Signed-off-by: David Leadbeater dgl@dgl.cx