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

feat: Go wasm #744

Merged
merged 37 commits into from
Feb 26, 2025
Merged

feat: Go wasm #744

merged 37 commits into from
Feb 26, 2025

Conversation

markphelps
Copy link
Contributor

@markphelps markphelps commented Feb 24, 2025

This PR redoes the Go SDK to use pure WASM to get around the issues with Go and musl v glibc issues as mentioned in #719 and other issues/PRs

High Level Changes

  1. Renamed flipt-engine-wasm to flipt-engine-wasm-js as it is JS specific since it uses wasm-bindgen to generate JS bindings
  2. Created flipt-engine-wasm which compiles to 'pure' wasm using the wasm32-wasip1 target
  3. Leverage wazero to handle the calls to / from wasm and Go without requiring CGo
  4. Implement the fetching of flag state in pure Go as WASM cant do that yet, similar to how we do in the JS/browser SDKs
  5. Changed some public facing API signatures for the Go SDK (see below as we need to decide if we want to do this now or not)

Go SDK API changes

⚠️ = breaking change

  1. ⚠️ I changed WithUpdateInterval signature to use a time.Duration instead of int as its more idiomatic
  2. I added the missing WithRef option
  3. ⚠️ I changed the New func to take in a context.Context as wazero requires one, although we could undo this change and just use context.Background.. however using a context here will allow the user to set a timeout on the initial state fetch if they wish
  4. ⚠️ I changed Close to also take in a context.Context for shutdown

TODO

  • implement FetchMode, specifically support Streaming 😬
  • Implement ErrorStrategy which means we'll need to capture the last error from fetch and hold it in the Go code
  • Update READMEs / docs
  • Benchmark against existing Go FFI based SDK
  • pprof to check for memory leaks
  • Decide what we want to do about potential breaking changes above. Either undo them or accept them and make this a new major version
  • Probably need to implement retries for fetching like we do currently in the FFI engine
  • Redo packaging for Go SDK (likely in another PR)

@codecov-commenter
Copy link

codecov-commenter commented Feb 24, 2025

Codecov Report

Attention: Patch coverage is 0% with 321 lines in your changes missing coverage. Please review.

Project coverage is 80.07%. Comparing base (f81523e) to head (0ceb451).

Files with missing lines Patch % Lines
flipt-engine-wasm/src/lib.rs 0.00% 191 Missing ⚠️
flipt-engine-wasm-js/src/lib.rs 0.00% 115 Missing ⚠️
flipt-engine-ffi/src/lib.rs 0.00% 15 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##           static-musl-build     #744      +/-   ##
=====================================================
- Coverage              84.62%   80.07%   -4.56%     
=====================================================
  Files                      7        8       +1     
  Lines                   3922     4145     +223     
=====================================================
  Hits                    3319     3319              
- Misses                   603      826     +223     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@markphelps
Copy link
Contributor Author

I'm going to open this up for review as its already large enough. Will tackle the remaining issues in other PRs before release

@markphelps markphelps marked this pull request as ready for review February 24, 2025 22:29
@markphelps markphelps requested a review from a team as a code owner February 24, 2025 22:29
@markphelps
Copy link
Contributor Author

cc @jalaziz @piclemx this should finally fix the issues with have multiple versions of the Go SDK, allowing us to collapse it down to one that should work cross-platform

@jalaziz
Copy link

jalaziz commented Feb 25, 2025

I'm so incredibly excited for this!!!! This will finally remove a fairly annoying road bump with our current usage of flipt <3

@markphelps
Copy link
Contributor Author

markphelps commented Feb 25, 2025

@erka @GeorgeMac sorry for the massive PR!!

If you get a chance I would love some 👀 on evaluation.go as that is where the meat of the changes are and a bunch of concurrency stuff going on

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: Mark Phelps <[email protected]>
@markphelps markphelps requested a review from erka February 26, 2025 17:31
Copy link
Collaborator

@erka erka left a comment

Choose a reason for hiding this comment

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

Overall it looks solid. I have only few comments about it.


// startStreaming starts the background streaming.
// Note: currently any errors cause this method to exit. We still need to implement retries for trying to reconnect.
func (e *EvaluationClient) startStreaming(ctx context.Context) {
Copy link
Collaborator

@erka erka Feb 26, 2025

Choose a reason for hiding this comment

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

Probably there should be some reconnect logic. It will work fine to the first connection/timeout error and then client will not get any updates from Flipt (for example, the restart will kill all streaming for the clients). I don't know if I can emulate this with current open-source version. I just make a wild guess here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah you are right again! i was thinking of doing the retry/reconnect logic in another PR before release as this one is already quite large. I have it as a todo on the PR description though.

@markphelps markphelps merged commit 3ab3ce2 into static-musl-build Feb 26, 2025
12 checks passed
@markphelps markphelps deleted the go-wasm branch February 26, 2025 23:37
markphelps added a commit that referenced this pull request Feb 27, 2025
* chore: split up package-ffi workflows by OS (#724)

* chore: split up package-ffi workflows by OS

Signed-off-by: Mark Phelps <[email protected]>

* chore: add back notify android test job

Signed-off-by: Mark Phelps <[email protected]>

* chore: revert back to dynamic lib for now

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to build actual static lib for musl

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix musl build

Signed-off-by: Mark Phelps <[email protected]>

* chore: turn of reqwest default features

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix cross install issue

Signed-off-by: Mark Phelps <[email protected]>

* chore: revert cross install back to GitHub

Signed-off-by: Mark Phelps <[email protected]>

* chore: add qemu

Signed-off-by: Mark Phelps <[email protected]>

* chore: only install qemu for hosts where we need cross-comp

Signed-off-by: Mark Phelps <[email protected]>

* chore: get rid of glibc builds

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to update tests to use static lib instead of dynamic lib

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix merge conflict

Signed-off-by: Mark Phelps <[email protected]>

* chore: static build flag

Signed-off-by: Mark Phelps <[email protected]>

* chore: more apk add

Signed-off-by: Mark Phelps <[email protected]>

* chore: get go tests working

Signed-off-by: Mark Phelps <[email protected]>

* chore: capture test results

Signed-off-by: Mark Phelps <[email protected]>

* chore: switch back to .so

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix package workflow

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix java file

Signed-off-by: Mark Phelps <[email protected]>

* chore: add musl shared libs

Signed-off-by: Mark Phelps <[email protected]>

* chore: set norelr

Signed-off-by: Mark Phelps <[email protected]>

* chore: refactor to try and get working with debian x86

Signed-off-by: Mark Phelps <[email protected]>

* build(deps): bump flipt-io/setup-action from 0.2.0 to 0.3.1 (#727)

Bumps [flipt-io/setup-action](https://github.com/flipt-io/setup-action) from 0.2.0 to 0.3.1.
- [Release notes](https://github.com/flipt-io/setup-action/releases)
- [Changelog](https://github.com/flipt-io/setup-action/blob/main/CHANGELOG.md)
- [Commits](flipt-io/setup-action@v0.2.0...v0.3.1)

---
updated-dependencies:
- dependency-name: flipt-io/setup-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump @types/react in /flipt-client-react (#729)

Bumps [@types/react](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react) from 19.0.8 to 19.0.10.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react)

---
updated-dependencies:
- dependency-name: "@types/react"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump @typescript-eslint/eslint-plugin (#730)

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 8.24.0 to 8.24.1.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.24.1/packages/eslint-plugin)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump @typescript-eslint/parser in /flipt-client-react (#728)

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 8.23.0 to 8.24.1.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.24.1/packages/parser)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump globals in /flipt-client-react (#731)

Bumps [globals](https://github.com/sindresorhus/globals) from 15.14.0 to 15.15.0.
- [Release notes](https://github.com/sindresorhus/globals/releases)
- [Commits](sindresorhus/globals@v15.14.0...v15.15.0)

---
updated-dependencies:
- dependency-name: globals
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump rollup in /flipt-client-react (#732)

Bumps [rollup](https://github.com/rollup/rollup) from 4.34.2 to 4.34.8.
- [Release notes](https://github.com/rollup/rollup/releases)
- [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md)
- [Commits](rollup/rollup@v4.34.2...v4.34.8)

---
updated-dependencies:
- dependency-name: rollup
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump prettier from 3.4.2 to 3.5.1 in /flipt-client-node (#734)

Bumps [prettier](https://github.com/prettier/prettier) from 3.4.2 to 3.5.1.
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](prettier/prettier@3.4.2...3.5.1)

---
updated-dependencies:
- dependency-name: prettier
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump rollup in /flipt-client-browser (#733)

Bumps [rollup](https://github.com/rollup/rollup) from 4.34.7 to 4.34.8.
- [Release notes](https://github.com/rollup/rollup/releases)
- [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md)
- [Commits](rollup/rollup@v4.34.7...v4.34.8)

---
updated-dependencies:
- dependency-name: rollup
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps-dev): bump @types/node in /flipt-client-node (#735)

Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 22.13.1 to 22.13.4.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* build(deps): bump com.android.tools.build:gradle (#736)

Bumps com.android.tools.build:gradle from 8.8.0 to 8.8.1.

---
updated-dependencies:
- dependency-name: com.android.tools.build:gradle
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: Mark Phelps <[email protected]>

* chore: cleanup

Signed-off-by: Mark Phelps <[email protected]>

* chore: holy s it worked

Signed-off-by: Mark Phelps <[email protected]>

* chore: rm verbose

Signed-off-by: Mark Phelps <[email protected]>

* chore: add gitignore for compiled libs

Signed-off-by: Mark Phelps <[email protected]>

* chore: build on debian:bullseye and link with musl-gcc

Signed-off-by: Mark Phelps <[email protected]>

* chore: test on more distros; add dagger build cache; support rlib and dylib

Signed-off-by: Mark Phelps <[email protected]>

* chore: rm duplicate flipt_engine.h

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix csharp and dart tests

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix csharp paths

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix ruby lib path

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to add output to gh summary run

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix csharp paths

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix ruby alpine build; make output a little nicer

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix csharp build

Signed-off-by: Mark Phelps <[email protected]>

* chore: rm debug for csharp; test on dotnet alpine image

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix dart build

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to debug go segfault

Signed-off-by: Mark Phelps <[email protected]>

* chore: modify build script to work with android and swift and ffi

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix engine lint errors

Signed-off-by: Mark Phelps <[email protected]>

* chore: rename exposed rust functions to _ffi

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix lint errors

Signed-off-by: Mark Phelps <[email protected]>

* chore: fix destroy_string call in swift

Signed-off-by: Mark Phelps <[email protected]>

* chore: update android wrapper

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix packaging names to be consistent

Signed-off-by: Mark Phelps <[email protected]>

* chore: rename (again) to _ffi

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to fix ordering of test output

Signed-off-by: Mark Phelps <[email protected]>

* feat: Go wasm (#744)

* chore: rm strip step

Signed-off-by: Mark Phelps <[email protected]>

* chore: split up linux and android workflows

Signed-off-by: Mark Phelps <[email protected]>

* chore: android try to ffix ndk build

Signed-off-by: Mark Phelps <[email protected]>

* chore: try to add cargo cache

Signed-off-by: Mark Phelps <[email protected]>

---------

Signed-off-by: Mark Phelps <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

4 participants