Skip to content

All result timestamps off by 1ms (without alignment turned on) #2932

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

Closed
juliusv opened this issue Jul 27, 2020 · 7 comments · Fixed by #2990
Closed

All result timestamps off by 1ms (without alignment turned on) #2932

juliusv opened this issue Jul 27, 2020 · 7 comments · Fixed by #2990

Comments

@juliusv
Copy link
Contributor

juliusv commented Jul 27, 2020

If you run a query like this against the latest Cortex release, started with the single-process config at https://github.com/raw/cortexproject/cortex/master/docs/configuration/single-process-config.yaml:

http://localhost:9009/api/prom/api/v1/query_range?query=time()&start=1595846748.806&end=1595846848.806&step=5

...then the first result sample should be at timestamp 1595846748.806, but Cortex reports it at 1595846748.805, and any subsequent samples are off by 1ms as well. Compare to the result by Prometheus at https://demo.promlabs.com/api/v1/query_range?query=time()&start=1595846748.806&end=1595846848.806&step=5.

I didn't turn on the step alignment middleware, so it shouldn't be related to that.

@juliusv
Copy link
Contributor Author

juliusv commented Jul 27, 2020

There was prometheus/prometheus#4941 2 years ago in upstream Prometheus to also fix an off-by-1ms rounding bug in the incoming timestamps. Is this maybe not picked up by Cortex yet? In Cortex's go.mod I see github.com/prometheus/prometheus v1.8.2-0.20200722151933-4a8531a64b32. If this is based off 4a8531a64b32 in Prometheus from July 22, 2020, then I don't see how Cortex and Prometheus would behave different in this regard.

@bwplotka
Copy link
Contributor

bwplotka commented Aug 4, 2020

Thanks!

Yea, we just need this: prometheus/prometheus#4941 then. The thing is that API package is almost impossible to import (not reusible much unfortunately), so both Thanos & Cortex has to manually apply this fix.

I wish we could somehow move Prometheus api.go into components for easier reuse 🤔

@juliusv
Copy link
Contributor Author

juliusv commented Aug 4, 2020

@bwplotka Totally agreed, the API package is really hard to reuse externally, without injecting a boatload of not-always-necessary stuff. Would be great to change that at some point as well :)

@pracucci
Copy link
Contributor

pracucci commented Aug 6, 2020

Thanks for reporting it! I'm fixing it...

@lilic
Copy link

lilic commented Oct 7, 2020

Since this was fixed any chance to update the results @juliusv to reflect that? https://promlabs.com/promql-compliance-tests/

@pstibrany
Copy link
Contributor

This bugfix is in Cortex 1.4 release.

@juliusv
Copy link
Contributor Author

juliusv commented Oct 7, 2020

@lilic Yep, I'm just waiting for one vendor to see if they can give me remote_write access so I can include them in my tests as well. But hope to publish an update in a couple of weeks :)

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

Successfully merging a pull request may close this issue.

5 participants