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-feature-flag): GO Feature Flag in process using GO module #546

Merged
merged 9 commits into from
Aug 14, 2024

Conversation

thomaspoignant
Copy link
Member

This PR

In this PR we create a new provider for GO Feature Flag in process.

GO Feature Flag can be run as a go module and it was possible to run it like this in https://github.com/open-feature/go-sdk-contrib/tree/main/providers/go-feature-flag BUT this brings a lot of unused dependencies in the provider that are not used if you are using the relay-proxy.

In another PR we will release a refactor that removes the dependency to GO Feature Flag to minimize the provider's dependencies and keep it lightweight if it uses the relay proxy.

Follow-up Tasks

Another PR will come with the refactor of https://github.com/open-feature/go-sdk-contrib/tree/main/providers/go-feature-flag.

@thomaspoignant thomaspoignant requested a review from a team as a code owner July 30, 2024 15:31
@thomaspoignant thomaspoignant changed the title feat: GO Feature Flag in process using GO module feat(go-feature-flag): GO Feature Flag in process using GO module Jul 30, 2024
@thomaspoignant
Copy link
Member Author

The test failing is in the open-telemetry hook, and from my early analysis, it may be because of the GO version (1.22.5).

Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
Signed-off-by: Thomas Poignant <[email protected]>
@toddbaert
Copy link
Member

I'll take a look at this tomorrow and see what I can do abou t the CI issue.

@toddbaert
Copy link
Member

@thomaspoignant I fixed the CI issue with some dep updates (done in another branch and merged to main).

@@ -0,0 +1,22 @@
module github.com/open-feature/go-sdk-contrib/providers/go-feature-flag-in-process

go 1.22.5
Copy link
Member

Choose a reason for hiding this comment

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

of course it's up do you... but consider the go SDK still only requires 1.21, so you may be excluding some adoption with this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I know, but the library is in 1.22.5 so I have no real choice here 😒

return nil, err
}
return &Provider{
goFeatureFlagInstance: goff,
Copy link
Member

Choose a reason for hiding this comment

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

So these are the same internals that power GoFF as a standalone server I guess? That's cool!

Do you plan to support in-process in other languages? I guess that would be a lot harder 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes exactly, this is the complete GO Feature Flag code behind the provider.

In-process for other languages is something I have in mind, but I haven't found the right approach for now, the rule library I use is mainly for GO only, and I am not sure how much effort it is to have the same compatibility level for other languages.

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

👍

@toddbaert
Copy link
Member

Can you also add a license file? These recently started causing issues in our releases: #554

@thomaspoignant thomaspoignant merged commit ee5b36c into main Aug 14, 2024
5 checks passed
@toddbaert toddbaert deleted the goff-in-process branch August 15, 2024 12:33
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