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

ruler: add /ruler/tenants service API #10738

Merged
merged 3 commits into from
Feb 26, 2025
Merged

ruler: add /ruler/tenants service API #10738

merged 3 commits into from
Feb 26, 2025

Conversation

narqo
Copy link
Contributor

@narqo narqo commented Feb 25, 2025

What this PR does

This PR adds a new /ruler/tenants endpoint to list tenants, the ruler service discovered in its storage. The idea here is to provide an external operator-component a way to observe which tenants have rules, without asking Mimir to retrieve the actual rules.

The API endpoint is complementary to the existing /store-gateway/tenants, /compactor/tenants, and /ingesters/tenants.

Checklist

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

Signed-off-by: Vladimir Varankin <[email protected]>
@narqo narqo requested review from a team as code owners February 25, 2025 12:12
@narqo narqo force-pushed the vldmr/ruler-api-tenants branch from b58a13f to 15d9580 Compare February 25, 2025 12:13
Signed-off-by: Vladimir Varankin <[email protected]>
@narqo narqo force-pushed the vldmr/ruler-api-tenants branch from 15d9580 to cd9853e Compare February 25, 2025 12:14
Copy link
Contributor

@colega colega left a comment

Choose a reason for hiding this comment

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

Can you add a couple of tests? Especially for requests sending JSON.

It's not clear from the code that we're creating a contract on the contents of the JSON returned.

for _, g := range rg {
groups = append(groups, ruleGroupData{
Name: g.GetName(),
Namespace: g.GetNamespace(),
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we include RuleCount: len(g.Rules) here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually wanted to do the same initially, but len(g.Rules) doesn't have any rules until we ask the Ruler to go to storage, and to populate them for every namespace and group — via the RuleStore.LoadRuleGroups — this is confusing, yes. I'd skip doing that (at least for now).

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, okay!

Signed-off-by: Vladimir Varankin <[email protected]>
@narqo
Copy link
Contributor Author

narqo commented Feb 25, 2025

Can you add a couple of tests? Especially for requests sending JSON.

Sure, good point. We seem to not see these service APIs as something we guarantee (at I haven't noticed any similar tests in other places), but I don't see why the behaviour around how limits work with this API shouldn't be covered.

@narqo narqo requested a review from colega February 25, 2025 17:40
@narqo narqo merged commit 34cdbbf into main Feb 26, 2025
28 checks passed
@narqo narqo deleted the vldmr/ruler-api-tenants branch February 26, 2025 15:36
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