Skip to content

007-OFEP-flag-change-events#25

Merged
toddbaert merged 8 commits intomainfrom
ofep/flag-change-events
Mar 13, 2023
Merged

007-OFEP-flag-change-events#25
toddbaert merged 8 commits intomainfrom
ofep/flag-change-events

Conversation

@toddbaert
Copy link
Copy Markdown
Member

@toddbaert toddbaert commented Jul 29, 2022

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 open-feature/js-sdk#316 and open-feature/js-sdk-contrib#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 toddbaert changed the title Ofep/flag change events 007-OFEP-flag-change-events Jul 29, 2022
Copy link
Copy Markdown
Member

@justinabrahms justinabrahms left a comment

Choose a reason for hiding this comment

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

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.

@toddbaert
Copy link
Copy Markdown
Member Author

toddbaert commented Aug 8, 2022

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
Copy link
Copy Markdown

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 toddbaert requested a review from weyert November 14, 2022 19:17
@toddbaert toddbaert force-pushed the ofep/flag-change-events branch from f6b1772 to a2b06b9 Compare November 14, 2022 19:20
@jakedoublev
Copy link
Copy Markdown

jakedoublev commented Nov 25, 2022

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
Copy link
Copy Markdown
Member Author

toddbaert commented Nov 28, 2022

@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
Copy link
Copy Markdown
Member

moredip commented Dec 1, 2022

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:

  • an event for when a provider is ready are pretty much a necessity to build a react-friendly API, because of the fact that client-side code will often try and use the provider before it's ready to provide flags.
  • an event for when the provider's configuration changes is very nice if you want the UI to reflect flag changes ASAP.
  • Overall, it was super easy to use the proposed API to achieve the two points above 👍
  • At least for React, it would be pretty important to have a removeHandler to pair with addHandler. React has a tendency to do lots of disposal of objects (component unmounting), and we need some way to clear out handlers during unmount.

Signed-off-by: Todd Baert <[email protected]>
@toddbaert
Copy link
Copy Markdown
Member Author

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.

Copy link
Copy Markdown
Member

@moredip moredip left a comment

Choose a reason for hiding this comment

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

Overall I think this is a good proposal. I have some minor questions about whether the events originate from a specific client or globally, but I think we can resolve that during the actual spec-ing part.

Co-authored-by: Michael Beemer <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert mentioned this pull request Feb 28, 2023
Co-authored-by: Justin Abrahms <[email protected]>
Signed-off-by: Todd Baert <[email protected]>
@toddbaert toddbaert merged commit daa9342 into main Mar 13, 2023
@toddbaert toddbaert deleted the ofep/flag-change-events branch March 13, 2023 20:20
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.