Skip to content
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

Added functionality to alter Webhook message #1422

Conversation

victor-cr
Copy link

To extend Webhook functionality it is nice to have a possibility to change JSON via template or static text.

@brian-brazil
Copy link
Contributor

Thanks for your PR, can you explain what you're trying to do here?

@victor-cr
Copy link
Author

I wanted to have control on the message body of Webhook. So I added an extra field into webhook_config to allow people modify current JSON to the format external system is expecting. With the change you are able to either put static text as message body or generate custom JSON by templates.

@brian-brazil
Copy link
Contributor

The intended way to do arbitrary integrations is to write something designed to take in the webhook, and then it will send out an appropriately encoded request to the system you wish to integrate with. It's not intended that the webhhook will directly integrate with arbitrary other protocols, as that would be intractable from a maintenance standpoint.

@victor-cr
Copy link
Author

I understand that it may be not the purpose of the alert manager. However, I think of several reasons why it might make sense:

  1. Other configurations like Slack, Email, PagerDuty have an ability to change message content
  2. Sometimes alert data contains sensitive information you would like to not to share for all webhooks
  3. This change will help people to remove extra layers of integration software between alert system and the target system with already implemented RESTful API (like JIRA, Confluence, etc)
  4. It is still optional. If configuration does not declare the field "message" it will work as you expect

@brian-brazil
Copy link
Contributor

Other configurations like Slack, Email, PagerDuty have an ability to change message content

They do, but creating a generic integration that can integrate with anything is far more involved than what this PR would add. We have chosen that our integration point is the webhook as-is, and do not wish to have more than one generic integration as that'd kinda defeats the purpose of having a single generic integration. Unfortunately we can't support everything out of the box.

Sometimes alert data contains sensitive information you would like to not to share for all webhooks

If the data is that sensitive, it would be unwise to send it to the Alertmanager at all as it's all acessible via the UI. Webhooks only get the alerts that are routed to them.

@victor-cr
Copy link
Author

Fine, I am not insisting on the change, it was quite helpful to me and I just wanted to share so it may be useful for others. If you consider that the change is not proper in terms of design view - it should be rejected.
Anyway, I am grateful for your time and patience.

@simonpasquier
Copy link
Member

@victor-cr thanks for your understanding. Based on the discussion, I'm closing this PR in order to keep the list of open requests up-to-date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants