Commit 324c544c authored by Mark Chao's avatar Mark Chao

Fix mail worker error in versioning middleware

Only assign version for ApplicationWorker
parent 280598e1
---
name: sidekiq_versioning
introduced_by_url:
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/232934
group: group::fulfillment
type: development
default_enabled: false
\ No newline at end of file
......@@ -19,6 +19,7 @@ module Gitlab
chain.add ::Labkit::Middleware::Sidekiq::Server
chain.add ::Gitlab::SidekiqMiddleware::InstrumentationLogger
chain.add ::Gitlab::SidekiqMiddleware::AdminMode::Server
chain.add ::Gitlab::SidekiqVersioning::Middleware
chain.add ::Gitlab::SidekiqStatus::ServerMiddleware
chain.add ::Gitlab::SidekiqMiddleware::WorkerContext::Server
chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Server
......
......@@ -5,10 +5,6 @@ module Gitlab
def self.install!
Sidekiq::Manager.prepend SidekiqVersioning::Manager
Sidekiq.server_middleware do |chain|
chain.add SidekiqVersioning::Middleware
end
# The Sidekiq client API always adds the queue to the Sidekiq queue
# list, but mail_room and gitlab-shell do not. This is only necessary
# for monitoring.
......
......@@ -4,7 +4,7 @@ module Gitlab
module SidekiqVersioning
class Middleware
def call(worker, job, queue)
worker.job_version = job['version']
worker.job_version = job['version'] if worker.is_a?(ApplicationWorker) && Feature.enabled?(:sidekiq_versioning)
yield
end
......
......@@ -51,6 +51,7 @@ RSpec.describe Gitlab::SidekiqMiddleware do
Gitlab::SidekiqMiddleware::BatchLoader,
Labkit::Middleware::Sidekiq::Server,
Gitlab::SidekiqMiddleware::InstrumentationLogger,
Gitlab::SidekiqVersioning::Middleware,
Gitlab::SidekiqStatus::ServerMiddleware,
Gitlab::SidekiqMiddleware::ServerMetrics,
Gitlab::SidekiqMiddleware::ArgumentsLogger,
......@@ -78,6 +79,41 @@ RSpec.describe Gitlab::SidekiqMiddleware do
end
end
shared_examples "a server middleware chain for mailer" do
let(:worker_class) { ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper }
let(:job_args) do
[
{
"job_class" => "ActionMailer::MailDeliveryJob",
"job_id" => "a180b47c-3fd6-41b8-81e9-34da61c3400e",
"provider_job_id" => nil,
"queue_name" => "mailers",
"priority" => nil,
"arguments" => [
"Notify",
"test_email",
"deliver_now",
{
"args" => [
"test@example.com",
"subject",
"body"
],
"_aj_symbol_keys" => ["args"]
}
],
"executions" => 0,
"exception_executions" => {},
"locale" => "en",
"timezone" => "UTC",
"enqueued_at" => "2020-07-27T07:43:31Z"
}
]
end
it_behaves_like "a server middleware chain"
end
context "all optional middlewares off" do
let(:metrics) { false }
let(:arguments_logger) { false }
......@@ -91,6 +127,7 @@ RSpec.describe Gitlab::SidekiqMiddleware do
end
it_behaves_like "a server middleware chain"
it_behaves_like "a server middleware chain for mailer"
end
context "all optional middlewares on" do
......@@ -100,6 +137,7 @@ RSpec.describe Gitlab::SidekiqMiddleware do
let(:disabled_sidekiq_middlewares) { [] }
it_behaves_like "a server middleware chain"
it_behaves_like "a server middleware chain for mailer"
context "server metrics" do
let(:gitaly_histogram) { double(:gitaly_histogram) }
......
......@@ -34,5 +34,27 @@ RSpec.describe Gitlab::SidekiqVersioning::Middleware do
it 'yields' do
expect { |b| call!(&b) }.to yield_control
end
context 'when worker is not ApplicationWorker' do
let(:worker_class) do
ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper
end
it 'does not err' do
expect { call! }.not_to raise_error
end
end
context 'when sidekiq_versioning is disabled' do
before do
stub_feature_flags(sidekiq_versioning: false)
end
it 'does not set job_version' do
expect(worker).not_to receive(:job_version=)
call!
end
end
end
end
......@@ -35,12 +35,6 @@ RSpec.describe Gitlab::SidekiqVersioning, :redis do
expect(Sidekiq::Manager).to include(Gitlab::SidekiqVersioning::Manager)
end
it 'adds the SidekiqVersioning::Middleware Sidekiq server middleware' do
described_class.install!
expect(Sidekiq.server_middleware.entries.map(&:klass)).to include(Gitlab::SidekiqVersioning::Middleware)
end
it 'registers all versionless and versioned queues with Redis' do
described_class.install!
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment