Skip to content

Publish ruler's ring metrics #3074

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
Merged
merged 1 commit into from
Aug 25, 2020
Merged

Publish ruler's ring metrics #3074

merged 1 commit into from
Aug 25, 2020

Conversation

lahsivjar
Copy link
Contributor

@lahsivjar lahsivjar commented Aug 21, 2020 •
edited
Loading

Signed-off-by: lahsivjar [email protected]

What this PR does: Currently ruler doesn't publish it's ring metrics since it is not registered. Also, the ruler ring is currently named as ring which has been changed in this MR to ruler so that the metrics is understandable.

Checklist

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

@lahsivjar lahsivjar marked this pull request as draft August 21, 2020 16:18
@lahsivjar lahsivjar marked this pull request as ready for review August 22, 2020 01:29
@lahsivjar
Copy link
Contributor Author

@gouthamve Please take a look at the PR whenever you can

pstibrany
pstibrany reviewed Aug 25, 2020
View reviewed changes
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! It looks good, with tiny minor comments.

Would you please rebase this PR on top of master, as there were some conflicting changes in gossip integration test? Thanks!

@pull-request-size pull-request-size bot added size/S and removed size/M labels Aug 25, 2020
@lahsivjar
Copy link
Contributor Author

Would you please rebase this PR on top of master, as there were some conflicting changes in gossip integration test?

@pstibrany Fixed all the comments and rebased.

pstibrany
pstibrany approved these changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@pstibrany pstibrany left a comment

Choose a reason for hiding this comment

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

Thank you!

bboreham
bboreham approved these changes Aug 25, 2020
View reviewed changes
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

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

Thanks!

pracucci
pracucci approved these changes Aug 25, 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.

Yes yes yes, thanks!

@pracucci pracucci merged commit 0602b9e into cortexproject:master Aug 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@pstibrany pstibrany pstibrany approved these changes

@pracucci pracucci pracucci approved these changes

@bboreham bboreham bboreham approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lahsivjar @pstibrany @pracucci @bboreham