Merge branch 'fix-actionmailer-job-metric-labels' into 'master'

Fix ActionMailer job metric labels

See merge request gitlab-org/gitlab!56552

app/workers/all_queues.yml

@@ -1781,9 +1781,9 @@
   :idempotent: true
   :tags: []
 - :name: mailers
-  :feature_category:
+  :feature_category: :issue_tracking
   :has_external_dependencies:
-  :urgency:
+  :urgency: low
   :resource_boundary:
   :weight: 2
   :idempotent:

lib/gitlab/sidekiq_config.rb

@@ -13,10 +13,17 @@ module Gitlab
       (EE_QUEUE_CONFIG_PATH if Gitlab.ee?)
     ].compact.freeze
 
-    DEFAULT_WORKERS = [
-      DummyWorker.new('default', weight: 1, tags: []),
-      DummyWorker.new('mailers', weight: 2, tags: [])
-    ].map { |worker| Gitlab::SidekiqConfig::Worker.new(worker, ee: false) }.freeze
+    # This maps workers not in our application code to queues. We need
+    # these queues in our YAML files to ensure we don't accidentally
+    # miss jobs from these queues.
+    #
+    # The default queue should be unused, which is why it maps to an
+    # invalid class name. We keep it in the YAML file for safety, just
+    # in case anything does get scheduled to run there.
+    DEFAULT_WORKERS = {
+      '_' => DummyWorker.new('default', weight: 1, tags: []),
+      'ActionMailer::MailDeliveryJob' => DummyWorker.new('mailers', feature_category: :issue_tracking, urgency: 'low', weight: 2, tags: [])
+    }.transform_values { |worker| Gitlab::SidekiqConfig::Worker.new(worker, ee: false) }.freeze
 
     class << self
       include Gitlab::SidekiqConfig::CliMethods
@@ -40,7 +47,7 @@ module Gitlab
       def workers
         @workers ||= begin
           result = []
-          result.concat(DEFAULT_WORKERS)
+          result.concat(DEFAULT_WORKERS.values)
           result.concat(find_workers(Rails.root.join('app', 'workers'), ee: false))
 
           if Gitlab.ee?

lib/gitlab/sidekiq_middleware/metrics_helper.rb

@@ -10,6 +10,7 @@ module Gitlab
 
       def create_labels(worker_class, queue, job)
         worker_name = (job['wrapped'].presence || worker_class).to_s
+        worker = find_worker(worker_name, worker_class)
 
         labels = { queue: queue.to_s,
                    worker: worker_name,
@@ -18,15 +19,15 @@ module Gitlab
                    feature_category: "",
                    boundary: "" }
 
-        return labels unless worker_class && worker_class.include?(WorkerAttributes)
+        return labels unless worker.respond_to?(:get_urgency)
 
-        labels[:urgency] = worker_class.get_urgency.to_s
-        labels[:external_dependencies] = bool_as_label(worker_class.worker_has_external_dependencies?)
+        labels[:urgency] = worker.get_urgency.to_s
+        labels[:external_dependencies] = bool_as_label(worker.worker_has_external_dependencies?)
 
-        feature_category = worker_class.get_feature_category
+        feature_category = worker.get_feature_category
         labels[:feature_category] = feature_category.to_s
 
-        resource_boundary = worker_class.get_worker_resource_boundary
+        resource_boundary = worker.get_worker_resource_boundary
         labels[:boundary] = resource_boundary == :unknown ? "" : resource_boundary.to_s
 
         labels
@@ -35,6 +36,10 @@ module Gitlab
       def bool_as_label(value)
         value ? TRUE_LABEL : FALSE_LABEL
       end
+
+      def find_worker(worker_name, worker_class)
+        Gitlab::SidekiqConfig::DEFAULT_WORKERS.fetch(worker_name, worker_class)
+      end
     end
   end
 end

spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb

@@ -229,6 +229,15 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do
       it_behaves_like "a metrics middleware"
     end
 
+    context 'for ActionMailer::MailDeliveryJob' do
+      let(:job) { { 'class' => ActionMailer::MailDeliveryJob } }
+      let(:worker) { ActionMailer::MailDeliveryJob.new }
+      let(:worker_class) { ActionMailer::MailDeliveryJob }
+      let(:labels) { default_labels.merge(feature_category: 'issue_tracking') }
+
+      it_behaves_like 'a metrics middleware'
+    end
+
     context "when workers are attributed" do
       def create_attributed_worker_class(urgency, external_dependencies, resource_boundary, category)
         Class.new do

spec/workers/every_sidekiq_worker_spec.rb

@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe 'Every Sidekiq worker' do
   let(:workers_without_defaults) do
-    Gitlab::SidekiqConfig.workers - Gitlab::SidekiqConfig::DEFAULT_WORKERS
+    Gitlab::SidekiqConfig.workers - Gitlab::SidekiqConfig::DEFAULT_WORKERS.values
   end
 
   it 'does not use the default queue' do