Make use of LabelHints.Limit for `LabelNames` and `LabelValues` requests (#8805) #10410

santileira · 2025-01-12T15:04:34Z

What this PR does

Make use of LabelHints.Limit for LabelNames and LabelValues on the StoreGatewayClient, Distributor and Ingester.

Which issue(s) this PR fixes or relates to

Fixes #8805

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.

aknuds1

To me it looks pretty good, but I suggest some improvements.

pkg/distributor/distributor.go

pkg/distributor/distributor_test.go

pkg/querier/distributor_queryable_test.go

pkg/querier/tenantfederation/merge_queryable_test.go

pkg/storegateway/bucket.go

pkg/distributor/distributor_test.go

aknuds1 · 2025-01-23T08:24:07Z

Can you rebase on main and squash your commits into one, please? Maybe rebasing will fix the CI failures, since I can't reproduce them locally.

WIP Add limit field on LabelNamesRequest and LabelValuesRequest Add support for limit on gateway.proto Rename variables to keep consistency Fix tests Add tests Add tests Add tests Add tests Add changelog Fix changelog Fix tests Fix imports empty commit Update pkg/distributor/distributor.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/querier/tenantfederation/merge_queryable_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/querier/tenantfederation/merge_queryable_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/storegateway/bucket.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/storegateway/bucket.go Co-authored-by: Arve Knudsen <[email protected]> Update CHANGELOG.md Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/distributor/distributor_test.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/ingester/client/compat.go Co-authored-by: Arve Knudsen <[email protected]> Update pkg/ingester/client/compat.go Co-authored-by: Arve Knudsen <[email protected]> Apply suggestions from code review Co-authored-by: Arve Knudsen <[email protected]> Apply suggestions from code review Co-authored-by: Arve Knudsen <[email protected]> Fix pipeline Fix tests empty commit

santileira · 2025-01-23T13:26:29Z

Can you rebase on main and squash your commits into one, please? Maybe rebasing will fix the CI failures, since I can't reproduce them locally.

done! could you check it again?

aknuds1

Looks much better, but there are still some issues.

CHANGELOG.md

pkg/ingester/client/compat.go

pkg/querier/blocks_store_queryable.go

pkg/querier/distributor_queryable.go

pkg/querier/distributor_queryable_test.go

Co-authored-by: Arve Knudsen <[email protected]>

santileira · 2025-01-24T19:38:16Z

Looks much better, but there are still some issues.

@aknuds1 I fixed them. Could you check it again?

santileira · 2025-01-28T14:45:54Z

@aknuds1 is this ready to merge?

aknuds1 · 2025-01-28T14:55:13Z

I plan on reviewing it soon, but it may have to wait until tomorrow. It's been busy.

santileira · 2025-01-28T16:08:04Z

I plan on reviewing it soon, but it may have to wait until tomorrow. It's been busy.

Makes sense. Thanks

aknuds1

After changing the changelog item into an ENHANCEMENT (and preferably addressing the two nits), I think this is good to go! @charleskorn do you also want to look it over?

pkg/querier/distributor_queryable_test.go

pkg/ingester/ingester.go

CHANGELOG.md

Stale

santileira · 2025-01-30T16:58:45Z

@aknuds1 done, could you review it again?

aknuds1

LGTM, thank you!

I'm going to let @charleskorn review. I think he's off work until Monday.

santileira · 2025-01-30T20:37:24Z

LGTM, thank you!

I'm going to let @charleskorn review. I think he's off work until Monday.

Amazing! Thanks

charleskorn

Thanks for working on this @santileira!

charleskorn · 2025-02-02T23:11:43Z

CHANGELOG.md

@@ -37,6 +37,7 @@
  * `cortex_ingester_tsdb_block_postings_for_matchers_cache_requests_total`
  * `cortex_ingester_tsdb_block_postings_for_matchers_cache_skips_total`
 * [ENHANCEMENT] Compactor: Shuffle users' order in `BlocksCleaner`. Prevents bucket indexes from going an extended period without cleanup during compactor restarts. #10513
+* [ENHANCEMENT] Distributor, querier, ingester and store-gateway: Make use of LabelHints.Limit for label names and values requests. #10410


I would phrase this in terms a user would understand: something like "add support for limit parameter for label names and values requests"

pkg/distributor/distributor_test.go

pkg/ingester/ingester.go

pkg/ingester/ingester_test.go

santileira · 2025-02-03T19:41:06Z

Thanks for working on this @santileira!

Done! @charleskorn could you review it again? Thanks

CHANGELOG.md

charleskorn

Thanks @santileira!

santileira · 2025-02-03T23:30:33Z

Thanks @santileira!

You are welcome!

santileira changed the title ~~Use LabelHints.Limit~~ Make use of LabelHints.Limit for label names and values requests Jan 18, 2025

santileira changed the title ~~Make use of LabelHints.Limit for label names and values requests~~ Make use of LabelHints.Limit for LabelNames and LabelValues requests Jan 19, 2025

santileira marked this pull request as ready for review January 19, 2025 20:09

santileira requested a review from a team as a code owner January 19, 2025 20:09

santileira mentioned this pull request Jan 19, 2025

Make use of LabelHints.Limit for label names and values requests #8805

Closed

santileira changed the title ~~Make use of LabelHints.Limit for LabelNames and LabelValues requests~~ Make use of LabelHints.Limit for LabelNames and LabelValues requests (#8805) Jan 19, 2025

aknuds1 reviewed Jan 21, 2025

View reviewed changes

aknuds1 requested review from aknuds1 and charleskorn January 21, 2025 13:11

aknuds1 added component/ingester component/store-gateway component/distributor component/querier labels Jan 21, 2025

santileira commented Jan 21, 2025

View reviewed changes

pkg/distributor/distributor_test.go Show resolved Hide resolved

santileira added 2 commits January 23, 2025 10:06

Rebase

69ea948

santileira force-pushed the santiago.leira/use-LabelHints.Limit branch from fccd555 to 69ea948 Compare January 23, 2025 13:07

aknuds1 previously requested changes Jan 24, 2025

View reviewed changes

santileira and others added 2 commits January 24, 2025 14:57

Apply suggestions from code review

74686a7

Co-authored-by: Arve Knudsen <[email protected]>

Addressed feedback PR

d546a4f

santileira requested a review from aknuds1 January 24, 2025 19:38

Merge branch 'main' into santiago.leira/use-LabelHints.Limit

261ae81

aknuds1 reviewed Jan 29, 2025

View reviewed changes

pkg/querier/distributor_queryable_test.go Outdated Show resolved Hide resolved

pkg/ingester/ingester.go Outdated Show resolved Hide resolved

CHANGELOG.md Outdated Show resolved Hide resolved

Addressed feedback PR

744e51e

aknuds1 approved these changes Jan 30, 2025

View reviewed changes

santileira mentioned this pull request Feb 2, 2025

Idea: expose query stats in Grafana #10192

Open

charleskorn reviewed Feb 2, 2025

View reviewed changes

Addressed feedback PR from charleskorn

ed95bbc

Fix test

b29de37

charleskorn reviewed Feb 3, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

ee2bcdc

charleskorn enabled auto-merge (squash) February 3, 2025 22:58

charleskorn approved these changes Feb 3, 2025

View reviewed changes

charleskorn merged commit ac097ac into grafana:main Feb 3, 2025
28 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make use of LabelHints.Limit for `LabelNames` and `LabelValues` requests (#8805) #10410

Make use of LabelHints.Limit for `LabelNames` and `LabelValues` requests (#8805) #10410

santileira commented Jan 12, 2025 •

edited

Loading

aknuds1 left a comment

aknuds1 commented Jan 23, 2025

santileira commented Jan 23, 2025

aknuds1 left a comment

santileira commented Jan 24, 2025

santileira commented Jan 28, 2025

aknuds1 commented Jan 28, 2025

santileira commented Jan 28, 2025

aknuds1 left a comment

santileira commented Jan 30, 2025

aknuds1 left a comment

santileira commented Jan 30, 2025

charleskorn left a comment

charleskorn Feb 2, 2025

santileira commented Feb 3, 2025

charleskorn left a comment

santileira commented Feb 3, 2025

Make use of LabelHints.Limit for LabelNames and LabelValues requests (#8805) #10410

Make use of LabelHints.Limit for LabelNames and LabelValues requests (#8805) #10410

Conversation

santileira commented Jan 12, 2025 • edited Loading

What this PR does

Which issue(s) this PR fixes or relates to

Checklist

aknuds1 left a comment

Choose a reason for hiding this comment

aknuds1 commented Jan 23, 2025

santileira commented Jan 23, 2025

aknuds1 left a comment

Choose a reason for hiding this comment

santileira commented Jan 24, 2025

santileira commented Jan 28, 2025

aknuds1 commented Jan 28, 2025

santileira commented Jan 28, 2025

aknuds1 left a comment

Choose a reason for hiding this comment

santileira commented Jan 30, 2025

aknuds1 left a comment

Choose a reason for hiding this comment

santileira commented Jan 30, 2025

charleskorn left a comment

Choose a reason for hiding this comment

charleskorn Feb 2, 2025

Choose a reason for hiding this comment

santileira commented Feb 3, 2025

charleskorn left a comment

Choose a reason for hiding this comment

santileira commented Feb 3, 2025

Make use of LabelHints.Limit for `LabelNames` and `LabelValues` requests (#8805) #10410

Make use of LabelHints.Limit for `LabelNames` and `LabelValues` requests (#8805) #10410

santileira commented Jan 12, 2025 •

edited

Loading