feat(launchdarkly): Implement openfeature.StateHandler interface for clean shutdowns

[dangra]

This PR

Implements openfeature.StateHandler interface to call LDClient.Close on shutdown.

How to test

It ensures the provider implements the openfeature.StateHandler interface the same way it is done for openfeature.FeatureProvider interface.

[gemini-code-assist]

Summary of Changes

Hello @dangra, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the LaunchDarkly OpenFeature provider by integrating proper shutdown procedures. By implementing the openfeature.StateHandler interface, the provider can now gracefully close the underlying LaunchDarkly client when the application shuts down, preventing resource leaks and ensuring a clean exit. This change improves the robustness and resource management of the provider.

Highlights

• Interface Implementation: The LaunchDarkly provider now implements the openfeature.StateHandler interface, ensuring proper lifecycle management.
• Clean Shutdowns: A Shutdown method has been added to the provider, which calls LDClient.Close() to gracefully shut down the LaunchDarkly client.
• LDClient Interface Update: The internal LDClient interface has been extended to include the Close() method, aligning with the new shutdown functionality.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request correctly implements the openfeature.StateHandler interface, enabling clean shutdowns of the LaunchDarkly client. The changes are well-structured and directly address the goal of the PR. I have one suggestion to improve the test coverage for the new shutdown functionality to ensure its correctness and prevent future regressions.

[erka]

Hey @dangra. Thank you for your contribution.

Could you share a bit more about why you want to do this? It seems a bit unfair that StateHandler is used this way. Since the provider gets the client from outside, I’d expect the client to be closed externally as well.

[dangra]

hi @erka, that is a fair observation.

My take is that I don't always setup LaunchDarkly as provider but I want to my code to be independent of the provider and be able to call openfeature.Shutdown() when stopping the service.

high level it looks like this:

func main() {
   setupOpenfeature(...)
   defer openfeature.Shutdown()
  
  ofc := openfeature.GetApiInstance().GetClient()
  if ofc.Boolean("flag-name") {
      callSomeStuff()
  }
}

func setupOpenfeature(...) {
  switch envvarOrConfig {
  case "launchdarkly":
    ldc, _ := ldclient.MakeClient(sdkey, 0)
    provider := launchdarkly.NewProvider(ldc)
    openfeature.SetProvider(provider)
  case "other":
     // setup some other for local development or tests
  }
}

That is a simplified structure, my setup function does a lot more open-feature related.

In fact, due to the recent extended outage caused by AWS that also hit LaunchDarkly, I have setup a LaunchDarkly Relay Proxy and with the use of Multi-Provider it points to upstream and the relay. I am mentioning this because it is two LD providers wrapped in a single provider that supports the StateHandler interface and works transparently.

openfeature.Shutdown and LDClient.Close are both supposed to be called as last thing before shutting down, they are pretty much aligned by definition.

For the sake of completeness, this is my workaround in case other needs it:

// The launchdarkly openfeature provider doesn't implement StateHandler interface
type LaunchdarklyProvider struct {
	*ofld.Provider
	ldClient *ld.LDClient
}

func (p *LaunchdarklyProvider) Init(evCtx openfeature.EvaluationContext) error {
	return nil
}

func (p *LaunchdarklyProvider) Shutdown() {
	_ = p.ldClient.Close()
}

func newLaunchdarklyProvider(ldClient *ld.LDClient) *LaunchdarklyProvider {
	return &LaunchdarklyProvider{
		ldClient: ldClient,
		Provider: ofld.NewProvider(ldClient),
	}
}

[erka]

@dangra thank you for sharing it.

I think it would be an easier and quicker approach to simply return the shutdown function from setupOpenfeature. Something like this:

func main() {
	shutdown := setupOpenfeature( /*...*/ )
	defer shutdown()

	ofc := openfeature.NewDefaultClient()
	if ofc.Boolean("flag-name") {
		callSomeStuff()
	}
}

func setupOpenfeature( /*...*/ ) func() {
	switch envvarOrConfig {
	case "launchdarkly":
		ldc, _ := ldclient.MakeClient(sdkey, 0)
		provider := launchdarkly.NewProvider(ldc)
		openfeature.SetProvider(provider)
		return func() {
			openfeature.Shutdown()
			ldc.Close()
		}
	case "other":
		// setup some other for local development or tests
	}
	return func() {
		openfeature.Shutdown()
	}
}

Another approach would be to create the LD client in the init function so that the provider owns and closes it.

[beeme1mr]

In fact, due to the recent extended outage caused by AWS that also hit LaunchDarkly, I have setup a LaunchDarkly Relay Proxy and with the use of Multi-Provider it points to upstream and the relay.

I don't want to hijack this PR but this use case stood out to me. It may make for an interesting blog if that's something you'd be interested in.

[kinyoklion]

In fact, due to the recent extended outage caused by AWS that also hit LaunchDarkly, I have setup a LaunchDarkly Relay Proxy and with the use of Multi-Provider it points to upstream and the relay.

I don't want to hijack this PR but this use case stood out to me. It may make for an interesting blog if that's something you'd be interested in.

I would be interested in understanding why you took this approach as well. Versus always using the proxy. If you are willing to share.

[dangra]

@erka I won't argue against your approach, I think I understand the motivation behind it. To me in practical terms it is about passing the ldclient.Client instance ownership to the openfeature provider and forget about it. I also understand this change is backwards incompatible and may surprise current and future users.

The only alternative I see other than closing this PR and if you are up to, is to have a NewProvider(...) option named WithCloseOnShutdown() (or something else) that enables the new behavior so users can opt-in.

would that be ok with you?

[dangra]

hey @beeme1mr and @kinyoklion ! 👋🏼

I work for Fly.io so my approach includes running the LD relay proxy as a Fly application. But the service that uses the LD flags is the same that powers the hosting platform and I don't want to rely on a circular dependency without an external fallback source.

Fly.io doesn't directly depend on AWS but LD does, and Fly relies on LD flags to fine tune its system. The code defaults aren't always in sync and the LD outage caused not ideal flag values to kick in for the extended period LD endpoints were down.

I plan to write a community post about this topic, hopefully can get to it this week 😅

[kinyoklion]

One thing to keep in mind is that for the LD SDKs close/shutdown is terminal. We added the support to our first-party providers, but because of this they are currently not completely compliant with the specification: https://github.com/open-feature/spec/blob/main/specification/sections/02-providers.md#requirement-252

Which expects a provider to be able to recover from a shutdown.

When we did add shutdown support I think we mostly moved to support construction of the SDK instance via the provider instead of only accepting a pre-built client. Which makes this shutdown relationship more expected.

https://github.com/launchdarkly/openfeature-dotnet-server/blob/ef4b1e58e01ca718b05cdb86d468d53712c70ac2/src/LaunchDarkly.OpenFeature.ServerProvider/Provider.cs#L56

This move also allows us to configure a wrapper name/version which helps us to understand OpenFeature adoption.

[erka]

@erka I won't argue against your approach, I think I understand the motivation behind it. To me in practical terms it is about passing the ldclient.Client instance ownership to the openfeature provider and forget about it. I also understand this change is backwards incompatible and may surprise current and future users.

@dangra Yeah... someone could be unhappy

The only alternative I see other than closing this PR and if you are up to, is to have a NewProvider(...) option named WithCloseOnShutdown() (or something else) that enables the new behavior so users can opt-in.

would that be ok with you?

Yes, this could be a solid middle-ground solution.

[dangra]

@erka thanks for approving. I am afraid I forgot to sign the commits so had to rebase and force push to fulfill DCO check.