Skip to content

Update AlertManager new notifiers recommendation#843

Merged
stuartnelson3 merged 1 commit intoprometheus:masterfrom
josedonizetti:update-am-notifiers-doc
Oct 6, 2017
Merged

Update AlertManager new notifiers recommendation#843
stuartnelson3 merged 1 commit intoprometheus:masterfrom
josedonizetti:update-am-notifiers-doc

Conversation

@josedonizetti
Copy link
Contributor

There are several PRs to add new notifiers on AlertManager, and usually, the team will answer that webhooks should be used for now, while a more flexible solution is planned. This PR updates the doc to reflect this opinion.

PRs of new notifiers:
prometheus/alertmanager#391
prometheus/alertmanager#600
prometheus/alertmanager#601
prometheus/alertmanager#1021
prometheus/alertmanager#885
prometheus/alertmanager#1003
prometheus/alertmanager#1012

@stuartnelson3

@stuartnelson3
Copy link
Contributor

@fabxc @brancz @brian-brazil

__Other receiver implementations available in version 0.0.4 of Alertmanager
are not implemented yet. We are gladly accepting any contributions to add them
to the new implementation.__
__We're not actively adding notifiers at the moment as we try to figure out something more flexible. For the time being, we recommend implementing custom notifiers via the webhook notifier.__
Copy link
Member

@brancz brancz Oct 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how much it's about being on the search for something more flexible, the jsonnet approach would definitely be more flexible but I also see it being more error prone and can easily get out of sync as users would need to update those themselves separate from an Alertmanager update.

I think this is primarily because we can't take on the additional maintenance burden, therefore we chose to support only the major providers out of the box, and additional support needs to be added through a webhook integration.

My 2 cents, but happy to hear what others have to say.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts are in line with @brancz

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool by me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if the best action is just to remove this section of the documentation. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave it given the frequency of requests, but remove the "as we try to figure out something more flexible. For the time being,"

@josedonizetti josedonizetti force-pushed the update-am-notifiers-doc branch from b12a385 to 70a3411 Compare October 6, 2017 05:22
@josedonizetti josedonizetti force-pushed the update-am-notifiers-doc branch from 70a3411 to 827b05d Compare October 6, 2017 05:25
@stuartnelson3 stuartnelson3 merged commit 6e5f7dc into prometheus:master Oct 6, 2017
aylei pushed a commit to aylei/docs that referenced this pull request Oct 28, 2019
* tools/lightning: the region-min-size config has been removed

* tools/lightning: included the IO concurrency settings and related metrics

* tools/lightning: removed mention of mixed deployment

* tools/lightning: added TiKV free space requirement

Added several FAQ entries explaining the tight requirements

* tools/lightning: removed one more sentence related to mixed deployment

* tools/lightning: stop using /tmp to store the engine files

* Apply suggestions from code review

Co-Authored-By: kennytm <kennytm@gmail.com>
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.

4 participants