Commit afa49321 authored by Alex Buijs's avatar Alex Buijs Committed by Rémy Coutable

Set an experimentation_id cookie on every request

Check if it exists and set it otherwise
parent 80e147a7
...@@ -14,6 +14,7 @@ class ApplicationController < ActionController::Base ...@@ -14,6 +14,7 @@ class ApplicationController < ActionController::Base
include SessionlessAuthentication include SessionlessAuthentication
include ConfirmEmailWarning include ConfirmEmailWarning
include Gitlab::Tracking::ControllerConcern include Gitlab::Tracking::ControllerConcern
include Gitlab::Experimentation::ControllerConcern
before_action :authenticate_user! before_action :authenticate_user!
before_action :enforce_terms!, if: :should_enforce_terms? before_action :enforce_terms!, if: :should_enforce_terms?
......
...@@ -37,7 +37,7 @@ describe Dashboard::ProjectsController do ...@@ -37,7 +37,7 @@ describe Dashboard::ProjectsController do
end end
it 'renders the welcome page if it has not dismissed onboarding' do it 'renders the welcome page if it has not dismissed onboarding' do
allow(controller).to receive(:cookies).and_return({ 'onboarding_dismissed' => 'false' }) cookies[:onboarding_dismissed] = 'false'
get :index get :index
...@@ -45,7 +45,7 @@ describe Dashboard::ProjectsController do ...@@ -45,7 +45,7 @@ describe Dashboard::ProjectsController do
end end
it 'renders the index template if it has dismissed the onboarding' do it 'renders the index template if it has dismissed the onboarding' do
allow(controller).to receive(:cookies).and_return({ 'onboarding_dismissed' => 'true' }) cookies[:onboarding_dismissed] = 'true'
get :index get :index
......
...@@ -17,7 +17,10 @@ describe Gitlab::Auth::GroupSaml::FailureHandler do ...@@ -17,7 +17,10 @@ describe Gitlab::Auth::GroupSaml::FailureHandler do
params = { params = {
'omniauth.error.strategy' => strategy, 'omniauth.error.strategy' => strategy,
'devise.mapping' => Devise.mappings[:user], 'devise.mapping' => Devise.mappings[:user],
'warden' => warden 'warden' => warden,
'action_dispatch.key_generator' => ActiveSupport::KeyGenerator.new('b2efbaccbdb9548217eebc73a896db73'), # necessary for setting signed cookies in lib/gitlab/experimentation.rb
'action_dispatch.signed_cookie_salt' => 'a4fb52b0ccb302eaef92bda18fedf5c3', # necessary for setting signed cookies in lib/gitlab/experimentation.rb
'action_dispatch.cookies_rotations' => OpenStruct.new(signed: []) # necessary for setting signed cookies in lib/gitlab/experimentation.rb
} }
Rack::MockRequest.env_for(path, params) Rack::MockRequest.env_for(path, params)
end end
......
# frozen_string_literal: true
# == 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`).
#
module Gitlab
module Experimentation
EXPERIMENTS = {
signup_flow: {
feature_toggle: :experimental_separate_sign_up_flow,
environment: ::Gitlab.dev_env_or_com?,
enabled_ratio: 0.1
}
}.freeze
# 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.
#
module ControllerConcern
extend ActiveSupport::Concern
included do
before_action :set_experimentation_subject_id_cookie
helper_method :experiment_enabled?
end
def set_experimentation_subject_id_cookie
return if cookies[:experimentation_subject_id].present?
cookies.permanent.signed[:experimentation_subject_id] = {
value: SecureRandom.uuid,
domain: :all,
secure: ::Gitlab.config.gitlab.https
}
end
def experiment_enabled?(experiment)
Experimentation.enabled?(experiment, experimentation_subject_index)
end
private
def experimentation_subject_index
experimentation_subject_id = cookies.signed[:experimentation_subject_id]
return if experimentation_subject_id.blank?
experimentation_subject_id.delete('-').hex % 100
end
end
class << self
def enabled?(experiment_key, experimentation_subject_index)
return false unless EXPERIMENTS.key?(experiment_key)
experiment = Experiment.new(EXPERIMENTS[experiment_key].merge(key: experiment_key))
experiment.feature_toggle_enabled? &&
experiment.enabled_for_environment? &&
experiment.enabled_for_experimentation_subject?(experimentation_subject_index)
end
end
Experiment = Struct.new(:key, :feature_toggle, :environment, :enabled_ratio, keyword_init: true) do
def feature_toggle_enabled?
return Feature.enabled?(key, default_enabled: true) if feature_toggle.nil?
Feature.enabled?(feature_toggle)
end
def enabled_for_environment?
return true if environment.nil?
environment
end
def enabled_for_experimentation_subject?(experimentation_subject_index)
return false if enabled_ratio.nil? || experimentation_subject_index.blank?
experimentation_subject_index <= enabled_ratio * 100
end
end
end
end
...@@ -189,7 +189,7 @@ describe Projects::DiscussionsController do ...@@ -189,7 +189,7 @@ describe Projects::DiscussionsController do
context "when vue_mr_discussions cookie is present" do context "when vue_mr_discussions cookie is present" do
before do before do
allow(controller).to receive(:cookies).and_return({ vue_mr_discussions: 'true' }) cookies[:vue_mr_discussions] = 'true'
end end
it "renders discussion with serializer" do it "renders discussion with serializer" do
......
# frozen_string_literal: true
require 'spec_helper'
describe Gitlab::Experimentation::ControllerConcern, type: :controller do
controller(ApplicationController) do
include Gitlab::Experimentation::ControllerConcern
def index
head :ok
end
end
describe '#set_experimentation_subject_id_cookie' do
before do
get :index
end
context 'cookie is present' do
before do
cookies[:experimentation_subject_id] = 'test'
end
it 'does not change the cookie' do
expect(cookies[:experimentation_subject_id]).to eq 'test'
end
end
context 'cookie is not present' do
it 'sets a permanent signed cookie' do
expect(cookies.permanent.signed[:experimentation_subject_id]).to be_present
end
end
end
describe '#experiment_enabled?' do
context 'cookie is not present' do
it 'calls Gitlab::Experimentation.enabled? with the name of the experiment and an experimentation_subject_index of nil' do
expect(Gitlab::Experimentation).to receive(:enabled?).with(:test_experiment, nil)
controller.experiment_enabled?(:test_experiment)
end
end
context 'cookie is present' do
before do
cookies.permanent.signed[:experimentation_subject_id] = 'abcd-1234'
get :index
end
it 'calls Gitlab::Experimentation.enabled? with the name of the experiment and an experimentation_subject_index of the modulo 100 of the hex value of the uuid' do
# 'abcd1234'.hex % 100 = 76
expect(Gitlab::Experimentation).to receive(:enabled?).with(:test_experiment, 76)
controller.experiment_enabled?(:test_experiment)
end
end
end
end
describe Gitlab::Experimentation do
before do
stub_const('Gitlab::Experimentation::EXPERIMENTS', {
test_experiment: {
feature_toggle: feature_toggle,
environment: environment,
enabled_ratio: enabled_ratio
}
})
stub_feature_flags(feature_toggle => true)
end
let(:feature_toggle) { :test_experiment_toggle }
let(:environment) { Rails.env.test? }
let(:enabled_ratio) { 0.1 }
describe '.enabled?' do
subject { described_class.enabled?(:test_experiment, experimentation_subject_index) }
let(:experimentation_subject_index) { 9 }
context 'feature toggle is enabled, we are on the right environment and we are selected' do
it { is_expected.to be_truthy }
end
describe 'experiment is not defined' do
it 'returns false' do
expect(described_class.enabled?(:missing_experiment, experimentation_subject_index)).to be_falsey
end
end
describe 'feature toggle' do
context 'feature toggle is not set' do
let(:feature_toggle) { nil }
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
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? }
it { is_expected.to be_falsey }
end
end
describe 'enabled ratio' do
context 'enabled ratio is not set' do
let(:enabled_ratio) { nil }
it { is_expected.to be_falsey }
end
context 'experimentation_subject_index is not set' do
let(:experimentation_subject_index) { nil }
it { is_expected.to be_falsey }
end
context 'experimentation_subject_index is an empty string' do
let(:experimentation_subject_index) { '' }
it { is_expected.to be_falsey }
end
context 'experimentation_subject_index outside enabled ratio' do
let(:experimentation_subject_index) { 11 }
it { is_expected.to be_falsey }
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment