Skip to content

Commit

Permalink
fix rounding of time to milliseconds (#5352)
Browse files Browse the repository at this point in the history
Current implementation of RoundToMilliseconds is broken due to improper handling of floating point operation in go.
Fixing it by using simple division and modulo operators
  • Loading branch information
sandeepsukhani authored Feb 9, 2022
1 parent 8e9009a commit 0afd113
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 3 deletions.
12 changes: 9 additions & 3 deletions pkg/util/conv.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package util

import (
"math"
"time"
"unsafe"

Expand All @@ -28,8 +27,15 @@ func MapToModelLabelSet(m map[string]string) model.LabelSet {
// RoundToMilliseconds returns milliseconds precision time from nanoseconds.
// from will be rounded down to the nearest milliseconds while through is rounded up.
func RoundToMilliseconds(from, through time.Time) (model.Time, model.Time) {
return model.Time(int64(math.Floor(float64(from.UnixNano()) / float64(time.Millisecond)))),
model.Time(int64(math.Ceil(float64(through.UnixNano()) / float64(time.Millisecond))))
fromMs := from.UnixNano() / int64(time.Millisecond)
throughMs := through.UnixNano() / int64(time.Millisecond)

// add a millisecond to the through time if the nanosecond offset within the second is not a multiple of milliseconds
if int64(through.Nanosecond())%int64(time.Millisecond) != 0 {
throughMs++
}

return model.Time(fromMs), model.Time(throughMs)
}

// LabelsToMetric converts a Labels to Metric
Expand Down
14 changes: 14 additions & 0 deletions pkg/util/conv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,20 @@ func TestRoundToMilliseconds(t *testing.T) {
model.Time(1),
model.Time(3),
},
{
"rounding large number in nanoseconds",
time.Unix(0, 1643958368442000064),
time.Unix(0, 1643958368443000064),
model.Time(1643958368442),
model.Time(1643958368444),
},
{
"already rounded large number in nanoseconds",
time.Unix(0, 1643958368442000000),
time.Unix(0, 1643958368443000000),
model.Time(1643958368442),
model.Time(1643958368443),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down

0 comments on commit 0afd113

Please sign in to comment.