Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support in spec for closure/shutdown #163

Closed
jakedoublev opened this issue Nov 25, 2022 · 7 comments
Closed

Support in spec for closure/shutdown #163

jakedoublev opened this issue Nov 25, 2022 · 7 comments

Comments

@jakedoublev
Copy link

jakedoublev commented Nov 25, 2022

Background

As it currently stands, it doesn't appear there is current support within the spec or providers for shutdown or closure functionality. Multiple flag vendors instruct within their documentation that feature flagging clients should be shut down upon closure of the application or other necessity within the implementation: LaunchDarkly client closing, Go Feature Flag deferred closing, Harness feature flags closing.

There are several reasons that a closing of the flag provision could be necessary:

  • a user who has since logged out should not continue to get flags targeted to them post-logout
  • memory leaks (especially lower-level languages/stacks)
  • ceasing of polling/streaming that many flagging tools utilize
  • some change in the app/service state that requires re-connecting to the Provider and therefore invalidation of the prior OpenFeature/Provider connection

Some of these reasons are even potential security issues (i.e. user information could remain inside OpenFeature and a Provider after logout in some cases). It's probably not a large or frequent issue, but security risks usually start that way.

Other Considerations

Post-shutdown behavior

The natural follow-up question if shutdown is provided is "what comes next?" That's a great question, and some thought is required to make sure it's handled well.

For example, take the case that an OpenFeature client is instantiated via a Provider and utilized in an application. Some state changes in the application that triggers the need for a shutdown, and with a pseudocode client.shutdown() available, the closing of the OpenFeature Client as well as the flag vendor downstream though the Provider is successful. Everything is now shut down.

With the client now closed, how should calls for flag values be handled? Throwing errors on flag state calls makes sense, but it's not the entire picture.

After shutdown, how should re-instantiation be handled? The metadata and context are defined for OpenFeature at instantiation of the OpenFeature client, so should those be cleared out as well on shutdown? In other words, does it make the most sense to clear everything out of the OpenFeature Client upon shutdown and render it unusable, throwing an error on every method?

This seems strict, but requiring re-instantiation of OpenFeature with new metadata likely makes the most sense if the flag vendor it is abstracting is also needing to be re-instantiated. A shut down client should not be functional in any way.

Shutdown/closing mean different things across flagging tools

This is perhaps the never ending question OpenFeature must handle: how can shutdown behavior be handled in a universally acceptable way? If a flag vendor does not support shutdown or closing, how can the Provider do so nonetheless? These answers will need to be handled on a case by case Provider basis with the expected behavior defined by the consensus at the OpenFeature spec level.

Hook to shut down?

In exploration of this issue, I looked into a hook as a potential option, but as they are related to flag call lifecycles and not the lifecycle of either the OpenFeature Client or Provider connection, hooks are not a viable solution.

Suggested Implementation

I propose a shutdown method on the OpenFeature Client that:

  • utilizes the Provider to trigger whatever function causes the vendor/flag-tool equivalent of close if available
  • will clear the metadata assigned at the time of OpenFeature instantiation
  • will clear the logger/telemetry collector assigned at the time of OpenFeature instantiation (and that connection if possible?)
  • will cause InactiveClientError or ShutdownClientError errors if any methods are invoked post-shutdown (or another informative error name)

The upside is an increase in robustness, durability, and flexibility, and the downside is not immediately clear. Perhaps a downside is a lack of universality in what "shutdown" means across the flag vendors, but that shouldn't mean OpenFeature's spec cannot define what "shutdown" means with some finality.

I would argue that this functionality is not bloat or a request to support an insignificant, random use-case but rather is one of the areas where the spec can mature. I'm sure there will be more areas like this identified as utilization of both feature flagging and OpenFeature specifically grows.

@toddbaert
Copy link
Member

Thanks for opening this issue @jakedoublev .

This is indeed something that needs to be addressed. There was a concensus that it could be deferred for a time, but with a focus shifting toward client providers and eventing, that time has likely come. I'd recommend you take a look at open-feature/ofep#30 and leave some of your thoughts there.

In the upcoming days I will try to coordinate an effort here and consolidate the interested contributors. I'll keep you apprised.

@jakedoublev
Copy link
Author

Apologies for creating this issue within the repo for spec and not ofep. Thanks for the feedback @toddbaert . I added comments as well on open-feature/ofep#25.

@toddbaert
Copy link
Member

@jakedoublev no need to apologize. Thanks again for your contribution!

I can't ping you in the OFEP, because you are not in the org (I can invite you if you're interested!) but I attempted to address some of your observations in open-feature/ofep#30 (comment), and I would appreciate your feedback.

@jakedoublev
Copy link
Author

@toddbaert I am interested! As I'm sure is universal - I'm not sure how much I can commit to contributing, but I'd like to be involved as much has I'm able. I encouraged my org to utilize OpenFeature at work (different GH account) and led development of a Provider to use internally as one wasn't yet available for our use case.

@toddbaert
Copy link
Member

@toddbaert I am interested! As I'm sure is universal - I'm not sure how much I can commit to contributing, but I'd like to be involved as much has I'm able. I encouraged my org to utilize OpenFeature at work (different GH account) and led development of a Provider to use internally as one wasn't yet available for our use case.

I will invite you to the org. Contribution level is always up to you.

I would love to understand your usage more. For many reasons, including CNCF graduation progress, understanding use-cases is very important!

@jakedoublev
Copy link
Author

Happy to share more async @toddbaert Feel free to shoot me a DM in the CNCF Slack and we can talk more there or via email. :)

@beeme1mr
Copy link
Member

The spec now defines shutdown behavior.

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

No branches or pull requests

3 participants