Skip to content
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

Playground to_unix_timestamp nanoseconds precision error #20325

Open
DimDroll opened this issue Apr 14, 2024 · 9 comments
Open

Playground to_unix_timestamp nanoseconds precision error #20325

DimDroll opened this issue Apr 14, 2024 · 9 comments
Labels
type: bug A code related bug. vrl: playground Changes to the VRL Web Playground.

Comments

@DimDroll
Copy link

DimDroll commented Apr 14, 2024

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
Copy link
Member

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 jszwedko transferred this issue from vectordotdev/vrl Apr 17, 2024
@jszwedko jszwedko added the vrl: playground Changes to the VRL Web Playground. label Apr 17, 2024
@jszwedko
Copy link
Member

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

@jszwedko jszwedko added the type: bug A code related bug. label Apr 17, 2024
@DimDroll
Copy link
Author

DimDroll commented Apr 24, 2024

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:
rustwasm/wasm-bindgen#2629
rustwasm/wasm-bindgen@d6d056c
https://github.com/rustwasm/wasm-bindgen/releases/tag/0.2.77

serde-wasm-bindgen also implemented it in:
RReverser/serde-wasm-bindgen#24
RReverser/serde-wasm-bindgen#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:
gyscos/zstd-rs#93
gyscos/zstd-rs#139
gyscos/zstd-rs#139

@jszwedko
Copy link
Member

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
Copy link
Author

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: [email protected]+zstd.1.5.6: In file included from zstd/lib/common/zstd_common.c:18:
warning: [email protected]+zstd.1.5.6: In file included from zstd/lib/common/zstd_internal.h:35:
warning: [email protected]+zstd.1.5.6: zstd/lib/common/xxhash.h:3157:22: error: called object type 'int' is not a function or function pointer
...
warning: [email protected]+zstd.1.5.6: 2 errors generated.
warning: [email protected]+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
Copy link
Member

Thanks @DimDroll . I actually see the same thing too. It looks like 0599d60 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
Copy link
Member

jszwedko commented Apr 24, 2024

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

@jszwedko
Copy link
Member

Opened #20369 to bump down the zstd dependency for now.

@DimDroll
Copy link
Author

DimDroll commented Apr 25, 2024

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:
microsoft/WSL#8691 (comment)
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A code related bug. vrl: playground Changes to the VRL Web Playground.
Projects
None yet
Development

No branches or pull requests

2 participants