Merge branch 'ee-move-worker-differences' into 'master'

EE: Refactor Sidekiq workers to reduce differences between CE and EE

See merge request gitlab-org/gitlab-ee!9215

app/workers/build_finished_worker.rb

@@ -9,18 +9,28 @@ class BuildFinishedWorker
   # rubocop: disable CodeReuse/ActiveRecord
   def perform(build_id)
     Ci::Build.find_by(id: build_id).try do |build|
-      UpdateBuildMinutesService.new(build.project, nil).execute(build)
+      process_build(build)
+    end
+  end
+  # rubocop: enable CodeReuse/ActiveRecord
 
-      # We execute that in sync as this access the files in order to access local file, and reduce IO
+  private
+
+  # Processes a single CI build that has finished.
+  #
+  # This logic resides in a separate method so that EE can extend it more
+  # easily.
+  #
+  # @param [Ci::Build] build The build to process.
+  def process_build(build)
+    # We execute these in sync to reduce IO.
     BuildTraceSectionsWorker.new.perform(build.id)
     BuildCoverageWorker.new.perform(build.id)
 
-      # We execute that async as this are two independent operations that can be executed after TraceSections and Coverage
+    # We execute these async as these are independent operations.
     BuildHooksWorker.perform_async(build.id)
     ArchiveTraceWorker.perform_async(build.id)
   end
-  end
-  # rubocop: enable CodeReuse/ActiveRecord
 end
 
 BuildFinishedWorker.prepend(EE::BuildFinishedWorker)

app/workers/delete_user_worker.rb

@@ -7,7 +7,7 @@ class DeleteUserWorker
     delete_user  = User.find(delete_user_id)
     current_user = User.find(current_user_id)
 
-    ::Users::DestroyService.new(current_user).execute(delete_user, options.symbolize_keys)
+    Users::DestroyService.new(current_user).execute(delete_user, options.symbolize_keys)
   rescue Gitlab::Access::AccessDeniedError => e
     Rails.logger.warn("User could not be destroyed: #{e}")
   end

app/workers/expire_pipeline_cache_worker.rb

@@ -11,23 +11,9 @@ class ExpirePipelineCacheWorker
     pipeline = Ci::Pipeline.find_by(id: pipeline_id)
     return unless pipeline
 
-    project = pipeline.project
     store = Gitlab::EtagCaching::Store.new
 
-    store.touch(project_pipelines_path(project))
-    store.touch(project_pipeline_path(project, pipeline))
-    store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil?
-    store.touch(new_merge_request_pipelines_path(project))
-    each_pipelines_merge_request_path(project, pipeline) do |path|
-      store.touch(path)
-    end
-
-    triggered_by = pipeline.triggered_by_pipeline
-    store.touch(project_pipeline_path(triggered_by.project, triggered_by)) if triggered_by
-
-    pipeline.triggered_pipelines.each do |triggered|
-      store.touch(project_pipeline_path(triggered.project, triggered))
-    end
+    update_etag_cache(pipeline, store)
 
     Gitlab::Cache::Ci::ProjectPipelineStatus.update_for_pipeline(pipeline)
   end
@@ -58,4 +44,25 @@ class ExpirePipelineCacheWorker
       yield(path)
     end
   end
+
+  # Updates ETag caches of a pipeline.
+  #
+  # This logic resides in a separate method so that EE can more easily extend
+  # it.
+  #
+  # @param [Ci::Pipeline] pipeline
+  # @param [Gitlab::EtagCaching::Store] store
+  def update_etag_cache(pipeline, store)
+    project = pipeline.project
+
+    store.touch(project_pipelines_path(project))
+    store.touch(project_pipeline_path(project, pipeline))
+    store.touch(commit_pipelines_path(project, pipeline.commit)) unless pipeline.commit.nil?
+    store.touch(new_merge_request_pipelines_path(project))
+    each_pipelines_merge_request_path(project, pipeline) do |path|
+      store.touch(path)
+    end
+  end
 end
+
+ExpirePipelineCacheWorker.prepend(EE::ExpirePipelineCacheWorker)

app/workers/stuck_import_jobs_worker.rb

@@ -10,16 +10,9 @@ class StuckImportJobsWorker
     import_state_without_jid_count = mark_import_states_without_jid_as_failed!
     import_state_with_jid_count = mark_import_states_with_jid_as_failed!
 
-    values = {
+    Gitlab::Metrics.add_event(:stuck_import_jobs,
                              projects_without_jid_count: import_state_without_jid_count,
-      projects_with_jid_count: import_state_with_jid_count
-    }
-
-    Gitlab::Metrics.add_event_with_values(:stuck_import_jobs, values)
-
-    stuck_import_jobs_worker_runs_counter.increment
-    import_state_without_jid_metric.set({}, import_state_without_jid_count)
-    import_state_with_jid_metric.set({}, import_state_with_jid_count)
+                             projects_with_jid_count: import_state_with_jid_count)
   end
 
   private
@@ -72,17 +65,4 @@ class StuckImportJobsWorker
   def error_message
     _("Import timed out. Import took longer than %{import_jobs_expiration} seconds") % { import_jobs_expiration: IMPORT_JOBS_EXPIRATION }
   end
-
-  def stuck_import_jobs_worker_runs_counter
-    @stuck_import_jobs_worker_runs_counter ||= Gitlab::Metrics.counter(:gitlab_stuck_import_jobs_worker_runs_total,
-                                                                       'Stuck import jobs worker runs count')
-  end
-
-  def import_state_without_jid_metric
-    @import_state_without_jid_metric ||= Gitlab::Metrics.gauge(:gitlab_projects_without_jid, 'Projects without Job ids')
-  end
-
-  def import_state_with_jid_metric
-    @import_state_with_jid_metric ||= Gitlab::Metrics.gauge(:gitlab_projects_with_jid, 'Projects with Job ids')
-  end
 end

ee/app/workers/ee/build_finished_worker.rb

@@ -2,14 +2,12 @@
 
 module EE
   module BuildFinishedWorker
-    # rubocop: disable CodeReuse/ActiveRecord
-    def perform(build_id)
+    def process_build(build)
+      UpdateBuildMinutesService.new(build.project, nil).execute(build)
+
       super
 
-      ::Ci::Build.find_by(id: build_id).try do |build|
-        ChatNotificationWorker.perform_async(build_id) if build.pipeline.chat?
-      end
+      ChatNotificationWorker.perform_async(build.id) if build.pipeline.chat?
     end
-    # rubocop: enable CodeReuse/ActiveRecord
   end
 end

ee/app/workers/ee/expire_pipeline_cache_worker.rb

+# frozen_string_literal: true
+
+module EE
+  module ExpirePipelineCacheWorker
+    extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
+
+    override :update_etag_cache
+    def update_etag_cache(pipeline, store)
+      super
+
+      triggered_by = pipeline.triggered_by_pipeline
+      store.touch(project_pipeline_path(triggered_by.project, triggered_by)) if triggered_by
+
+      pipeline.triggered_pipelines.each do |triggered|
+        store.touch(project_pipeline_path(triggered.project, triggered))
+      end
+    end
+  end
+end
+
+ExpirePipelineCacheWorker.prepend(EE::ExpirePipelineCacheWorker)

lib/gitlab/metrics/influx_db.rb

@@ -150,10 +150,6 @@ module Gitlab
           current_transaction&.add_event(*args)
         end
 
-        def add_event_with_values(*args)
-          current_transaction&.add_event_with_values(*args)
-        end
-
         # Returns the prefix to use for the name of a series.
         def series_prefix
           @series_prefix ||= Sidekiq.server? ? 'sidekiq_' : 'rails_'

lib/gitlab/metrics/transaction.rb

@@ -79,15 +79,6 @@ module Gitlab
         @metrics << Metric.new(EVENT_SERIES, { count: 1 }, tags.merge(event: event_name), :event)
       end
 
-      #
-      # Deprecated
-      def add_event_with_values(event_name, values, tags = {})
-        @metrics << Metric.new(EVENT_SERIES,
-                               { count: 1 }.merge(values),
-                               { event: event_name }.merge(tags),
-                               :event)
-      end
-
       # Returns a MethodCall object for the given name.
       def method_call_for(name, module_name, method_name)
         unless method = @methods[name]

spec/lib/gitlab/metrics/web_transaction_spec.rb

@@ -155,30 +155,6 @@ describe Gitlab::Metrics::WebTransaction do
     end
   end
 
-  describe '#add_event_with_values' do
-    it 'adds a metric' do
-      transaction.add_event_with_values(:meow, {})
-
-      expect(transaction.metrics[0]).to be_an_instance_of(Gitlab::Metrics::Metric)
-    end
-
-    it 'tracks values for every event' do
-      transaction.add_event_with_values(:meow, { number: 10 })
-
-      metric = transaction.metrics[0]
-
-      expect(metric.values).to eq(count: 1, number: 10)
-    end
-
-    it 'allows tracking of custom tags' do
-      transaction.add_event_with_values(:meow, {}, animal: 'cat')
-
-      metric = transaction.metrics[0]
-
-      expect(metric.tags).to eq(event: :meow, animal: 'cat')
-    end
-  end
-
   describe '#labels' do
     context 'when request goes to Grape endpoint' do
       before do