Unify query frontend instant and range protobufs into one #6180
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ben Ye <benye@amazon.com>
yeya24
commented
Aug 29, 2024
|
|
||
| message PrometheusData { | ||
| string ResultType = 1 [(gogoproto.jsontag) = "resultType"]; | ||
| tripperware.PrometheusQueryResult Result = 2 [(gogoproto.nullable) = false, (gogoproto.jsontag) = "result"]; |
There was a problem hiding this comment.
This is the main change. Previous definition here uses repated tripperware.SampleStream which supports Matrix type only. This change makes it possible to support vector and other types so supporting both instant and range queries using the same type.
There was a problem hiding this comment.
Looks good to me. The new PrometheusResponse is pretty much the same as what we had for PrometheusInstantQueryResponse so should be easy to use for both types.
afhassan
approved these changes
Aug 29, 2024
yeya24
requested review from
alanprot,
danielblando,
friedrichg and
CharlieTLe
This was referenced Sep 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does:
Unify queryfrontend instant and range query protobuf into a single protobuf. This is kind of a tech debt we introduced before. There is no need to keep two separate protobuf files for different query types as the response format is the same.
This change makes it easier to support #5527 as Querier can only have one Codec to encode response to protobuf format, instead of having two Codecs for instant and range query.
This change has no impact for migration and anything else as the protobuf types used in QFE are not even used over the wire. Just for serialization and deserialization in the same process so we can change it without worrying about backward compatibility.
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]I didn't add new tests since this PR is mainly moving protobufs to another place. Existing tests should pass.