utils.FromUnixMilli has floating point rounding issues
Describe the bug
The utils.FromUnixMilli helper function internally converts the integer to a float and back again.
https://github.com/Icinga/icingadb/blob/9c2dcd2502b7effb3c88950fbb3e7fc24defb76d/pkg/utils/utils.go#L24
By doing so, it adds additional precision which might result in a wrongly rounded number.
To Reproduce
While working on something totally different within icinga-notifications when I realized that my times.UnixMilli-based comparison simply didn't made any sense. After lots of debugging and enabling PostgreSQL's debug logs, I found an off-by-one issue with my timestamps and was able to reproduce it by adding another test case to TestUnixMilli_MarshalJSON.
diff --git a/pkg/types/unix_milli_test.go b/pkg/types/unix_milli_test.go
index 985fa2a..818cff0 100644
--- a/pkg/types/unix_milli_test.go
+++ b/pkg/types/unix_milli_test.go
@@ -1,6 +1,7 @@
package types
import (
+ "github.com/icinga/icingadb/pkg/utils"
"github.com/stretchr/testify/require"
"testing"
"time"
@@ -16,6 +17,7 @@ func TestUnixMilli_MarshalJSON(t *testing.T) {
{"zero", UnixMilli{}, `null`},
{"epoch", UnixMilli(time.Unix(0, 0)), `0`},
{"nonzero", UnixMilli(time.Unix(1234567890, 62500000)), `1234567890062`},
+ {"ilovefloats", UnixMilli(utils.FromUnixMilli(1714489037339)), `1714489037339`},
}
for _, st := range subtests {
Running this test results in an error, returning a number off by one.
=== RUN TestUnixMilli_MarshalJSON/ilovefloats
unix_milli_test.go:29:
Error Trace: …/icingadb/pkg/types/unix_milli_test.go:29
Error: Not equal:
expected: "1714489037339"
actual : "1714489037338"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-1714489037339
+1714489037338
Test: TestUnixMilli_MarshalJSON/ilovefloats
--- FAIL: TestUnixMilli_MarshalJSON (0.00s)
--- FAIL: TestUnixMilli_MarshalJSON/ilovefloats (0.00s)
Expected :1714489037339
Actual :1714489037338
Or, making it even more obvious by creating a test for utils.FromUnixMilli:
diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
index b0ea54b..de34c76 100644
--- a/pkg/utils/utils_test.go
+++ b/pkg/utils/utils_test.go
@@ -3,6 +3,7 @@ package utils
import (
"github.com/stretchr/testify/require"
"testing"
+ "time"
)
func TestChanFromSlice(t *testing.T) {
@@ -52,3 +53,8 @@ func requireClosedEmpty(t *testing.T, ch <-chan int) {
require.Fail(t, "receiving should not block")
}
}
+
+func TestFromUnixMilli_Float(t *testing.T) {
+ const ms = 1714489037339
+ require.Equal(t, time.UnixMilli(ms), FromUnixMilli(ms))
+}
=== RUN TestFromUnixMilli_Float
utils_test.go:59:
Error Trace: …/icingadb/pkg/utils/utils_test.go:59
Error: Not equal:
expected: time.Date(2024, time.April, 30, 16, 57, 17, 339000000, time.Local)
actual : time.Date(2024, time.April, 30, 16, 57, 17, 338999986, time.Local)
Diff:
--- Expected
+++ Actual
@@ -1,2 +1,2 @@
-(time.Time) 2024-04-30 16:57:17.339 +0200 CEST
+(time.Time) 2024-04-30 16:57:17.338999986 +0200 CEST
Test: TestFromUnixMilli_Float
--- FAIL: TestFromUnixMilli_Float (0.00s)
Expected :time.Date(2024, time.April, 30, 16, 57, 17, 339000000, time.Local)
Actual :time.Date(2024, time.April, 30, 16, 57, 17, 338999986, time.Local)
Expected behavior
No additional floating point precision should be added and later cut off while being cast back to an integer.
Your Environment
Include as many relevant details about the environment you experienced the problem in
- Icinga DB version: current
main - Icinga 2 version: N/A
Additional context
#744 and #747 might solve this by switching to Go's native time.UnixMilli.