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

Query Frontend: add new field for dense native histogram format #6199

Merged
merged 4 commits into from
Dec 4, 2024

Conversation

yeya24
Copy link
Contributor

@yeya24 yeya24 commented Sep 9, 2024

What this PR does:

#6151 added support for Ruler to evaluate rules via sending gRPC requests to Query Frontend.

An issue we found out is that the current JSON format query response from Query Frontend doesn't allow Ruler to access the whole information of native histograms. As what Prometheus returns in its Query response is a more compact format of native histogram, not its full format.

With another in progress PR #5527, we can make it possible to return protobuf query response in Query Frontend through Querier. This could make it possible for Ruler to get the full representation of native histograms.

This PR adds a new rawHistograms field to Query Frontend protobuf. This field should never be set when using JSON codec and should be only used when using Protobuf codec asked by Ruler.

Checklist

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

@yeya24 yeya24 force-pushed the add-histograms-raw branch from 57472b1 to 4061437 Compare December 3, 2024 05:28
@yeya24 yeya24 marked this pull request as ready for review December 3, 2024 05:28
@yeya24
Copy link
Contributor Author

yeya24 commented Dec 3, 2024

@SungJin1212 Would you like to take a look? This PR tries to return dense native histogram format if it is protobuf codec with cortex internal set to true.

Then you can continue the work in #6345 to implement everything end to end.

Since there is no actual client asking for this format now I didn't add an integration test

Signed-off-by: Ben Ye <[email protected]>
Signed-off-by: Ben Ye <[email protected]>
@SungJin1212
Copy link
Contributor

LGTM! Thanks.

Signed-off-by: Ben Ye <[email protected]>
@yeya24 yeya24 force-pushed the add-histograms-raw branch from ddba207 to f49e90e Compare December 3, 2024 18:29
@yeya24 yeya24 enabled auto-merge (squash) December 4, 2024 01:25
@yeya24 yeya24 merged commit 320e475 into cortexproject:master Dec 4, 2024
16 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.

2 participants