feat(api): implement launch darkly node sdk #3441
Conversation
p-fernandez
requested review from
davidsoderberg,
LetItRock,
ainouzgali,
scopsy,
BiswaViraj and
djabarovgeorge
NV-2323 Implement Launch Darkly Node SDK
We created to create a provider abstraction that would help us to replace easily the FF provider if needed. Definition of done
|
| private service: IFeatureFlagsService; | ||
|
|
||
| constructor() { | ||
| // TODO: In the future we can replace the object key here for an environment variable |
There was a problem hiding this comment.
This would allow users to implement or choose their own feature flags provider.
| const featureFlagsService = { | ||
| provide: FeatureFlagsService, | ||
| useFactory: async () => { | ||
| return new FeatureFlagsService(); | ||
| }, | ||
| }; |
There was a problem hiding this comment.
probably we don't need this provider and can pass the FeatureFlagsService to the providers array
| }, | ||
| }; | ||
|
|
||
| const providers = [LaunchDarklyService, featureFlagsService]; |
There was a problem hiding this comment.
I'm wondering whether these two should be added to the application-generic package instead
There was a problem hiding this comment.
Good point, in that way we can also use it inside the worker, ws and other services
There was a problem hiding this comment.
I have a fear of application-generic becoming a trash can where everything shared ends up there.
I am biased regarding the usage of feature flags in the worker, as it is the lowest level of our system, and I consider feature flags discrimination should be as early as possible in the API layer. On the other side I would understand the need to use feature flags for the WS app. Though one could say that should be controlled in the client (Web app) and to retrieve the logic from the client we have 2 options: to call to our API and have a one place of responsibility or use the Launch Darkly client SDK package.
There was a problem hiding this comment.
This is how the application-generic is used right now 🥲
I feel like it might be useful for all the apps, I can imagine a situation where we want the request to go through the API layer and just feature flag something in the Worker layer, let's say new calculations for the timed digest functionality, for some users we will calculate it with "old logic" and for others using the "new logic" (just the example).
There was a problem hiding this comment.
maybe let's start like this, and then if we will see the need then we can move to the application-generic
|
|
||
| constructor() { | ||
| // TODO: In the future we can replace the object key here for an environment variable | ||
| const service = featureFlagsProviders[FeatureFlagsProvidersEnum.LAUNCH_DARKLY]; |
There was a problem hiding this comment.
If we want to let the user have the ability to choose a provider, maybe it will be a good idea to introduce an environment variable with the provider name e.g
FEATURE_FLAG_PROVIDER=LaunchDarkly.
And update the current environment variable key LAUNCH_DARKLY_SDK_KEY to a generic name like FEATURE_FLAG_SDK_KEY, that way the user could choose from existing sdk's a provide that he wants to use.
There was a problem hiding this comment.
Exactly is what I'd like to do in the future. 👍🏻
p-fernandez
force-pushed
the
nv-2323-implement-launch-darkly-node-sdk
branch
from
752a446
to
5c48630
Compare
What change does this PR introduce?
Implements a Feature Flags service that instantiates a selected provider service, in the current case, the Launch Darkly service. Is implemented in a way that won't block execution if the feature flags provider is not set.
Why was this change needed?
Abstraction of the feature flags provider.
Other information (Screenshots)