Merge branch 'allow-setting-experiment-ratio-through-chatops' into 'master'

Allow setting the experiment enabled ratio through chatops

See merge request gitlab-org/gitlab!29612

doc/development/experiment_guide/index.md

@@ -32,10 +32,9 @@ The author then adds a comment to this piece of code and adds a link to the issu
       #...
     },
     # Add your experiment here:
-    sign_up_flow: {
-      feature_toggle: :experimental_sign_up_flow, # Feature flag that will be used
-      environment: ::Gitlab.dev_env_or_com?, # Target environment
-      enabled_ratio: 0.1 # Percentage of users that will be part of the experiment. 10% of the users would be part of this experiments.
+    signup_flow: {
+      environment: ::Gitlab.dev_env_or_com?, # Target environment, defaults to enabled for development and GitLab.com
+      tracking_category: 'Growth::Acquisition::Experiment::SignUpFlow' # Used for providing the category when setting up tracking data
     }
   }.freeze
   ```
@@ -46,7 +45,7 @@ The author then adds a comment to this piece of code and adds a link to the issu
   class RegistrationController < Applicationcontroller
    def show
      # experiment_enabled?(:feature_name) is also available in views and helpers
-     if experiment_enabled?(:sign_up_flow)
+     if experiment_enabled?(:signup_flow)
        # render the experiment
      else
        # render the original version
@@ -57,11 +56,16 @@ The author then adds a comment to this piece of code and adds a link to the issu
 
 - Track necessary events. See the [telemetry guide](../../telemetry/index.md) for details.
 - After the merge request is merged, use [`chatops`](../../ci/chatops/README.md) in the
-[appropriate channel](../feature_flags/controls.md#where-to-run-commands) to enable the feature flag and start the experiment.
+[appropriate channel](../feature_flags/controls.md#where-to-run-commands) to start the experiment for 10% of the users.
+The feature flag should have the name of the experiment with the `_experiment_percentage` suffix appended.
 For visibility, please also share any commands run against production in the `#s_growth` channel:
 
   ```shell
-  /chatops run feature set --project=gitlab-org/gitlab experimental_sign_up_flow true
+  /chatops run feature set signup_flow_experiment_percentage 10
   ```
 
-  If you notice issues with the experiment, you can disable the experiment by setting the feature flag to `false` again.
+  If you notice issues with the experiment, you can disable the experiment by removing the feature flag:
+
+  ```shell
+  /chatops run feature delete signup_flow_experiment_percentage
+  ```

lib/gitlab/experimentation.rb

@@ -2,48 +2,44 @@
 
 # == Experimentation
 #
-# Utility module used for A/B testing experimental features. Define your experiments in the `EXPERIMENTS` constant.
-# The feature_toggle and environment keys are optional. If the feature_toggle is not set, a feature with the name of
-# the experiment will be checked, with a default value of true. The enabled_ratio is required and should be
-# the ratio for the number of users for which this experiment is enabled. For example: a ratio of 0.1 will
-# enable the experiment for 10% of the users (determined by the `experimentation_subject_index`).
+# Utility module for A/B testing experimental features. Define your experiments in the `EXPERIMENTS` constant.
+# Experiment options:
+# - environment (optional, defaults to enabled for development and GitLab.com)
+# - tracking_category (optional, used to set the category when tracking an experiment event)
+#
+# The experiment is controlled by a Feature Flag (https://docs.gitlab.com/ee/development/feature_flags/controls.html),
+# which is named "#{key}_experiment_percentage" and *must* be set with a percentage and not be used for other purposes.
+# To enable the experiment for 10% of the users (determined by the `experimentation_subject_index` value from a cookie):
+#
+# chatops: `/chatops run feature set key_experiment_percentage 10`
+# console: `Feature.get(:key_experiment_percentage).enable_percentage_of_time(10)`
+#
+# To disable the experiment:
+#
+# chatops: `/chatops run feature delete key_experiment_percentage`
+# console: `Feature.get(:key_experiment_percentage).remove`
 #
 module Gitlab
   module Experimentation
     EXPERIMENTS = {
       signup_flow: {
-        feature_toggle: :experimental_separate_sign_up_flow,
-        environment: ::Gitlab.dev_env_or_com?,
-        enabled_ratio: 1,
         tracking_category: 'Growth::Acquisition::Experiment::SignUpFlow'
       },
       paid_signup_flow: {
-        feature_toggle: :paid_signup_flow,
-        environment: ::Gitlab.dev_env_or_com?,
-        enabled_ratio: 1,
         tracking_category: 'Growth::Acquisition::Experiment::PaidSignUpFlow'
       },
       suggest_pipeline: {
-        feature_toggle: :suggest_pipeline,
-        environment: ::Gitlab.dev_env_or_com?,
-        enabled_ratio: 0.1,
         tracking_category: 'Growth::Expansion::Experiment::SuggestPipeline'
       },
       ci_notification_dot: {
-        feature_toggle: :ci_notification_dot,
-        environment: ::Gitlab.dev_env_or_com?,
-        enabled_ratio: 0.1,
         tracking_category: 'Growth::Expansion::Experiment::CiNotificationDot'
       },
       buy_ci_minutes_version_a: {
-        feature_toggle: :buy_ci_minutes_version_a,
-        environment: ::Gitlab.dev_env_or_com?,
-        enabled_ratio: 0.2,
         tracking_category: 'Growth::Expansion::Experiment::BuyCiMinutesVersionA'
       }
     }.freeze
 
-    # Controller concern that checks if an experimentation_subject_id cookie is present and sets it if absent.
+    # Controller concern that checks if an `experimentation_subject_id cookie` is present and sets it if absent.
     # Used for A/B testing of experimental features. Exposes the `experiment_enabled?(experiment_name)` method
     # to controllers and views. It returns true when the experiment is enabled and the user is selected as part
     # of the experimental group.
@@ -144,7 +140,7 @@ module Gitlab
         return false unless EXPERIMENTS.key?(experiment_key)
 
         experiment = experiment(experiment_key)
-        experiment.feature_toggle_enabled? && experiment.enabled_for_environment?
+        experiment.enabled? && experiment.enabled_for_environment?
       end
 
       def enabled_for_user?(experiment_key, experimentation_subject_index)
@@ -153,23 +149,28 @@ module Gitlab
       end
     end
 
-    Experiment = Struct.new(:key, :feature_toggle, :environment, :enabled_ratio, :tracking_category, keyword_init: true) do
-      def feature_toggle_enabled?
-        return Feature.enabled?(key, default_enabled: true) if feature_toggle.nil?
-
-        Feature.enabled?(feature_toggle)
+    Experiment = Struct.new(:key, :environment, :tracking_category, keyword_init: true) do
+      def enabled?
+        experiment_percentage.positive?
       end
 
       def enabled_for_environment?
-        return true if environment.nil?
+        return ::Gitlab.dev_env_or_com? if environment.nil?
 
         environment
       end
 
       def enabled_for_experimentation_subject?(experimentation_subject_index)
-        return false if enabled_ratio.nil? || experimentation_subject_index.blank?
+        return false if experimentation_subject_index.blank?
+
+        experimentation_subject_index <= experiment_percentage
+      end
+
+      private
 
-        experimentation_subject_index <= enabled_ratio * 100
+      # When a feature does not exist, the `percentage_of_time_value` method will return 0
+      def experiment_percentage
+        @experiment_percentage ||= Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value
       end
     end
   end

spec/lib/gitlab/experimentation_spec.rb

@@ -6,19 +6,16 @@ describe Gitlab::Experimentation do
   before do
     stub_const('Gitlab::Experimentation::EXPERIMENTS', {
       test_experiment: {
-        feature_toggle: feature_toggle,
         environment: environment,
-        enabled_ratio: enabled_ratio,
         tracking_category: 'Team'
       }
     })
 
-    stub_feature_flags(feature_toggle => true)
+    allow(Feature).to receive(:get).with(:test_experiment_experiment_percentage).and_return double(percentage_of_time_value: enabled_percentage)
   end
 
-  let(:feature_toggle) { :test_experiment_toggle }
   let(:environment) { Rails.env.test? }
-  let(:enabled_ratio) { 0.1 }
+  let(:enabled_percentage) { 10 }
 
   describe Gitlab::Experimentation::ControllerConcern, type: :controller do
     controller(ApplicationController) do
@@ -251,44 +248,16 @@ describe Gitlab::Experimentation do
       end
     end
 
-    describe 'feature toggle' do
-      context 'feature toggle is not set' do
-        let(:feature_toggle) { nil }
+    describe 'experiment is disabled' do
+      let(:enabled_percentage) { 0 }
 
-        it { is_expected.to be_truthy }
-      end
-
-      context 'feature toggle is not set, but a feature with the experiment key as name does exist' do
-        before do
-          stub_feature_flags(test_experiment: false)
-        end
-
-        let(:feature_toggle) { nil }
-
-        it { is_expected.to be_falsey }
-      end
-
-      context 'feature toggle is disabled' do
-        before do
-          stub_feature_flags(feature_toggle => false)
-        end
-
-        it { is_expected.to be_falsey }
-      end
+      it { is_expected.to be_falsey }
     end
 
-    describe 'environment' do
-      context 'environment is not set' do
-        let(:environment) { nil }
-
-        it { is_expected.to be_truthy }
-      end
-
-      context 'we are on the wrong environment' do
-        let(:environment) { ::Gitlab.com? }
+    describe 'we are on the wrong environment' do
+      let(:environment) { ::Gitlab.com? }
 
-        it { is_expected.to be_falsey }
-      end
+      it { is_expected.to be_falsey }
     end
   end
 
@@ -312,12 +281,6 @@ describe Gitlab::Experimentation do
 
       it { is_expected.to be_truthy }
 
-      context 'enabled ratio is not set' do
-        let(:enabled_ratio) { nil }
-
-        it { is_expected.to be_falsey }
-      end
-
       describe 'experimentation_subject_index' do
         context 'experimentation_subject_index is not set' do
           let(:experimentation_subject_index) { nil }