Commit aef9d142 authored by Dylan Griffith's avatar Dylan Griffith

Merge branch 'nicolasdular/experiment-subjects' into 'master'

Add Experiment Subjects

See merge request gitlab-org/gitlab!48256
parents 7516adb5 3cdc8904
...@@ -109,15 +109,6 @@ class InvitesController < ApplicationController ...@@ -109,15 +109,6 @@ class InvitesController < ApplicationController
end end
def track_invitation_reminders_experiment(action) def track_invitation_reminders_experiment(action)
return unless Gitlab::Experimentation.enabled?(:invitation_reminders) track_experiment_event(:invitation_reminders, action, subject: member)
property = Gitlab::Experimentation.enabled_for_attribute?(:invitation_reminders, member.invite_email) ? 'experimental_group' : 'control_group'
Gitlab::Tracking.event(
Gitlab::Experimentation.experiment(:invitation_reminders).tracking_category,
action,
property: property,
label: Digest::MD5.hexdigest(member.to_global_id.to_s)
)
end end
end end
...@@ -64,11 +64,11 @@ module Emails ...@@ -64,11 +64,11 @@ module Emails
layout: 'unknown_user_mailer' layout: 'unknown_user_mailer'
) )
if Gitlab::Experimentation.enabled?(:invitation_reminders) if Gitlab::Experimentation.active?(:invitation_reminders)
Gitlab::Tracking.event( Gitlab::Tracking.event(
Gitlab::Experimentation.experiment(:invitation_reminders).tracking_category, Gitlab::Experimentation.get_experiment(:invitation_reminders).tracking_category,
'sent', 'sent',
property: Gitlab::Experimentation.enabled_for_attribute?(:invitation_reminders, member.invite_email) ? 'experimental_group' : 'control_group', property: Gitlab::Experimentation.in_experiment_group?(:invitation_reminders, subject: member.invite_email) ? 'experimental_group' : 'control_group',
label: Digest::MD5.hexdigest(member.to_global_id.to_s) label: Digest::MD5.hexdigest(member.to_global_id.to_s)
) )
end end
......
...@@ -25,7 +25,7 @@ module Members ...@@ -25,7 +25,7 @@ module Members
private private
def experiment_enabled? def experiment_enabled?
Gitlab::Experimentation.enabled_for_attribute?(:invitation_reminders, invitation.invite_email) Gitlab::Experimentation.in_experiment_group?(:invitation_reminders, subject: invitation.invite_email)
end end
def days_after_invitation_sent def days_after_invitation_sent
......
...@@ -8,7 +8,7 @@ class MemberInvitationReminderEmailsWorker # rubocop:disable Scalability/Idempot ...@@ -8,7 +8,7 @@ class MemberInvitationReminderEmailsWorker # rubocop:disable Scalability/Idempot
urgency :low urgency :low
def perform def perform
return unless Gitlab::Experimentation.enabled?(:invitation_reminders) return unless Gitlab::Experimentation.active?(:invitation_reminders)
Member.not_accepted_invitations.not_expired.last_ten_days_excluding_today.find_in_batches do |invitations| Member.not_accepted_invitations.not_expired.last_ten_days_excluding_today.find_in_batches do |invitations|
invitations.each do |invitation| invitations.each do |invitation|
......
...@@ -56,59 +56,90 @@ addressed. ...@@ -56,59 +56,90 @@ addressed.
1. Use the experiment in the code. 1. Use the experiment in the code.
Experiments can be performed on a `subject`. The `subject` that gets provided needs to respond to `to_global_id` or `to_s`.
The resulting string is bucketed and assigned to either the control or the experimental group. It's therefore necessary to always provide the same `subject` for an experiment to have the same experience.
- Use this standard for the experiment in a controller: - Use this standard for the experiment in a controller:
```ruby Experiment run for a user:
class RegistrationController < ApplicationController
```ruby
class ProjectController < ApplicationController
def show def show
# experiment_enabled?(:experiment_key) is also available in views and helpers # experiment_enabled?(:experiment_key) is also available in views and helpers
if experiment_enabled?(:signup_flow, subject: current_user)
# render the experiment
else
# render the original version
end
end
end
```
or experiment run for a namespace:
```ruby
if experiment_enabled?(:signup_flow, subject: namespace)
# experiment code
else
# control code
end
```
When no subject is given, it falls back to a cookie that gets set and is consistent until
the cookie gets deleted.
```ruby
class RegistrationController < ApplicationController
def show
# falls back to a cookie
if experiment_enabled?(:signup_flow) if experiment_enabled?(:signup_flow)
# render the experiment # render the experiment
else else
# render the original version # render the original version
end end
end end
end end
``` ```
- Make the experiment available to the frontend in a controller: - Make the experiment available to the frontend in a controller:
```ruby ```ruby
before_action do before_action do
push_frontend_experiment(:signup_flow) push_frontend_experiment(:signup_flow, subject: current_user)
end end
``` ```
The above checks whether the experiment is enabled and push the result to the frontend. The above checks whether the experiment is enabled and pushes the result to the frontend.
You can check the state of the feature flag in JavaScript: You can check the state of the feature flag in JavaScript:
```javascript ```javascript
import { isExperimentEnabled } from '~/experimentation'; import { isExperimentEnabled } from '~/experimentation';
if ( isExperimentEnabled('signupFlow') ) { if ( isExperimentEnabled('signupFlow') ) {
// ... // ...
} }
``` ```
- It is also possible to run an experiment outside of the controller scope, for example in a worker: - It is also possible to run an experiment outside of the controller scope, for example in a worker:
```ruby ```ruby
class SomeWorker class SomeWorker
def perform def perform
# Check if the experiment is enabled at all (the percentage_of_time_value > 0) # Check if the experiment is active at all (the percentage_of_time_value > 0)
return unless Gitlab::Experimentation.enabled?(:experiment_key) return unless Gitlab::Experimentation.active?(:experiment_key)
# Since we cannot access cookies in a worker, we need to bucket models based on a unique, unchanging attribute instead. # Since we cannot access cookies in a worker, we need to bucket models based on a unique, unchanging attribute instead.
# Use the following method to check if the experiment is enabled for a certain attribute, for example a username or email address: # It is therefore necessery to always provide the same subject.
if Gitlab::Experimentation.enabled_for_attribute?(:experiment_key, some_attribute) if Gitlab::Experimentation.in_experiment_group?(:experiment_key, subject: user)
# execute experimental code # execute experimental code
else else
# execute control code # execute control code
end end
end end
end end
``` ```
### Implement the tracking events ### Implement the tracking events
...@@ -122,7 +153,7 @@ The framework provides the following helper method that is available in controll ...@@ -122,7 +153,7 @@ The framework provides the following helper method that is available in controll
```ruby ```ruby
before_action do before_action do
track_experiment_event(:signup_flow, 'action', 'value') track_experiment_event(:signup_flow, 'action', 'value', subject: current_user)
end end
``` ```
...@@ -132,7 +163,7 @@ Which can be tested as follows: ...@@ -132,7 +163,7 @@ Which can be tested as follows:
context 'when the experiment is active and the user is in the experimental group' do context 'when the experiment is active and the user is in the experimental group' do
before do before do
stub_experiment(signup_flow: true) stub_experiment(signup_flow: true)
stub_experiment_for_user(signup_flow: true) stub_experiment_for_subject(signup_flow: true)
end end
it 'tracks an event', :snowplow do it 'tracks an event', :snowplow do
...@@ -155,8 +186,8 @@ The framework provides the following helper method that is available in controll ...@@ -155,8 +186,8 @@ The framework provides the following helper method that is available in controll
```ruby ```ruby
before_action do before_action do
push_frontend_experiment(:signup_flow) push_frontend_experiment(:signup_flow, subject: current_user)
frontend_experimentation_tracking_data(:signup_flow, 'action', 'value') frontend_experimentation_tracking_data(:signup_flow, 'action', 'value', subject: current_user)
end end
``` ```
...@@ -255,7 +286,7 @@ Along with the tracking of backend and frontend events and the [recording of exp ...@@ -255,7 +286,7 @@ Along with the tracking of backend and frontend events and the [recording of exp
- **Experimental experience:** Show an in-product nudge to see if it causes more people to sign up for trials. - **Experimental experience:** Show an in-product nudge to see if it causes more people to sign up for trials.
- **Conversion event:** The user starts a trial. - **Conversion event:** The user starts a trial.
The `record_experiment_conversion_event` helper method is available to all controllers, and enables us to easily record the conversion event for the current user, regardless of whether they are in the control or experimental group: The `record_experiment_conversion_event` helper method is available to all controllers. It enables us to record the conversion event for the current user, regardless of whether they are in the control or experimental group:
```ruby ```ruby
before_action do before_action do
...@@ -296,7 +327,7 @@ context 'when the experiment is active' do ...@@ -296,7 +327,7 @@ context 'when the experiment is active' do
context 'when the user is in the experimental group' do context 'when the user is in the experimental group' do
before do before do
stub_experiment_for_user(signup_flow: true) stub_experiment_for_subject(signup_flow: true)
end end
it { is_expected.to do_experimental_thing } it { is_expected.to do_experimental_thing }
...@@ -304,7 +335,7 @@ context 'when the experiment is active' do ...@@ -304,7 +335,7 @@ context 'when the experiment is active' do
context 'when the user is in the control group' do context 'when the user is in the control group' do
before do before do
stub_experiment_for_user(signup_flow: false) stub_experiment_for_subject(signup_flow: false)
end end
it { is_expected.to do_control_thing } it { is_expected.to do_control_thing }
......
...@@ -32,7 +32,7 @@ module BillingPlansHelper ...@@ -32,7 +32,7 @@ module BillingPlansHelper
end end
def experiment_tracking_data_for_button_click(button_label) def experiment_tracking_data_for_button_click(button_label)
return {} unless Gitlab::Experimentation.enabled?(:contact_sales_btn_in_app) return {} unless Gitlab::Experimentation.active?(:contact_sales_btn_in_app)
{ {
track: { track: {
......
...@@ -12,7 +12,7 @@ module API ...@@ -12,7 +12,7 @@ module API
end end
get do get do
experiments = Gitlab::Experimentation::EXPERIMENTS.keys.map do |experiment_key| experiments = Gitlab::Experimentation::EXPERIMENTS.keys.map do |experiment_key|
{ key: experiment_key, enabled: Gitlab::Experimentation.enabled?(experiment_key) } { key: experiment_key, enabled: Gitlab::Experimentation.active?(experiment_key) }
end end
present experiments, with: EE::API::Entities::Experiment, current_user: current_user present experiments, with: EE::API::Entities::Experiment, current_user: current_user
......
...@@ -16,7 +16,7 @@ RSpec.describe Registrations::GroupsController do ...@@ -16,7 +16,7 @@ RSpec.describe Registrations::GroupsController do
context 'with an authenticated user' do context 'with an authenticated user' do
before do before do
sign_in(user) sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it { is_expected.to have_gitlab_http_status(:ok) } it { is_expected.to have_gitlab_http_status(:ok) }
...@@ -37,7 +37,7 @@ RSpec.describe Registrations::GroupsController do ...@@ -37,7 +37,7 @@ RSpec.describe Registrations::GroupsController do
context 'with the experiment not enabled for user' do context 'with the experiment not enabled for user' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
...@@ -58,7 +58,7 @@ RSpec.describe Registrations::GroupsController do ...@@ -58,7 +58,7 @@ RSpec.describe Registrations::GroupsController do
context 'with an authenticated user' do context 'with an authenticated user' do
before do before do
sign_in(user) sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it 'creates a group' do it 'creates a group' do
...@@ -122,7 +122,7 @@ RSpec.describe Registrations::GroupsController do ...@@ -122,7 +122,7 @@ RSpec.describe Registrations::GroupsController do
context 'with the experiment not enabled for user' do context 'with the experiment not enabled for user' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
......
...@@ -17,7 +17,7 @@ RSpec.describe Registrations::ProjectsController do ...@@ -17,7 +17,7 @@ RSpec.describe Registrations::ProjectsController do
context 'with an authenticated user' do context 'with an authenticated user' do
before do before do
sign_in(user) sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
...@@ -39,7 +39,7 @@ RSpec.describe Registrations::ProjectsController do ...@@ -39,7 +39,7 @@ RSpec.describe Registrations::ProjectsController do
context 'with the experiment not enabled for user' do context 'with the experiment not enabled for user' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
...@@ -61,7 +61,7 @@ RSpec.describe Registrations::ProjectsController do ...@@ -61,7 +61,7 @@ RSpec.describe Registrations::ProjectsController do
before do before do
namespace.add_owner(user) namespace.add_owner(user)
sign_in(user) sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it 'creates a new project, a "Learn GitLab" project, sets a cookie and redirects to the experience level page' do it 'creates a new project, a "Learn GitLab" project, sets a cookie and redirects to the experience level page' do
...@@ -88,7 +88,7 @@ RSpec.describe Registrations::ProjectsController do ...@@ -88,7 +88,7 @@ RSpec.describe Registrations::ProjectsController do
context 'with the experiment not enabled for user' do context 'with the experiment not enabled for user' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
......
...@@ -76,7 +76,7 @@ RSpec.describe Registrations::WelcomeController do ...@@ -76,7 +76,7 @@ RSpec.describe Registrations::WelcomeController do
context 'when part of the onboarding issues experiment' do context 'when part of the onboarding issues experiment' do
before do before do
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it { is_expected.to redirect_to new_users_sign_up_group_path } it { is_expected.to redirect_to new_users_sign_up_group_path }
...@@ -123,7 +123,7 @@ RSpec.describe Registrations::WelcomeController do ...@@ -123,7 +123,7 @@ RSpec.describe Registrations::WelcomeController do
sign_in(user) sign_in(user)
allow(::Gitlab).to receive(:com?).and_return(on_gitlab_com) allow(::Gitlab).to receive(:com?).and_return(on_gitlab_com)
stub_experiment(onboarding_issues: experiment_enabled) stub_experiment(onboarding_issues: experiment_enabled)
stub_experiment_for_user(onboarding_issues: experiment_enabled_for_user) stub_experiment_for_subject(onboarding_issues: experiment_enabled_for_user)
allow(controller.helpers).to receive(:in_subscription_flow?).and_return(in_subscription_flow) allow(controller.helpers).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
allow(controller.helpers).to receive(:in_invitation_flow?).and_return(in_invitation_flow) allow(controller.helpers).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
allow(controller.helpers).to receive(:in_oauth_flow?).and_return(in_oauth_flow) allow(controller.helpers).to receive(:in_oauth_flow?).and_return(in_oauth_flow)
......
...@@ -148,7 +148,7 @@ RSpec.describe TrialsController do ...@@ -148,7 +148,7 @@ RSpec.describe TrialsController do
context 'when the group-only trials experiment is active' do context 'when the group-only trials experiment is active' do
before do before do
stub_experiment(group_only_trials: true) stub_experiment(group_only_trials: true)
stub_experiment_for_user(group_only_trials: user_is_in_experiment?) stub_experiment_for_subject(group_only_trials: user_is_in_experiment?)
end end
def expected_group_type def expected_group_type
......
...@@ -18,7 +18,7 @@ RSpec.describe 'Billing plan pages', :feature do ...@@ -18,7 +18,7 @@ RSpec.describe 'Billing plan pages', :feature do
end end
before do before do
stub_experiment_for_user(contact_sales_btn_in_app: true) stub_experiment_for_subject(contact_sales_btn_in_app: true)
stub_full_request("#{EE::SUBSCRIPTIONS_URL}/gitlab_plans?plan=#{plan.name}") stub_full_request("#{EE::SUBSCRIPTIONS_URL}/gitlab_plans?plan=#{plan.name}")
.to_return(status: 200, body: plans_data.to_json) .to_return(status: 200, body: plans_data.to_json)
stub_application_setting(check_namespace_plan: true) stub_application_setting(check_namespace_plan: true)
......
...@@ -5,7 +5,7 @@ require 'spec_helper' ...@@ -5,7 +5,7 @@ require 'spec_helper'
RSpec.describe 'User sees new onboarding flow', :js do RSpec.describe 'User sees new onboarding flow', :js do
before do before do
stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 200) stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 200)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
allow(Gitlab).to receive(:com?).and_return(true) allow(Gitlab).to receive(:com?).and_return(true)
gitlab_sign_in(:user) gitlab_sign_in(:user)
visit users_sign_up_welcome_path visit users_sign_up_welcome_path
......
...@@ -17,7 +17,7 @@ RSpec.describe 'Welcome screen', :js do ...@@ -17,7 +17,7 @@ RSpec.describe 'Welcome screen', :js do
allow_any_instance_of(EE::WelcomeHelper).to receive(:in_invitation_flow?).and_return(in_invitation_flow) allow_any_instance_of(EE::WelcomeHelper).to receive(:in_invitation_flow?).and_return(in_invitation_flow)
allow_any_instance_of(EE::WelcomeHelper).to receive(:in_subscription_flow?).and_return(in_subscription_flow) allow_any_instance_of(EE::WelcomeHelper).to receive(:in_subscription_flow?).and_return(in_subscription_flow)
allow_any_instance_of(EE::WelcomeHelper).to receive(:in_trial_flow?).and_return(in_trial_flow) allow_any_instance_of(EE::WelcomeHelper).to receive(:in_trial_flow?).and_return(in_trial_flow)
stub_experiment_for_user(onboarding_issues: part_of_onboarding_issues_experiment) stub_experiment_for_subject(onboarding_issues: part_of_onboarding_issues_experiment)
visit users_sign_up_welcome_path visit users_sign_up_welcome_path
end end
......
...@@ -87,23 +87,49 @@ module Gitlab ...@@ -87,23 +87,49 @@ module Gitlab
}.freeze }.freeze
class << self class << self
def experiment(key) def get_experiment(experiment_key)
Gitlab::Experimentation::Experiment.new(key, **EXPERIMENTS[key]) return unless EXPERIMENTS.key?(experiment_key)
::Gitlab::Experimentation::Experiment.new(experiment_key, **EXPERIMENTS[experiment_key])
end end
def enabled?(experiment_key) def active?(experiment_key)
return false unless EXPERIMENTS.key?(experiment_key) experiment = get_experiment(experiment_key)
return false unless experiment
experiment(experiment_key).enabled? experiment.active?
end end
def enabled_for_attribute?(experiment_key, attribute) def in_experiment_group?(experiment_key, subject:)
index = Digest::SHA1.hexdigest(attribute).hex % 100 return false if subject.blank?
enabled_for_value?(experiment_key, index) return false unless active?(experiment_key)
experiment = get_experiment(experiment_key)
return false unless experiment
experiment.enabled_for_index?(index_for_subject(experiment, subject))
end
private
def index_for_subject(experiment, subject)
index = if experiment.use_backwards_compatible_subject_index
Digest::SHA1.hexdigest(subject_id(subject)).hex
else
Zlib.crc32("#{experiment.key}#{subject_id(subject)}")
end
index % 100
end end
def enabled_for_value?(experiment_key, value) def subject_id(subject)
enabled?(experiment_key) && experiment(experiment_key).enabled_for_index?(value) if subject.respond_to?(:to_global_id)
subject.to_global_id.to_s
elsif subject.respond_to?(:to_s)
subject.to_s
else
raise ArgumentError.new('Subject must respond to `to_global_id` or `to_s`')
end
end end
end end
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'zlib' require 'zlib'
# 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 # Used for A/B testing of experimental features. Exposes the `experiment_enabled?(experiment_name, subject: nil)` method
# to controllers and views. It returns true when the experiment is enabled and the user is selected as part # to controllers and views. It returns true when the experiment is enabled and the user is selected as part
# of the experimental group. # of the experimental group.
# #
...@@ -28,55 +28,56 @@ module Gitlab ...@@ -28,55 +28,56 @@ module Gitlab
} }
end end
def push_frontend_experiment(experiment_key) def push_frontend_experiment(experiment_key, subject: nil)
var_name = experiment_key.to_s.camelize(:lower) var_name = experiment_key.to_s.camelize(:lower)
enabled = experiment_enabled?(experiment_key)
enabled = experiment_enabled?(experiment_key, subject: subject)
gon.push({ experiments: { var_name => enabled } }, true) gon.push({ experiments: { var_name => enabled } }, true)
end end
def experiment_enabled?(experiment_key) def experiment_enabled?(experiment_key, subject: nil)
return true if forced_enabled?(experiment_key)
return false if dnt_enabled? return false if dnt_enabled?
return true if Experimentation.enabled_for_value?(experiment_key, experimentation_subject_index(experiment_key)) subject ||= fallback_experimentation_subject_index(experiment_key)
return true if forced_enabled?(experiment_key)
false Experimentation.in_experiment_group?(experiment_key, subject: subject)
end end
def track_experiment_event(experiment_key, action, value = nil) def track_experiment_event(experiment_key, action, value = nil, subject: nil)
return if dnt_enabled? return if dnt_enabled?
track_experiment_event_for(experiment_key, action, value) do |tracking_data| track_experiment_event_for(experiment_key, action, value, subject: subject) do |tracking_data|
::Gitlab::Tracking.event(tracking_data.delete(:category), tracking_data.delete(:action), **tracking_data) ::Gitlab::Tracking.event(tracking_data.delete(:category), tracking_data.delete(:action), **tracking_data)
end end
end end
def frontend_experimentation_tracking_data(experiment_key, action, value = nil) def frontend_experimentation_tracking_data(experiment_key, action, value = nil, subject: nil)
return if dnt_enabled? return if dnt_enabled?
track_experiment_event_for(experiment_key, action, value) do |tracking_data| track_experiment_event_for(experiment_key, action, value, subject: subject) do |tracking_data|
gon.push(tracking_data: tracking_data) gon.push(tracking_data: tracking_data)
end end
end end
def record_experiment_user(experiment_key) def record_experiment_user(experiment_key)
return if dnt_enabled? return if dnt_enabled?
return unless Experimentation.enabled?(experiment_key) && current_user return unless Experimentation.active?(experiment_key) && current_user
::Experiment.add_user(experiment_key, tracking_group(experiment_key), current_user) ::Experiment.add_user(experiment_key, tracking_group(experiment_key, nil, subject: current_user), current_user)
end end
def record_experiment_conversion_event(experiment_key) def record_experiment_conversion_event(experiment_key)
return if dnt_enabled? return if dnt_enabled?
return unless current_user return unless current_user
return unless Experimentation.enabled?(experiment_key) return unless Experimentation.active?(experiment_key)
::Experiment.record_conversion_event(experiment_key, current_user) ::Experiment.record_conversion_event(experiment_key, current_user)
end end
def experiment_tracking_category_and_group(experiment_key) def experiment_tracking_category_and_group(experiment_key, subject: nil)
"#{tracking_category(experiment_key)}:#{tracking_group(experiment_key, '_group')}" "#{tracking_category(experiment_key)}:#{tracking_group(experiment_key, '_group', subject: subject)}"
end end
private private
...@@ -89,40 +90,41 @@ module Gitlab ...@@ -89,40 +90,41 @@ module Gitlab
cookies.signed[:experimentation_subject_id] cookies.signed[:experimentation_subject_id]
end end
def experimentation_subject_index(experiment_key) def fallback_experimentation_subject_index(experiment_key)
return if experimentation_subject_id.blank? return if experimentation_subject_id.blank?
if Experimentation.experiment(experiment_key).use_backwards_compatible_subject_index if Experimentation.get_experiment(experiment_key).use_backwards_compatible_subject_index
experimentation_subject_id.delete('-').hex % 100 experimentation_subject_id.delete('-')
else else
Zlib.crc32("#{experiment_key}#{experimentation_subject_id}") % 100 experimentation_subject_id
end end
end end
def track_experiment_event_for(experiment_key, action, value) def track_experiment_event_for(experiment_key, action, value, subject: nil)
return unless Experimentation.enabled?(experiment_key) return unless Experimentation.active?(experiment_key)
yield experimentation_tracking_data(experiment_key, action, value) yield experimentation_tracking_data(experiment_key, action, value, subject: subject)
end end
def experimentation_tracking_data(experiment_key, action, value) def experimentation_tracking_data(experiment_key, action, value, subject: nil)
{ {
category: tracking_category(experiment_key), category: tracking_category(experiment_key),
action: action, action: action,
property: tracking_group(experiment_key, "_group"), property: tracking_group(experiment_key, "_group", subject: subject),
label: experimentation_subject_id, label: tracking_label(subject),
value: value value: value
}.compact }.compact
end end
def tracking_category(experiment_key) def tracking_category(experiment_key)
Experimentation.experiment(experiment_key).tracking_category Experimentation.get_experiment(experiment_key).tracking_category
end end
def tracking_group(experiment_key, suffix = nil) def tracking_group(experiment_key, suffix = nil, subject: nil)
return unless Experimentation.enabled?(experiment_key) return unless Experimentation.active?(experiment_key)
group = experiment_enabled?(experiment_key) ? GROUP_EXPERIMENTAL : GROUP_CONTROL subject ||= fallback_experimentation_subject_index(experiment_key)
group = experiment_enabled?(experiment_key, subject: subject) ? GROUP_EXPERIMENTAL : GROUP_CONTROL
suffix ? "#{group}#{suffix}" : group suffix ? "#{group}#{suffix}" : group
end end
...@@ -130,6 +132,16 @@ module Gitlab ...@@ -130,6 +132,16 @@ module Gitlab
def forced_enabled?(experiment_key) def forced_enabled?(experiment_key)
params.has_key?(:force_experiment) && params[:force_experiment] == experiment_key.to_s params.has_key?(:force_experiment) && params[:force_experiment] == experiment_key.to_s
end end
def tracking_label(subject)
return experimentation_subject_id if subject.blank?
if subject.respond_to?(:to_global_id)
Digest::MD5.hexdigest(subject.to_global_id.to_s)
else
Digest::MD5.hexdigest(subject.to_s)
end
end
end end
end end
end end
...@@ -3,16 +3,17 @@ ...@@ -3,16 +3,17 @@
module Gitlab module Gitlab
module Experimentation module Experimentation
class Experiment class Experiment
attr_reader :tracking_category, :use_backwards_compatible_subject_index attr_reader :key, :tracking_category, :use_backwards_compatible_subject_index
def initialize(key, **params) def initialize(key, **params)
@key = key
@tracking_category = params[:tracking_category] @tracking_category = params[:tracking_category]
@use_backwards_compatible_subject_index = params[:use_backwards_compatible_subject_index] @use_backwards_compatible_subject_index = params[:use_backwards_compatible_subject_index]
@experiment_percentage = Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value # rubocop:disable Gitlab/AvoidFeatureGet @experiment_percentage = Feature.get(:"#{key}_experiment_percentage").percentage_of_time_value # rubocop:disable Gitlab/AvoidFeatureGet
end end
def enabled? def active?
::Gitlab.dev_env_or_com? && experiment_percentage > 0 ::Gitlab.dev_env_or_com? && experiment_percentage > 0
end end
......
...@@ -333,7 +333,7 @@ RSpec.describe GroupsController, factory_default: :keep do ...@@ -333,7 +333,7 @@ RSpec.describe GroupsController, factory_default: :keep do
context 'and the user is part of the control group' do context 'and the user is part of the control group' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it 'tracks the event with the "created_namespace" action with the "control_group" property', :snowplow do it 'tracks the event with the "created_namespace" action with the "control_group" property', :snowplow do
...@@ -350,7 +350,7 @@ RSpec.describe GroupsController, factory_default: :keep do ...@@ -350,7 +350,7 @@ RSpec.describe GroupsController, factory_default: :keep do
context 'and the user is part of the experimental group' do context 'and the user is part of the experimental group' do
before do before do
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it 'tracks the event with the "created_namespace" action with the "experimental_group" property', :snowplow do it 'tracks the event with the "created_namespace" action with the "experimental_group" property', :snowplow do
......
...@@ -25,7 +25,7 @@ RSpec.describe InvitesController, :snowplow do ...@@ -25,7 +25,7 @@ RSpec.describe InvitesController, :snowplow do
shared_examples "tracks the 'accepted' event for the invitation reminders experiment" do shared_examples "tracks the 'accepted' event for the invitation reminders experiment" do
before do before do
stub_experiment(invitation_reminders: true) stub_experiment(invitation_reminders: true)
allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).with(:invitation_reminders, member.invite_email).and_return(experimental_group) stub_experiment_for_subject(invitation_reminders: experimental_group)
end end
context 'when in the control group' do context 'when in the control group' do
......
...@@ -18,7 +18,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do ...@@ -18,7 +18,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
describe 'pushing tracking_data to Gon' do describe 'pushing tracking_data to Gon' do
before do before do
stub_experiment(jobs_empty_state: experiment_active) stub_experiment(jobs_empty_state: experiment_active)
stub_experiment_for_user(jobs_empty_state: in_experiment_group) stub_experiment_for_subject(jobs_empty_state: in_experiment_group)
get_index get_index
end end
......
...@@ -19,7 +19,7 @@ RSpec.describe Registrations::ExperienceLevelsController do ...@@ -19,7 +19,7 @@ RSpec.describe Registrations::ExperienceLevelsController do
context 'with an authenticated user' do context 'with an authenticated user' do
before do before do
sign_in(user) sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
it { is_expected.to have_gitlab_http_status(:ok) } it { is_expected.to have_gitlab_http_status(:ok) }
...@@ -28,7 +28,7 @@ RSpec.describe Registrations::ExperienceLevelsController do ...@@ -28,7 +28,7 @@ RSpec.describe Registrations::ExperienceLevelsController do
context 'when not part of the onboarding issues experiment' do context 'when not part of the onboarding issues experiment' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
...@@ -47,12 +47,12 @@ RSpec.describe Registrations::ExperienceLevelsController do ...@@ -47,12 +47,12 @@ RSpec.describe Registrations::ExperienceLevelsController do
context 'with an authenticated user' do context 'with an authenticated user' do
before do before do
sign_in(user) sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
end end
context 'when not part of the onboarding issues experiment' do context 'when not part of the onboarding issues experiment' do
before do before do
stub_experiment_for_user(onboarding_issues: false) stub_experiment_for_subject(onboarding_issues: false)
end end
it { is_expected.to have_gitlab_http_status(:not_found) } it { is_expected.to have_gitlab_http_status(:not_found) }
...@@ -90,7 +90,7 @@ RSpec.describe Registrations::ExperienceLevelsController do ...@@ -90,7 +90,7 @@ RSpec.describe Registrations::ExperienceLevelsController do
let(:issues_board) { build(:board, id: 123, project: project) } let(:issues_board) { build(:board, id: 123, project: project) }
before do before do
stub_experiment_for_user( stub_experiment_for_subject(
onboarding_issues: true, onboarding_issues: true,
default_to_issues_board: default_to_issues_board_xp? default_to_issues_board: default_to_issues_board_xp?
) )
......
...@@ -125,7 +125,7 @@ RSpec.describe RootController do ...@@ -125,7 +125,7 @@ RSpec.describe RootController do
context 'when experiment is enabled' do context 'when experiment is enabled' do
before do before do
stub_experiment_for_user(customize_homepage: true) stub_experiment_for_subject(customize_homepage: true)
end end
it 'renders the default dashboard' do it 'renders the default dashboard' do
......
...@@ -28,7 +28,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do ...@@ -28,7 +28,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
context 'with no jobs' do context 'with no jobs' do
before do before do
stub_experiment(jobs_empty_state: experiment_active) stub_experiment(jobs_empty_state: experiment_active)
stub_experiment_for_user(jobs_empty_state: in_experiment_group) stub_experiment_for_subject(jobs_empty_state: in_experiment_group)
visit project_jobs_path(project) visit project_jobs_path(project)
end end
......
...@@ -9,7 +9,7 @@ RSpec.describe 'Experience level screen' do ...@@ -9,7 +9,7 @@ RSpec.describe 'Experience level screen' do
before do before do
group.add_owner(user) group.add_owner(user)
gitlab_sign_in(user) gitlab_sign_in(user)
stub_experiment_for_user(onboarding_issues: true) stub_experiment_for_subject(onboarding_issues: true)
visit users_sign_up_experience_level_path(namespace_path: group.to_param) visit users_sign_up_experience_level_path(namespace_path: group.to_param)
end end
......
...@@ -20,14 +20,14 @@ RSpec.describe Gitlab::Experimentation::Experiment do ...@@ -20,14 +20,14 @@ RSpec.describe Gitlab::Experimentation::Experiment do
subject(:experiment) { described_class.new(:experiment_key, **params) } subject(:experiment) { described_class.new(:experiment_key, **params) }
describe '#enabled?' do describe '#active?' do
before do before do
allow(Gitlab).to receive(:dev_env_or_com?).and_return(on_gitlab_com) allow(Gitlab).to receive(:dev_env_or_com?).and_return(on_gitlab_com)
end end
subject { experiment.enabled? } subject { experiment.active? }
where(:on_gitlab_com, :percentage, :is_enabled) do where(:on_gitlab_com, :percentage, :is_active) do
true | 0 | false true | 0 | false
true | 10 | true true | 10 | true
false | 0 | false false | 0 | false
...@@ -35,7 +35,7 @@ RSpec.describe Gitlab::Experimentation::Experiment do ...@@ -35,7 +35,7 @@ RSpec.describe Gitlab::Experimentation::Experiment do
end end
with_them do with_them do
it { is_expected.to eq(is_enabled) } it { is_expected.to eq(is_active) }
end end
end end
......
...@@ -28,7 +28,7 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do ...@@ -28,7 +28,7 @@ RSpec.describe Gitlab::Experimentation::EXPERIMENTS do
end end
end end
RSpec.describe Gitlab::Experimentation, :snowplow do RSpec.describe Gitlab::Experimentation do
before do before do
stub_const('Gitlab::Experimentation::EXPERIMENTS', { stub_const('Gitlab::Experimentation::EXPERIMENTS', {
backwards_compatible_test_experiment: { backwards_compatible_test_experiment: {
...@@ -47,92 +47,131 @@ RSpec.describe Gitlab::Experimentation, :snowplow do ...@@ -47,92 +47,131 @@ RSpec.describe Gitlab::Experimentation, :snowplow do
let(:enabled_percentage) { 10 } let(:enabled_percentage) { 10 }
describe '.enabled?' do describe '.get_experiment' do
subject { described_class.enabled?(:test_experiment) } subject { described_class.get_experiment(:test_experiment) }
context 'feature toggle is enabled and we are selected' do context 'returns experiment' do
it { is_expected.to be_truthy } it { is_expected.to be_instance_of(Gitlab::Experimentation::Experiment) }
end
context 'experiment is not defined' do
subject { described_class.get_experiment(:missing_experiment) }
it { is_expected.to be_nil }
end
end
describe '.active?' do
subject { described_class.active?(:test_experiment) }
context 'feature toggle is enabled' do
it { is_expected.to eq(true) }
end end
describe 'experiment is not defined' do describe 'experiment is not defined' do
it 'returns false' do it 'returns false' do
expect(described_class.enabled?(:missing_experiment)).to be_falsey expect(described_class.active?(:missing_experiment)).to eq(false)
end end
end end
describe 'experiment is disabled' do describe 'experiment is disabled' do
let(:enabled_percentage) { 0 } let(:enabled_percentage) { 0 }
it { is_expected.to be_falsey } it { is_expected.to eq(false) }
end end
end end
describe '.enabled_for_value?' do describe '.in_experiment_group?' do
subject { described_class.enabled_for_value?(:test_experiment, experimentation_subject_index) } context 'with new index calculation' do
let(:enabled_percentage) { 50 }
let(:experiment_subject) { 'z' } # Zlib.crc32('test_experimentz') % 100 = 33
let(:experimentation_subject_index) { 9 } subject { described_class.in_experiment_group?(:test_experiment, subject: experiment_subject) }
context 'experiment is disabled' do
before do
allow(described_class).to receive(:enabled?).and_return(false)
end
it { is_expected.to be_falsey } context 'when experiment is active' do
end context 'when subject is part of the experiment' do
it { is_expected.to eq(true) }
end
context 'experiment is enabled' do context 'when subject is not part of the experiment' do
before do let(:experiment_subject) { 'a' } # Zlib.crc32('test_experimenta') % 100 = 61
allow(described_class).to receive(:enabled?).and_return(true)
end
it { is_expected.to be_truthy } it { is_expected.to eq(false) }
end
describe 'experimentation_subject_index' do context 'when subject has a global_id' do
context 'experimentation_subject_index is not set' do let(:experiment_subject) { double(:subject, to_global_id: 'z') }
let(:experimentation_subject_index) { nil }
it { is_expected.to be_falsey } it { is_expected.to eq(true) }
end end
context 'experimentation_subject_index is an empty string' do context 'when subject is nil' do
let(:experimentation_subject_index) { '' } let(:experiment_subject) { nil }
it { is_expected.to be_falsey } it { is_expected.to eq(false) }
end end
context 'experimentation_subject_index outside enabled ratio' do context 'when subject is an empty string' do
let(:experimentation_subject_index) { 11 } let(:experiment_subject) { '' }
it { is_expected.to be_falsey } it { is_expected.to eq(false) }
end end
end end
context 'when experiment is not active' do
before do
allow(described_class).to receive(:active?).and_return(false)
end
it { is_expected.to eq(false) }
end
end end
end
describe '.enabled_for_attribute?' do context 'with backwards compatible index calculation' do
subject { described_class.enabled_for_attribute?(:test_experiment, attribute) } let(:experiment_subject) { 'abcd' } # Digest::SHA1.hexdigest('abcd').hex % 100 = 7
let(:attribute) { 'abcd' } # Digest::SHA1.hexdigest('abcd').hex % 100 = 7 subject { described_class.in_experiment_group?(:backwards_compatible_test_experiment, subject: experiment_subject) }
context 'experiment is disabled' do context 'when experiment is active' do
before do before do
allow(described_class).to receive(:enabled?).and_return(false) allow(described_class).to receive(:active?).and_return(true)
end end
it { is_expected.to be false } context 'when subject is part of the experiment' do
end it { is_expected.to eq(true) }
end
context 'experiment is enabled' do context 'when subject is not part of the experiment' do
before do let(:experiment_subject) { 'abc' } # Digest::SHA1.hexdigest('abc').hex % 100 = 17
allow(described_class).to receive(:enabled?).and_return(true)
end
it { is_expected.to be true } it { is_expected.to eq(false) }
end
context 'when subject has a global_id' do
let(:experiment_subject) { double(:subject, to_global_id: 'abcd') }
it { is_expected.to eq(true) }
end
context 'outside enabled ratio' do context 'when subject is nil' do
let(:attribute) { 'abc' } # Digest::SHA1.hexdigest('abc').hex % 100 = 17 let(:experiment_subject) { nil }
it { is_expected.to eq(false) }
end
context 'when subject is an empty string' do
let(:experiment_subject) { '' }
it { is_expected.to eq(false) }
end
end
context 'when experiment is not active' do
before do
allow(described_class).to receive(:active?).and_return(false)
end
it { is_expected.to be false } it { is_expected.to eq(false) }
end end
end end
end end
......
...@@ -1453,7 +1453,7 @@ RSpec.describe Notify do ...@@ -1453,7 +1453,7 @@ RSpec.describe Notify do
shared_examples "tracks the 'sent' event for the invitation reminders experiment" do shared_examples "tracks the 'sent' event for the invitation reminders experiment" do
before do before do
stub_experiment(invitation_reminders: true) stub_experiment(invitation_reminders: true)
allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).with(:invitation_reminders, group_member.invite_email).and_return(experimental_group) allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(:invitation_reminders, subject: group_member.invite_email).and_return(experimental_group)
end end
it "tracks the 'sent' event", :snowplow do it "tracks the 'sent' event", :snowplow do
......
...@@ -11,7 +11,7 @@ RSpec.describe Members::InvitationReminderEmailService do ...@@ -11,7 +11,7 @@ RSpec.describe Members::InvitationReminderEmailService do
context 'when the experiment is disabled' do context 'when the experiment is disabled' do
before do before do
allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).and_return(false) allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_return(false)
invitation.expires_at = frozen_time + 2.days invitation.expires_at = frozen_time + 2.days
end end
...@@ -26,7 +26,7 @@ RSpec.describe Members::InvitationReminderEmailService do ...@@ -26,7 +26,7 @@ RSpec.describe Members::InvitationReminderEmailService do
context 'when the experiment is enabled' do context 'when the experiment is enabled' do
before do before do
allow(Gitlab::Experimentation).to receive(:enabled_for_attribute?).and_return(true) allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_return(true)
invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days invitation.expires_at = frozen_time + expires_at_days.days if expires_at_days
end end
......
...@@ -3,15 +3,15 @@ ...@@ -3,15 +3,15 @@
module StubExperiments module StubExperiments
# Stub Experiment with `key: true/false` # Stub Experiment with `key: true/false`
# #
# @param [Hash] experiment where key is feature name and value is boolean whether enabled or not. # @param [Hash] experiment where key is feature name and value is boolean whether active or not.
# #
# Examples # Examples
# - `stub_experiment(signup_flow: false)` ... Disable `signup_flow` experiment globally. # - `stub_experiment(signup_flow: false)` ... Disables `signup_flow` experiment.
def stub_experiment(experiments) def stub_experiment(experiments)
allow(Gitlab::Experimentation).to receive(:enabled?).and_call_original allow(Gitlab::Experimentation).to receive(:active?).and_call_original
experiments.each do |experiment_key, enabled| experiments.each do |experiment_key, enabled|
allow(Gitlab::Experimentation).to receive(:enabled?).with(experiment_key) { enabled } allow(Gitlab::Experimentation).to receive(:active?).with(experiment_key) { enabled }
end end
end end
...@@ -20,12 +20,12 @@ module StubExperiments ...@@ -20,12 +20,12 @@ module StubExperiments
# @param [Hash] experiment where key is feature name and value is boolean whether enabled or not. # @param [Hash] experiment where key is feature name and value is boolean whether enabled or not.
# #
# Examples # Examples
# - `stub_experiment_for_user(signup_flow: false)` ... Disable `signup_flow` experiment for user. # - `stub_experiment_for_subject(signup_flow: false)` ... Disable `signup_flow` experiment for user.
def stub_experiment_for_user(experiments) def stub_experiment_for_subject(experiments)
allow(Gitlab::Experimentation).to receive(:enabled_for_value?).and_call_original allow(Gitlab::Experimentation).to receive(:in_experiment_group?).and_call_original
experiments.each do |experiment_key, enabled| experiments.each do |experiment_key, enabled|
allow(Gitlab::Experimentation).to receive(:enabled_for_value?).with(experiment_key, anything) { enabled } allow(Gitlab::Experimentation).to receive(:in_experiment_group?).with(experiment_key, anything) { enabled }
end end
end end
end end
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
RSpec.shared_examples 'issuable invite members experiments' do RSpec.shared_examples 'issuable invite members experiments' do
context 'when invite_members_version_a experiment is enabled' do context 'when invite_members_version_a experiment is enabled' do
before do before do
stub_experiment_for_user(invite_members_version_a: true) stub_experiment_for_subject(invite_members_version_a: true)
end end
it 'shows a link for inviting members and follows through to the members page' do it 'shows a link for inviting members and follows through to the members page' do
...@@ -28,7 +28,7 @@ RSpec.shared_examples 'issuable invite members experiments' do ...@@ -28,7 +28,7 @@ RSpec.shared_examples 'issuable invite members experiments' do
context 'when invite_members_version_b experiment is enabled' do context 'when invite_members_version_b experiment is enabled' do
before do before do
stub_experiment_for_user(invite_members_version_b: true) stub_experiment_for_subject(invite_members_version_b: true)
end end
it 'shows a link for inviting members and follows through to modal' do it 'shows a link for inviting members and follows through to modal' do
......
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