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

Fixes the registration of the Alertmanager alert receiving API metrics #3065

Merged
merged 5 commits into from
Aug 24, 2020

Conversation

gotjosh
Copy link
Contributor

@gotjosh gotjosh commented Aug 20, 2020

What this PR does:

Fixes the registration of the Alertmanager API metrics for receiving alerts. Additionally, it removes a couple of TODOs I noticed along the way - they should be a no-op.

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Contributor

@pracucci pracucci left a comment

Choose a reason for hiding this comment

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

Oh yes, very good! I'm familiar with the problem (I submitted the PR prometheus/alertmanager#2200) and your changes LGTM. Thanks!

@gouthamve
Copy link
Contributor

We need to make sure that two instances of AM are not created, but can't see that anywhere in the code, so we're good!

@gouthamve gouthamve merged commit a87c25f into cortexproject:master Aug 24, 2020
@gotjosh
Copy link
Contributor Author

gotjosh commented Aug 24, 2020

We need to make sure that two instances of AM are not created

Even if that would happen we only ever keep a singleton of the metrics.

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

Successfully merging this pull request may close these issues.

3 participants