Transform featureFlags into configFlags

[FelixMalfait]

Context

The line between feature flag and environment variables is blurry.
Ex:
If I set a config var like FREE_TRIAL_LENGTH, maybe I will want to overwrite it at the user-level?
If I create a variable GOOGLE_API_KEY maybe I want people who self-host to be able to set its value directly from the admin panel, etc.
Is IS_BILLING_ENABLED a feature flag or a config?

See how Flagsmith blends both concepts under a single object:

As of today featureFlags are used:

• in seeds (some are added automatically in development environment / condition the creation of tables)
• on the frontend, loaded as a relationship through the currentWorkspace
• as a Gate() decorator on the backend
• via direct access on the backend through the typeorm repository

And environment variables are used:

• on the frontend through ClientConfig (loaded before login)
• across the backend, sometimes when typeorm isn't loaded (in that case those variables cannot be transformer into feature flags)

Proposal

We need to extend FeatureFlag to become ConfigFlag, that means:

• being able to store server-level configuration, not just workspace-level configs.
• being able to store values, not just a true/false state

Steps:

• Rename FeatureFlag entity to configFlag, add unique index to make sure there can't be two same configVars without a workspaceId, rename value column to state, and introduce a new column called value
• Also introduce a new column type: FEATURE_FLAG, ALERT_BANNER, CONFIG_VALUE, CONFIG_VALUE_SECRET
• Create a configFlag service with a get method and refactor existing code. We want to keep a great developer experience for "features flags" in particular
• It should have a natural fallback mechanism: workspace-level feature entry takes precedence over server-level entry which takes precedence over environment variable value (coming from .env file or system env)
• It should cache the database values efficiently, we want to make sure we don't add a big performance overhead but put this in cache and clear the cache wisely
• Refactor ClientConfig. Introduce a column on configValues "accessibleToFrontend"(default to FALSE ; and if CONFIG_VALUE_SECRET cannot ever be sent to frontend). Rename Endpoint.
• Cleanup existing code / vars, improve seeding mechanism

Use-cases

• Implement alert banners Implement Non-dismissable Action-required Alerts in UI #4723
• Overwrite config vars at at a workspace level (e.g. set a different token expiration length for a given workspace)
• Admin panel Admin Panel (master issue) #4499

[capaj]

it should cache the database values efficiently, we want to make sure we don't add a big performance overhead but put this in cache and clear the cache wisely**

How exactly do you want the caching to be implemented?
Just in memory cache to avoid going into the DB every time? How long should it be cached for? 5 minutes for example?

[FelixMalfait]

@capaj Tbh I didn't really think about it in details while writing the issue.
The issue with in-memory is that it would be hard to invalidate it. We have a cache service powered by redis, that could make invalidation easier, but it won't be as efficient as in-memory. So maybe we should just do in-memory and set 5 minutes yes (forgetting about cache invalidation then? what do you think? we'll need it when we do the admin panel but can maybe afford to do it just in-memory as long as we have 1 server only it will work / for people who self-host...)
It's the kind of issue that is not straightforward and will probably requirement judgement to adapt and not follow exactly.
I also edited to add a refactoring part on clientConfig, where I gave a direction but not 100% sure it will need a bit of exploration to make the best choices

[capaj]

I can make it async and configurable. Default to in memory, but you could just supply a redis URL in env var to use redis. Redis is perfect as caching layer.

When you say cache service-do you mean something like upstash?
I was looking into deps earlier and there is no redis client in deps.

[FelixMalfait]

@capaj No I meant that we already have an abstraction layer called "Cache Storage . Service" you can find the file in the project. Env var is REDIS_HOST

[capaj]

can you link me to where it is defined?
I just searched for getRedisHost usages and it's only used in one place in

twenty/packages/twenty-server/src/integrations/message-queue/message-queue.module-factory.ts

Line 35 in 735e75b

const host = environmentService.getRedisHost();

[FelixMalfait]

https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/integrations/cache-storage/cache-storage.service.ts
https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/integrations/cache-storage/cache-storage.module.ts
https://docs.nestjs.com/techniques/caching

The file you're looking at is not up-to-date btw (not on main branch anymore)

[FelixMalfait]

What we could do eventually is put an additional layer for the config so that the first time we ask for a config flag it's retrieved from redis but all flags are stored in a var, so that if we access other feature flag/config values in the same request, it takes the in-memory value and not do other redis calls

[saadfrhan]

Hi @FelixMalfait, can you assign me this issue? I will try to solve it.

[FelixMalfait]

Thanks @saadfrhan ; @capaj are you working on it already?

[capaj]

No, I have not started on this. Assign this to anyone else. I was only considering it.

…

On Tue, 16 Apr 2024, 09:45 Félix Malfait, ***@***.***> wrote: Thanks @saadfrhan <https://github.com/saadfrhan> ; @capaj <https://github.com/capaj> are you working on it already? — Reply to this email directly, view it on GitHub <#4562 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ6WITSI6C3AEJSW3VXT4DY5TJILAVCNFSM6AAAAABE36XILOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJYGQ2DQNZRGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[FelixMalfait]

Hey @saadfrhan this is not an easy one. Ping me on Discord if you need guidance! Thanks

[saadfrhan]

Yes. I will ping you if I stuck somewhere. Thanks!

[saadfrhan]

I apologize. I thought I could solve it.