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.

Sign up for GitHub

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

Jump to bottom

Add integration label to notification metrics #3056

Merged
merged 2 commits into from Aug 24, 2020
Merged

Add integration label to notification metrics #3056

merged 2 commits into from Aug 24, 2020

Conversation

gouthamve
Copy link
Contributor

Also, add a metric for when creation of rules manager would fail.

Signed-off-by: Goutham Veeramachaneni gouthamve@gmail.com

gotjosh
gotjosh approved these changes Aug 19, 2020
View reviewed changes
Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

Great catch LGTM! I'm assuming this was introduced upstream but given we have custom metric struct we need to manually update to be on par as I don't see where exactly is the integration being set.

pkg/alertmanager/multitenant.go Show resolved Hide resolved
pkg/ruler/manager.go Outdated Show resolved Hide resolved
pracucci
pracucci reviewed Aug 19, 2020
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
pkg/ruler/manager.go Outdated Show resolved Hide resolved
pkg/ruler/manager.go Outdated Show resolved Hide resolved
pkg/ruler/manager.go Outdated Show resolved Hide resolved
@gouthamve
Add integration label to notification metrics
950221f 
Also, add a metric for when creation of rules manager would fail.

Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
pracucci
pracucci approved these changes Aug 24, 2020
View reviewed changes
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.

LGTM (with a nit about the CHANGELOG entry). Thanks!

pkg/ruler/ruler.go Show resolved Hide resolved
@gouthamve
Address feedback
c3727b3 
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve gouthamve merged commit 2cb22f8 into cortexproject:master Aug 24, 2020
@gouthamve gouthamve deleted the metrics-for-ruler-rejig branch August 24, 2020 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gotjosh gotjosh gotjosh approved these changes

@pracucci pracucci pracucci approved these changes

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@gouthamve @gotjosh @pracucci