Merge branch 'feature/gb/refactor-register-build-metrics' into 'master'

Refactor runner build request / registration metrics

See merge request gitlab-org/gitlab!54953

app/services/ci/register_job_service.rb

@@ -6,15 +6,11 @@ module Ci
   class RegisterJobService
     attr_reader :runner
 
-    JOB_QUEUE_DURATION_SECONDS_BUCKETS = [1, 3, 10, 30, 60, 300, 900, 1800, 3600].freeze
-    JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET = 5.freeze
-    METRICS_SHARD_TAG_PREFIX = 'metrics_shard::'
-    DEFAULT_METRICS_SHARD = 'default'
-
     Result = Struct.new(:build, :build_json, :valid?)
 
     def initialize(runner)
       @runner = runner
+      @metrics = ::Gitlab::Ci::Queue::Metrics.new(runner)
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
@@ -48,7 +44,7 @@ module Ci
         next unless result
 
         if result.valid?
-          register_success(result.build)
+          @metrics.register_success(result.build)
 
           return result
         else
@@ -58,7 +54,7 @@ module Ci
         end
       end
 
-      register_failure
+      @metrics.register_failure
       Result.new(nil, nil, valid)
     end
     # rubocop: enable CodeReuse/ActiveRecord
@@ -189,48 +185,6 @@ module Ci
       builds
     end
 
-    def register_failure
-      failed_attempt_counter.increment
-      attempt_counter.increment
-    end
-
-    def register_success(job)
-      labels = { shared_runner: runner.instance_type?,
-                 jobs_running_for_project: jobs_running_for_project(job),
-                 shard: DEFAULT_METRICS_SHARD }
-
-      if runner.instance_type?
-        shard = runner.tag_list.sort.find { |name| name.starts_with?(METRICS_SHARD_TAG_PREFIX) }
-        labels[:shard] = shard.gsub(METRICS_SHARD_TAG_PREFIX, '') if shard
-      end
-
-      job_queue_duration_seconds.observe(labels, Time.current - job.queued_at) unless job.queued_at.nil?
-      attempt_counter.increment
-    end
-
-    # rubocop: disable CodeReuse/ActiveRecord
-    def jobs_running_for_project(job)
-      return '+Inf' unless runner.instance_type?
-
-      # excluding currently started job
-      running_jobs_count = job.project.builds.running.where(runner: Ci::Runner.instance_type)
-                              .limit(JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET + 1).count - 1
-      running_jobs_count < JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET ? running_jobs_count : "#{JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET}+"
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    def failed_attempt_counter
-      @failed_attempt_counter ||= Gitlab::Metrics.counter(:job_register_attempts_failed_total, "Counts the times a runner tries to register a job")
-    end
-
-    def attempt_counter
-      @attempt_counter ||= Gitlab::Metrics.counter(:job_register_attempts_total, "Counts the times a runner tries to register a job")
-    end
-
-    def job_queue_duration_seconds
-      @job_queue_duration_seconds ||= Gitlab::Metrics.histogram(:job_queue_duration_seconds, 'Request handling execution time', {}, JOB_QUEUE_DURATION_SECONDS_BUCKETS)
-    end
-
     def pre_assign_runner_checks
       {
         missing_dependency_failure: -> (build, _) { !build.has_valid_build_dependencies? },

lib/gitlab/ci/queue/metrics.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Queue
+      class Metrics
+        extend Gitlab::Utils::StrongMemoize
+
+        QUEUE_DURATION_SECONDS_BUCKETS = [1, 3, 10, 30, 60, 300, 900, 1800, 3600].freeze
+        METRICS_SHARD_TAG_PREFIX = 'metrics_shard::'
+        DEFAULT_METRICS_SHARD = 'default'
+        JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET = 5.freeze
+
+        attr_reader :runner
+
+        def initialize(runner)
+          @runner = runner
+        end
+
+        def register_failure
+          self.class.failed_attempt_counter.increment
+          self.class.attempt_counter.increment
+        end
+
+        def register_success(job)
+          labels = { shared_runner: runner.instance_type?,
+                     jobs_running_for_project: jobs_running_for_project(job),
+                     shard: DEFAULT_METRICS_SHARD }
+
+          if runner.instance_type?
+            shard = runner.tag_list.sort.find { |name| name.starts_with?(METRICS_SHARD_TAG_PREFIX) }
+            labels[:shard] = shard.gsub(METRICS_SHARD_TAG_PREFIX, '') if shard
+          end
+
+          self.class.job_queue_duration_seconds.observe(labels, Time.current - job.queued_at) unless job.queued_at.nil?
+          self.class.attempt_counter.increment
+        end
+
+        # rubocop: disable CodeReuse/ActiveRecord
+        def jobs_running_for_project(job)
+          return '+Inf' unless runner.instance_type?
+
+          # excluding currently started job
+          running_jobs_count = job.project.builds.running.where(runner: ::Ci::Runner.instance_type)
+                                  .limit(JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET + 1).count - 1
+          running_jobs_count < JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET ? running_jobs_count : "#{JOBS_RUNNING_FOR_PROJECT_MAX_BUCKET}+"
+        end
+        # rubocop: enable CodeReuse/ActiveRecord
+
+        def self.failed_attempt_counter
+          strong_memoize(:failed_attempt_counter) do
+            name = :job_register_attempts_failed_total
+            comment = 'Counts the times a runner tries to register a job'
+
+            Gitlab::Metrics.counter(name, comment)
+          end
+        end
+
+        def self.attempt_counter
+          strong_memoize(:attempt_counter) do
+            name = :job_register_attempts_total
+            comment = 'Counts the times a runner tries to register a job'
+
+            Gitlab::Metrics.counter(name, comment)
+          end
+        end
+
+        def self.job_queue_duration_seconds
+          strong_memoize(:job_queue_duration_seconds) do
+            name = :job_queue_duration_seconds
+            comment = 'Request handling execution time'
+            labels = {}
+            buckets = QUEUE_DURATION_SECONDS_BUCKETS
+
+            Gitlab::Metrics.histogram(name, comment, labels, buckets)
+          end
+        end
+      end
+    end
+  end
+end

spec/services/ci/register_job_service_spec.rb

@@ -590,22 +590,14 @@ module Ci
 
       before do
         allow(Time).to receive(:now).and_return(current_time)
-
-        # Stub defaults for any metrics other than the ones we're testing
-        allow(Gitlab::Metrics).to receive(:counter)
-                                    .with(any_args)
-                                    .and_return(Gitlab::Metrics::NullMetric.instance)
-        allow(Gitlab::Metrics).to receive(:histogram)
-                                    .with(any_args)
-                                    .and_return(Gitlab::Metrics::NullMetric.instance)
-
         # Stub tested metrics
-        allow(Gitlab::Metrics).to receive(:counter)
-                                    .with(:job_register_attempts_total, anything)
-                                    .and_return(attempt_counter)
-        allow(Gitlab::Metrics).to receive(:histogram)
-                                    .with(:job_queue_duration_seconds, anything, anything, anything)
-                                    .and_return(job_queue_duration_seconds)
+        allow(Gitlab::Ci::Queue::Metrics)
+          .to receive(:attempt_counter)
+          .and_return(attempt_counter)
+
+        allow(Gitlab::Ci::Queue::Metrics)
+          .to receive(:job_queue_duration_seconds)
+          .and_return(job_queue_duration_seconds)
 
         project.update!(shared_runners_enabled: true)
         pending_job.update!(created_at: current_time - 3600, queued_at: current_time - 1800)
@@ -655,7 +647,7 @@ module Ci
       context 'when shared runner is used' do
         let(:runner) { create(:ci_runner, :instance, tag_list: %w(tag1 tag2)) }
         let(:expected_shared_runner) { true }
-        let(:expected_shard) { Ci::RegisterJobService::DEFAULT_METRICS_SHARD }
+        let(:expected_shard) { ::Gitlab::Ci::Queue::Metrics::DEFAULT_METRICS_SHARD }
         let(:expected_jobs_running_for_project_first_job) { 0 }
         let(:expected_jobs_running_for_project_third_job) { 2 }
 
@@ -694,7 +686,7 @@ module Ci
       context 'when specific runner is used' do
         let(:runner) { create(:ci_runner, :project, projects: [project], tag_list: %w(tag1 metrics_shard::shard_tag tag2)) }
         let(:expected_shared_runner) { false }
-        let(:expected_shard) { Ci::RegisterJobService::DEFAULT_METRICS_SHARD }
+        let(:expected_shard) { ::Gitlab::Ci::Queue::Metrics::DEFAULT_METRICS_SHARD }
         let(:expected_jobs_running_for_project_first_job) { '+Inf' }
         let(:expected_jobs_running_for_project_third_job) { '+Inf' }