-
Notifications
You must be signed in to change notification settings - Fork 7
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 bytes fetched in query API #39
Query bytes fetched in query API #39
Conversation
5e3b938
to
172eb92
Compare
this is WIP |
172eb92
to
35708f8
Compare
16e38b4
to
f4cd1e5
Compare
b7d97e6
to
094bf09
Compare
ready for review now |
Can we run some queries for which we know roughly how many time series/data samples are fetched as a basic correctness verification? Perhaps, use |
@christopherzli Can you also explain your approach in the PR description? |
Please check the verification section. Count(up) is 173 and series fetched is 174. want to call out if same series live in different store api(receive/obj storage), it will be counted separately as of now |
094bf09
to
4c6757b
Compare
Can you run more queries (with range queries as well) to verify the results? |
add thanos stats queried
4c6757b
to
c8b83d2
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.
this is great, I'd suggest to add some unit tests and communicate about this API change before merging it: https://github.com/thanos-io/thanos/pull/7390/files#r1615148051
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.
as we discussed, please work with @yuchen-db to make sure query bytes fetched show up in bronson UI in oregon-dev, thanks!
jenkins merge |
Changes
Leverage the existing series stats counter field which was used for metrics, and add query bytes fetched to the field and save it as part of response
Verification
we can see the latter two returns very similar results