Merge branch 'mwaw/337169-update-usage-ping-enabled-check-in-redis-ordinary-counters' into 'master'

Use SSOT for checking if service ping is enabled via license or in application settings [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!67103

app/services/git/branch_hooks_service.rb

@@ -95,7 +95,7 @@ module Git
     end
 
     def track_ci_config_change_event
-      return unless Gitlab::CurrentSettings.usage_ping_enabled?
+      return unless ::ServicePing::ServicePingSettings.enabled?
       return unless default_branch?
 
       commits_changing_ci_config.each do |commit|

app/services/service_ping/service_ping_settings.rb

@@ -5,12 +5,10 @@ module ServicePing
     extend self
 
     def product_intelligence_enabled?
-      pings_enabled? && !User.single_user&.requires_usage_stats_consent?
+      enabled? && !User.single_user&.requires_usage_stats_consent?
     end
 
-    private
-
-    def pings_enabled?
+    def enabled?
       ::Gitlab::CurrentSettings.usage_ping_enabled?
     end
   end

ee/app/services/ee/service_ping/service_ping_settings.rb

@@ -5,10 +5,8 @@ module EE
     module ServicePingSettings
       extend ::Gitlab::Utils::Override
 
-      private
-
-      override :pings_enabled?
-      def pings_enabled?
+      override :enabled?
+      def enabled?
         ::License.current&.customer_service_enabled? || super
       end
     end

ee/lib/ee/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -12,11 +12,6 @@ module EE
           def valid_context_list
             super + License.all_plans
           end
-
-          override :usage_ping_enabled?
-          def usage_ping_enabled?
-            ::License.current&.customer_service_enabled? || super
-          end
         end
       end
     end

ee/lib/gitlab/usage_data_counters/network_policy_counter.rb

@@ -7,7 +7,7 @@ module Gitlab::UsageDataCounters
 
     class << self
       def add(forwards, drops)
-        return unless Gitlab::CurrentSettings.usage_ping_enabled?
+        return unless ::ServicePing::ServicePingSettings.enabled?
 
         Gitlab::Redis::SharedState.with do |redis|
           redis.multi do

ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -103,19 +103,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
         stub_application_setting(usage_ping_enabled: false)
       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
         before do
           # License.current.customer_service_enabled? == true
@@ -128,14 +115,6 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
           described_class.track_event(context_event, values: entity1, time: Date.current)
         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

ee/spec/services/ee/service_ping/service_ping_settings_spec.rb

@@ -38,4 +38,24 @@ RSpec.describe ServicePing::ServicePingSettings do
       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

lib/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -117,7 +117,7 @@ module Gitlab
         private
 
         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)
           Gitlab::ErrorTracking.track_and_raise_for_dev_exception(UnknownEvent.new("Unknown event #{event_name}")) unless event.present?
@@ -131,10 +131,6 @@ module Gitlab
           Gitlab::ErrorTracking.track_and_raise_for_dev_exception(e)
         end
 
-        def usage_ping_enabled?
-          Gitlab::CurrentSettings.usage_ping_enabled?
-        end
-
         # The array of valid context on which we allow tracking
         def valid_context_list
           Plan.all_plans

lib/gitlab/usage_data_counters/redis_counter.rb

@@ -4,13 +4,13 @@ module Gitlab
   module UsageDataCounters
     module RedisCounter
       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) }
       end
 
       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) }
       end

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -143,7 +143,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
 
       context 'when usage_ping is disabled' 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)
 
@@ -153,7 +153,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
 
       context 'when usage_ping is enabled' do
         before do
-          stub_application_setting(usage_ping_enabled: true)
+          allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(true)
         end
 
         it 'tracks event when using symbol' do

spec/lib/gitlab/usage_data_counters/redis_counter_spec.rb

@@ -8,12 +8,12 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
   subject { Class.new.extend(described_class) }
 
   before do
-    stub_application_setting(usage_ping_enabled: setting_value)
+    allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(service_ping_enabled)
   end
 
   describe '.increment' do
     context 'when usage_ping is disabled' do
-      let(:setting_value) { false }
+      let(:service_ping_enabled) { false }
 
       it 'counter is not increased' do
         expect do
@@ -23,7 +23,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
     end
 
     context 'when usage_ping is enabled' do
-      let(:setting_value) { true }
+      let(:service_ping_enabled) { true }
 
       it 'counter is increased' do
         expect do
@@ -35,7 +35,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
 
   describe '.increment_by' do
     context 'when usage_ping is disabled' do
-      let(:setting_value) { false }
+      let(:service_ping_enabled) { false }
 
       it 'counter is not increased' do
         expect do
@@ -45,7 +45,7 @@ RSpec.describe Gitlab::UsageDataCounters::RedisCounter, :clean_gitlab_redis_shar
     end
 
     context 'when usage_ping is enabled' do
-      let(:setting_value) { true }
+      let(:service_ping_enabled) { true }
 
       it 'counter is increased' do
         expect do

spec/services/git/branch_hooks_service_spec.rb

@@ -134,7 +134,7 @@ RSpec.describe Git::BranchHooksService, :clean_gitlab_redis_shared_state do
 
         context 'when usage ping is disabled' do
           before do
-            stub_application_setting(usage_ping_enabled: false)
+            allow(::ServicePing::ServicePingSettings).to receive(:enabled?).and_return(false)
           end
 
           it 'does not track the event' do

spec/services/service_ping/service_ping_settings_spec.rb

@@ -27,4 +27,20 @@ RSpec.describe ServicePing::ServicePingSettings do
       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