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

Adds major version 10 featuring WASM #3

Merged
merged 6 commits into from
Dec 12, 2022
Merged

Adds major version 10 featuring WASM #3

merged 6 commits into from
Dec 12, 2022

Conversation

scottopell
Copy link
Collaborator

This refactors all existing code into v5 (FFI) and adds a new v10 (WASM).

Currently this also does not add the pre-built wasm binary blobs, that will be done as part of #1

This has a few minor cleanup TODOs left, and a few VRL features left to support to reach parity with V5 in terms of features.

@scottopell scottopell marked this pull request as ready for review December 12, 2022 22:07
@scottopell scottopell requested a review from gh123man December 12, 2022 22:08
README.md Outdated
## Usage

### Feature Support

| | V5 | V10 |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think V5 and V10 are flipped here as V5 is the CGO variant and supports Kind?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, good catch.

README.md Outdated
| Secrets | ❌ | ❌ |
| Metadata | ❌ | ❌ |
| Timezones | ❌ | ❌ |
| Requires CGO | ❌ | ✅ |
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

too big to fit in this table - but IIRC, the WASM version has a limited exposure to the VRL std lib right (some functions dont work)? May be worth a note.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, good call, added.

| Timezones | ❌ | ❌ |
| Requires CGO | ❌ | ✅ |

\* "Basic" API currently means:
Copy link
Owner

@gh123man gh123man Dec 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    • compile

"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
)

//go:embed target/wasm32-wasi/release/vrl_bridge.wasm
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is cool

Copy link
Owner

@gh123man gh123man left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀
Just some minor comments. I trust your WASM integration as I don't know in great detail how that all works. But if the tests work 👍 and im sure anything else will be caught with more development as m(if?) we address the TODOs

@scottopell scottopell merged commit b387deb into main Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants