Commit c4c53e27 authored by Mikołaj Wawrzyniak's avatar Mikołaj Wawrzyniak Committed by Alex Kalderimis

Use a SSOT to check if service ping is enabled

Instead of checking the application settings directly, this is abstracted to
a single source of truth.

For projects with licenses, in EE we check if the license requires tracking.

With this change, events will be stored locally even if the service ping is
disabled for licensed users. These events will not be sent as part of the service
ping if the service ping is disabled.
parent 0bdfe446
...@@ -95,7 +95,7 @@ module Git ...@@ -95,7 +95,7 @@ module Git
end end
def track_ci_config_change_event def track_ci_config_change_event
return unless Gitlab::CurrentSettings.usage_ping_enabled? return unless ::ServicePing::ServicePingSettings.enabled?
return unless default_branch? return unless default_branch?
commits_changing_ci_config.each do |commit| commits_changing_ci_config.each do |commit|
......
...@@ -5,12 +5,10 @@ module ServicePing ...@@ -5,12 +5,10 @@ module ServicePing
extend self extend self
def product_intelligence_enabled? def product_intelligence_enabled?
pings_enabled? && !User.single_user&.requires_usage_stats_consent? enabled? && !User.single_user&.requires_usage_stats_consent?
end end
private def enabled?
def pings_enabled?
::Gitlab::CurrentSettings.usage_ping_enabled? ::Gitlab::CurrentSettings.usage_ping_enabled?
end end
end end
......
...@@ -5,10 +5,8 @@ module EE ...@@ -5,10 +5,8 @@ module EE
module ServicePingSettings module ServicePingSettings
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
private override :enabled?
def enabled?
override :pings_enabled?
def pings_enabled?
::License.current&.customer_service_enabled? || super ::License.current&.customer_service_enabled? || super
end end
end end
......
...@@ -12,11 +12,6 @@ module EE ...@@ -12,11 +12,6 @@ module EE
def valid_context_list def valid_context_list
super + License.all_plans super + License.all_plans
end end
override :usage_ping_enabled?
def usage_ping_enabled?
::License.current&.customer_service_enabled? || super
end
end end
end end
end end
......
...@@ -7,7 +7,7 @@ module Gitlab::UsageDataCounters ...@@ -7,7 +7,7 @@ module Gitlab::UsageDataCounters
class << self class << self
def add(forwards, drops) def add(forwards, drops)
return unless Gitlab::CurrentSettings.usage_ping_enabled? return unless ::ServicePing::ServicePingSettings.enabled?
Gitlab::Redis::SharedState.with do |redis| Gitlab::Redis::SharedState.with do |redis|
redis.multi do redis.multi do
......
...@@ -103,19 +103,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s ...@@ -103,19 +103,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
stub_application_setting(usage_ping_enabled: false) stub_application_setting(usage_ping_enabled: false)
end end
context 'with license usage ping disabled' do
before do
# License.current.customer_service_enabled? == true
create_current_license(operational_metrics_enabled: false)
end
it 'does not track the event' do
expect(Gitlab::Redis::HLL).not_to receive(:add)
described_class.track_event(context_event, values: entity1, time: Date.current)
end
end
context 'with license usage ping enabled' do context 'with license usage ping enabled' do
before do before do
# License.current.customer_service_enabled? == true # License.current.customer_service_enabled? == true
...@@ -128,14 +115,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s ...@@ -128,14 +115,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
described_class.track_event(context_event, values: entity1, time: Date.current) described_class.track_event(context_event, values: entity1, time: Date.current)
end end
end end
context 'without a license', :without_license do
it 'does not track the event' do
expect(Gitlab::Redis::HLL).not_to receive(:add)
described_class.track_event(context_event, values: entity1, time: Date.current)
end
end
end end
end end
end end
...@@ -38,4 +38,24 @@ RSpec.describe ServicePing::ServicePingSettings do ...@@ -38,4 +38,24 @@ RSpec.describe ServicePing::ServicePingSettings do
end end
end end
end end
describe '#enabled?' do
where(:usage_ping_enabled, :customer_service_enabled, :expected_enabled) do
true | true | true
false | true | true
true | false | true
false | false | false
end
with_them do
before do
stub_config_setting(usage_ping_enabled: usage_ping_enabled)
create_current_license(operational_metrics_enabled: customer_service_enabled)
end
it 'has the correct enabled?' do
expect(described_class.enabled?).to eq(expected_enabled)
end
end
end
end end
...@@ -117,7 +117,7 @@ module Gitlab ...@@ -117,7 +117,7 @@ module Gitlab
private private
def track(values, event_name, context: '', time: Time.zone.now) def track(values, event_name, context: '', time: Time.zone.now)
return unless usage_ping_enabled? return unless ::ServicePing::ServicePingSettings.enabled?
event = event_for(event_name) event = event_for(event_name)
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new("Unknown event #{event_name}")) unless event.present? Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new("Unknown event #{event_name}")) unless event.present?
...@@ -131,10 +131,6 @@ module Gitlab ...@@ -131,10 +131,6 @@ module Gitlab
Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e) Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
end end
def usage_ping_enabled?
Gitlab::CurrentSettings.usage_ping_enabled?
end
# The array of valid context on which we allow tracking # The array of valid context on which we allow tracking
def valid_context_list def valid_context_list
Plan.all_plans Plan.all_plans
......
...@@ -4,13 +4,13 @@ module Gitlab ...@@ -4,13 +4,13 @@ module Gitlab
module UsageDataCounters module UsageDataCounters
module RedisCounter module RedisCounter
def increment(redis_counter_key) def increment(redis_counter_key)
return unless Gitlab::CurrentSettings.usage_ping_enabled return unless ::ServicePing::ServicePingSettings.enabled?
Gitlab::Redis::SharedState.with { |redis| redis.incr(redis_counter_key) } Gitlab::Redis::SharedState.with { |redis| redis.incr(redis_counter_key) }
end end
def increment_by(redis_counter_key, incr) def increment_by(redis_counter_key, incr)
return unless Gitlab::CurrentSettings.usage_ping_enabled return unless ::ServicePing::ServicePingSettings.enabled?
Gitlab::Redis::SharedState.with { |redis| redis.incrby(redis_counter_key, incr) } Gitlab::Redis::SharedState.with { |redis| redis.incrby(redis_counter_key, incr) }
end end
......
...@@ -143,7 +143,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s ...@@ -143,7 +143,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
context 'when usage_ping is disabled' do context 'when usage_ping is disabled' do
it 'does not track the event' do it 'does not track the event' do
stub_application_setting(usage_ping_enabled: false) allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(false)
described_class.track_event(weekly_event, values: entity1, time: Date.current) described_class.track_event(weekly_event, values: entity1, time: Date.current)
...@@ -153,7 +153,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s ...@@ -153,7 +153,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
context 'when usage_ping is enabled' do context 'when usage_ping is enabled' do
before do before do
stub_application_setting(usage_ping_enabled: true) allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(true)
end end
it 'tracks event when using symbol' do it 'tracks event when using symbol' do
......
...@@ -8,12 +8,12 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar ...@@ -8,12 +8,12 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
subject { Class.new.extend(described_class) } subject { Class.new.extend(described_class) }
before do before do
stub_application_setting(usage_ping_enabled: setting_value) allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(service_ping_enabled)
end end
describe '.increment' do describe '.increment' do
context 'when usage_ping is disabled' do context 'when usage_ping is disabled' do
let(:setting_value) { false } let(:service_ping_enabled) { false }
it 'counter is not increased' do it 'counter is not increased' do
expect do expect do
...@@ -23,7 +23,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar ...@@ -23,7 +23,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
end end
context 'when usage_ping is enabled' do context 'when usage_ping is enabled' do
let(:setting_value) { true } let(:service_ping_enabled) { true }
it 'counter is increased' do it 'counter is increased' do
expect do expect do
...@@ -35,7 +35,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar ...@@ -35,7 +35,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
describe '.increment_by' do describe '.increment_by' do
context 'when usage_ping is disabled' do context 'when usage_ping is disabled' do
let(:setting_value) { false } let(:service_ping_enabled) { false }
it 'counter is not increased' do it 'counter is not increased' do
expect do expect do
...@@ -45,7 +45,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar ...@@ -45,7 +45,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
end end
context 'when usage_ping is enabled' do context 'when usage_ping is enabled' do
let(:setting_value) { true } let(:service_ping_enabled) { true }
it 'counter is increased' do it 'counter is increased' do
expect do expect do
......
...@@ -134,7 +134,7 @@ RSpec.describe Git::BranchHooksService, :clean_gitlab_redis_shared_state do ...@@ -134,7 +134,7 @@ RSpec.describe Git::BranchHooksService, :clean_gitlab_redis_shared_state do
context 'when usage ping is disabled' do context 'when usage ping is disabled' do
before do before do
stub_application_setting(usage_ping_enabled: false) allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(false)
end end
it 'does not track the event' do it 'does not track the event' do
......
...@@ -27,4 +27,20 @@ RSpec.describe ServicePing::ServicePingSettings do ...@@ -27,4 +27,20 @@ RSpec.describe ServicePing::ServicePingSettings do
end end
end end
end end
describe '#enabled?' do
describe 'has the correct enabled' do
it 'when false' do
stub_config_setting(usage_ping_enabled: false)
expect(described_class.enabled?).to eq(false)
end
it 'when true' do
stub_config_setting(usage_ping_enabled: true)
expect(described_class.enabled?).to eq(true)
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