Commit 36e41dc4 authored by Dallas Reedy's avatar Dallas Reedy

Include experiment key in subject index calc

Before this change, we always bucketed the same users into the same
groups (control or experimental) for the same percentage rollout
regardless of the experiment key. After this change, the same user may
end up in the experimental group for one experiment and the control
group for a different experiment even at the same percentage rollout.
parent 734e1dfa
# frozen_string_literal: true # frozen_string_literal: true
require 'zlib'
# == Experimentation # == Experimentation
# #
# Utility module for A/B testing experimental features. Define your experiments in the `EXPERIMENTS` constant. # Utility module for A/B testing experimental features. Define your experiments in the `EXPERIMENTS` constant.
# Experiment options: # Experiment options:
# - environment (optional, defaults to enabled for development and GitLab.com) # - environment (optional, defaults to enabled for development and GitLab.com)
# - tracking_category (optional, used to set the category when tracking an experiment event) # - tracking_category (optional, used to set the category when tracking an experiment event)
# - calculate_subject_index_from_uuid_only (optional, set this to true if you need backwards compatibility)
# #
# The experiment is controlled by a Feature Flag (https://docs.gitlab.com/ee/development/feature_flags/controls.html), # The experiment is controlled by a Feature Flag (https://docs.gitlab.com/ee/development/feature_flags/controls.html),
# which is named "#{experiment_key}_experiment_percentage" and *must* be set with a percentage and not be used for other purposes. # which is named "#{experiment_key}_experiment_percentage" and *must* be set with a percentage and not be used for other purposes.
...@@ -31,46 +34,59 @@ module Gitlab ...@@ -31,46 +34,59 @@ module Gitlab
module Experimentation module Experimentation
EXPERIMENTS = { EXPERIMENTS = {
signup_flow: { signup_flow: {
tracking_category: 'Growth::Acquisition::Experiment::SignUpFlow' tracking_category: 'Growth::Acquisition::Experiment::SignUpFlow',
calculate_subject_index_from_uuid_only: true
}, },
onboarding_issues: { onboarding_issues: {
tracking_category: 'Growth::Conversion::Experiment::OnboardingIssues' tracking_category: 'Growth::Conversion::Experiment::OnboardingIssues',
calculate_subject_index_from_uuid_only: true
}, },
ci_notification_dot: { ci_notification_dot: {
tracking_category: 'Growth::Expansion::Experiment::CiNotificationDot' tracking_category: 'Growth::Expansion::Experiment::CiNotificationDot',
calculate_subject_index_from_uuid_only: true
}, },
upgrade_link_in_user_menu_a: { upgrade_link_in_user_menu_a: {
tracking_category: 'Growth::Expansion::Experiment::UpgradeLinkInUserMenuA' tracking_category: 'Growth::Expansion::Experiment::UpgradeLinkInUserMenuA',
calculate_subject_index_from_uuid_only: true
}, },
invite_members_version_a: { invite_members_version_a: {
tracking_category: 'Growth::Expansion::Experiment::InviteMembersVersionA' tracking_category: 'Growth::Expansion::Experiment::InviteMembersVersionA',
calculate_subject_index_from_uuid_only: true
}, },
invite_members_version_b: { invite_members_version_b: {
tracking_category: 'Growth::Expansion::Experiment::InviteMembersVersionB' tracking_category: 'Growth::Expansion::Experiment::InviteMembersVersionB',
calculate_subject_index_from_uuid_only: true
}, },
invite_members_empty_group_version_a: { invite_members_empty_group_version_a: {
tracking_category: 'Growth::Expansion::Experiment::InviteMembersEmptyGroupVersionA' tracking_category: 'Growth::Expansion::Experiment::InviteMembersEmptyGroupVersionA'
}, },
new_create_project_ui: { new_create_project_ui: {
tracking_category: 'Manage::Import::Experiment::NewCreateProjectUi' tracking_category: 'Manage::Import::Experiment::NewCreateProjectUi',
calculate_subject_index_from_uuid_only: true
}, },
contact_sales_btn_in_app: { contact_sales_btn_in_app: {
tracking_category: 'Growth::Conversion::Experiment::ContactSalesInApp' tracking_category: 'Growth::Conversion::Experiment::ContactSalesInApp',
calculate_subject_index_from_uuid_only: true
}, },
customize_homepage: { customize_homepage: {
tracking_category: 'Growth::Expansion::Experiment::CustomizeHomepage' tracking_category: 'Growth::Expansion::Experiment::CustomizeHomepage',
calculate_subject_index_from_uuid_only: true
}, },
invite_email: { invite_email: {
tracking_category: 'Growth::Acquisition::Experiment::InviteEmail' tracking_category: 'Growth::Acquisition::Experiment::InviteEmail',
calculate_subject_index_from_uuid_only: true
}, },
invitation_reminders: { invitation_reminders: {
tracking_category: 'Growth::Acquisition::Experiment::InvitationReminders' tracking_category: 'Growth::Acquisition::Experiment::InvitationReminders',
calculate_subject_index_from_uuid_only: true
}, },
group_only_trials: { group_only_trials: {
tracking_category: 'Growth::Conversion::Experiment::GroupOnlyTrials' tracking_category: 'Growth::Conversion::Experiment::GroupOnlyTrials',
calculate_subject_index_from_uuid_only: true
}, },
default_to_issues_board: { default_to_issues_board: {
tracking_category: 'Growth::Conversion::Experiment::DefaultToIssuesBoard' tracking_category: 'Growth::Conversion::Experiment::DefaultToIssuesBoard',
calculate_subject_index_from_uuid_only: true
} }
}.freeze }.freeze
...@@ -110,7 +126,8 @@ module Gitlab ...@@ -110,7 +126,8 @@ module Gitlab
def experiment_enabled?(experiment_key) def experiment_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) return true if Experimentation.enabled_for_value?(experiment_key,
experimentation_subject_index(experiment_key))
return true if forced_enabled?(experiment_key) return true if forced_enabled?(experiment_key)
false false
...@@ -153,10 +170,14 @@ module Gitlab ...@@ -153,10 +170,14 @@ module Gitlab
cookies.signed[:experimentation_subject_id] cookies.signed[:experimentation_subject_id]
end end
def experimentation_subject_index def experimentation_subject_index(experiment_key)
return if experimentation_subject_id.blank? return if experimentation_subject_id.blank?
experimentation_subject_id.delete('-').hex % 100 if Experimentation.experiment(experiment_key).calculate_subject_index_from_uuid_only
experimentation_subject_id.delete('-').hex % 100
else
Zlib.crc32("#{experiment_key}#{experimentation_subject_id}") % 100
end
end end
def track_experiment_event_for(experiment_key, action, value) def track_experiment_event_for(experiment_key, action, value)
...@@ -209,13 +230,18 @@ module Gitlab ...@@ -209,13 +230,18 @@ module Gitlab
enabled_for_value?(experiment_key, index) enabled_for_value?(experiment_key, index)
end end
def enabled_for_value?(experiment_key, experimentation_subject_index) def enabled_for_value?(experiment_key, value)
enabled?(experiment_key) && enabled?(experiment_key) && experiment(experiment_key).enabled_for_index?(value)
experiment(experiment_key).enabled_for_index?(experimentation_subject_index)
end end
end end
Experiment = Struct.new(:key, :environment, :tracking_category, keyword_init: true) do Experiment = Struct.new(
:key,
:environment,
:tracking_category,
:calculate_subject_index_from_uuid_only,
keyword_init: true
) do
def enabled? def enabled?
experiment_percentage > 0 experiment_percentage > 0
end end
......
...@@ -5,12 +5,18 @@ require 'spec_helper' ...@@ -5,12 +5,18 @@ require 'spec_helper'
RSpec.describe Gitlab::Experimentation, :snowplow do RSpec.describe Gitlab::Experimentation, :snowplow do
before do before do
stub_const('Gitlab::Experimentation::EXPERIMENTS', { stub_const('Gitlab::Experimentation::EXPERIMENTS', {
backwards_compatible_test_experiment: {
environment: environment,
tracking_category: 'Team',
calculate_subject_index_from_uuid_only: true
},
test_experiment: { test_experiment: {
environment: environment, environment: environment,
tracking_category: 'Team' tracking_category: 'Team'
} }
}) })
Feature.enable_percentage_of_time(:backwards_compatible_test_experiment_experiment_percentage, enabled_percentage)
Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage) Feature.enable_percentage_of_time(:test_experiment_experiment_percentage, enabled_percentage)
end end
...@@ -89,20 +95,28 @@ RSpec.describe Gitlab::Experimentation, :snowplow do ...@@ -89,20 +95,28 @@ RSpec.describe Gitlab::Experimentation, :snowplow do
context 'cookie is not present' do context 'cookie is not present' do
it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and an experimentation_subject_index of nil' do it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and an experimentation_subject_index of nil' do
expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(:test_experiment, nil) expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(:test_experiment, nil)
controller.experiment_enabled?(:test_experiment) subject
end end
end end
context 'cookie is present' do context 'cookie is present' do
using RSpec::Parameterized::TableSyntax
before do before do
cookies.permanent.signed[:experimentation_subject_id] = 'abcd-1234' cookies.permanent.signed[:experimentation_subject_id] = 'abcd-1234'
get :index get :index
end end
it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and an experimentation_subject_index of the modulo 100 of the hex value of the uuid' do where(:experiment_key, :index_value) do
# 'abcd1234'.hex % 100 = 76 :test_experiment | 40 # Zlib.crc32('test_experimentabcd-1234') % 100 = 40
expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(:test_experiment, 76) :backwards_compatible_test_experiment | 76 # 'abcd1234'.hex % 100 = 76
controller.experiment_enabled?(:test_experiment) end
with_them do
it 'calls Gitlab::Experimentation.enabled_for_value? with the name of the experiment and the calculated experimentation_subject_index based on the uuid' do
expect(Gitlab::Experimentation).to receive(:enabled_for_value?).with(experiment_key, index_value)
controller.experiment_enabled?(experiment_key)
end
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