open-feature / ofep

A focal point for OpenFeature research, proposals and requests for comments
https://openfeature.dev
20 stars 15 forks source link

007-OFEP-flag-change-events #25

Closed toddbaert closed 1 year ago

toddbaert commented 2 years ago

A proposal for subscribing to flag configuration changes.

UPDATE (Nov 14)

I've made significant changes to this OFEP, taking in to account things discovered in https://github.com/open-feature/js-sdk/pull/316 and https://github.com/open-feature/js-sdk-contrib/pull/142

I also opened a much shorter OFEP that attempts to address some specific concerns about feature-discovery.

UPDATE (Feb 2)

I've added a bit more detail here around flag change events and answered some questions! Please take a look.

toddbaert commented 2 years ago

I think more broadly, we should have some guidance for SDKs which don't support all OF operations. Like.. if folks don't support doubles or events.. how should they behave.

Agreed. Thoughts on how to accomplish this? Maybe a non-normative section of the spec or something that discusses the general rules around this?

UPDATE: I think https://github.com/open-feature/ofep/blob/main/OFEP-provider-metadata-capability-discovery.md has addressed this.

InTheCloudDan commented 2 years ago

I think more broadly, we should have some guidance for SDKs which don't support all OF operations. Like.. if folks don't support doubles or events.. how should they behave.

Agreed. Thoughts on how to accomplish this? Maybe a non-normative section of the spec or something that discusses the general rules around this?

We talked about this some on the community call. I think @beeme1mr was going to follow up with some ideas around naming? One thing that was talked about is that Events like this may not even fit in the Core (just current example name) and maybe should be an Extension to the spec, because if it's in Core every provider should in theory have support for it.

toddbaert commented 1 year ago

I've added this OFEP to address some of the concerns brought up here.

cc @InTheCloudDan @justinabrahms @beeme1mr

jakedoublev commented 1 year ago

I'm curious about this proposal given the varying ways flag state changes occur in the vendor SDK's and therefore Providers - polling, streaming, on-fetch, etc. Not to throw a wrench in things, but I'm unsure how best to implement a specific collection of events and their corresponding handlers when we the individual events themselves are so varied across the flagging landscape of vendors/tools.

It seems like a flag state hook could be created that sets up an event listener within. Pseudocode: OpenFeature.addHooks(new EventListenerHook(root)) // "root" is a variable/state node that React or whatever implementation could listen for changes to abstracted away from OpenFeature or client.addHooks(new EventListenerHook(someCallbackDefinedByImplementor)) // where the implementor passes some state-change logic into the event listener hook constructor.

Maybe instead of adding a method to the OpenFeature client and enforcing a strictness on flag vendors/libraries, we could explore development of a library of optional hooks, like the ever-expanding collection of React hooks?

I think an additional alternative to building out a full event-processing framework within the OpenFeature client would be to look at events in a more black and white context for the moment, considering handling only an event that is analogous to a "breaking change" in Provider/OpenFeature Client state. If shutdown is enabled (see relevant other issue and PR ), then the implementor of OpenFeature in-app/in-service can shut down the client and reinstantiate if any event is significant enough to require doing so. This seems like a baby step in the right direction.

toddbaert commented 1 year ago

@jakedoublev

I'm curious about this proposal given the varying ways flag state changes occur in the vendor SDK's and therefore Providers - polling, streaming, on-fetch, etc. Not to throw a wrench in things, but I'm unsure how best to implement a specific collection of events and their corresponding handlers when we the individual events themselves are so varied across the flagging landscape of vendors/tools.

This is the challenge. This proposal attempts to keep things very simple. The only event types defined would be: PROVIDER_READY, PROVIDER_CONFIGURATION_CHANGED, PROVIDER_SHUTDOWN and perhaps PROVIDER_ERROR.

The vendor SDKs I've investigated fall into 2 groups with respect to events: those that are capable of emitting some kind of event or firing some callback when the backend configuration is changed, and those that are not. This proposal seems to work effectively for the first category. SDKs in the second category simply wouldn't support events if this proposal was implemented.

The data received with events (particularly relevant with flag change events) is highly variable, but that's intentionally not addressed by this proposal. If we do attach data to flag change events, it will probably be loosely structured (https://github.com/open-feature/ofep/blob/main/007-OFEP-provider-flag-metadata.md is relevant here) and optional.

It seems like a flag state hook could be created that sets up an event listener within. Pseudocode: OpenFeature.addHooks(new EventListenerHook(root)) // "root" is a variable/state node that React or whatever implementation could listen for changes to abstracted away from OpenFeature or client.addHooks(new EventListenerHook(someCallbackDefinedByImplementor)) // where the implementor passes some state-change logic into the event listener hook constructor.

I think this is an interesting idea. Currently hooks only run as part of a evaluation. If we wanted hooks to run with particular events, apart from an evaluation, and have such hooks be the primary means of adding event handlers to OpenFeature objects, I think we'd still have to define some of the interfaces in this proposal. Specifically, we'd still have to define an API that providers could use fire events. The SDK would then run the handlers defined in the hooks. We also might want to extend the hook interface to specifically define a new stage (or stages) for such events.

I think the difference with what you propose is that there wouldn't be more methods on the client, but instead the application author would use hooks to attach handlers for events.

Maybe instead of adding a method to the OpenFeature client and enforcing a strictness on flag vendors/libraries, we could explore development of a library of optional hooks, like the ever-expanding collection of React hooks?

Ya, definitely an interesting proposal. I will think about it a bit more.

I think an additional alternative to building out a full event-processing framework within the OpenFeature client would be to look at events in a more black and white context for the moment, considering handling only an event that is analogous to a "breaking change" in Provider/OpenFeature Client state. If shutdown is enabled (see relevant other issue and PR ), then the implementor of OpenFeature in-app/in-service can shut down the client and reinstantiate if any event is significant enough to require doing so. This seems like a baby step in the right direction.

I believe that we need a means of defining a callback that should run when the backend flag configuration changes. Maybe this is another means of doing that, but I'm not 100% sure I understand what you're proposing in this paragraph.

moredip commented 1 year ago

FWIW, I've experimented with using @toddbaert's js-sdk eventing PoC to create a more idiomatic react hooks API for Open Feature - here's my PoC.

Some observations based on that experiment:

toddbaert commented 1 year ago

I think more broadly, we should have some guidance for SDKs which don't support all OF operations. Like.. if folks don't support doubles or events.. how should they behave.

Agreed. Thoughts on how to accomplish this? Maybe a non-normative section of the spec or something that discusses the general rules around this?

We talked about this some on the community call. I think @beeme1mr was going to follow up with some ideas around naming? One thing that was talked about is that Events like this may not even fit in the Core (just current example name) and maybe should be an Extension to the spec, because if it's in Core every provider should in theory have support for it.

I think https://github.com/open-feature/ofep/blob/main/OFEP-provider-metadata-capability-discovery.md has addressed this.