Clarify Feature Flag usage
In the secure retro we discussed feature flags being a bit unclear. Raising this issue here to discuss potential ways to improve how feature flags are coded.
Unfortunately, the way we do feature flags in GitLab still causes confusion sometimes, and leads to after-the-fact fixes like !24505 (merged).
There are a few of things about feature flags which are confusing:
- They are tri-state:
enabled
,disabled
andundefined
, the last of which is handled differently depending on how they are checked.- Feature flags are checked implicitly by
feature_available?
methods, though in non-obvious ways. In effect, we complect features with feature flags. I can understand why this can be nice, and useful, but I think it is also surprising.- There's not a lot of consistency about where feature flag checks should be carried out. Ideally, this would happen in one place; often, it's multiple (e.g., in various controllers and/or templates).
enabled | disabled | undefined | |
---|---|---|---|
feature_available? (implicit check) |
|||
alpha/beta_feature_available? (implicit check) |
|||
Feature.enabled? |
|||
Feature.enabled?(default_enabled: true) |
|||
Feature.enabled?(default_enabled: false) |
With all that said, I don't know how we could redesign our feature flag usage so that engineers fall into the pit of success, rather than the pit of despair!