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

Remove unused metrics exported by ring lifecycler and client #162

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

pracucci
Copy link
Contributor

What this PR does:
This PR proposes to remove some metrics exported by ring client and lifecycler. This proposed change originates from a discussion we had in grafana/mimir#1750 about some high cardinality metrics exposed by large Mimir clusters (running hundreds of instances).

In this PR I propose to remove the following metrics, which don't look to be much useful:

  • member_ring_tokens_owned: actual number of tokens (ring virtual nodes) registered by the ring lifecycler (e.g. 128)
  • member_ring_tokens_to_own: configured number of tokens (ring virtual nodes) in the ring lifecycler (e.g. 128)
  • ring_tokens_owned: exposes how many tokens (ring virtual nodes) that member has registered in the ring (e.g. 128)
  • ring_member_ownership_percent: exposes the % of tokens owned in the ring (exposed by web UI too)

I checked Mimir / Loki / Tempo mixins and I can't find any reference of these metrics.

Which issue(s) this PR fixes:
Part of grafana/mimir#1750

Checklist

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

Signed-off-by: Marco Pracucci <marco@pracucci.com>
Signed-off-by: Marco Pracucci <marco@pracucci.com>
@pracucci pracucci requested review from a team April 28, 2022 15:18
@bboreham
Copy link
Contributor

FWIW I used to watch the ring ownership when doing an ingester rollout back when it handed-off tokens from a leaving ingester to a joining one. But now we replace ingesters in-place and retain the same tokens it doesn't seem very interesting.

@pracucci
Copy link
Contributor Author

Got a LGTM on Slack from @owen-d (Loki) and @kvrhdn (Tempo). Going to merge it, but please reach out if this change is causing you any trouble and I can revert the metrics you need!

@pracucci pracucci merged commit 25baa36 into main Apr 29, 2022
@pracucci pracucci deleted the remove-ring-client-high-cardinality-metrics branch April 29, 2022 12:15
@pracucci pracucci mentioned this pull request Apr 29, 2022
3 tasks
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.

2 participants