Vendor prometheus 2.12.0 and alertmanager 0.19.0#1597
Vendor prometheus 2.12.0 and alertmanager 0.19.0#1597bboreham merged 13 commits intocortexproject:masterfrom
Conversation
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
|
The |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
codesome
force-pushed
the
vendor-prometheus
branch
from
b5a0203 to
e016238
Compare
|
With the last commit I believe I fixed all alertmanager API changes. But now alertmanager 0.18.0 uses an older version of prometheus/common (0.4.1), and is not compatible with the latest (0.6.0) which the prometheus pulls. It is fixed in alertmanager master, but vendoring that is downgrading prometheus unusually. |
|
Opened this PR prometheus/alertmanager#2008 to fix the prometheus downgrade. |
jtlisi
left a comment
jtlisi
left a comment
There was a problem hiding this comment.
The function to register the routes needs to be changed, otherwise this looks good to me.
pkg/alertmanager/alertmanager.go
Outdated
| webReload := make(chan chan error) | ||
| ui.Register(am.router.WithPrefix(am.cfg.ExternalURL.Path), webReload, log.With(am.logger, "component", "ui")) | ||
| am.api.Register(am.router.WithPrefix(path.Join(am.cfg.ExternalURL.Path, "/api/v1"))) | ||
| am.api.Register(am.router, path.Join(am.cfg.ExternalURL.Path, "/api/v1")) |
There was a problem hiding this comment.
This Register will handle registering the correct api prefix, no need to pass this.
| am.api.Register(am.router, path.Join(am.cfg.ExternalURL.Path, "/api/v1")) | |
| am.api.Register(am.router, am.cfg.ExternalURL.Path) |
|
|
||
| // buildReceiverIntegrations builds a list of integration notifiers off of a | ||
| // receiver config. | ||
| // Taken from https://github.com/prometheus/alertmanager/blob/94d875f1227b29abece661db1a68c001122d1da5/cmd/alertmanager/main.go#L112-L159. |
There was a problem hiding this comment.
We should look into exporting this upstream function if we can.
There was a problem hiding this comment.
Looking at old alertmanager releases, it was actually exported in some different package. It was un-exported and moved there in recent releases. I will attempt exporting from alertmanager.
There was a problem hiding this comment.
Tried to export it in upstream by putting it in the right place (alertmanager/notify), but running into a lot of cyclic import conflicts (tried to remove most, but hit a dead-end to fix it without duplicating some code). If its fine we could leave it as it is here.
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
codesome
changed the title
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
codesome
force-pushed
the
vendor-prometheus
branch
from
ea93970 to
0934e8e
Compare
|
This PR is ready for another review now. I have vendored prometheus 2.12.0 and alertmanager 0.19.0-rc.0 (will do 0.19.0 in this PR when it's out but I don't expect it to be different), and finally no conflicts with prometheus 2.12.0! |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
|
Updated alertmanager to 0.19.0 (no changes from rc0) and addressed all reviews. Waiting to cut 0.2 I believe. |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
bboreham
left a comment
bboreham
left a comment
There was a problem hiding this comment.
Dropping -alertmanager.configs.auto-slack-root needs to go in CHANGELOG as a breaking change.
Also I would list this update to 2.12.0 / 0.19.0 in CHANGELOG as it will be of interest to consumers.
|
@bboreham Done |
Signed-off-by: Ganesh Vernekar <cs15btech11018@iith.ac.in>
codesome
force-pushed
the
vendor-prometheus
branch
from
06b89b8 to
0200fa1
Compare
|
Was it essential here to update jsoniter ? |
|
Did not really see into that and did not expect the bugs that it's causing. Sorry about that! I see @gouthamve already has some advancements on this prometheus/prometheus#6030 |
4 participants
With this, we no longer vendor prometheus/tsdb repo as it was merged with prometheus.
prometheus/alertmanager is also upgraded to 0.18.0 as a part of this. I have managed to fix all the API changes except 1, still trying to figure out what has to be done for that.