Update development guide about feature flags

This does: - moves feature flag testing into a single place - heavily refactors `development` guide for feature flags - document a `bin/feature-flag` workflow

Update development guide about feature flags
This does: - moves feature flag testing into a single place - heavily refactors `development` guide for feature flags - document a `bin/feature-flag` workflow
2ddcaba1 · Kamil Trzciński · Marcel Amirault · 3c6ee540 · 2ddcaba1 · 2ddcaba1
Commit 2ddcaba1 authored Jul 29, 2020 by Kamil Trzciński Committed by Marcel Amirault Jul 29, 2020
6 changed files
--- a/doc/development/feature_flags/development.md
+++ b/doc/development/feature_flags/development.md
 # Developing with feature flags
-In general, it's better to have a group- or user-based gate, and you should prefer
+This document provides guidelines on how to use feature flags
-it over the use of percentage gates. This would make debugging easier, as you
+in the GitLab codebase to conditionally enable features
-filter for example logs and errors based on actors too. Furthermore, this allows
+and test them.
-for enabling for the `gitlab-org` or `gitlab-com` group first, while the rest of
+Features that are developed and merged behind a feature flag
+should not include a changelog entry. The entry should be added either in the merge
+request removing the feature flag or the merge request where the default value of
+the feature flag is set to enabled. If the feature contains any database migrations, it
+*should* include a changelog entry for the database changes.
+CAUTION: **Caution:**
+All newly-introduced feature flags should be [disabled by default](process.md#feature-flags-in-gitlab-development).
+NOTE: **Note:**
+This document is the subject of continued work as part of an epic to [improve internal usage of Feature Flags](https://gitlab.com/groups/gitlab-org/-/epics/3551). Raise any suggestions as new issues and attach them to the epic.
+## Types of feature flags
+Currently, only a single type of feature flag is available.
+Additional feature flag types will be provided in the future,
+with descriptions for their usage.
+### `development` type
+`development` feature flags are short-lived feature flags,
+used so that unfinished code can be deployed in production.
+A `development` feature flag should have a rollout issue,
+ideally created using the [Feature Flag Roll Out template](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.gitlab/issue_templates/Feature%20Flag%20Roll%20Out.md).
+NOTE: **Note:**
+This is the default type used when calling `Feature.enabled?`.
+## Feature flag definition and validation
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/229161) in GitLab 13.3.
+During development (`RAILS_ENV=development`) or testing (`RAILS_ENV=test`) all feature flag usage is being strictly validated.
+This process is meant to ensure consistent feature flag usage in the codebase. All feature flags **must**:
+- Be known. Only use feature flags that are explicitly defined.
+- Not be defined twice. They have to be defined either in FOSS or EE, but not both.
+- Use a valid and consistent `type:` across all invocations.
+- Use the same `default_enabled:` across all invocations.
+- Have an owner.
+All feature flags known to GitLab are self-documented in YAML files stored in:
+- [`config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/config/feature_flags)
+- [`ee/config/feature_flags`](https://gitlab.com/gitlab-org/gitlab/-/tree/master/ee/config/feature_flags)
+Each feature flag is defined in a separate YAML file consisting of a number of fields:
+| Field               | Required | Description                                                    |
+|---------------------|----------|----------------------------------------------------------------|
+| `name`              | yes      | Name of the feature flag.                                      |
+| `type`              | yes      | Type of feature flag.                                          |
+| `default_enabled`   | yes      | The default state of the feature flag that is strongly validated, with `default_enabled:` passed as an argument. |
+| `introduced_by_url` | no       | The URL to the Merge Request that introduced the feature flag. |
+| `rollout_issue_url` | no       | The URL to the Issue covering the feature flag rollout.        |
+| `group`             | no       | The [group](https://about.gitlab.com/handbook/product/product-categories/#devops-stages) that owns the feature flag. |
+TIP: **Tip:**
+All validations are skipped when running in `RAILS_ENV=production`.
+## Create a new feature flag
+The GitLab codebase provides [`bin/feature-flag`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/bin/feature-flag),
+a dedicated tool to create new feature flag definitions.
+The tool asks various questions about the new feature flag, then creates
+a YAML definition in `config/feature_flags` or `ee/config/feature_flags`.
+Only feature flags that have a YAML definition file can be used when running the development or testing environments.
+```shell
+$ bin/feature-flag my-feature-flag
+>> 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-feature-flag%60&
+>> Paste URL here, or enter to skip:
+?>
+create config/feature_flags/development/my_feature_flag.yml
+---
+name: my_feature_flag
+introduced_by_url:
+rollout_issue_url:
+group: group::memory
+type: development
+default_enabled: false
+```
+TIP: **Tip:**
+To create a feature flag that is only used in EE, add the `--ee` flag: `bin/feature-flag --ee`
+## Develop with a feature flag
+There are two main ways of using Feature Flags in the GitLab codebase:
+- [Backend code (Rails)](#backend)
+- [Frontend code (VueJS)](#frontend)
+### Backend
+The feature flag interface is defined in [`lib/feature.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/feature.rb).
+This interface provides a set of methods to check if the feature flag is enabled or disabled:
+```ruby
+if Feature.enabled?(:my_feature_flag, project)
+  # execute code if feature flag is enabled
+else
+  # execute code if feature flag is disabled
+end
+if Feature.disabled?(:my_feature_flag, project)
+  # execute code if feature flag is disabled
+end
+```
+In rare cases you may want to make a feature enabled by default. If so, explain the reasoning
+in the merge request. Use `default_enabled: true` when checking the feature flag state:
+```ruby
+if Feature.enabled?(:feature_flag, project, default_enabled: true)
+  # execute code if feature flag is enabled
+else
+  # execute code if feature flag is disabled
+end
+if Feature.disabled?(:my_feature_flag, project, default_enabled: true)
+  # execute code if feature flag is disabled
+end
+```
+### Frontend
+Use the `push_frontend_feature_flag` method for frontend code, which is
+available to all controllers that inherit from `ApplicationController`. You can use
+this method to expose the state of a feature flag, for example:
+```ruby
+before_action do
+  # Prefer to scope it per project or user e.g.
+  push_frontend_feature_flag(:vim_bindings, project)
+end
+def index
+  # ...
+end
+def edit
+  # ...
+end
+```
+You can then check the state of the feature flag in JavaScript as follows:
+```javascript
+if ( gon.features.vimBindings ) {
+  // ...
+}
+```
+The name of the feature flag in JavaScript is always camelCase,
+so checking for `gon.features.vim_bindings` would not work.
+See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about
+how to access feature flags in a Vue component.
+In rare cases you may want to make a feature enabled by default. If so, explain the reasoning
+in the merge request. Use `default_enabled: true` when checking the feature flag state:
+```ruby
+before_action do
+  # Prefer to scope it per project or user e.g.
+  push_frontend_feature_flag(:vim_bindings, project, default_enabled: true)
+end
+```
+### Feature actors
+**It is strongly advised to use actors with feature flags.** Actors provide a simple
+way to enable a feature flag only for a given project, group or user. This makes debugging
+easier, as you can filter logs and errors for example, based on actors. This also makes it possible
+to enable the feature on the `gitlab-org` or `gitlab-com` groups first, while the rest of
 the users aren't impacted.
+Actors also provide an easy way to do a percentage rollout of a feature in a sticky way.
+If a 1% rollout enabled a feature for a specific actor, that actor will continue to have the feature enabled at
+10%, 50%, and 100%.
+GitLab currently supports the following models as feature flag actors:
+- `User`
+- `Project`
+- `Group`
+The actor is a second parameter of the `Feature.enabled?` call. The
+same actor type must be used consistently for all invocations of `Feature.enabled?`.
 ```ruby
-# Good
 Feature.enabled?(:feature_flag, project)
+Feature.enabled?(:feature_flag, group)
-# Avoid, if possible
+Feature.enabled?(:feature_flag, user)
-Feature.enabled?(:feature_flag)
 ```
+### Enable additional objects as actors
 To use feature gates based on actors, the model needs to respond to
 `flipper_id`. For example, to enable for the Foo model:
@@ -26,32 +224,18 @@ end
 Only models that `include FeatureGate` or expose `flipper_id` method can be
 used as an actor for `Feature.enabled?`.
-Features that are developed and are intended to be merged behind a feature flag
+### Feature flags for licensed features
-should not include a changelog entry. The entry should either be added in the merge
-request removing the feature flag or the merge request where the default value of
-the feature flag is set to true. If the feature contains any DB migration it
-should include a changelog entry for DB changes.
-NOTE: **Note:**
+If a feature is license-gated, there's no need to add an additional
-All newly-introduced feature flags should be [off by default](./process.md#feature-flags-in-gitlab-development).
+explicit feature flag check since the flag will be checked as part of the
+`License.feature_available?` call. Similarly, there's no need to "clean up" a
-In rare cases you may need to set the feature flag on by default. If so, please explain the reasoning
+feature flag once the feature has reached general availability.
-in the merge request. To enable an active feature flag, use `default_enabled: true` when checking:
-```ruby
-Feature.enabled?(:feature_flag, project, default_enabled: true)
-```
 The [`Project#feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/app/models/project_feature.rb#L63-68),
 [`Namespace#feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/ee/namespace.rb#L71-85) (EE), and
 [`License.feature_available?`](https://gitlab.com/gitlab-org/gitlab/blob/4cc1c62918aa4c31750cb21dfb1a6c3492d71080/ee/app/models/license.rb#L293-300) (EE) methods all implicitly check for
 a by default enabled feature flag with the same name as the provided argument.
-For example if a feature is license-gated, there's no need to add an additional
-explicit feature flag check since the flag will be checked as part of the
-`License.feature_available?` call. Similarly, there's no need to "clean up" a
-feature flag once the feature has reached general availability.
 You'd still want to use an explicit `Feature.enabled?` check if your new feature
 isn't gated by a License or Plan.
@@ -73,10 +257,7 @@ GitLab.com and self-managed instances, you should use the
 method, according to our [definitions](https://about.gitlab.com/handbook/product/gitlab-the-product/#alpha-beta-ga). This ensures the feature is disabled unless the feature flag is
 _explicitly_ enabled.
-## Feature groups
+### Feature groups
-Starting from GitLab 9.4 we support feature groups via
-[Flipper groups](https://github.com/jnunemaker/flipper/blob/v0.10.2/docs/Gates.md#2-group).
 Feature groups must be defined statically in `lib/feature.rb` (in the
 `.register_feature_groups` method), but their implementation can obviously be
@@ -85,50 +266,144 @@ dynamic (querying the DB etc.).
 Once defined in `lib/feature.rb`, you will be able to activate a
 feature for a given feature group via the [`feature_group` parameter of the features API](../../api/features.md#set-or-create-a-feature)
-### Frontend
+### Enabling a feature flag locally (in development)
-For frontend code you can use the method `push_frontend_feature_flag`, which is
+In the rails console (`rails c`), enter the following command to enable a feature flag:
-available to all controllers that inherit from `ApplicationController`. Using
-this method you can expose the state of a feature flag as follows:
 ```ruby
-before_action do
+Feature.enable(:feature_flag_name)
-  # Prefer to scope it per project or user e.g.
+```
-  push_frontend_feature_flag(:vim_bindings, project)
-  # Avoid, if possible
+Similarly, the following command will disable a feature flag:
-  push_frontend_feature_flag(:vim_bindings)
-end
-def index
+```ruby
-  # ...
+Feature.disable(:feature_flag_name)
-end
+```
-def edit
+You can also enable a feature flag for a given gate:
-  # ...
-end
+```ruby
+Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project"))
 ```
-You can then check for the state of the feature flag in JavaScript as follows:
+## Feature flags in tests
-```javascript
+Introducing a feature flag into the codebase creates an additional codepath that should be tested.
-if ( gon.features.vimBindings ) {
+It is strongly advised to test all code affected by a feature flag, both when **enabled** and **disabled**
-  // ...
+to ensure the feature works properly.
-}
+NOTE: **Note:**
+When using the testing environment, all feature flags are enabled by default.
+To disable a feature flag in a test, use the `stub_feature_flags`
+helper. For example, to globally disable the `ci_live_trace` feature
+flag in a test:
+```ruby
+stub_feature_flags(ci_live_trace: false)
+Feature.enabled?(:ci_live_trace) # => false
 ```
-The name of the feature flag in JavaScript will always be camelCased, meaning
+If you wish to set up a test where a feature flag is enabled only
-that checking for `gon.features.vim_bindings` would not work.
+for some actors and not others, you can specify this in options
+passed to the helper. For example, to enable the `ci_live_trace`
+feature flag for a specific project:
-See the [Vue guide](../fe_guide/vue.md#accessing-feature-flags) for details about
+```ruby
-how to access feature flags in a Vue component.
+project1, project2 = build_list(:project, 2)
-### Specs
+# Feature will only be enabled for project1
+stub_feature_flags(ci_live_trace: project1)
+Feature.enabled?(:ci_live_trace) # => false
+Feature.enabled?(:ci_live_trace, project1) # => true
+Feature.enabled?(:ci_live_trace, project2) # => false
+```
+The behavior of FlipperGate is as follows:
+1. You can enable an override for a specified actor to be enabled
+1. You can disable (remove) an override for a specified actor,
+   falling back to default state
+1. There's no way to model that you explicitly disable a specified actor
+```ruby
+Feature.enable(:my_feature)
+Feature.disable(:my_feature, project1)
+Feature.enabled?(:my_feature) # => true
+Feature.enabled?(:my_feature, project1) # => true
+Feature.disable(:my_feature2)
+Feature.enable(:my_feature2, project1)
+Feature.enabled?(:my_feature2) # => false
+Feature.enabled?(:my_feature2, project1) # => true
+```
+### `stub_feature_flags` vs `Feature.enable*`
+It is preferred to use `stub_feature_flags` to enable feature flags
+in the testing environment. This method provides a simple and well described
+interface for simple use cases.
+However, in some cases more complex behavior needs to be tested,
+like percentage rollouts of feature flags. This can be done using
+`.enable_percentage_of_time` or `.enable_percentage_of_actors`:
+```ruby
+# Good: feature needs to be explicitly disabled, as it is enabled by default if not defined
+stub_feature_flags(my_feature: false)
+stub_feature_flags(my_feature: true)
+stub_feature_flags(my_feature: project)
+stub_feature_flags(my_feature: [project, project2])
+# Bad
+Feature.enable(:my_feature_2)
+# Good: enable my_feature for 50% of time
+Feature.enable_percentage_of_time(:my_feature_3, 50)
+# Good: enable my_feature for 50% of actors/gates/things
+Feature.enable_percentage_of_actors(:my_feature_4, 50)
+```
+Each feature flag that has a defined state will be persisted
+during test execution time:
+```ruby
+Feature.persisted_names.include?('my_feature') => true
+Feature.persisted_names.include?('my_feature_2') => true
+Feature.persisted_names.include?('my_feature_3') => true
+Feature.persisted_names.include?('my_feature_4') => true
+```
+### Stubbing actor
+When you want to enable a feature flag for a specific actor only,
+you can stub its representation. A gate that is passed
+as an argument to `Feature.enabled?` and `Feature.disabled?` must be an object
+that includes `FeatureGate`.
+In specs you can use the `stub_feature_flag_gate` method that allows you to
+quickly create a custom actor:
+```ruby
+gate = stub_feature_flag_gate('CustomActor')
+stub_feature_flags(ci_live_trace: gate)
+Feature.enabled?(:ci_live_trace) # => false
+Feature.enabled?(:ci_live_trace, gate) # => true
+```
+### Controlling feature flags engine in tests
 Our Flipper engine in the test environment works in a memory mode `Flipper::Adapters::Memory`.
 `production` and `development` modes use `Flipper::Adapters::ActiveRecord`.
-### `stub_feature_flags: true` (default and preferred)
+You can control whether the `Flipper::Adapters::Memory` or `ActiveRecord` mode is being used.
+#### `stub_feature_flags: true` (default and preferred)
 In this mode Flipper is configured to use `Flipper::Adapters::Memory` and mark all feature
 flags to be on-by-default and persisted on a first use. This overwrites the `default_enabled:`
@@ -148,23 +423,3 @@ a mode that is used by `production` and `development`.
 You should use this mode only when you really want to tests aspects of Flipper
 with how it interacts with `ActiveRecord`.
-### Enabling a feature flag (in development)
-In the rails console (`rails c`), enter the following command to enable your feature flag
-```ruby
-Feature.enable(:feature_flag_name)
-```
-Similarly, the following command will disable a feature flag:
-```ruby
-Feature.disable(:feature_flag_name)
-```
-You can as well enable feature flag for a given gate:
-```ruby
-Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project"))
-```
--- a/doc/development/new_fe_guide/tips.md
+++ b/doc/development/new_fe_guide/tips.md
@@ -10,12 +10,12 @@ yarn clean
 ## Creating feature flags in development
-The process for creating a feature flag is the same as [enabling a feature flag in development](../feature_flags/development.md#enabling-a-feature-flag-in-development).
+The process for creating a feature flag is the same as [enabling a feature flag in development](../feature_flags/development.md#enabling-a-feature-flag-locally-in-development).
 Your feature flag can now be:
 - [Made available to the frontend](../feature_flags/development.md#frontend) via the `gon`
- Queried in [tests](../feature_flags/development.md#specs)
+- Queried in [tests](../feature_flags/development.md#feature-flags-in-tests)
 - Queried in HAML templates and Ruby files via the `Feature.enabled?(:my_shiny_new_feature_flag)` method
 ### More on feature flags

--- a/doc/development/service_measurement.md
+++ b/doc/development/service_measurement.md
@@ -71,7 +71,7 @@ In order to actually use it, you need to enable measuring for the desired servic
 ### Enabling measurement using feature flags
 In the following example, the `:gitlab_service_measuring_projects_import_service`
-[feature flag](feature_flags/development.md#enabling-a-feature-flag-in-development) is used to enable the measuring feature
+[feature flag](feature_flags/development.md#enabling-a-feature-flag-locally-in-development) is used to enable the measuring feature
 for `Projects::ImportService`.
 From ChatOps:

--- a/doc/development/testing_guide/best_practices.md
+++ b/doc/development/testing_guide/best_practices.md
@@ -315,109 +315,7 @@ end
 ### Feature flags in tests
-All feature flags are stubbed to be enabled by default in our Ruby-based
+This section was moved to [developing with feature flags](../feature_flags/development.md).
-tests.
-To disable a feature flag in a test, use the `stub_feature_flags`
-helper. For example, to globally disable the `ci_live_trace` feature
-flag in a test:
-```ruby
-stub_feature_flags(ci_live_trace: false)
-Feature.enabled?(:ci_live_trace) # => false
-```
-If you wish to set up a test where a feature flag is enabled only
-for some actors and not others, you can specify this in options
-passed to the helper. For example, to enable the `ci_live_trace`
-feature flag for a specific project:
-```ruby
-project1, project2 = build_list(:project, 2)
-# Feature will only be enabled for project1
-stub_feature_flags(ci_live_trace: project1)
-Feature.enabled?(:ci_live_trace) # => false
-Feature.enabled?(:ci_live_trace, project1) # => true
-Feature.enabled?(:ci_live_trace, project2) # => false
-```
-This represents an actual behavior of FlipperGate:
-1. You can enable an override for a specified actor to be enabled
-1. You can disable (remove) an override for a specified actor,
-   falling back to default state
-1. There's no way to model that you explicitly disable a specified actor
-```ruby
-Feature.enable(:my_feature)
-Feature.disable(:my_feature, project1)
-Feature.enabled?(:my_feature) # => true
-Feature.enabled?(:my_feature, project1) # => true
-```
-```ruby
-Feature.disable(:my_feature2)
-Feature.enable(:my_feature2, project1)
-Feature.enabled?(:my_feature2) # => false
-Feature.enabled?(:my_feature2, project1) # => true
-```
-#### `stub_feature_flags` vs `Feature.enable*`
-It is preferred to use `stub_feature_flags` for enabling feature flags
-in testing environment. This method provides a simple and well described
-interface for a simple use-cases.
-However, in some cases a more complex behaviors needs to be tested,
-like a feature flag percentage rollouts. This can be achieved using
-the `.enable_percentage_of_time` and `.enable_percentage_of_actors`
-```ruby
-# Good: feature needs to be explicitly disabled, as it is enabled by default if not defined
-stub_feature_flags(my_feature: false)
-stub_feature_flags(my_feature: true)
-stub_feature_flags(my_feature: project)
-stub_feature_flags(my_feature: [project, project2])
-# Bad
-Feature.enable(:my_feature_2)
-# Good: enable my_feature for 50% of time
-Feature.enable_percentage_of_time(:my_feature_3, 50)
-# Good: enable my_feature for 50% of actors/gates/things
-Feature.enable_percentage_of_actors(:my_feature_4, 50)
-```
-Each feature flag that has a defined state will be persisted
-for test execution time:
-```ruby
-Feature.persisted_names.include?('my_feature') => true
-Feature.persisted_names.include?('my_feature_2') => true
-Feature.persisted_names.include?('my_feature_3') => true
-Feature.persisted_names.include?('my_feature_4') => true
-```
-#### Stubbing gate
-It is required that a gate that is passed as an argument to `Feature.enabled?`
-and `Feature.disabled?` is an object that includes `FeatureGate`.
-In specs you can use a `stub_feature_flag_gate` method that allows you to have
-quickly your custom gate:
-```ruby
-gate = stub_feature_flag_gate('CustomActor')
-stub_feature_flags(ci_live_trace: gate)
-Feature.enabled?(:ci_live_trace) # => false
-Feature.enabled?(:ci_live_trace, gate) # => true
-```
 ### Pristine test environments

--- a/doc/user/analytics/index.md
+++ b/doc/user/analytics/index.md
@@ -26,9 +26,9 @@ The following analytics features are available at the group level:
 - [Insights](../group/insights/index.md). **(ULTIMATE)**
 - [Issues](../group/issues_analytics/index.md). **(PREMIUM)**
 - [Productivity](productivity_analytics.md), enabled with the `productivity_analytics`
-  [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-in-development). **(PREMIUM)**
+  [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development). **(PREMIUM)**
 - [Value Stream](value_stream_analytics.md), enabled with the `cycle_analytics`
-  [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-in-development). **(PREMIUM)**
+  [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development). **(PREMIUM)**
 ## Project-level analytics
@@ -40,4 +40,4 @@ The following analytics features are available at the project level:
 - [Issues](../group/issues_analytics/index.md). **(PREMIUM)**
 - [Repository](repository_analytics.md).
 - [Value Stream](value_stream_analytics.md), enabled with the `cycle_analytics`
-  [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-in-development). **(STARTER)**
+  [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development). **(STARTER)**
--- a/doc/user/group/index.md
+++ b/doc/user/group/index.md
@@ -278,7 +278,7 @@ The group details view also shows the number of the following items created in t
 - Issues.
 - Members.
-These Group Activity Analytics can be enabled with the `group_activity_analytics` [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-in-development).
+These Group Activity Analytics can be enabled with the `group_activity_analytics` [feature flag](../../development/feature_flags/development.md#enabling-a-feature-flag-locally-in-development).
 ![Recent Group Activity](img/group_activity_analytics_v12_10.png)