feat: add isolated api instances#243
Conversation
5d5bab8 to
ea362ff
Compare
There was a problem hiding this comment.
Code Review
This pull request refactors the OpenFeature SDK to support multiple isolated instances by moving core logic from the OpenFeatureAPI singleton into a new OpenFeatureInstance base class. Users can now create independent instances using OpenFeatureAPI.createInstance(), each maintaining its own provider, context, and hooks. A new identity-based registry ensures that a provider is not bound to multiple instances simultaneously. Feedback highlighted a bug in setProviderInternal where re-setting the same provider instance would lead to an unintended shutdown, as well as a thread-safety concern in clearProvider due to missing mutex synchronization.
565b450 to
cbd8361
Compare
5a604fb to
023a067
Compare
bencehornak
left a comment
There was a problem hiding this comment.
I usually find it a good practice to make only interfaces public and the implementing classes internal.
val OpenFeatureAPI: OpenFeatureAPIInterface = OpenFeatureImpl()
fun createIsolatedOpenFeatureAPI(): OpenFeatureAPIInterface = OpenFeatureImpl()
interface OpenFeatureAPIInterface {...}
private /* or internal */ class OpenFeatureImpl {...}This is what the official Kotlin libs often do too:
public interface MutableStateFlow<T> : StateFlow<T>, MutableSharedFlow<T> {...}
public fun <T> MutableStateFlow(value: T): MutableStateFlow<T> = StateFlowImpl(value ?: NULL)
private class StateFlowImpl<T> {...}This allows separating what's public and what's internal. Additionally, it's easier to mock the interfaces in unit tests than objects.
| var hooks: List<Hook<*>> = listOf() | ||
| private set | ||
|
|
||
| object OpenFeatureAPI : OpenFeatureInstance() { |
There was a problem hiding this comment.
I would prefer val OpenFeatureAPI = OpenFeatureInstance(), because this is not an actual inheritance (no behavior is overridden).
There was a problem hiding this comment.
You're right, no behavior is overridden. We keep object for binary compatibility because it generates a JVM class with a static final INSTANCE field that existing compiled consumers reference. A top-level val removes that class entirely causing NoClassDefFoundError at runtime.
|
If it was a green field project, I'd suggest the names: val OpenFeature: OpenFeatureAPI = OpenFeatureImpl() // BREAKING CHANGE
fun createIsolatedOpenFeatureAPI(): OpenFeatureAPI = OpenFeatureImpl()
interface OpenFeatureAPI {...}
private /* or internal */ class OpenFeatureImpl {...}instead of the names I used in my previous comment: val OpenFeatureAPI: OpenFeatureAPIInterface = OpenFeatureImpl()
fun createIsolatedOpenFeatureAPI(): OpenFeatureAPIInterface = OpenFeatureImpl()
interface OpenFeatureAPIInterface {...}
private /* or internal */ class OpenFeatureImpl {...}But it's a breaking change, so I'm not sure if it's worth annoying all current lib users. |
|
Thanks for the review @bencehornak! interface + internal impl pattern: I agree that's the ideal Kotlin pattern for new APIs. For this PR though, I updated moving |
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
… method Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
023a067 to
899775f
Compare
|
@marcozabel I think breaking changes are still allowed, esp. in the jvm compatibility layer, since we are pre 1.0. However, you are right, we should try to avoid them, if possible. One more thought on my previous line of thought: with the following trick both the Kotlin ( @file:JvmName("OpenFeatureAPI")
@JvmName("INSTANCE")
val OpenFeatureAPI = OpenFeatureInstance()IMO this pattern describes the specs better than the current proposal, because the convenience OpenFeatureAPI is an instance of And this pattern would allow the smooth introduction of the separation between the interface and implementation classes in the same PR without noticeable impact from the outside. |
bencehornak
left a comment
There was a problem hiding this comment.
Looks good otherwise, see my suggestions above.
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
bencehornak
left a comment
There was a problem hiding this comment.
One more thing: would you mind adding a Kotlin and a Java snippet to the README about the two usage patterns? (Singleton+factory)
Signed-off-by: marcozabel <marco.zabel@dynatrace.com>
| @get:JvmName("INSTANCE") | ||
| @JvmField |
There was a problem hiding this comment.
TIA, these two are apparently mutually exclusive. This way the the singleton will need to be accessed like OpenFeatureAPI.OpenFeatureAPI from Java code (as opposed to OpenFeatureAPI.INSTANCE) in versions after this:
public final class dev/openfeature/kotlin/sdk/OpenFeatureAPI {
- public static final fun INSTANCE ()Ldev/openfeature/kotlin/sdk/OpenFeatureAPIInstance;
+ public static final field OpenFeatureAPI Ldev/openfeature/kotlin/sdk/OpenFeatureAPIInstance;
...
}I'm fine with this, and I wouldn't worry too much about changing the JVM binary interface. We are in the 0.x range, where every version is allowed to contain breaking changes according to the semver specs.
| /** | ||
| * Global singleton entry point for the OpenFeature SDK. | ||
| * | ||
| * Use this directly for typical single-provider usage. For isolated, independent instances | ||
| * (e.g., for DI frameworks or testing), use [createOpenFeatureAPIInstance]. | ||
| * | ||
| * This is an instance of [OpenFeatureAPIInstance], just like any instance returned by | ||
| * the [createOpenFeatureAPIInstance] factory method. | ||
| * | ||
| * @apiNote Isolated API instances (per spec section 1.8) are experimental and subject to change. | ||
| */ | ||
| @JvmField | ||
| val OpenFeatureAPI: OpenFeatureAPIInstance = OpenFeatureAPIInstance() | ||
|
|
||
| /** | ||
| * Create a new, independent [OpenFeatureAPIInstance] with its own provider, context, hooks, | ||
| * and events — completely isolated from the global [OpenFeatureAPI] singleton and other instances. | ||
| * | ||
| * @return a new [OpenFeatureAPIInstance] | ||
| */ | ||
| fun createOpenFeatureAPIInstance(): OpenFeatureAPIInstance = OpenFeatureAPIInstance() |
There was a problem hiding this comment.
nit: @apiNote is javadoc, afaik it doesn't work with kdoc. And the note rather belongs to the factory method, doesn't it?
| /** | |
| * Global singleton entry point for the OpenFeature SDK. | |
| * | |
| * Use this directly for typical single-provider usage. For isolated, independent instances | |
| * (e.g., for DI frameworks or testing), use [createOpenFeatureAPIInstance]. | |
| * | |
| * This is an instance of [OpenFeatureAPIInstance], just like any instance returned by | |
| * the [createOpenFeatureAPIInstance] factory method. | |
| * | |
| * @apiNote Isolated API instances (per spec section 1.8) are experimental and subject to change. | |
| */ | |
| @JvmField | |
| val OpenFeatureAPI: OpenFeatureAPIInstance = OpenFeatureAPIInstance() | |
| /** | |
| * Create a new, independent [OpenFeatureAPIInstance] with its own provider, context, hooks, | |
| * and events — completely isolated from the global [OpenFeatureAPI] singleton and other instances. | |
| * | |
| * @return a new [OpenFeatureAPIInstance] | |
| */ | |
| fun createOpenFeatureAPIInstance(): OpenFeatureAPIInstance = OpenFeatureAPIInstance() | |
| /** | |
| * Global singleton entry point for the OpenFeature SDK. | |
| * | |
| * Use this directly for typical single-provider usage. For isolated, independent instances | |
| * (e.g., for DI frameworks or testing), use [createOpenFeatureAPIInstance]. | |
| * | |
| * This is an instance of [OpenFeatureAPIInstance], just like any instance returned by | |
| * the [createOpenFeatureAPIInstance] factory method. | |
| */ | |
| @JvmField | |
| val OpenFeatureAPI: OpenFeatureAPIInstance = OpenFeatureAPIInstance() | |
| /** | |
| * Create a new, independent [OpenFeatureAPIInstance] with its own provider, context, hooks, | |
| * and events — completely isolated from the global [OpenFeatureAPI] singleton and other instances. | |
| * | |
| * **Note:** isolated API instances (per spec section 1.8) are experimental and subject to change. | |
| * | |
| * @return a new [OpenFeatureAPIInstance] | |
| */ | |
| fun createOpenFeatureAPIInstance(): OpenFeatureAPIInstance = OpenFeatureAPIInstance() |
This PR
This PR introduces support for creating isolated OpenFeature API instances, each with their own providers, hooks, context, and event handling - enabling multi-tenant or side-by-side usage without shared global state.
Related Issues
#229
Notes
Follow-up Tasks
How to test