feat: improving SDK e2e test strategy

[Kavindu-Dodan]

This PR

Propose to improve OpenFeature SDK e2e test strategy by decoupling tests from flagd and introducing a self-contained provider built into SDK implementation

Feel free to comment, propose or approve the ofep based on your experience

Current dependency issue

flowchart
    A[SDK]--> B[SDK-Contrib] --> C[flagd]
    A[SDK] -.-> |e2e test dependency| C[Flagd]

Proposed change

flowchart LR 
    subgraph SDK 
    A[e2e Tests] -.-> B[In-memory provider]
    end

---

Update

• A POC implementation in go-sdk : https://github.com/open-feature/go-sdk/pull/192

• If approved, we will introduce this to OpenFeature spec through an Appendix : https://github.com/open-feature/spec/pull/199

  Note - While this OFEP is being reviewed, I will attempt to create a POC implementation of the proposed provider in one of the SDK I maintain.

[toddbaert]

I dont want us to build a "flagd" in each SDK - but I think what you're proposing (a simple in memory provider) per SDK makes sense. Indeed, as @thisthat points out, similar things have been implemented by OTel (such as the in-memory metrics exporters) and they fulfill similar roles (unit testing, etc).

I have highlighted a few additional requirements I think would be necessary to make this fully usable. I hope they make sense.

[Kavindu-Dodan]

@thisthat & @toddbaert thank you for the feedback

I have implemented a POC implementation of the proposed InMemory provider in go-sdk - https://github.com/open-feature/go-sdk/pull/192

[toddbaert]

@thisthat & @toddbaert thank you for the feedback

I have implemented a POC implementation of the proposed InMemory provider in go-sdk - open-feature/go-sdk#192

After looking at @Kavindu-Dodan 's go implementation, I agree we should do this in all SDKs.

[toddbaert]

@Kavindu-Dodan I think it's a good idea to actually add the exported in-memory provider as a spec requirement. What do you think? I think it would be a good testing utility even for consumers. We could add it as a new section, or maybe some kind of appendix.

[Kavindu-Dodan]

@Kavindu-Dodan I think it's a good idea to actually add the exported in-memory provider as a spec requirement. What do you think? I think it would be a good testing utility even for consumers. We could add it as a new section, or maybe some kind of appendix.

I think this is a good idea. Adding an appendix section with extra details and recommendations will benefit the community

Opened a spec pr - https://github.com/open-feature/spec/pull/199

[federicobond]

After looking at @Kavindu-Dodan 's go implementation, I agree we should do this in all SDKs.

Agree too. The only thing I'm not convinced about is whether flags for the in-memory provider should be set statically at initialization only or a mutable interface would be more helpful in practice.

For example, it might be helpful for some tests to allow the provider to change the resolution of a flag without having to set a whole new provider. Think cascading flag overrides for tests, where a flag is set for a whole test class and another at the method level, while keeping the previous one.

[Kavindu-Dodan]

After looking at @Kavindu-Dodan 's go implementation, I agree we should do this in all SDKs.

Agree too. The only thing I'm not convinced about is whether flags for the in-memory provider should be set statically at initialization only or a mutable interface would be more helpful in practice.

For example, it might be helpful for some tests to allow the provider to change the resolution of a flag without having to set a whole new provider. Think cascading flag overrides for tests, where a flag is set for a whole test class and another at the method level, while keeping the previous one.

I think the in-memory provider can be easily expanded if we see the need. For example, the provider constructor can accept flag storage that can be mutated independently. However, I am not sure whether we must define this through OFEP itself 🤔

[beeme1mr]

I wonder if we could use this to automagically build out a support feature matrix and update a readme. Basically, if there are e2e tests for events, it's supported by the SDK. Definitely out of scope of this OFEP, but the idea may be worth exploring.

[Kavindu-Dodan]

I wonder if we could use this to automagically build out a support feature matrix and update a readme. Basically, if there are e2e tests for events, it's supported by the SDK. Definitely out of scope of this OFEP, but the idea may be worth exploring.

If I have to do this, I would rather create an OpenFeature GitHub action 🤔 Reason is, there is a considerable effort to do this in each language (Check feature completeness -> Update readme -> Create Git PR )

[tcarrio]

Just to clarify, does this mean that the in-memory provider will be included in the SDK, or part of the repository.

The latter would be my preference. This is something that we can accomplish without tying it directly into the same package and publishing the provider separately, but we do retain atomic changes.

That way no one has to unnecessarily install the in-memory provider code unless they actually wanted to use an in-memory provider (which I can't think of many cases that actually makes sense in production).

[toddbaert]

Just to clarify, does this mean that the in-memory provider will be included in the SDK, or part of the repository.

The latter would be my preference. This is something that we can accomplish without tying it directly into the same package and publishing the provider separately, but we do retain atomic changes.

That way no one has to unnecessarily install the in-memory provider code unless they actually wanted to use an in-memory provider (which I can't think of many cases that actually makes sense in production).

I see your point, and I'd like to come to a consensus on this before accepting/merging this OFEP.

I tend to think it would be better to actually include it in the SDK. Here are my reasons why:

• The provider will be useful to others, not just SDK authors. I think Integrations will leverage it for testing. If we don't ship it the SDK, but instead only keep it in the repo, this won't be possible, or we'll have to convert SDK repos into monorepos which publish multiple components (headache++)
• The provider should be very small from a code perspective, and have no dependencies. Shipping it with the SDK should not result in a significantly larger binary. Static linking (as in Go) and tree shaking (as in JS) can be leveraged to prevent its being included in consuming applications if we build and package it with that in mind.
• We currently don't separate the API which application authors target from the toolkit used to create integrations, like OTel does. We may do this in the future (by publishing a OpenFeature API package). I would put the in-memory provider in the category of "toolkit used to create integrations", so for now, I think it should be included.

[Kavindu-Dodan]

Just to clarify, does this mean that the in-memory provider will be included in the SDK, or part of the repository. The latter would be my preference. This is something that we can accomplish without tying it directly into the same package and publishing the provider separately, but we do retain atomic changes. That way no one has to unnecessarily install the in-memory provider code unless they actually wanted to use an in-memory provider (which I can't think of many cases that actually makes sense in production).

I see your point, and I'd like to come to a consensus on this before accepting/merging this OFEP.

I tend to think it would be better to actually include it in the SDK. Here are my reasons why:

• The provider will be useful to others, not just SDK authorsl. I think Integrations will leverage it for testing. If we don't ship it the SDK, but instead only keep it in the repo, this won't be possible, or we'll have to convert SDK repos into monorepos.
• The provider should be very small from a code perspective, and have no dependencies. Shipping it with the SDK should not result in a significantly larger binary. Static linking (as in Go) and tree shaking (as in JS) can be leveraged to prevent its being included in consuming applications if we build and package it with that in mind.
• We currently don't separate the API which application authors target from the toolkit used to create integrations, like OTel does. We may do this in the future (by publishing a OpenFeature API package). I would put the in-memory provider in the category of "toolkit used to create integrations", so for now, I think it should be included.

Thank you for feedback @tcarrio & @toddbaert

I think a dedicated artifact/binary release brings more maintenance effort. And I updated the OFEP to emphasize the no dependencies requirement. This should make it lightweight and should have no(minimal) impact on our current SDK binary.

[Kavindu-Dodan]

Updated status to APPROVED given the number of approvals on the PR

If there is no further feedback, or objections on the proposal, this OFPE will be merged on next Tuesday (25th July)