diff --git a/changelogs/unreleased/119107-respect-dnt-for-experiments.yml b/changelogs/unreleased/119107-respect-dnt-for-experiments.yml new file mode 100644 index 0000000000000000000000000000000000000000..0132599d40bc817167b6efbc24a23e8f0e74a519 --- /dev/null +++ b/changelogs/unreleased/119107-respect-dnt-for-experiments.yml @@ -0,0 +1,5 @@ +--- +title: 'Use DNT: 1 as an experiment opt-out mechanism' +merge_request: 22100 +author: +type: other diff --git a/lib/gitlab/experimentation.rb b/lib/gitlab/experimentation.rb index 7c59267c0b62111dc9ce7533ea4cc1aea25deb2c..30c8eaf605a88d541a07661c5992537b5eb66d72 100644 --- a/lib/gitlab/experimentation.rb +++ b/lib/gitlab/experimentation.rb @@ -40,7 +40,7 @@ module ControllerConcern extend ActiveSupport::Concern included do - before_action :set_experimentation_subject_id_cookie + before_action :set_experimentation_subject_id_cookie, unless: :dnt_enabled? helper_method :experiment_enabled? end @@ -56,7 +56,12 @@ def set_experimentation_subject_id_cookie end def experiment_enabled?(experiment_key) - Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index) || forced_enabled?(experiment_key) + return false if dnt_enabled? + + return true if Experimentation.enabled_for_user?(experiment_key, experimentation_subject_index) + return true if forced_enabled?(experiment_key) + + false end def track_experiment_event(experiment_key, action, value = nil) @@ -73,6 +78,10 @@ def frontend_experimentation_tracking_data(experiment_key, action, value = nil) private + def dnt_enabled? + Gitlab::Utils.to_boolean(request.headers['DNT']) + end + def experimentation_subject_id cookies.signed[:experimentation_subject_id] end diff --git a/spec/lib/gitlab/experimentation_spec.rb b/spec/lib/gitlab/experimentation_spec.rb index 1506794cbb546e16221a3921a3a669a115e84624..a39c50ab038f49fe8d6330f686d99523f127fb7b 100644 --- a/spec/lib/gitlab/experimentation_spec.rb +++ b/spec/lib/gitlab/experimentation_spec.rb @@ -30,7 +30,12 @@ def index end describe '#set_experimentation_subject_id_cookie' do + let(:do_not_track) { nil } + let(:cookie) { cookies.permanent.signed[:experimentation_subject_id] } + before do + request.headers['DNT'] = do_not_track if do_not_track.present? + get :index end @@ -46,12 +51,30 @@ def index context 'cookie is not present' do it 'sets a permanent signed cookie' do - expect(cookies.permanent.signed[:experimentation_subject_id]).to be_present + expect(cookie).to be_present + end + + context 'DNT: 0' do + let(:do_not_Track) { '0' } + + it 'sets a permanent signed cookie' do + expect(cookie).to be_present + end + end + + context 'DNT: 1' do + let(:do_not_track) { '1' } + + it 'does nothing' do + expect(cookie).not_to be_present + end end end end describe '#experiment_enabled?' do + subject { controller.experiment_enabled?(:test_experiment) } + context 'cookie is not present' do it 'calls Gitlab::Experimentation.enabled_for_user? with the name of the experiment and an experimentation_subject_index of nil' do expect(Gitlab::Experimentation).to receive(:enabled_for_user?).with(:test_experiment, nil) @@ -72,11 +95,25 @@ def index end end + it 'returns true when DNT: 0 is set in the request' do + allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true } + controller.request.headers['DNT'] = '0' + + is_expected.to be_truthy + end + + it 'returns false when DNT: 1 is set in the request' do + allow(Gitlab::Experimentation).to receive(:enabled_for_user?) { true } + controller.request.headers['DNT'] = '1' + + is_expected.to be_falsy + end + describe 'URL parameter to force enable experiment' do - it 'returns true' do + it 'returns true unconditionally' do get :index, params: { force_experiment: :test_experiment } - expect(controller.experiment_enabled?(:test_experiment)).to be_truthy + is_expected.to be_truthy end end end