IS-310: Setup GrowthBook for BE

[harishv7]

Problem

We are currently feature flagging using env variables which can get messy as we rollout more features. We also do not have an easy way to do things like percentage rollouts.

Closes IS-310

Solution

This PR introduces GrowthBook as a feature flagging system. It sets up some basic code that intialises the SDK and adds growthbook as an req property. Each time we do this, it creates a new instance of GrowthBook.

Note that growthbook can also be imported directly into the file/service where required too. The above req property injection helps us to perform conditional handling starting at controller level.

Breaking Changes

• Yes - this PR contains breaking changes
  • Details ...
• No - this PR is backwards compatible

Tests

-[ ] Create a feature flag on GrowthBook - either boolean or string or object
-[ ] Use the isOn or getFeatureValue SDK functions in req.growthbook object to retrieve the created flag
-[ ] Perform a conditional flow in one of the routes and see the behaviour

Deploy Notes

Please remove existing env var of WHITELISTED_GIT_SERVICE_REPOS from EB.

Deprecated environment variables: WHITELISTED_GIT_SERVICE_REPOS

New feature flags
ggs_whitelisted_repos: ensure correct values are set on GB portal

New environment variables:

• GROWTHBOOK_CLIENT_KEY: required for SDK to connect to GrowthBook

New dependencies:

• @growthbook/growthbook: GrowthBook SDK
• cross-fetch: Polyfill for client side libs
• eventsource: Polyfill for client side libs

[QiluXie]

lgtm. Added a few nit, non-blocking comments.

[QiluXie]

lgtm. Thanks @harishv7 for addressing the comments quickly.

[seaerchin]

my main problem with this approach is that suddenly alot of middleware ignorant stuff suddenly has to become middleware-aware

this leads to our codebase being resistant to change - if for every new feature, i'd have to pass sessionData down 3-4 layers, it gets very untenable very quickly. this is already felt most clearly when we have a nested router, and i don't want to make the problem worse than it already is.

given that this is already approved, i don't have an issue with it functionality-wise. maintainability-wise, i feel like the more direct approach of just having a global singleton might be the best - we don't actually need to pass it clientKey to getNewGrowthbookInstance as it's an env var + this allows us to trim away the deps passing of sessionData

[seaerchin]

might benefit from using logger here because we want to actually debug cases in prod, no?

[harishv7]

updated

[harishv7]

my main problem with this approach is that suddenly alot of middleware ignorant stuff suddenly has to become middleware-aware

this leads to our codebase being resistant to change - if for every new feature, i'd have to pass sessionData down 3-4 layers, it gets very untenable very quickly. this is already felt most clearly when we have a nested router, and i don't want to make the problem worse than it already is.

given that this is already approved, i don't have an issue with it functionality-wise. maintainability-wise, i feel like the more direct approach of just having a global singleton might be the best - we don't actually need to pass it clientKey to getNewGrowthbookInstance as it's an env var + this allows us to trim away the deps passing of sessionData

@seaerchin Isn't sessionData already being passed down? It's analogous to the context of a request which is short-lived till that operation finishes

just having a global singleton might be the best

The SDK is the singleton, every time we do a new GrowthBook(), behind the scenes it is actually using a singleton.

We need to instantiate a new "wrapper" over this singleton for every request since it belongs to the session of the user. It should be tied to the user's request. Hence, think of it as being part of the session

[harishv7]

my main problem with this approach is that suddenly alot of middleware ignorant stuff suddenly has to become middleware-aware
this leads to our codebase being resistant to change - if for every new feature, i'd have to pass sessionData down 3-4 layers, it gets very untenable very quickly. this is already felt most clearly when we have a nested router, and i don't want to make the problem worse than it already is.
given that this is already approved, i don't have an issue with it functionality-wise. maintainability-wise, i feel like the more direct approach of just having a global singleton might be the best - we don't actually need to pass it clientKey to getNewGrowthbookInstance as it's an env var + this allows us to trim away the deps passing of sessionData

@seaerchin Isn't sessionData already being passed down? It's analogous to the context of a request which is short-lived till that operation finishes

just having a global singleton might be the best

The SDK is the singleton, every time we do a new GrowthBook(), behind the scenes it is actually using a singleton

I clarified the part on singleton with GB team:

"Our JS and React SDKs create a shared, cached singleton across all SDK instances and each new SDK instance is just a thin, short-lived wrapper around that singleton. This allows each SDK instance to be user-specific (based on attributes) while still tapping into the central feature repository."

[seaerchin]

@seaerchin Isn't sessionData already being passed down? It's analogous to the context of a request which is short-lived till that operation finishes

tbh context is ok but the stuff that we're passing sessionData too seems too deeply nested to be aware of context (session data here). it should only be that our topmost layer (routes) are aware of the context, extract necessary values and passes them on. if we can either 1. make growthbook standalone or 2. extract required stuff from growthbook, i'm ok

The SDK is the singleton, every time we do a new GrowthBook(), behind the scenes it is actually using a singleton.

We need to instantiate a new "wrapper" over this singleton for every request since it belongs to the session of the user. It should be tied to the user's request. Hence, think of it as being part of the session

yep, i think growthbook is structured as class instance (pass properties to segment) -> acc instance

[seaerchin]

fyi, got build errors - might wanan fix prior to merge

[harishv7]

fyi, got build errors - might wanan fix prior to merge

fixed

[harishv7]

@seaerchin refactored methods to have finer depedencies