Always use generated queue name for worker matching

When we match against workers, the `name` attribute is tricky. It means
the queue name, but which queue name?

1. The actual queue name (after applying worker routing rules)?
2. The generated queue name (based on the worker class name and queue
   namespace)?

Before this commit, it was 1, but that was confusing: it would start as
`default` (before the attribute was set), then change to the generated
queue name, then change again to the queue name based on worker routing
rules. Because the routing rules are evaluated on application load, this
was hard to debug and could give unexpected results.

Instead, we say that the `name` attribute always means item 2. This is
also the name present in `all_queues.yml`, and this file is referenced
in the documentation. That means it's more consistent and predictable,
and doesn't require changes to the router itself.

app/workers/concerns/application_worker.rb

@@ -54,6 +54,10 @@ module ApplicationWorker
       subclass.after_set_class_attribute { subclass.set_queue }
     end
 
+    def generated_queue_name
+      Gitlab::SidekiqConfig::WorkerRouter.queue_name_from_worker_name(self)
+    end
+
     override :validate_worker_attributes!
     def validate_worker_attributes!
       super

doc/administration/operations/extra_sidekiq_routing.md

@@ -103,9 +103,11 @@ based on a subset of worker attributes:
 - `worker_name` - the worker name. The other attributes are typically more useful as
   they are more general, but this is available in case a particular worker needs
   to be selected.
-- `name` - the queue name. The other attributes are typically more useful as
-  they are more general, but this is available in case a particular queue needs
-  to be selected.
+- `name` - the queue name generated from the worker name. The other attributes
+  are typically more useful as they are more general, but this is available in
+  case a particular queue needs to be selected. Because this is generated from
+  the worker name, it does not change based on the result of other routing
+  rules.
 - `resource_boundary` - if the queue is bound by `cpu`, `memory`, or
   `unknown`. For example, the `ProjectExportWorker` is memory bound as it has
   to load data in memory before saving it for export.

ee/spec/lib/gitlab/sidekiq_config_spec.rb

@@ -46,7 +46,7 @@ RSpec.describe Gitlab::SidekiqConfig do
   describe '.workers_for_all_queues_yml' do
     it 'returns a tuple with EE workers second' do
       expect(described_class.workers_for_all_queues_yml.second)
-        .to include(an_object_having_attributes(queue: 'repository_update_mirror'))
+        .to include(an_object_having_attributes(generated_queue_name: 'repository_update_mirror'))
     end
   end
 

lib/gitlab/sidekiq_config/dummy_worker.rb

@@ -5,7 +5,6 @@ module Gitlab
     # For queues that don't have explicit workers - default and mailers
     class DummyWorker
       ATTRIBUTE_METHODS = {
-        queue: :queue,
         name: :name,
         feature_category: :get_feature_category,
         has_external_dependencies: :worker_has_external_dependencies?,
@@ -20,6 +19,10 @@ module Gitlab
         @attributes = attributes
       end
 
+      def generated_queue_name
+        @attributes[:queue]
+      end
+
       def queue_namespace
         nil
       end

lib/gitlab/sidekiq_config/worker.rb

@@ -6,9 +6,11 @@ module Gitlab
       include Comparable
 
       attr_reader :klass
-      delegate :feature_category_not_owned?, :get_feature_category, :get_sidekiq_options,
-               :get_tags, :get_urgency, :get_weight, :get_worker_resource_boundary,
-               :idempotent?, :queue, :queue_namespace, :worker_has_external_dependencies?,
+
+      delegate :feature_category_not_owned?, :generated_queue_name, :get_feature_category,
+               :get_sidekiq_options, :get_tags, :get_urgency, :get_weight,
+               :get_worker_resource_boundary, :idempotent?, :queue_namespace,
+               :worker_has_external_dependencies?,
                to: :klass
 
       def initialize(klass, ee:)
@@ -35,7 +37,7 @@ module Gitlab
 
       # Put namespaced queues first
       def to_sort
-        [queue_namespace ? 0 : 1, queue]
+        [queue_namespace ? 0 : 1, generated_queue_name]
       end
 
       # YAML representation
@@ -45,7 +47,7 @@ module Gitlab
 
       def to_yaml
         {
-          name: queue,
+          name: generated_queue_name,
           worker_name: klass.name,
           feature_category: get_feature_category,
           has_external_dependencies: worker_has_external_dependencies?,
@@ -62,7 +64,7 @@ module Gitlab
       end
 
       def queue_and_weight
-        [queue, get_weight]
+        [generated_queue_name, get_weight]
       end
 
       def retries

spec/lib/gitlab/sidekiq_config/worker_router_spec.rb

@@ -114,6 +114,13 @@ RSpec.describe Gitlab::SidekiqConfig::WorkerRouter do
         ['resource_boundary=cpu', 'queue_b'],
         ['tags=expensive', 'queue_c']
       ] | 'queue_foo'
+      # Match by generated queue name
+      [
+        ['name=foo_bar', 'queue_foo'],
+        ['feature_category=feature_a|urgency=low', 'queue_a'],
+        ['resource_boundary=cpu', 'queue_b'],
+        ['tags=expensive', 'queue_c']
+      ] | 'queue_foo'
     end
   end
 

spec/lib/gitlab/sidekiq_config/worker_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
     namespace = queue.include?(':') && queue.split(':').first
     inner_worker = double(
       name: attributes[:worker_name] || 'Foo::BarWorker',
-      queue: queue,
+      generated_queue_name: queue,
       queue_namespace: namespace,
       get_feature_category: attributes[:feature_category],
       get_weight: attributes[:weight],
@@ -48,9 +48,9 @@ RSpec.describe Gitlab::SidekiqConfig::Worker do
 
   describe 'delegations' do
     [
-      :feature_category_not_owned?, :get_feature_category, :get_weight,
-      :get_worker_resource_boundary, :get_urgency, :queue,
-      :queue_namespace, :worker_has_external_dependencies?
+      :feature_category_not_owned?, :generated_queue_name,
+      :get_feature_category, :get_weight, :get_worker_resource_boundary,
+      :get_urgency, :queue_namespace, :worker_has_external_dependencies?
     ].each do |meth|
       it "delegates #{meth} to the worker class" do
         worker = double

spec/lib/gitlab/sidekiq_config_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Gitlab::SidekiqConfig do
   describe '.workers_for_all_queues_yml' do
     it 'returns a tuple with FOSS workers first' do
       expect(described_class.workers_for_all_queues_yml.first)
-        .to include(an_object_having_attributes(queue: 'post_receive'))
+        .to include(an_object_having_attributes(generated_queue_name: 'post_receive'))
     end
   end
 

spec/workers/every_sidekiq_worker_spec.rb

@@ -8,17 +8,17 @@ RSpec.describe 'Every Sidekiq worker' do
   end
 
   it 'does not use the default queue' do
-    expect(workers_without_defaults.map(&:queue)).not_to include('default')
+    expect(workers_without_defaults.map(&:generated_queue_name)).not_to include('default')
   end
 
   it 'uses the cronjob queue when the worker runs as a cronjob' do
-    expect(Gitlab::SidekiqConfig.cron_workers.map(&:queue)).to all(start_with('cronjob:'))
+    expect(Gitlab::SidekiqConfig.cron_workers.map(&:generated_queue_name)).to all(start_with('cronjob:'))
   end
 
   it 'has its queue in Gitlab::SidekiqConfig::QUEUE_CONFIG_PATHS', :aggregate_failures do
     file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set
 
-    worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set
+    worker_queues = Gitlab::SidekiqConfig.workers.map(&:generated_queue_name).to_set
     worker_queues << ActionMailer::MailDeliveryJob.new.queue_name
     worker_queues << 'default'
 
@@ -33,7 +33,7 @@ RSpec.describe 'Every Sidekiq worker' do
     config_queues = Gitlab::SidekiqConfig.config_queues.to_set
 
     Gitlab::SidekiqConfig.workers.each do |worker|
-      queue = worker.queue
+      queue = worker.generated_queue_name
       queue_namespace = queue.split(':').first
 
       expect(config_queues).to include(queue).or(include(queue_namespace))