Skip to content

Query ingesters only for GetLabelNames and GetLabelValues if start time parameter is not specified #6618

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

Merged
merged 9 commits into from
Mar 19, 2025

Conversation

afhassan
Copy link
Contributor

@afhassan afhassan commented Feb 28, 2025

What this PR does:
GetSeries, GetLabelNames, and GetLabelValues API endpoints have optional start and end time parameters. If user does not specify a start or end time parameter, we set them to MinTime and MaxTime.

This causes GetLabelNames and GetLabelValues to fail if start or end are not specified due to exceeding maxQueryLength limit

the query time range exceeds the limit (query length: 1348893h1m53.351551616s, limit: 720h0m0s)

For GetSeries, we fallback to querying only ingesters if the start time is not specified. This PR changes the behaviour for both GetLabelNames and GetLabelValues to also query ingesters if start time parameter is not specified.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

@afhassan
Copy link
Contributor Author

Should we truncate response if ignoreMaxQueryLength is true?

@afhassan afhassan force-pushed the fix-get-labels-range-limit branch from c05250c to 700b1c2 Compare February 28, 2025 08:13
@pull-request-size pull-request-size bot added size/S and removed size/L labels Mar 13, 2025
@afhassan afhassan force-pushed the fix-get-labels-range-limit branch from cc37538 to 3e5e76a Compare March 13, 2025 16:50
@danielblando
Copy link
Contributor

Seems we had a discussion in the past about this break change for GetSeries. #4976

Can you update docs as we did them?

Signed-off-by: Ahmed Hassan <[email protected]>
@afhassan afhassan changed the title Truncate GetLabels and GetSeries to max query length if no time provided Query ingesters only for GetLabelNames and GetLabelValues if start time parameter is not specified Mar 13, 2025
Signed-off-by: Ahmed Hassan <[email protected]>
@danielblando
Copy link
Contributor

Should we truncate response if ignoreMaxQueryLength is true?

Do we truncate for GetSeries?

I think if the purpose of this PR is to avoid OOM, we should still truncate. If customer wants, it can explicit suggest the time range

@afhassan
Copy link
Contributor Author

Do we truncate for GetSeries?

Yes we still truncate since we completely skip enforcing the max query length limit. I do agree it is better to still truncate.

@danielblando danielblando requested a review from yeya24 March 18, 2025 18:13
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 good. But I think we need to add a changelog

afhassan and others added 2 commits March 18, 2025 18:12
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.

Thanks

@yeya24 yeya24 merged commit 268f089 into cortexproject:master Mar 19, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants