Fixed query start/end timestamp rounding #2990
Conversation
|
👍 |
| input: "1543578564.705", | ||
| result: time.Unix(1543578564, 705*1e6), | ||
| }, | ||
| } |
There was a problem hiding this comment.
What about `input: "2015-06-03T13:21:58.56789Z" ... should that be rounded on milliseconds precision?
There was a problem hiding this comment.
I copied the exact implementation and tests of Prometheus. Should we keep it 1-1 with Prometheus or change it? I'm open to discuss it.
There was a problem hiding this comment.
No strong opinion on milliseconds rounding... We can keep it as-is to be consistent with Prometheus, and think about it when (if) someone hits that. Then we can clarify at both places (depending on what Prometheus decides)
There was a problem hiding this comment.
I've added a test. I believe we should just use the Prometheus implementation as reference implementation (we want to be 100% compatible with Prometheus) and thus I've written the expected value with the current Prometheus rounding.
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
Signed-off-by: Marco Pracucci <[email protected]>
pracucci
force-pushed
the
fix-query-start-end-time-parsing
branch
from
ffe6e4c to
572b261
Compare
What this PR does:
The good @juliusv reported we're not rounding query start/end timestamps like Prometheus. This PR fixes it, specifically introduces the change done in Prometheus almost 3y ago prometheus/prometheus#4941.
Which issue(s) this PR fixes:
Fixes #2932
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]