Commit f547efec authored by Nicolas Dular's avatar Nicolas Dular Committed by Dmytro Zaporozhets (DZ)

Respect DNT when tracking experiments

While we have not enabled experiments for users with DNT set to '1', we
were sending snowplow events and recorded the users within the control
group. This is not only against the idea of DNT, but also influences our
experimentation data.
parent fb224e3b
---
title: Respect DNT when tracking experiments
merge_request: 44420
author:
type: fixed
...@@ -114,18 +114,23 @@ module Gitlab ...@@ -114,18 +114,23 @@ module Gitlab
end end
def track_experiment_event(experiment_key, action, value = nil) def track_experiment_event(experiment_key, action, value = nil)
return if dnt_enabled?
track_experiment_event_for(experiment_key, action, value) do |tracking_data| track_experiment_event_for(experiment_key, action, value) 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)
return if dnt_enabled?
track_experiment_event_for(experiment_key, action, value) do |tracking_data| track_experiment_event_for(experiment_key, action, value) 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 unless Experimentation.enabled?(experiment_key) && current_user return unless Experimentation.enabled?(experiment_key) && current_user
::Experiment.add_user(experiment_key, tracking_group(experiment_key), current_user) ::Experiment.add_user(experiment_key, tracking_group(experiment_key), current_user)
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Experimentation do RSpec.describe Gitlab::Experimentation, :snowplow do
before do before do
stub_const('Gitlab::Experimentation::EXPERIMENTS', { stub_const('Gitlab::Experimentation::EXPERIMENTS', {
test_experiment: { test_experiment: {
...@@ -141,13 +141,14 @@ RSpec.describe Gitlab::Experimentation do ...@@ -141,13 +141,14 @@ RSpec.describe Gitlab::Experimentation do
end end
it 'tracks the event with the right parameters' do it 'tracks the event with the right parameters' do
expect(Gitlab::Tracking).to receive(:event).with( controller.track_experiment_event(:test_experiment, 'start', 1)
'Team',
'start', expect_snowplow_event(
category: 'Team',
action: 'start',
property: 'experimental_group', property: 'experimental_group',
value: 'team_id' value: 1
) )
controller.track_experiment_event(:test_experiment, 'start', 'team_id')
end end
end end
...@@ -157,13 +158,43 @@ RSpec.describe Gitlab::Experimentation do ...@@ -157,13 +158,43 @@ RSpec.describe Gitlab::Experimentation do
end end
it 'tracks the event with the right parameters' do it 'tracks the event with the right parameters' do
expect(Gitlab::Tracking).to receive(:event).with( controller.track_experiment_event(:test_experiment, 'start', 1)
'Team',
'start', expect_snowplow_event(
category: 'Team',
action: 'start',
property: 'control_group', property: 'control_group',
value: 'team_id' value: 1
) )
controller.track_experiment_event(:test_experiment, 'start', 'team_id') end
end
context 'do not track is disabled' do
before do
request.headers['DNT'] = '0'
end
it 'does track the event' do
controller.track_experiment_event(:test_experiment, 'start', 1)
expect_snowplow_event(
category: 'Team',
action: 'start',
property: 'control_group',
value: 1
)
end
end
context 'do not track enabled' do
before do
request.headers['DNT'] = '1'
end
it 'does not track the event' do
controller.track_experiment_event(:test_experiment, 'start', 1)
expect_no_snowplow_event
end end
end end
end end
...@@ -174,8 +205,9 @@ RSpec.describe Gitlab::Experimentation do ...@@ -174,8 +205,9 @@ RSpec.describe Gitlab::Experimentation do
end end
it 'does not track the event' do it 'does not track the event' do
expect(Gitlab::Tracking).not_to receive(:event)
controller.track_experiment_event(:test_experiment, 'start') controller.track_experiment_event(:test_experiment, 'start')
expect_no_snowplow_event
end end
end end
end end
...@@ -234,6 +266,36 @@ RSpec.describe Gitlab::Experimentation do ...@@ -234,6 +266,36 @@ RSpec.describe Gitlab::Experimentation do
) )
end end
end end
context 'do not track disabled' do
before do
request.headers['DNT'] = '0'
end
it 'pushes the right parameters to gon' do
controller.frontend_experimentation_tracking_data(:test_experiment, 'start')
expect(Gon.tracking_data).to eq(
{
category: 'Team',
action: 'start',
property: 'control_group'
}
)
end
end
context 'do not track enabled' do
before do
request.headers['DNT'] = '1'
end
it 'does not push data to gon' do
controller.frontend_experimentation_tracking_data(:test_experiment, 'start')
expect(Gon.method_defined?(:tracking_data)).to be_falsey
end
end
end end
context 'when the experiment is disabled' do context 'when the experiment is disabled' do
...@@ -308,6 +370,39 @@ RSpec.describe Gitlab::Experimentation do ...@@ -308,6 +370,39 @@ RSpec.describe Gitlab::Experimentation do
controller.record_experiment_user(:test_experiment) controller.record_experiment_user(:test_experiment)
end end
end end
context 'do not track' do
before do
allow(controller).to receive(:current_user).and_return(user)
allow_next_instance_of(described_class) do |instance|
allow(instance).to receive(:experiment_enabled?).with(:test_experiment).and_return(false)
end
end
context 'is disabled' do
before do
request.headers['DNT'] = '0'
end
it 'calls add_user on the Experiment model' do
expect(::Experiment).to receive(:add_user).with(:test_experiment, :control, user)
controller.record_experiment_user(:test_experiment)
end
end
context 'is enabled' do
before do
request.headers['DNT'] = '1'
end
it 'does not call add_user on the Experiment model' do
expect(::Experiment).not_to receive(:add_user)
controller.record_experiment_user(:test_experiment)
end
end
end
end end
describe '#experiment_tracking_category_and_group' do describe '#experiment_tracking_category_and_group' 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