Draft: PoC: Track all development feature flags in YAML (RUN AS-IF-FOSS)
Description of the proposal
I used the New static analysis check
which seemed the most accurate.
For very long time I consider feature flag to be super handy mechanism, but also a ~"technical debt": #30228 (closed).
There's currently an ongoing effort to spread more knowledge how to use feature flags by Engineers at GitLab: gitlab-com/www-gitlab-com#7048 (closed)
This kind of builds on top of these two issues to have a better discoverability and manageability of feature flag.
This Proof of Concept introduces a config/feature-flags/my-feature.yaml
and
ee/config/feature-flags/my-feature.yaml
that tries to describe each feature flag
that we use in GitLab.
What this MR does?
This makes the following changes to process and code:
- Asks to create issue that tracks rollout and removal of FF
- Intentionally adds
default_enabled
to YAML to make it in the future a single source of truth whether the feature flag is enabled or disabled by default - Adds
bin/feature-flag
script to easily create a new definition - Changes how we stub "feature-flags" for specs, to ensure that normal Flipper semantics could be always used
- The lack of definition does not break
production
, it raises an exception indevelopment/test
environments only - It expects that each feature flag is explicitly defined
YAML
syntax
The name: rugged_commit_is_ancestor
introduced_by_url: # a URL to Merge Request introducing a feature flag
rollout_issue_url: # a URL to Issue discussing rollout of a feature flag
author: # likely not needed, as this can be easily taken from MR and Issue
group: 'group::apm' # a group that introduced a given feature flag
type: other # a type of feature flag: like FF for licensed feature, or just development/rollout type
default_enabled: false # a default state of feature flag if not defined
All the fields like _url
are considered optional. For example,
for licensed
features we might not specify rollout_issue_url
,
but may introduced_by_url
The types
I currently proposed the following types of feature flags:
-
licensed
: Licensed feature, likeee/app/models/license.rb
:contribution_analytics
-
config
: Feature flag used to configure features, like gitlab-com/gl-infra/scalability#327 (closed) -
development
: Temporary feature flag used for feature development, flags that are considered to be temporary and are ~"technical debt" according to that definition: #30228 (closed) -
other
: Other
bin/feature-flags
script
The ➜ gitlab-rails bin/feature-flag my-new-feature
>> Please specify the index for the category of your change:
1. licensed: Licensed feature
2. config: Feature flag used to configure features
3. development: Temporary feature flag used for feature development
4. other: Other
?> 3
>> Please specify the group introducing feature flag, like `group::apm`:
?> group::memory
>> Open this URL and fill the rest of details:
https://gitlab.com/gitlab-org/gitlab/-/issues/new?issue[title]=%5BFeature+flag%5D+Rollout+of+%60my-new-feature%60%60&
>> Paste URL here, or enter to skip:
?> https://gitlab.com/gitlab-org/gitlab/-/issues/12321
create config/feature-flags/my-new-feature.yml
---
name: my-new-feature
introduced_by_url:
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/12321
author:
group: group::memory
type: development
default_enabled: false
The idea of rollout
My vision for that is:
- Remove all usages of
allow(Feature)
andexpect(Feature)
from specs, ensure that we always usestub_feature_flags
- Create all definitions of current feature flags as YAML and change process how we use a new feature flag
- Ensure that
default_enabled
is coherent with YAML definition - Remove
, default_enabled: true
from code, as this makes us to copy the same line over and over and possibly have a cases where we don't change all places that we we should definedefault_enabled
The intent
My intent of proposing that change is:
- See clearly what feature flags are used and can be easily tracked by issues and YAML definition
- Make it easy to quickly compare changes between two versions of GitLab, see what new feature flags were added/removed/changed
- Make it easy to track by PM the defaults of feature flags as used by on-premise installations
- Make it more descriptive on how and where we use feature flags
- Make it easy to know who owns the given FF and make it easier to collaborate on their usage and rollout
- Maybe in the future we also figure out some consistent naming for feature flags :)
Check-list
-
Make sure this MR enables a static analysis check rule for new usage but ignores current offenses -
Mention this proposal in the relevant Slack channels (e.g. #development
,#backend
,#frontend
) -
If there is a choice to make between two potential styles, set up an emoji vote in the MR: - CHOICE_A:
🅰 - CHOICE_B:
🅱 - Vote yourself for both choices so that people know these are the choices
- CHOICE_A:
-
The MR doesn't have significant objections, and is getting a majority of 👍 vs👎 (remember that we don't need to reach a consensus) -
(If applicable) One style is getting a majority of vote (compared to the other choice) -
(If applicable) Update the MR with the chosen style -
Create a follow-up issue to fix the current offenses as a separate iteration: ISSUE_LINK -
Follow the review process as usual -
Once approved and merged by a maintainer, mention it again: -
In the relevant Slack channels (e.g. #development
,#backend
,#frontend
) -
(Optional depending on the impact of the change) In the Engineering Week in Review
-