feat: add proposal for dispose functionality

[weyert]

Add the ability to dispose of resources consumed by the OpenFeature Provider. For example, when a provider uses a setInterval or registers an event handler then they should be released and currently this is not possible

[beeme1mr]

Hey @weyert, just to confirm, the application author would need to call the dispose method themselves, right? I think this would work fine in most cases. However, OpenFeature should likely call dispose on the previously registered provider when a new one is set.

[beeme1mr]

@weyert, could you please update your proposal with the suggestions mentioned above? Also, please remember to sign-off on your comments. We're not able to merge without it.

[weyert]

Sure, hope to catch up on it next week

[beeme1mr]

@weyert, this will likely need to be something we tackle post KubeCon. However, since it should be a non-breaking change, that shouldn't be an issue.

[beeme1mr]

Hey @weyert, would you be able to update the proposal based on the feedback and sign-off on the commits?

[weyert]

Hey @weyert, would you be able to update the proposal based on the feedback and sign-off on the commits?

Sorry, more important work stuff got in the way. I will make some time this weekend to work on it.

[toddbaert]

This issue adds some relevant discussion to this OFEP. I will attempt to address some of the excellent points made in that issue here to keep discussion centralized. I'll try to keep my thoughts concise.

• I think both https://github.com/open-feature/spec/issues/163 and this OFEP suggest adding the shutdown method to the client. I think instead it should be added to the top-level API object, (ex: OpenFeatureAPI.shutdown()). My reasoning for this is because this would shut down the provider, which would impact all clients. Because the effect is global, I think it should be implemented on the global object.
• I think after shutdown is called, if a provider is put into a state that causes it to throw or otherwise abnormally function, clients should essentially behave as they would in any case when a provider doesn't resolve values as expected; by returning the default.
• We may want to implement another function to "re-initialize" the configured provider, which would again put it in a ready state.
• Implicit in the above is that the state of "readiness" (shutdown or not) is maintained in primarily in the provider, which I think is the optimal approach.

All the above seems compatible (though not completely implemented) with https://github.com/open-feature/ofep/pull/25, and with the eventing-POC in the js-sdk. I could attempt to do an implementation of the above there if it would be helpful. I could also update this OFEP if that would be helpful to you @weyert .

cc @weyert @beeme1mr @justinabrahms @moredip @jakedoublev @skyerus @tcarrio @benjiro @kinyoklion @agentgonzo

[beeme1mr]

• We may want to implement another function to "re-initialize" the configured provider, which would again put it in a ready state.

This doesn't seem necessary. You could just reregister a provider to reinitialize a provider that was shutdown.

[toddbaert]

This doesn't seem necessary. You could just reregister a provider to reinitialize a provider that was shutdown.

I had the same thought. We could do it this way, but re-registering the same provider feels a bit odd. Maybe it actually makes the most sense though. I don't feel too strongly about that one either way.

[jakedoublev]

• I think both Support in spec for closure/shutdown spec#163 and this OFEP suggest adding the shutdown method to the client. I think instead it should be added to the top-level API object, (ex: OpenFeatureAPI.shutdown()). My reasoning for this is because this would shut down the provider, which would impact all clients. Because the effect is global, I think it should be implemented on the global object.

This is really interesting. I hadn't experienced a use case of multiple clients simultaneously consuming one provider and one OpenFeatureAPI but your suggestion makes sense in light of that capability and use case. I agree that it makes the most sense to ensure a shutdown call closes all clients.

[tcarrio]

• I think both Support in spec for closure/shutdown spec#163 and this OFEP suggest adding the shutdown method to the client. I think instead it should be added to the top-level API object, (ex: OpenFeatureAPI.shutdown()). My reasoning for this is because this would shut down the provider, which would impact all clients. Because the effect is global, I think it should be implemented on the global object.

This is really interesting. I hadn't experienced a use case of multiple clients simultaneously consuming one provider and one OpenFeatureAPI but your suggestion makes sense in light of that capability and use case. I agree that it makes the most sense to ensure a shutdown call closes all clients.

OpenFeature is currently designed that you can only have a single instance of the API, and one or more clients. So the case of many clients will need to be handled.

[kinyoklion]

This doesn't seem necessary. You could just reregister a provider to reinitialize a provider that was shutdown.

I had the same thought. We could do it this way, but re-registering the same provider feels a bit odd. Maybe it actually makes the most sense though. I don't feel too strongly about that one either way.

I think registering a provider makes sense. I am doubtful registering the same provider (as in same instance) would make sense, as the provider would already be closed, so you would still need some means to tell it not to be closed. (The way my provider currently works, if it were to be closed somehow, then it couldn't really be started again.)

[toddbaert]

I think registering a provider makes sense. I am doubtful registering the same provider (as in same instance) would make sense, as the provider would already be closed, so you would still need some means to tell it not to be closed.

Great point. If some providers implement a means of "re-initialization", I think that's their business, and we can leave the SDK out of it. The SDK will simply use whatever provider is set, and if the state of readiness is maintained with the provider, we don't need to define "re-initialization" behavior on the SDK.

[toddbaert]

@jakedoublev with regards to multiple clients, this bit of doc may be helpful.

[beeme1mr]

Hey @weyert, would you be able to address the feedback and sign off your commits? This is a pre-req for some upcoming client-side changes. Thanks!

[weyert]

Hey @weyert, would you be able to address the feedback and sign off your commits? This is a pre-req for some upcoming client-side changes. Thanks!

I have applied the suggestions and signed them off. I will have a closer look what is needs to be done based on the comments

[toddbaert]

@weyert if you give me access to your fork, I can push the contents here up to it, and then we can revive this PR. Let me know!