alertmanager

alertmanager copied to clipboard

When writing custom templates, the current set of functions is fairly limited.

We could register the functions from https://github.com/Masterminds/sprig to offer the template authors an extensive set of helper functions. Docs of the functions is available here: https://masterminds.github.io/sprig/

If you are interested, I would be happy to send a patch.

A thumbsup does to not do justice to how badly I want this to be added.

I'll share what I wrote in #3770:

I've spent some time thinking about this and my preference is that we don't add Sprig. Instead, we implement template functions as needed. We can also implement the most common functions in prometheus/common so they can be used in both Prometheus and Alertmanager.

The reasons I don't think we should add Sprig are the following:

1. We need to maintain an allow list of functions that we consider safe, and disable functions that do filesystem operations, network calls, etc.
2. Sprig adds 11 new dependencies to Alertmanager. I would like to reduce the number of dependencies we have over time, not increase them.
3. We need to track Sprig for updates and vet changes to existing functions.
4. If Sprig is ever deprecated or archived we need to implement every single function ourselves anyway, because users now depend on them.

And just to make sure there is no misunderstanding, I think we do need to add more useful functions to templates. But I think we should add our own, and not be dependent on another package for it (whether Sprig or something else).

@grobinson-grafana makes sense.

Would it make sense to collect a subset of "safe" sprig functions that we could add as an initial set?

Hi! 👋 Please feel free to put together a list of template functions that you think are missing. These shouldn't be functions copied from Sprig verbatim, but think about the kind of functions you are missing when writing templates in Alertmanager. It might be easier to pretend that Sprig doesn't exist 🙂

@oncilla that approach is already implemented in proposed PR.

@TheMeier ack. But AFAICS, you are still importing sprig. If I understand @grobinson-grafana correctly, he wants to have an original implementation in alertmanager code, rather than importing it.

(I'm not agreeing with the point FWIW)

@oncilla I misunderstood your comment, makes sense now