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

querier: Add a flag to limit time range for metadata APIs (Labels and Series) #3147

Merged
merged 7 commits into from
Sep 21, 2020
Merged

querier: Add a flag to limit time range for metadata APIs (Labels and Series) #3147

merged 7 commits into from
Sep 21, 2020

Conversation

kakkoyun
Copy link
Member

@kakkoyun kakkoyun commented Sep 9, 2020 •
edited
Loading

Signed-off-by: Kemal Akkoyun kakkoyun@gmail.com

WIP

Ref #3119
Fixes #1811

Changes

  • Add query.metadata.default-time-range flag to specify the default look-back duration for retrieving labels through Labels APIs when time range parameters are not specified.

Verification

  • make test-local

@kakkoyun kakkoyun marked this pull request as draft September 9, 2020 15:57
@kakkoyun kakkoyun mentioned this pull request Sep 11, 2020
Closed
@kakkoyun kakkoyun force-pushed the limit_label_apis branch 3 times, most recently from f84bd95 to c751c96 Compare September 17, 2020 16:00
@kakkoyun kakkoyun changed the title [WIP]: Limit time window for label APIs by default Limit time window for label APIs by default Sep 17, 2020
@kakkoyun kakkoyun changed the title Limit time window for label APIs by default querier: Add a flag to limit time range for label APIs Sep 17, 2020
@kakkoyun kakkoyun marked this pull request as ready for review September 17, 2020 16:03
yeya24
yeya24 approved these changes Sep 17, 2020
View reviewed changes
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Looks perfect!

CHANGELOG.md Show resolved Hide resolved
onprem
onprem approved these changes Sep 18, 2020
View reviewed changes
Copy link
Member

@onprem onprem left a comment

Choose a reason for hiding this comment

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

LGTM!

@kakkoyun
Limit the default time window for label name and label value APIs
0d74172 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Keep the existing behaviour as default
9ba0e09 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Add label API endpoint tests
c3f3193 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Fix changelog entry
1ebbb5c 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Copy link
Member Author

friendly ping @bwplotka

bwplotka
bwplotka reviewed Sep 21, 2020
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Amazing, thanks! Some suggestion, otherwise LGTM 👍

cmd/thanos/query.go Outdated Show resolved Hide resolved
pkg/api/query/v1.go Outdated Show resolved Hide resolved
pkg/api/query/v1.go Outdated Show resolved Hide resolved
@kakkoyun kakkoyun changed the title querier: Add a flag to limit time range for label APIs querier: Add a flag to limit time range for metadata APIs (Labels and Series) Sep 21, 2020
@kakkoyun
Address review issues
de92bdc 
Implement time ranges for series API

Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Update changelog
cca1b5e 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
@kakkoyun
Copy link
Member Author

@bwplotka Addressed the raised issues and included series API time range as well.

@kakkoyun
Fix formatting
a3862c1 
Signed-off-by: Kemal Akkoyun <kakkoyun@gmail.com>
bwplotka
bwplotka approved these changes Sep 21, 2020
View reviewed changes
Copy link
Member

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

LGTM, 👍

Thanks! 💪

I think it would be amazing to actually propose changing the default for this being ~1d or something in next release (:

@kakkoyun kakkoyun merged commit 68d0ad2 into thanos-io:master Sep 21, 2020
@kakkoyun kakkoyun deleted the limit_label_apis branch September 21, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bwplotka bwplotka bwplotka approved these changes

@yeya24 yeya24 yeya24 approved these changes

@onprem onprem onprem approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

querier: Limit LabelNames; LabelValues to certain time period.
4 participants
@kakkoyun @bwplotka @yeya24 @onprem