Merge branch '241466-refactor-track-unique-events-class' into 'master'

Resolve "Refactor track unique events class"

See merge request gitlab-org/gitlab!40729

lib/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -84,6 +84,10 @@ module Gitlab
           end
         end
 
+        def known_event?(event_name)
+          event_for(event_name).present?
+        end
+
         private
 
         # Allow to add totals for events that are in the same redis slot, category and have the same aggregation level

lib/gitlab/usage_data_counters/known_events.yml

@@ -104,3 +104,15 @@
   category: search
   redis_slot: search
   aggregation: weekly
+- name: wiki_action
+  category: source_code
+  aggregation: daily
+- name: design_action
+  category: source_code
+  aggregation: daily
+- name: project_action
+  category: source_code
+  aggregation: daily
+- name: merge_request_action
+  category: source_code
+  aggregation: daily

lib/gitlab/usage_data_counters/track_unique_actions.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module UsageDataCounters
-    module TrackUniqueActions
-      KEY_EXPIRY_LENGTH = 29.days
-
-      class << self
-        def track_action(action:, author_id:, time: Time.zone.now)
-          return unless Gitlab::CurrentSettings.usage_ping_enabled
-
-          target_key = key(action, time)
-
-          add_key(target_key, author_id)
-        end
-
-        def count_unique(action:, date_from:, date_to:)
-          keys = (date_from.to_date..date_to.to_date).map { |date| key(action, date) }
-
-          Gitlab::Redis::HLL.count(keys: keys)
-        end
-
-        private
-
-        def key(action, date)
-          year_day = date.strftime('%G-%j')
-          "#{year_day}-{#{action}}"
-        end
-
-        def add_key(key, value)
-          Gitlab::Redis::HLL.add(key: key, value: value, expiry: KEY_EXPIRY_LENGTH)
-        end
-      end
-    end
-  end
-end

lib/gitlab/usage_data_counters/track_unique_events.rb

@@ -38,11 +38,13 @@ module Gitlab
           transformed_target = transform_target(event_target)
           transformed_action = transform_action(event_action, transformed_target)
 
-          Gitlab::UsageDataCounters::TrackUniqueActions.track_action(action: transformed_action, author_id: author_id, time: time)
+          return unless Gitlab::UsageDataCounters::HLLRedisCounter.known_event?(transformed_action.to_s)
+
+          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(author_id, transformed_action.to_s, time)
         end
 
         def count_unique_events(event_action:, date_from:, date_to:)
-          Gitlab::UsageDataCounters::TrackUniqueActions.count_unique(action: event_action, date_from: date_from, date_to: date_to)
+          Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: event_action.to_s, start_date: date_from, end_date: date_to)
         end
 
         private

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
 
   describe '.categories' do
     it 'gets all unique category names' do
-      expect(described_class.categories).to contain_exactly('analytics', 'compliance', 'ide_edit', 'search')
+      expect(described_class.categories).to contain_exactly('analytics', 'compliance', 'ide_edit', 'search', 'source_code')
     end
   end
 

spec/lib/gitlab/usage_data_counters/track_unique_actions_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::UsageDataCounters::TrackUniqueActions, :clean_gitlab_redis_shared_state do
-  let(:time) { Time.zone.now }
-  let(:action) { 'example_action' }
-
-  def track_action(params)
-    described_class.track_action(params)
-  end
-
-  def count_unique(params)
-    described_class.count_unique(params)
-  end
-
-  context 'tracking an event' do
-    context 'when tracking successfully' do
-      it 'tracks and counts the events as expected' do
-        stub_application_setting(usage_ping_enabled: true)
-
-        aggregate_failures do
-          expect(track_action(action: action, author_id: 1)).to be_truthy
-          expect(track_action(action: action, author_id: 1)).to be_truthy
-          expect(track_action(action: action, author_id: 2)).to be_truthy
-          expect(track_action(action: action, author_id: 3, time: time - 3.days)).to be_truthy
-
-          expect(count_unique(action: action, date_from: time, date_to: Date.today)).to eq(2)
-          expect(count_unique(action: action, date_from: time - 5.days, date_to: Date.tomorrow)).to eq(3)
-        end
-      end
-    end
-
-    context 'when tracking unsuccessfully' do
-      it 'does not track the event' do
-        stub_application_setting(usage_ping_enabled: false)
-
-        expect(track_action(action: action, author_id: 2)).to be_nil
-        expect(count_unique(action: action, date_from: time, date_to: Date.today)).to eq(0)
-      end
-    end
-  end
-end

spec/lib/gitlab/usage_data_counters/track_unique_events_spec.rb

@@ -33,17 +33,14 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueEvents, :clean_gitlab_redis
             expect(track_event(event_action: :pushed, event_target: project, author_id: 2)).to be_truthy
             expect(track_event(event_action: :pushed, event_target: project, author_id: 3)).to be_truthy
             expect(track_event(event_action: :pushed, event_target: project, author_id: 4, time: time - 3.days)).to be_truthy
-            expect(track_event(event_action: :created, event_target: project, author_id: 5, time: time - 3.days)).to be_truthy
 
             expect(track_event(event_action: :destroyed, event_target: design, author_id: 3)).to be_truthy
             expect(track_event(event_action: :created, event_target: design, author_id: 4)).to be_truthy
             expect(track_event(event_action: :updated, event_target: design, author_id: 5)).to be_truthy
-            expect(track_event(event_action: :pushed, event_target: design, author_id: 6)).to be_truthy
 
             expect(track_event(event_action: :destroyed, event_target: wiki, author_id: 5)).to be_truthy
             expect(track_event(event_action: :created, event_target: wiki, author_id: 3)).to be_truthy
             expect(track_event(event_action: :updated, event_target: wiki, author_id: 4)).to be_truthy
-            expect(track_event(event_action: :pushed, event_target: wiki, author_id: 6)).to be_truthy
 
             expect(count_unique(event_action: described_class::PUSH_ACTION, date_from: time, date_to: Date.today)).to eq(3)
             expect(count_unique(event_action: described_class::PUSH_ACTION, date_from: time - 5.days, date_to: Date.tomorrow)).to eq(4)
@@ -58,17 +55,13 @@ RSpec.describe Gitlab::UsageDataCounters::TrackUniqueEvents, :clean_gitlab_redis
     context 'when tracking unsuccessfully' do
       using RSpec::Parameterized::TableSyntax
 
-      where(:application_setting, :target, :action) do
-        true  | Project         | :invalid_action
-        false | Project         | :pushed
-        true  | :invalid_target | :pushed
+      where(:target, :action) do
+        Project         | :invalid_action
+        :invalid_target | :pushed
+        Project         | :created
       end
 
       with_them do
-        before do
-          stub_application_setting(usage_ping_enabled: application_setting)
-        end
-
         it 'returns the expected values' do
           expect(track_event(event_action: action, event_target: target, author_id: 2)).to be_nil
           expect(count_unique(event_action: described_class::PUSH_ACTION, date_from: time, date_to: Date.today)).to eq(0)

spec/lib/gitlab/usage_data_spec.rb

@@ -974,14 +974,14 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
     before do
       counter = Gitlab::UsageDataCounters::TrackUniqueEvents
       merge_request = Event::TARGET_TYPES[:merge_request]
-      project = Event::TARGET_TYPES[:project]
+      design = Event::TARGET_TYPES[:design]
 
       counter.track_event(event_action: :commented, event_target: merge_request, author_id: 1, time: time)
       counter.track_event(event_action: :opened, event_target: merge_request, author_id: 1, time: time)
       counter.track_event(event_action: :merged, event_target: merge_request, author_id: 2, time: time)
       counter.track_event(event_action: :closed, event_target: merge_request, author_id: 3, time: time)
       counter.track_event(event_action: :opened, event_target: merge_request, author_id: 4, time: time - 3.days)
-      counter.track_event(event_action: :created, event_target: project, author_id: 5, time: time)
+      counter.track_event(event_action: :created, event_target: design, author_id: 5, time: time)
     end
 
     it 'returns the distinct count of users using merge requests (via events table) within the specified time period' do
@@ -1015,7 +1015,6 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
       counter.track_event(event_action: :pushed, event_target: project, author_id: 2)
       counter.track_event(event_action: :pushed, event_target: project, author_id: 3)
       counter.track_event(event_action: :pushed, event_target: project, author_id: 4, time: time - 3.days)
-      counter.track_event(event_action: :created, event_target: project, author_id: 5, time: time - 3.days)
       counter.track_event(event_action: :created, event_target: wiki, author_id: 3)
       counter.track_event(event_action: :created, event_target: design, author_id: 3)
 
@@ -1145,6 +1144,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
     subject { described_class.redis_hll_counters }
 
     let(:categories) { ::Gitlab::UsageDataCounters::HLLRedisCounter.categories }
+    let(:ineligible_total_categories) { ['source_code'] }
 
     it 'has all know_events' do
       expect(subject).to have_key(:redis_hll_counters)
@@ -1152,7 +1152,11 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
       expect(subject[:redis_hll_counters].keys).to match_array(categories)
 
       categories.each do |category|
-        keys = ::Gitlab::UsageDataCounters::HLLRedisCounter.events_for_category(category) + ["#{category}_total_unique_counts_weekly", "#{category}_total_unique_counts_monthly"]
+        keys = ::Gitlab::UsageDataCounters::HLLRedisCounter.events_for_category(category)
+
+        if ineligible_total_categories.exclude?(category)
+          keys.append("#{category}_total_unique_counts_weekly", "#{category}_total_unique_counts_monthly")
+        end
 
         expect(subject[:redis_hll_counters][category].keys).to match_array(keys)
       end