-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Split labels/series API endpoints in query frontend #3276
Conversation
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
a8dc6f6
to
00c67cf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work 🎉 I just skimmed over and looking good so far.
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
2557f99
to
7d6c334
Compare
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
Signed-off-by: Ben Ye <yb532204897@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yeya24 Great work. It has taken some time to review the whole the PR. Overall it looks amazing. Thanks a lot for the contribution.
I have only added a small nit. Other than that we should test by integrating it with whole stack. It would be really nice to run it with quickstart
script. WDYT?
Yes, that's what I want as well. Since this task is trivial, I will open an issue and let someone else contribute. Done: #3304 |
Signed-off-by: Ben Ye <yb532204897@gmail.com>
5f5e987
to
35380a9
Compare
Signed-off-by: Ben Ye <yb532204897@gmail.com>
While I am working on the caching part, I find that all response format must be protobuf because they will be marshaled in the results cache middlware. I pushed another commit to switch the labels and series response to protobuf. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing work, some post merge comments 🤗
} | ||
} | ||
|
||
func (c labelsCodec) MergeResponse(responses ...queryrange.Response) (queryrange.Response, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing exported comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some commentary why queryrange.Response
is used for labels would be nice ;p
case *ThanosSeriesResponse: | ||
seriesData := make([]labelpb.LabelSet, 0) | ||
|
||
// seriesString is used in soring so we don't have to calculate the string of label sets again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
soring
* split metadata endpoints Signed-off-by: Ben Ye <yb532204897@gmail.com> * add metadata codec implementation Signed-off-by: Ben Ye <yb532204897@gmail.com> * cleanup go mod Signed-off-by: Ben Ye <yb532204897@gmail.com> * add back fix for thanos-io#3240 Signed-off-by: Ben Ye <yb532204897@gmail.com> * check set key exist Signed-off-by: Ben Ye <yb532204897@gmail.com> * update Signed-off-by: Ben Ye <yb532204897@gmail.com> * add default metadata range flag Signed-off-by: Ben Ye <yb532204897@gmail.com> * fix linting issues Signed-off-by: Ben Ye <yb532204897@gmail.com> * refactor flags and add test cases Signed-off-by: Ben Ye <yb532204897@gmail.com> * fix go lint issue Signed-off-by: Ben Ye <yb532204897@gmail.com> * add roundtrip tripperware tests for labels and series requests Signed-off-by: Ben Ye <yb532204897@gmail.com> * fix all unit tests and e2e tests Signed-off-by: Ben Ye <yb532204897@gmail.com> * fix lint issues Signed-off-by: Ben Ye <yb532204897@gmail.com> * add nolint unparam Signed-off-by: Ben Ye <yb532204897@gmail.com> * update flags and changelog Signed-off-by: Ben Ye <yb532204897@gmail.com> * switch response format to protobuf Signed-off-by: Ben Ye <yb532204897@gmail.com>
Changes
This pr adds splitting and retry middleware support for all metadata API endpoints (label_name, label_value, series). This is the first step of #3230. Will add caching support in another pr since this pr is already very large.
Verification
Tests updated.