Update experiment grouping on subsequent record_experiment_user calls
The Problem
The way we currently have the new experiments
and experiment_users
tables set up is likely to cause issues when experiments roll out to more & more users over time.
Here’s the relevant code from the updated lib/gitlab/experimentation.rb
file:
module Gitlab
module Experimentation
def record_experiment_user(experiment_key)
return unless Experimentation.enabled?(experiment_key) && current_user
::Experiment.add_user(experiment_key, tracking_group(experiment_key), current_user)
end
end
end
And here are the related bits from the new app/models/experiment.rb
file:
class Experiment < ApplicationRecord
has_many :experiment_users
def self.add_user(name, group_type, user)
experiment = find_or_create_by(name: name)
return unless experiment
return if experiment.experiment_users.where(user: user).exists?
group_type == ::Gitlab::Experimentation::GROUP_CONTROL ? experiment.add_control_user(user) : experiment.add_experimental_user(user)
end
end
The problem really comes into play on this line from Experiment.add_user
:
return if experiment.experiment_users.where(user: user).exists?
What that line is doing is exiting the method early if we’ve already recorded the given user’s id
for the given experiment name. It does not take into account what the group_type
was or what it is now.
What will happen is that when we first roll out an experiment we might open it up to 10% of users, and then as we gain insights or gain confidence in the experiment code, we might ramp that up to 25% of users. When we change from 10% to 25% we’ll begin including users in the “experiment” group who were previously in the “control” group. That means we’ve already recorded them in the experiment_users
database and we’ve already recorded them as being in the “control” group. Even though they are now in the “experiment” group, we’ll see that we’ve already recorded this user id
for this experiment name
and we’ll skip recording them again (even though they’re now in the “experiment” group_type
).
Proposed Solution
Rather than bailing out on the method altogether, we should run an update
on the found experiment_users
row to change the recorded group_type
from “control” to “experiment”. We should also add a group_type_updated_at
timestamp column to keep track of when the user’s group_type
was changed from “control” to “experiment”.