Add feature flag for Support Flow repeated question groups

[tangrammer]

Context

Relates #2127

https://github.com/akvo/akvo-lumen/commit/ecbb395947a2dcebfe60e7a857380ad563c7f981

The new columns created by RQGs should be invisible with no feature flag.

Once included in client-side we need to adapt backend to it.

• So far I was thinking in dataset and library endpoint

[iperdomo]

In a pairing session with @valllllll2000 we checked the available options for making code changes behind a feature flag. There are several options:

• Use a feature flag As a Service - e.g. https://launchdarkly.com/
• Use a query string parameter, e.g. https://lumen.akvotest.org/?rqg=true
• Use a configuration per user in Auth0 via app_metadata - https://auth0.com/docs/users/concepts/overview-user-metadata
• Use a configuration map in config.edn
• Use a merged configuration in config.edn via k8s ConfigMap
• Use a new table environment per tenant and expose those records in the /env endpoint

Desired properties

We understand that the flag should be short-lived. We would like:

• We want to change a flag enable/disable in a single place
• That flag configuration needs to be made available to client and backend code with an easy API
• We don't need to redeploy the service in order to make config changes
• We don't need to restart the service in order to make config changes

Feature flag as a Service

There are services like launchdarkly.com that solves all the problems related to features flags.

Pros:

• A service that we can just™ use
• They have experienced problems and edge cases that we can't foresee

Cons:

• We'll add a new 3rd party dependency to our code
• We don't have a huge user base, do we don't need segmenting by market/user etc
• $75/month for the starter package

Query-string parameter

We can enable a different code-path by adding a query-string parameter, e.g. /?rqg=true. Some team members have experience with this.

Pros:

• Easy to understand, to enable something add a little bit of text at the end of the URL

Cons:

• The flag needs to be carried in all requests from the client to the backend. If the client misses to send the flag, the backend will not execute the different code-path
• In our specific example: If a user imported a Flow form using the feature flag, it will lead to cells containing an array of values. If another user visits the same Dataset without the flag, it could lead to inconsistent results. This could be only a minor annoyance, but also could lead us to some effort wasted in bug-hunting.

Use a configuration per user in Auth0 via app_metadata

We have experience with this approach. Previous flags were attached to a user configuration.

Pros:

• We have experienced and it works

Cons:

• It's the same case as the query-string parameter. One user with the flag enabled imports, and another user then visits the same Dataset. Inconsistency on code-paths operating on the same objects.

Use a configuration map in config.edn

We configure the system with config.edn. We could add the configuration for one instance there.

Pros:

• Really easy to use. Add a new key with a map and some other properties
• We already read the configuration and expose part it to the client via the /env endpoint
• This is the model that Flow uses, we're familiar with it.

Cons:

• In order to enable/disable a property for a given instance we require a re-deploy. We affect the availability of all instances instead of just one. The code has not changed, we only want to enable/disable some configuration

Use a merged configuration in config.edn via k8s ConfigMap

This is a variation of the previous approach. Instead of having a fixed configuration in config.edn we could merge with with some other configuration exposed via a k8s ConfigMap.

Pros:

• Similar to the previous one, easy to use and we already expose some keys via /env endpoint

Cons:

• In order to make a config change we need require a service restart (e.g. kill the pod)

Use a new table environment per tenant and expose those records in the /env endpoint

In this approach we add a new table to the tenant database environment (or env) and expose its content via the /env endpoint.

Pros:

• Similar to the config map one, we already have some code, we're just adding a dynamic part based on a table
• For the backend means a SQL query to know if something is enabled or not
• For the client is accessing the configuration via /env
• In order to make a change (enable/disable/add/remove) we execute a query to the database.
• No restart or redeploy required, just a browser refresh

Cons:

• We're adding a new table, we need to maintain it
• Adding/removing/enabling/disabling a flag means executing a SQL query in the production database.

From the available list, I vote for the last one (new table env with configuration per instance).

[dlebrero]

The pro/cons of each option misses the granularity of the feature flag. The first option is per segment, the second "a la carte", third per user, last per instance.

For testing "a la carte" is awesome. Per segment can implement all of the above options.

The pro/cons should also include which options are reusable for Flow or other services.

I am not endorsing any particular option.

And now talking without understanding the details, but please be careful to not overdo feature flags. From the description above it seems that we want to feature flag the import process. I would argue that we do not need a feature flag for that. All users seeing a new column with a piece of json seems like little risk.

[tangrammer]

@valllllll2000 @iperdomo @dlebrero my only concern about using last approach Use a new table environment is that so far we don't connect to the db if user isn't authenticated and keycloak authorized. My feeling is that not connecting to DB without having a not-boot-user is good, but maybe I'm overestimating that 🤔

[iperdomo]

Thanks for the feedback

The pro/cons of each option misses the granularity of the feature flag. The first option is per segment, the second "a la carte", third per user, last per instance.

Who needs that now?

For testing "a la carte" is awesome. Per segment can implement all of the above options.

a la carte is awesome but we don't need it

The pro/cons should also include which options are reusable for Flow or other services.

Also not a requirement right now. Flow already uses its own way via System.properties.

I am not endorsing any particular option.

Thanks :+1:

And now talking without understanding the details, but please be careful to not overdo feature flags. From the description above it seems that we want to feature flag the import process. I would argue that we do not need a feature flag for that. All users seeing a new column with a piece of json seems like little risk.

That's why having a key -> value is the simplest way IMO.

[iperdomo]

my only concern about using last approach Use a new table environment is that so far we don't connect to the db if user isn't authenticated and keycloak authorized. My feeling is that not connecting to DB without having a not-boot-user is good, but maybe I'm overestimating tha

For my this is not a concern. If we want to optimize (for a reason I really don't understand) those DB calls, we can add a query parameter to include the dynamic part. By default _pre-authentication /env only respond with static config settings (IdP, piwik, etc). Post-authentication /env?dynamic=true or something similar that actual makes the DB and includes the environment: {} key.

[janagombitova]

@tangrammer and @iperdomo I believe we can close this issue. Is that correct or do you need to do something more related to the feature flag we have in place?

[iperdomo]

Nothing pending