Add metrics for build queue push/pop operations

app/services/ci/update_build_queue_service.rb

@@ -17,10 +17,10 @@ module Ci
       raise InvalidQueueTransition unless transition.to == 'pending'
 
       transition.within_transaction do
-        ::Ci::PendingBuild.create!(build: build, project: build.project)
+        ::Ci::PendingBuild.create!(build: build, project: build.project).tap do
+          metrics.increment_queue_operation(:build_queue_push)
+        end
       end
-
-      # TODO increment pending builds counter
     end
 
     ##
@@ -30,10 +30,14 @@ module Ci
       raise InvalidQueueTransition unless transition.from == 'pending'
 
       transition.within_transaction do
-        ::Ci::PendingBuild.find_by(build_id: build.id)&.destroy! # rubocop:disable CodeReuse/ActiveRecord
-      end
+        ::Ci::PendingBuild.find_by(build_id: build.id).tap do |queued| # rubocop:disable CodeReuse/ActiveRecord
+          next unless queued.present?
 
-      # TODO decrement pending builds counter
+          queued.destroy!
+
+          metrics.increment_queue_operation(:build_queue_pop)
+        end
+      end
     end
 
     ##

lib/gitlab/ci/queue/metrics.rb

@@ -20,6 +20,8 @@ module Gitlab
           :build_can_pick,
           :build_not_pick,
           :build_not_pending,
+          :build_queue_push,
+          :build_queue_pop,
           :build_temporary_locked,
           :build_conflict_lock,
           :build_conflict_exception,
@@ -77,11 +79,7 @@ module Gitlab
         # rubocop: enable CodeReuse/ActiveRecord
 
         def increment_queue_operation(operation)
-          if !Rails.env.production? && !OPERATION_COUNTERS.include?(operation)
-            raise ArgumentError, "unknown queue operation: #{operation}"
-          end
-
-          self.class.queue_operations_total.increment(operation: operation)
+          self.class.increment_queue_operation(operation)
         end
 
         def observe_queue_depth(queue, size)
@@ -121,6 +119,14 @@ module Gitlab
           result
         end
 
+        def self.increment_queue_operation(operation)
+          if !Rails.env.production? && !OPERATION_COUNTERS.include?(operation)
+            raise ArgumentError, "unknown queue operation: #{operation}"
+          end
+
+          queue_operations_total.increment(operation: operation)
+        end
+
         def self.observe_active_runners(runners_proc)
           return unless Feature.enabled?(:gitlab_ci_builds_queuing_metrics, default_enabled: false)
 

spec/services/ci/update_build_queue_service_spec.rb

@@ -18,6 +18,18 @@ RSpec.describe Ci::UpdateBuildQueueService do
       expect(queued.project).to eq project
     end
 
+    it 'increments queue push metric' do
+      metrics = spy('metrics')
+      transition = double(to: 'pending')
+      allow(transition).to receive(:within_transaction).and_yield
+
+      described_class.new(metrics).push(build, transition)
+
+      expect(metrics)
+        .to have_received(:increment_queue_operation)
+        .with(:build_queue_push)
+    end
+
     it 'raises an error when invalid transition is detected' do
       transition = double(to: 'created')
 
@@ -27,10 +39,10 @@ RSpec.describe Ci::UpdateBuildQueueService do
   end
 
   describe '#pop' do
-    it 'it removes pending build in a transaction' do
+    it 'removes pending build in a transaction' do
       transition = double(from: 'pending')
       allow(transition).to receive(:within_transaction).and_yield
-      queued = Ci::PendingBuild.create!(build: build, project: project)
+      Ci::PendingBuild.create!(build: build, project: project)
 
       dequeued = subject.pop(build, transition)
 
@@ -39,6 +51,19 @@ RSpec.describe Ci::UpdateBuildQueueService do
       expect(dequeued).to be_destroyed
     end
 
+    it 'increments queue pop metric' do
+      metrics = spy('metrics')
+      transition = double(from: 'pending')
+      allow(transition).to receive(:within_transaction).and_yield
+      Ci::PendingBuild.create!(build: build, project: project)
+
+      described_class.new(metrics).pop(build, transition)
+
+      expect(metrics)
+        .to have_received(:increment_queue_operation)
+        .with(:build_queue_pop)
+    end
+
     it 'does nothing if there is no pending build to remove' do
       transition = double(from: 'pending')
       allow(transition).to receive(:within_transaction).and_yield
@@ -59,13 +84,13 @@ RSpec.describe Ci::UpdateBuildQueueService do
   describe '#tick' do
     shared_examples 'refreshes runner' do
       it 'ticks runner queue value' do
-        expect { subject.tick!(build) }.to change { runner.ensure_runner_queue_value }
+        expect { subject.tick(build) }.to change { runner.ensure_runner_queue_value }
       end
     end
 
     shared_examples 'does not refresh runner' do
       it 'ticks runner queue value' do
-        expect { subject.tick!(build) }.not_to change { runner.ensure_runner_queue_value }
+        expect { subject.tick(build) }.not_to change { runner.ensure_runner_queue_value }
       end
     end