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: error in #1904 - /series with SkipChunks not use start/end args. #2011

Closed
IKSIN opened this issue Jan 20, 2020 · 6 comments · Fixed by #2015
Closed

query: error in #1904 - /series with SkipChunks not use start/end args. #2011

IKSIN opened this issue Jan 20, 2020 · 6 comments · Fixed by #2015

Comments

@IKSIN
Copy link
Contributor

IKSIN commented Jan 20, 2020

Thanos, Prometheus and Golang version used:
thanos v0.10.0 with #1904

Object Storage Provider:

What happened:
/series return all data (not considering selected time range in Grafana)

What you expected to happen:
Return data only in selected time range

How to reproduce it (as minimally and precisely as possible):
Request /series to prometheus with start/end args and compare with same request to thanos-query

Full logs to relevant components:

Anything else we need to know:

I try to set SkipChunks=false and get vallid response

@GiedriusS
Copy link
Member

Thanks for the report! At first glance, it seems like the problem is here: https://github.com/thanos-io/thanos/pull/1904/files#diff-83652b446ecd7da3f50669bd4a6d44baR664-R667 - q.Encode() should be called after adding all of the parameters. For some reason, we call it before adding start/end times. @yeya24 does that sound about right?

@yeya24
Copy link
Contributor

yeya24 commented Jan 20, 2020

Thanks for the report. I will send a fix ASAP

@yeya24
Copy link
Contributor

yeya24 commented Jan 20, 2020

Thanks for the report! At first glance, it seems like the problem is here: https://github.com/thanos-io/thanos/pull/1904/files#diff-83652b446ecd7da3f50669bd4a6d44baR664-R667 - q.Encode() should be called after adding all of the parameters. For some reason, we call it before adding start/end times. @yeya24 does that sound about right?

I think you are right! Thanks! I need to test it locally first

@GiedriusS
Copy link
Member

@yeya24 let me know if you need any help. It would be nice to release 0.10.1 with the one other fix that we have for alerting rules + this one :p

@bwplotka
Copy link
Member

This should be fixed, we will be releasing 0.10.1 with this.

@bwplotka
Copy link
Member

cc @GiedriusS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants