Merge branch 'am-adjust-redis-hll-track_event-to-accept-hash' into 'master'

Allow hash options to track_event [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!49828

app/controllers/concerns/redis_tracking.rb

@@ -33,7 +33,7 @@ module RedisTracking
     return unless metric_feature_enabled?(feature, feature_default_enabled)
     return unless visitor_id
 
-    Gitlab::UsageDataCounters::HLLRedisCounter.track_event(visitor_id, event_name)
+    Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name, values: visitor_id)
   end
 
   def trackable_request?

app/services/packages/create_event_service.rb

@@ -5,7 +5,7 @@ module Packages
     def execute
       if Feature.enabled?(:collect_package_events_redis)
         ::Packages::Event.unique_counters_for(event_scope, event_name, originator_type).each do |event_name|
-          ::Gitlab::UsageDataCounters::HLLRedisCounter.track_event(current_user.id, event_name)
+          ::Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name, values: current_user.id)
         end
 
         ::Packages::Event.counters_for(event_scope, event_name, originator_type).each do |event_name|

doc/development/product_analytics/usage_ping.md

@@ -474,19 +474,19 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF
    api.trackRedisHllUserEvent('my_already_defined_event_name'),
    ```
 
-1. Track event using base module `Gitlab::UsageDataCounters::HLLRedisCounter.track_event(values, event_name)`.
+1. Track event using base module `Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name, values:)`.
 
    Arguments:
 
-   - `values`: One value or array of values we count. For example: user_id, visitor_id, user_ids.
    - `event_name`: event name.
+   - `values`: One value or array of values we count. For example: user_id, visitor_id, user_ids.
 
-1. Track event on context level using base module `Gitlab::UsageDataCounters::HLLRedisCounter.track_event_in_context(entity_id, event_name, context)`.
+1. Track event on context level using base module `Gitlab::UsageDataCounters::HLLRedisCounter.track_event_in_context(event_name, values:, context:)`.
 
    Arguments:
 
-   - `entity_id`: value we count. For example: user_id, visitor_id.
    - `event_name`: event name.
+   - `values`: values we count. For example: user_id, visitor_id.
    - `context`: context value. Allowed values are `default`, `free`, `bronze`, `silver`, `gold`, `starter`, `premium`, `ultimate`
 
 1. Get event data using `Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names:, start_date:, end_date:, context: '')`.
@@ -503,8 +503,8 @@ Implemented using Redis methods [PFADD](https://redis.io/commands/pfadd) and [PF
 Trigger events in rails console by using `track_event` method
 
    ```ruby
-   Gitlab::UsageDataCounters::HLLRedisCounter.track_event(1, 'g_compliance_audit_events')
-   Gitlab::UsageDataCounters::HLLRedisCounter.track_event(2, 'g_compliance_audit_events')
+   Gitlab::UsageDataCounters::HLLRedisCounter.track_event('g_compliance_audit_events', values: 1)
+   Gitlab::UsageDataCounters::HLLRedisCounter.track_event('g_compliance_audit_events', values: [2, 3])
    ```
 
 Next, get the unique events for the current week.
@@ -595,7 +595,7 @@ redis_usage_data { ::Gitlab::UsageCounters::PodLogs.usage_totals[:total] }
 # Define events in common.yml https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data_counters/known_events/common.yml
 
 # Tracking events
-Gitlab::UsageDataCounters::HLLRedisCounter.track_event(visitor_id, 'expand_vulnerabilities')
+Gitlab::UsageDataCounters::HLLRedisCounter.track_event('expand_vulnerabilities', values: visitor_id)
 
 # Get unique events for metric
 redis_usage_data { Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: 'expand_vulnerabilities', start_date: 28.days.ago, end_date: Date.current) }

ee/app/models/ee/ci/build.rb

@@ -180,7 +180,7 @@ module EE
         return unless ::Feature.enabled?(:usage_data_i_ci_secrets_management_vault_build_created, default_enabled: true)
         return unless ci_secrets_management_available? && secrets?
 
-        ::Gitlab::UsageDataCounters::HLLRedisCounter.track_event(user_id, 'i_ci_secrets_management_vault_build_created')
+        ::Gitlab::UsageDataCounters::HLLRedisCounter.track_event('i_ci_secrets_management_vault_build_created', values: user_id)
       end
     end
   end

ee/spec/controllers/ee/search_controller_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe SearchController do
 
           it 'does not track if there is no license available' do
             stub_licensed_features(elastic_search: false)
-            expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with(instance_of(String), target_id)
+            expect(Gitlab::UsageDataCounters::HLLRedisCounter).not_to receive(:track_event).with(target_id, values: instance_of(String))
 
             get :show, params: request_params, format: :html
           end

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

@@ -52,7 +52,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
             expect(kwargs[:key]).to match(/^#{context}\_.*/)
           end
 
-          described_class.track_event_in_context(entity, event_name, context)
+          described_class.track_event_in_context(event_name, values: entity, context: context)
         end
       end
     end
@@ -61,18 +61,18 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
       it 'is not incrementing the counter' do
         expect(Gitlab::Redis::HLL).not_to receive(:add)
 
-        described_class.track_event_in_context(entity1, context_event, '')
+        described_class.track_event_in_context(context_event, values: entity1, context: '')
       end
     end
   end
 
   describe '.unique_events' do
     before do
-      described_class.track_event_in_context([entity1, entity3], context_event, default_context, 2.days.ago)
-      described_class.track_event_in_context(entity3, context_event, ultimate_context, 2.days.ago)
-      described_class.track_event_in_context(entity3, context_event, gold_context, 2.days.ago)
-      described_class.track_event_in_context(entity3, context_event, invalid_context, 2.days.ago)
-      described_class.track_event_in_context([entity1, entity2], context_event, '', 2.weeks.ago)
+      described_class.track_event_in_context(context_event, values: [entity1, entity3], context: default_context, time: 2.days.ago)
+      described_class.track_event_in_context(context_event, values: entity3, context: ultimate_context, time: 2.days.ago)
+      described_class.track_event_in_context(context_event, values: entity3, context: gold_context, time: 2.days.ago)
+      described_class.track_event_in_context(context_event, values: entity3, context: invalid_context, time: 2.days.ago)
+      described_class.track_event_in_context(context_event, values: [entity1, entity2], context: '', time: 2.weeks.ago)
     end
 
     context 'with correct arguments' do

ee/spec/models/ci/build_spec.rb

@@ -578,7 +578,7 @@ RSpec.describe Ci::Build do
           it 'tracks unique users' do
             ci_build = build(:ci_build, secrets: valid_secrets)
 
-            expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(ci_build.user_id, 'i_ci_secrets_management_vault_build_created')
+            expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with('i_ci_secrets_management_vault_build_created', values: ci_build.user_id)
 
             ci_build.save!
           end

ee/spec/requests/api/audit_events_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe API::AuditEvents do
 
       it 'tracks 3 separate events' do
         expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).exactly(3).times
-                                                                  .with(current_user.id, 'a_compliance_audit_events_api')
+                                                                  .with('a_compliance_audit_events_api', values: current_user.id)
 
         subject
       end

lib/api/helpers.rb

@@ -570,7 +570,7 @@ module API
 
       return unless Feature.enabled?(feature_flag, default_enabled: true)
 
-      Gitlab::UsageDataCounters::HLLRedisCounter.track_event(values, event_name)
+      Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name, values: values)
     rescue => error
       Gitlab::AppLogger.warn("Redis tracking event failed for event: #{event_name}, message: #{error.message}")
     end

lib/gitlab/analytics/unique_visits.rb

@@ -4,7 +4,7 @@ module Gitlab
   module Analytics
     class UniqueVisits
       def track_visit(visitor_id, target_id, time = Time.zone.now)
-        Gitlab::UsageDataCounters::HLLRedisCounter.track_event(visitor_id, target_id, time)
+        Gitlab::UsageDataCounters::HLLRedisCounter.track_event(target_id, values: visitor_id, time: time)
       end
 
       # Returns number of unique visitors for given targets in given time frame

lib/gitlab/usage_data_counters/editor_unique_counter.rb

@@ -53,7 +53,7 @@ module Gitlab
           return unless Feature.enabled?(:track_editor_edit_actions, default_enabled: true)
           return unless author
 
-          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(author.id, action, time)
+          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(action, values: author.id, time: time)
         end
 
         def count_unique(actions, date_from, date_to)

lib/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -39,20 +39,31 @@ module Gitlab
       #
       # Usage:
       #
-      # * Track event: Gitlab::UsageDataCounters::HLLRedisCounter.track_event(user_id, 'g_compliance_dashboard')
+      # * Track event: Gitlab::UsageDataCounters::HLLRedisCounter.track_event('g_compliance_dashboard', values: user_id)
       # * Get unique counts per user: Gitlab::UsageDataCounters::HLLRedisCounter.unique_events(event_names: 'g_compliance_dashboard', start_date: 28.days.ago, end_date: Date.current)
       class << self
         include Gitlab::Utils::UsageData
 
-        def track_event(value, event_name, time = Time.zone.now)
-          track(value, event_name, time: time)
-        end
-
-        def track_event_in_context(value, event_name, context, time = Time.zone.now)
+        # Track unique events
+        #
+        # event_name - The event name.
+        # values - One or multiple values counted.
+        # time - Time of the action, set to Time.current.
+        def track_event(event_name, values:, time: Time.current)
+          track(values, event_name, time: time)
+        end
+
+        # Track unique events
+        #
+        # event_name - The event name.
+        # values - One or multiple values counted.
+        # context - Event context, plan level tracking.
+        # time - Time of the action, set to Time.current.
+        def track_event_in_context(event_name, values:, context:, time: Time.zone.now)
           return if context.blank?
           return unless context.in?(valid_context_list)
 
-          track(value, event_name, context: context, time: time)
+          track(values, event_name, context: context, time: time)
         end
 
         def unique_events(event_names:, start_date:, end_date:, context: '')
@@ -114,13 +125,13 @@ module Gitlab
 
         private
 
-        def track(value, event_name, context: '', time: Time.zone.now)
+        def track(values, event_name, context: '', time: Time.zone.now)
           return unless Gitlab::CurrentSettings.usage_ping_enabled?
 
           event = event_for(event_name)
           raise UnknownEvent, "Unknown event #{event_name}" unless event.present?
 
-          Gitlab::Redis::HLL.add(key: redis_key(event, time, context), value: value, expiry: expiry(event))
+          Gitlab::Redis::HLL.add(key: redis_key(event, time, context), value: values, expiry: expiry(event))
         end
 
         # The array of valid context on which we allow tracking

lib/gitlab/usage_data_counters/issue_activity_unique_counter.rb

@@ -148,7 +148,7 @@ module Gitlab
           return unless Feature.enabled?(:track_issue_activity_actions, default_enabled: true)
           return unless author
 
-          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(author.id, action, time)
+          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(action, values: author.id, time: time)
         end
       end
     end

lib/gitlab/usage_data_counters/track_unique_events.rb

@@ -43,7 +43,7 @@ module Gitlab
 
           return unless Gitlab::UsageDataCounters::HLLRedisCounter.known_event?(transformed_action.to_s)
 
-          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(author_id, transformed_action.to_s, time)
+          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(transformed_action.to_s, values: author_id, time: time)
 
           track_git_write_action(author_id, transformed_action, time)
         end
@@ -73,7 +73,7 @@ module Gitlab
         def track_git_write_action(author_id, transformed_action, time)
           return unless GIT_WRITE_ACTIONS.include?(transformed_action)
 
-          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(author_id, GIT_WRITE_ACTION, time)
+          Gitlab::UsageDataCounters::HLLRedisCounter.track_event(GIT_WRITE_ACTION, values: author_id, time: time)
         end
       end
     end

lib/gitlab/utils/usage_data.rb

@@ -122,7 +122,7 @@ module Gitlab
       def track_usage_event(event_name, values)
         return unless Feature.enabled?(:"usage_data_#{event_name}", default_enabled: true)
 
-        Gitlab::UsageDataCounters::HLLRedisCounter.track_event(values, event_name.to_s)
+        Gitlab::UsageDataCounters::HLLRedisCounter.track_event(event_name.to_s, values: values)
       end
 
       private

spec/controllers/concerns/redis_tracking_spec.rb

@@ -42,7 +42,7 @@ RSpec.describe RedisTracking do
 
   def expect_tracking
     expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event)
-      .with(instance_of(String), 'g_compliance_approval_rules')
+      .with('g_compliance_approval_rules', values: instance_of(String))
   end
 
   def expect_no_tracking

spec/lib/api/helpers_spec.rb

@@ -205,7 +205,7 @@ RSpec.describe API::Helpers do
       end
 
       it 'tracks redis hll event' do
-        expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(value, event_name)
+        expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(event_name, values: value)
 
         subject.increment_unique_values(event_name, value)
       end

spec/lib/gitlab/utils/usage_data_spec.rb

@@ -347,7 +347,7 @@ RSpec.describe Gitlab::Utils::UsageData do
       end
 
       it 'tracks redis hll event' do
-        expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(value, event_name)
+        expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(event_name, values: value)
 
         described_class.track_usage_event(event_name, value)
       end

spec/services/packages/create_event_service_spec.rb

@@ -70,7 +70,7 @@ RSpec.describe Packages::CreateEventService do
       end
 
       it 'tracks the event' do
-        expect(::Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(user.id, /package/)
+        expect(::Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(/package/, values: user.id)
 
         subject
       end

spec/support/shared_examples/controllers/unique_hll_events_examples.rb

@@ -7,7 +7,7 @@
 
 RSpec.shared_examples 'tracking unique hll events' do |feature_flag|
   it 'tracks unique event' do
-    expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(expected_type, target_id)
+    expect(Gitlab::UsageDataCounters::HLLRedisCounter).to receive(:track_event).with(target_id, values: expected_type)
 
     request
   end

spec/support/shared_examples/lib/gitlab/usage_data_counters/incident_management_activity_shared_examples.rb

@@ -13,7 +13,7 @@ RSpec.shared_examples 'an incident management tracked event' do |event|
 
       expect(Gitlab::UsageDataCounters::HLLRedisCounter)
         .to receive(:track_event)
-        .with(current_user.id, event.to_s)
+        .with(event.to_s, values: current_user.id)
         .and_call_original
 
       expect { subject }