vectordotdev / vector

A high-performance observability data pipeline.
https://vector.dev
Mozilla Public License 2.0
17.83k stars 1.58k forks source link

Playground to_unix_timestamp nanoseconds precision error #20325

Open DimDroll opened 6 months ago

DimDroll commented 6 months ago

Hello Vector Team,

It seems there is some sort of integer overflow happening during nanoseconds parsing:

.A = to_unix_timestamp(t'1970-04-15T05:59:59.254740992Z', "nanoseconds")
# returns:
#   "A": 9007199254740992,
.B = to_unix_timestamp(t'1970-04-15T05:59:59.254740993Z', "nanoseconds")
# returns the same:
#   "B": 9007199254740992,

I assume this is something JavaScript related as in VRL CLI it works just fine. This is first magical number when it starts occurring "9007199254740993". Here is the relevant artice that I found: https://medium.com/@vinaymahamuni/interesting-bug-finding-related-to-javascript-e31de4dac02d

I spent significant time researching this issue because I assumed playground works for the most part 1-1 with VRL.

If it is something that can't be fixed maybe it should be added as note here: https://vector.dev/docs/reference/vrl/#learn And as info box in the top right in the Playground UI as well.

Some more examples:

# works fine up until timestamp is within microsecond precision:
# this is expected
.C = to_unix_timestamp(t'2021-01-01T01:01:01.111119000Z', "nanoseconds")
# returns: 1609462861111118000
# at nanosecond level it ignores them:
.D = to_unix_timestamp(t'2021-01-01T01:01:01.111119001Z', "nanoseconds")
# returns the same: 1609462861111119000
# up until 232rd nanosecond where it starts rounding it to 400:
.E = to_unix_timestamp(t'2021-01-01T01:01:01.111119232Z', "nanoseconds")
# returns: 1609462861111119400
# from 489 rounds to 600
.F = to_unix_timestamp(t'2021-01-01T01:01:01.111119489Z', "nanoseconds")
# returns: 1609462861111119600
# from 744 rounds to 000 and increments microsecond
.G = to_unix_timestamp(t'2021-01-01T01:01:01.111119743Z', "nanoseconds")
# returns: 1609462861111119600
# rounds to next microsecond 
.H = to_unix_timestamp(t'2021-01-01T01:01:01.111119999Z', "nanoseconds")
# returns: 1609462861111120000

Following became irrelevant when I found that this is not Rust related issue.

https://timclicks.dev/convert-a-unix-timestamp-to-rust/

Unfortunately, I'm not Rust proficient enough to suggest a solution, but when I tried to replicate the same issue in Rust playground I couldn't do it:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=83364f8d960975b24bcd70f3dcddbb09

Simplified version of the code which does not replicate the issue:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=af0ae97da4f852c2f94e96e9be7d12e3_
jszwedko commented 6 months ago

Thanks for this thorough report @DimDroll

I think this might be related to the fact that the VRL Playground compiles VRL into a WASM module which is then executed in the browser 🤔 . I agree if we aren't able to resolve it we should at least call it out.

jszwedko commented 6 months ago

(I transferred this issue to Vector since that is where the VRL playground code lives)

DimDroll commented 6 months ago

Spent some time trying to understand the issue with WASM and precision, but realized that this issue is out of my depth.

So here is the research I did if it helps anyone else: List of BigInt related issue in wasm: https://github.com/search?q=org%3Arustwasm+bigint&type=issues

wasm-bingden supports BigInt since 0.2.77: https://github.com/rustwasm/wasm-bindgen/pull/2629 https://github.com/rustwasm/wasm-bindgen/commit/d6d056cdc83a8b336c2e8aaa761a5fe7f82818fa https://github.com/rustwasm/wasm-bindgen/releases/tag/0.2.77

serde-wasm-bindgen also implemented it in: https://github.com/RReverser/serde-wasm-bindgen/pull/24 https://github.com/RReverser/serde-wasm-bindgen/issues/21 https://github.com/RReverser/serde-wasm-bindgen/releases/tag/v0.4.0

Mozilla added it to standards: https://bugs.webkit.org/show_bug.cgi?id=213528

WASM and JS features that added BigInt: https://v8.dev/features/wasm-bigint https://v8.dev/features/bigint Demo: https://sauleau.com/notes/wasm-bigint.html

Browsers that support WASM with BigInt: https://caniuse.com/wasm-bigint

I tried to see if the issue replicates locally, but following this manual: https://github.com/vectordotdev/vector/blob/master/lib/vector-vrl/web-playground/README.md I was not able to build VRL Playground on Windows WSL 2 Ubuntu 22.04 presumably because: error: failed to run custom build command for zstd-sys v2.0.10+zstd.1.5.6 On clang 16 as mentioned in the doc.

Presumably because zstd needs to be build like this: zstd = { version = "0.11.1", optional = true, default-features = false } But I don't know nor understand what package tries to include it and where I can set default features to false: https://github.com/gyscos/zstd-rs/issues/93 https://github.com/gyscos/zstd-rs/pull/139 https://github.com/gyscos/zstd-rs/pull/139

jszwedko commented 6 months ago

I tried to see if the issue replicates locally, but following this manual: https://github.com/vectordotdev/vector/blob/master/lib/vector-vrl/web-playground/README.md I was not able to build VRL Playground on Windows WSL 2 Ubuntu 22.04 presumably because: error: failed to run custom build command for zstd-sys v2.0.10+zstd.1.5.6 On clang 16 as mentioned in the doc.

Are there more errors in the log output that indicate the issue it had building zstd-sys? My best guess would be the lack of some build tools. Usually the build output makes that clear.

DimDroll commented 6 months ago

Thank you Jesse for taking the time to look at this,

Replication steps from my side are:

git clone https://github.com/vectordotdev/vector.git
cd vector/lib/vector-vrl/web-playground/
cargo install --locked --version 0.10.3 wasm-pack
wasm-pack build --target web --out-dir public/pkg

Here are excerpts from the error output:

...
   Compiling vector-vrl-web-playground v0.1.0 (/opt/vector/vector/lib/vector-vrl/web-playground)
The following warnings were emitted during compilation:

warning: zstd-sys@2.0.10+zstd.1.5.6: In file included from zstd/lib/common/zstd_common.c:18:
warning: zstd-sys@2.0.10+zstd.1.5.6: In file included from zstd/lib/common/zstd_internal.h:35:
warning: zstd-sys@2.0.10+zstd.1.5.6: zstd/lib/common/xxhash.h:3157:22: error: called object type 'int' is not a function or function pointer
...
warning: zstd-sys@2.0.10+zstd.1.5.6: 2 errors generated.
warning: zstd-sys@2.0.10+zstd.1.5.6: ToolExecError: Command "clang" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=wasm32-unknown-unknown" "-I" "wasm-shim/" "-I" "zstd/lib/" "-I" "zstd/lib/common" "-fvisibility=hidden" "-DXXH_STATIC_ASSERT=0" "-DZSTD_LIB_DEPRECATED=0" "-DXXH_PRIVATE_API=" "-DZSTDLIB_VISIBILITY=" "-DZSTDERRORLIB_VISIBILITY=" "-o" "/opt/vector/vector/target/wasm32-unknown-unknown/release/build/zstd-sys-e00ba60dc74396c0/out/zstd/lib/compress/zstd_opt.o" "-c" "zstd/lib/compress/zstd_opt.c" with args "clang" did not execute successfully (status code exit status: 1).
...
error: failed to run custom build command for `zstd-sys v2.0.10+zstd.1.5.6`

Caused by:
  process didn't exit successfully: `/opt/vector/vector/target/release/build/zstd-sys-757700ba3b855699/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=ZSTD_SYS_USE_PKG_CONFIG
  cargo:rustc-cfg=feature="std"
...
  error occurred: Command "clang" "-O3" "-ffunction-sections" "-fdata-sections" "-fPIC" "--target=wasm32-unknown-unknown" "-I" "wasm-shim/" "-I" "zstd/lib/" "-I" "zstd/lib/common" "-fvisibility=hidden" "-DXXH_STATIC_ASSERT=0" "-DZSTD_LIB_DEPRECATED=0" "-DXXH_PRIVATE_API=" "-DZSTDLIB_VISIBILITY=" "-DZSTDERRORLIB_VISIBILITY=" "-o" "/opt/vector/vector/target/wasm32-unknown-unknown/release/build/zstd-sys-e00ba60dc74396c0/out/zstd/lib/decompress/zstd_decompress_block.o" "-c" "zstd/lib/decompress/zstd_decompress_block.c" with args "clang" did not execute successfully (status code exit status: 1).

warning: build failed, waiting for other jobs to finish...
Error: Compiling your crate to WebAssembly failed
Caused by: failed to execute `cargo build`: exited with exit status: 101
  full command: cd "/opt/vector/vector/lib/vector-vrl/web-playground" && "cargo" "build" "--lib" "--release" "--target" "wasm32-unknown-unknown"

There are a lot more details in it, so I've attached full log in the file: wasm-pack_build_zstd-sys_error.log

jszwedko commented 6 months ago

Thanks @DimDroll . I actually see the same thing too. It looks like 0599d60758 broke it. Checking out the commit before that one builds. I'll dig into that a bit and revert the zstd update if necessary.

jszwedko commented 6 months ago

Looks like someone else discovered it too, https://github.com/gyscos/zstd-rs/issues/271, and there is an open a PR to fix it: https://github.com/gyscos/zstd-rs/pull/274. I'll bump back down in the meanwhile.

jszwedko commented 6 months ago

Opened https://github.com/vectordotdev/vector/pull/20369 to bump down the zstd dependency for now.

DimDroll commented 6 months ago

Thank you,

I was finally able to build playground and replicate the issue there as well.

For those stumbling into the thread and building it in WSL too, I had to apply this fix for WSL2: https://github.com/microsoft/WSL/issues/8691#issuecomment-1255847633 In order to access http://127.0.0.1:8000/ from Windows's env with Chrome Browser.

p.s. @jszwedko I really appreciate your lightning speed replies and changes. thank you.