diff options
Diffstat (limited to 'spec/lib/gitlab/sidekiq_middleware')
6 files changed, 539 insertions, 121 deletions
diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb index 0285467ecab..a10a8883591 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/duplicate_job_spec.rb @@ -18,14 +18,43 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi end describe '#schedule' do - it 'calls schedule on the strategy' do - expect do |block| - expect_next_instance_of(Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting) do |strategy| - expect(strategy).to receive(:schedule).with(job, &block) + shared_examples 'scheduling with deduplication class' do |strategy_class| + it 'calls schedule on the strategy' do + expect do |block| + expect_next_instance_of("Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::#{strategy_class}".constantize) do |strategy| + expect(strategy).to receive(:schedule).with(job, &block) + end + + duplicate_job.schedule(&block) + end.to yield_control + end + end + + it_behaves_like 'scheduling with deduplication class', 'UntilExecuting' + + context 'when the deduplication depends on a FF' do + before do + skip_feature_flags_yaml_validation + skip_default_enabled_yaml_check + + allow(AuthorizedProjectsWorker).to receive(:get_deduplication_options).and_return(feature_flag: :my_feature_flag) + end + + context 'when the feature flag is enabled' do + before do + stub_feature_flags(my_feature_flag: true) end - duplicate_job.schedule(&block) - end.to yield_control + it_behaves_like 'scheduling with deduplication class', 'UntilExecuting' + end + + context 'when the feature flag is disabled' do + before do + stub_feature_flags(my_feature_flag: false) + end + + it_behaves_like 'scheduling with deduplication class', 'None' + end end end @@ -51,6 +80,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi .from([nil, -2]) .to(['123', be_within(1).of(described_class::DUPLICATE_KEY_TTL)]) end + + it "adds the idempotency key to the jobs payload" do + expect { duplicate_job.check! }.to change { job['idempotency_key'] }.from(nil).to(idempotency_key) + end end context 'when there was already a job with same arguments in the same queue' do @@ -81,14 +114,39 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob, :clean_gi context 'when the key exists in redis' do before do - set_idempotency_key(idempotency_key, 'existing-key') + set_idempotency_key(idempotency_key, 'existing-jid') end - it 'removes the key from redis' do - expect { duplicate_job.delete! } - .to change { read_idempotency_key_with_ttl(idempotency_key) } - .from(['existing-key', -1]) - .to([nil, -2]) + shared_examples 'deleting the duplicate job' do + it 'removes the key from redis' do + expect { duplicate_job.delete! } + .to change { read_idempotency_key_with_ttl(idempotency_key) } + .from(['existing-jid', -1]) + .to([nil, -2]) + end + end + + context 'when the idempotency key is not part of the job' do + it_behaves_like 'deleting the duplicate job' + + it 'recalculates the idempotency hash' do + expect(duplicate_job).to receive(:idempotency_hash).and_call_original + + duplicate_job.delete! + end + end + + context 'when the idempotency key is part of the job' do + let(:idempotency_key) { 'not the same as what we calculate' } + let(:job) { super().merge('idempotency_key' => idempotency_key) } + + it_behaves_like 'deleting the duplicate job' + + it 'does not recalculate the idempotency hash' do + expect(duplicate_job).not_to receive(:idempotency_hash) + + duplicate_job.delete! + end end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/instrumentation_logger_spec.rb b/spec/lib/gitlab/sidekiq_middleware/instrumentation_logger_spec.rb index eb9ba50cdcd..8cf65e1be5b 100644 --- a/spec/lib/gitlab/sidekiq_middleware/instrumentation_logger_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/instrumentation_logger_spec.rb @@ -24,58 +24,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::InstrumentationLogger do stub_const('TestWorker', worker) end - describe '.keys' do - it 'returns all available payload keys' do - expected_keys = [ - :cpu_s, - :gitaly_calls, - :gitaly_duration_s, - :rugged_calls, - :rugged_duration_s, - :elasticsearch_calls, - :elasticsearch_duration_s, - :elasticsearch_timed_out_count, - :mem_objects, - :mem_bytes, - :mem_mallocs, - :redis_calls, - :redis_duration_s, - :redis_read_bytes, - :redis_write_bytes, - :redis_action_cable_calls, - :redis_action_cable_duration_s, - :redis_action_cable_read_bytes, - :redis_action_cable_write_bytes, - :redis_cache_calls, - :redis_cache_duration_s, - :redis_cache_read_bytes, - :redis_cache_write_bytes, - :redis_queues_calls, - :redis_queues_duration_s, - :redis_queues_read_bytes, - :redis_queues_write_bytes, - :redis_shared_state_calls, - :redis_shared_state_duration_s, - :redis_shared_state_read_bytes, - :redis_shared_state_write_bytes, - :db_count, - :db_write_count, - :db_cached_count, - :external_http_count, - :external_http_duration_s, - :rack_attack_redis_count, - :rack_attack_redis_duration_s - ] - - expect(described_class.keys).to include(*expected_keys) - end - end - describe '#call', :request_store do let(:instrumentation_values) do { cpu_s: 10, - unknown_attribute: 123, db_count: 0, db_cached_count: 0, db_write_count: 0, @@ -90,12 +42,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::InstrumentationLogger do end end - it 'merges correct instrumentation data in the job' do + it 'merges all instrumentation data in the job' do expect { |b| subject.call(worker, job, queue, &b) }.to yield_control - expected_values = instrumentation_values.except(:unknown_attribute) - - expect(job[:instrumentation]).to eq(expected_values) + expect(job[:instrumentation]).to eq(instrumentation_values) end end end diff --git a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb index 95be76ce351..34b4541f339 100644 --- a/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/server_metrics_spec.rb @@ -107,5 +107,110 @@ RSpec.describe Gitlab::SidekiqMiddleware::ServerMetrics do let(:job_status) { :done } let(:labels_with_job_status) { labels.merge(job_status: job_status.to_s) } end + + context 'DB load balancing' do + using RSpec::Parameterized::TableSyntax + + subject { described_class.new } + + let(:queue) { :test } + let(:worker_class) { worker.class } + let(:job) { {} } + let(:job_status) { :done } + let(:labels_with_job_status) { default_labels.merge(job_status: job_status.to_s) } + let(:default_labels) do + { queue: queue.to_s, + worker: worker_class.to_s, + boundary: "", + external_dependencies: "no", + feature_category: "", + urgency: "low" } + end + + before do + stub_const('TestWorker', Class.new) + TestWorker.class_eval do + include Sidekiq::Worker + include WorkerAttributes + end + end + + let(:worker) { TestWorker.new } + + include_context 'server metrics with mocked prometheus' + + context 'when load_balancing is enabled' do + let(:load_balancing_metric) { double('load balancing metric') } + + include_context 'clear DB Load Balancing configuration' + + before do + allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(true) + allow(Gitlab::Metrics).to receive(:counter).with(:sidekiq_load_balancing_count, anything).and_return(load_balancing_metric) + end + + describe '#initialize' do + it 'sets load_balancing metrics' do + expect(Gitlab::Metrics).to receive(:counter).with(:sidekiq_load_balancing_count, anything).and_return(load_balancing_metric) + + subject + end + end + + describe '#call' do + include_context 'server metrics call' + + context 'when :database_chosen is provided' do + where(:database_chosen) do + %w[primary retry replica] + end + + with_them do + context "when #{params[:database_chosen]} is used" do + let(:labels_with_load_balancing) do + labels_with_job_status.merge(database_chosen: database_chosen, data_consistency: 'delayed') + end + + before do + job[:database_chosen] = database_chosen + job[:data_consistency] = 'delayed' + allow(load_balancing_metric).to receive(:increment) + end + + it 'increment sidekiq_load_balancing_count' do + expect(load_balancing_metric).to receive(:increment).with(labels_with_load_balancing, 1) + + described_class.new.call(worker, job, :test) { nil } + end + end + end + end + + context 'when :database_chosen is not provided' do + it 'does not increment sidekiq_load_balancing_count' do + expect(load_balancing_metric).not_to receive(:increment) + + described_class.new.call(worker, job, :test) { nil } + end + end + end + end + + context 'when load_balancing is disabled' do + include_context 'clear DB Load Balancing configuration' + + before do + allow(::Gitlab::Database::LoadBalancing).to receive(:enable?).and_return(false) + end + + describe '#initialize' do + it 'doesnt set load_balancing metrics' do + expect(Gitlab::Metrics).not_to receive(:counter).with(:sidekiq_load_balancing_count, anything) + + subject + end + end + end + end end # rubocop: enable RSpec/MultipleMemoizedHelpers diff --git a/spec/lib/gitlab/sidekiq_middleware/size_limiter/compressor_spec.rb b/spec/lib/gitlab/sidekiq_middleware/size_limiter/compressor_spec.rb new file mode 100644 index 00000000000..b9b58683459 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/size_limiter/compressor_spec.rb @@ -0,0 +1,200 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Compressor do + using RSpec::Parameterized::TableSyntax + + let(:base_payload) do + { + "class" => "ARandomWorker", + "queue" => "a_worker", + "retry" => true, + "jid" => "d774900367dc8b2962b2479c", + "created_at" => 1234567890, + "enqueued_at" => 1234567890 + } + end + + describe '.compressed?' do + where(:job, :result) do + {} | false + base_payload.merge("args" => [123, 'hello', ['world']]) | false + base_payload.merge("args" => ['eJzLSM3JyQcABiwCFQ=='], 'compressed' => true) | true + end + + with_them do + it 'returns whether the job payload is compressed' do + expect(described_class.compressed?(job)).to eql(result) + end + end + end + + describe '.compress' do + where(:args) do + [ + nil, + [], + ['hello'], + [ + { + "job_class" => "SomeWorker", + "job_id" => "b4a577edbccf1d805744efa9", + "provider_job_id" => nil, + "queue_name" => "default", + "arguments" => ["some", ["argument"]], + "executions" => 0, + "locale" => "en", + "attempt_number" => 1 + }, + nil, + 'hello', + 12345678901234567890, + ['nice'] + ], + [ + '2021-05-13_09:59:37.57483 [35mrails-background-jobs : [0m{"severity":"ERROR","time":"2021-05-13T09:59:37.574Z"', + 'bonne journée - ขอให้มีความสุขในวันนี้ - một ngày mới tốt lành - 좋은 하루 되세요 - ごきげんよう', + '🤝 - 🦊' + ] + ] + end + + with_them do + let(:payload) { base_payload.merge("args" => args) } + + it 'injects compressed data' do + serialized_args = Sidekiq.dump_json(args) + described_class.compress(payload, serialized_args) + + expect(payload['args'].length).to be(1) + expect(payload['args'].first).to be_a(String) + expect(payload['compressed']).to be(true) + expect(payload['original_job_size_bytes']).to eql(serialized_args.bytesize) + expect do + Sidekiq.dump_json(payload) + end.not_to raise_error + end + + it 'can decompress the payload' do + original_payload = payload.deep_dup + + described_class.compress(payload, Sidekiq.dump_json(args)) + described_class.decompress(payload) + + expect(payload).to eql(original_payload) + end + end + end + + describe '.decompress' do + context 'job payload is not compressed' do + let(:payload) { base_payload.merge("args" => ['hello']) } + + it 'preserves the payload after decompression' do + original_payload = payload.deep_dup + + described_class.decompress(payload) + + expect(payload).to eql(original_payload) + end + end + + context 'job payload is compressed with a default level' do + let(:payload) do + base_payload.merge( + 'args' => ['eF6LVspIzcnJV9JRKs8vyklRigUAMq0FqQ=='], + 'compressed' => true + ) + end + + it 'decompresses and clean up the job payload' do + described_class.decompress(payload) + + expect(payload['args']).to eql(%w[hello world]) + expect(payload).not_to have_key('compressed') + end + end + + context 'job payload is compressed with a different level' do + let(:payload) do + base_payload.merge( + 'args' => [Base64.strict_encode64(Zlib::Deflate.deflate(Sidekiq.dump_json(%w[hello world]), 9))], + 'compressed' => true + ) + end + + it 'decompresses and clean up the job payload' do + described_class.decompress(payload) + + expect(payload['args']).to eql(%w[hello world]) + expect(payload).not_to have_key('compressed') + end + end + + context 'job payload argument list is malformed' do + let(:payload) do + base_payload.merge( + 'args' => ['eNqLVspIzcnJV9JRKs8vyklRigUAMq0FqQ==', 'something else'], + 'compressed' => true + ) + end + + it 'tracks the conflicting exception' do + expect(::Gitlab::ErrorTracking).to receive(:track_and_raise_exception).with( + be_a(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor::PayloadDecompressionConflictError) + ) + + described_class.decompress(payload) + + expect(payload['args']).to eql(%w[hello world]) + expect(payload).not_to have_key('compressed') + end + end + + context 'job payload is not a valid base64 string' do + let(:payload) do + base_payload.merge( + 'args' => ['hello123'], + 'compressed' => true + ) + end + + it 'raises an exception' do + expect do + described_class.decompress(payload) + end.to raise_error(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor::PayloadDecompressionError) + end + end + + context 'job payload compression does not contain a valid Gzip header' do + let(:payload) do + base_payload.merge( + 'args' => ['aGVsbG8='], + 'compressed' => true + ) + end + + it 'raises an exception' do + expect do + described_class.decompress(payload) + end.to raise_error(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor::PayloadDecompressionError) + end + end + + context 'job payload compression does not contain a valid Gzip body' do + let(:payload) do + base_payload.merge( + 'args' => ["eNqLVspIzcnJVw=="], + 'compressed' => true + ) + end + + it 'raises an exception' do + expect do + described_class.decompress(payload) + end.to raise_error(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor::PayloadDecompressionError) + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_middleware/size_limiter/server_spec.rb b/spec/lib/gitlab/sidekiq_middleware/size_limiter/server_spec.rb new file mode 100644 index 00000000000..91b8ef97ab4 --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/size_limiter/server_spec.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require 'spec_helper' + +# rubocop: disable RSpec/MultipleMemoizedHelpers +RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Server, :clean_gitlab_redis_queues do + subject(:middleware) { described_class.new } + + let(:worker) { Class.new } + let(:job) do + { + "class" => "ARandomWorker", + "queue" => "a_worker", + "args" => %w[Hello World], + "created_at" => 1234567890, + "enqueued_at" => 1234567890 + } + end + + before do + allow(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor).to receive(:compress) + end + + it 'yields block' do + expect { |b| subject.call(worker, job, :test, &b) }.to yield_control.once + end + + it 'calls the Compressor' do + expect(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor).to receive(:decompress).with(job) + + subject.call(worker, job, :test) {} + end +end diff --git a/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb b/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb index 3140686c908..4fbe59c3c27 100644 --- a/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/size_limiter/validator_spec.rb @@ -3,6 +3,21 @@ require 'spec_helper' RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do + let(:base_payload) do + { + "class" => "ARandomWorker", + "queue" => "a_worker", + "retry" => true, + "jid" => "d774900367dc8b2962b2479c", + "created_at" => 1234567890, + "enqueued_at" => 1234567890 + } + end + + def job_payload(args = {}) + base_payload.merge('args' => args) + end + let(:worker_class) do Class.new do def self.name @@ -24,8 +39,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'does not log a warning message' do expect(::Sidekiq.logger).not_to receive(:warn) - described_class.new(TestSizeLimiterWorker, {}, mode: 'track') - described_class.new(TestSizeLimiterWorker, {}, mode: 'raise') + described_class.new(TestSizeLimiterWorker, job_payload, mode: 'track') + described_class.new(TestSizeLimiterWorker, job_payload, mode: 'compress') end end @@ -33,7 +48,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'defaults to track mode and logs a warning message' do expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter mode: invalid. Fallback to track mode.') - validator = described_class.new(TestSizeLimiterWorker, {}, mode: 'invalid') + validator = described_class.new(TestSizeLimiterWorker, job_payload, mode: 'invalid') expect(validator.mode).to eql('track') end @@ -43,7 +58,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'defaults to track mode' do expect(::Sidekiq.logger).not_to receive(:warn) - validator = described_class.new(TestSizeLimiterWorker, {}) + validator = described_class.new(TestSizeLimiterWorker, job_payload) expect(validator.mode).to eql('track') end @@ -53,8 +68,8 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'does not log a warning message' do expect(::Sidekiq.logger).not_to receive(:warn) - described_class.new(TestSizeLimiterWorker, {}, size_limit: 300) - described_class.new(TestSizeLimiterWorker, {}, size_limit: 0) + described_class.new(TestSizeLimiterWorker, job_payload, size_limit: 300) + described_class.new(TestSizeLimiterWorker, job_payload, size_limit: 0) end end @@ -62,7 +77,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'defaults to 0 and logs a warning message' do expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter limit: -1') - described_class.new(TestSizeLimiterWorker, {}, size_limit: -1) + described_class.new(TestSizeLimiterWorker, job_payload, size_limit: -1) end end @@ -70,15 +85,63 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'defaults to 0' do expect(::Sidekiq.logger).not_to receive(:warn) - validator = described_class.new(TestSizeLimiterWorker, {}) + validator = described_class.new(TestSizeLimiterWorker, job_payload) expect(validator.size_limit).to be(0) end end + + context 'when the compression threshold is valid' do + it 'does not log a warning message' do + expect(::Sidekiq.logger).not_to receive(:warn) + + described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 300) + described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 1) + end + end + + context 'when the compression threshold is negative' do + it 'logs a warning message' do + expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter compression threshold: -1') + + described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: -1) + end + + it 'falls back to the default' do + validator = described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: -1) + + expect(validator.compression_threshold).to be(100_000) + end + end + + context 'when the compression threshold is zero' do + it 'logs a warning message' do + expect(::Sidekiq.logger).to receive(:warn).with('Invalid Sidekiq size limiter compression threshold: 0') + + described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 0) + end + + it 'falls back to the default' do + validator = described_class.new(TestSizeLimiterWorker, job_payload, compression_threshold: 0) + + expect(validator.compression_threshold).to be(100_000) + end + end + + context 'when the compression threshold is empty' do + it 'defaults to 100_000' do + expect(::Sidekiq.logger).not_to receive(:warn) + + validator = described_class.new(TestSizeLimiterWorker, job_payload) + + expect(validator.compression_threshold).to be(100_000) + end + end end shared_examples 'validate limit job payload size' do context 'in track mode' do + let(:compression_threshold) { nil } let(:mode) { 'track' } context 'when size limit negative' do @@ -87,11 +150,11 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'does not track jobs' do expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) end it 'does not raise exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error + expect { validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) }.not_to raise_error end end @@ -101,11 +164,13 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'does not track jobs' do expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) end it 'does not raise exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error + expect do + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + end.not_to raise_error end end @@ -117,11 +182,13 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do be_a(Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError) ) - validate.call(TestSizeLimiterWorker, { a: 'a' * 100 }) + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 100)) end it 'does not raise an exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error + expect do + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + end.not_to raise_error end context 'when the worker has big_payload attribute' do @@ -132,13 +199,17 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'does not track jobs' do expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) - validate.call('TestSizeLimiterWorker', { a: 'a' * 300 }) + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + validate.call('TestSizeLimiterWorker', job_payload(a: 'a' * 300)) end it 'does not raise an exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error - expect { validate.call('TestSizeLimiterWorker', { a: 'a' * 300 }) }.not_to raise_error + expect do + validate.call(TestSizeLimiterWorker, job_payload(a: 'a' * 300)) + end.not_to raise_error + expect do + validate.call('TestSizeLimiterWorker', job_payload(a: 'a' * 300)) + end.not_to raise_error end end end @@ -149,63 +220,60 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do it 'does not track job' do expect(Gitlab::ErrorTracking).not_to receive(:track_exception) - validate.call(TestSizeLimiterWorker, { a: 'a' }) + validate.call(TestSizeLimiterWorker, job_payload(a: 'a')) end it 'does not raise an exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' }) }.not_to raise_error + expect { validate.call(TestSizeLimiterWorker, job_payload(a: 'a')) }.not_to raise_error end end end - context 'in raise mode' do - let(:mode) { 'raise' } - - context 'when size limit is negative' do - let(:size_limit) { -1 } - - it 'does not raise exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error - end - end + context 'in compress mode' do + let(:mode) { 'compress' } - context 'when size limit is 0' do - let(:size_limit) { 0 } + context 'when job size is less than compression threshold' do + let(:size_limit) { 50 } + let(:compression_threshold) { 30 } + let(:job) { job_payload(a: 'a' * 10) } - it 'does not raise exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error + it 'does not raise an exception' do + expect(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor).not_to receive(:compress) + expect { validate.call(TestSizeLimiterWorker, job_payload(a: 'a')) }.not_to raise_error end end - context 'when job size is bigger than size limit' do + context 'when job size is bigger than compression threshold and less than size limit after compressed' do let(:size_limit) { 50 } + let(:compression_threshold) { 30 } + let(:args) { { a: 'a' * 300 } } + let(:job) { job_payload(args) } - it 'raises an exception' do - expect do - validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) - end.to raise_error( - Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError, - /TestSizeLimiterWorker job exceeds payload size limit/i - ) - end - - context 'when the worker has big_payload attribute' do - before do - worker_class.big_payload! - end + it 'does not raise an exception' do + expect(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor).to receive(:compress).with( + job, Sidekiq.dump_json(args) + ).and_return('a' * 40) - it 'does not raise an exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' * 300 }) }.not_to raise_error - expect { validate.call('TestSizeLimiterWorker', { a: 'a' * 300 }) }.not_to raise_error - end + expect do + validate.call(TestSizeLimiterWorker, job) + end.not_to raise_error end end - context 'when job size is less than size limit' do + context 'when job size is bigger than compression threshold and bigger than size limit after compressed' do let(:size_limit) { 50 } + let(:compression_threshold) { 30 } + let(:args) { { a: 'a' * 3000 } } + let(:job) { job_payload(args) } it 'does not raise an exception' do - expect { validate.call(TestSizeLimiterWorker, { a: 'a' }) }.not_to raise_error + expect(::Gitlab::SidekiqMiddleware::SizeLimiter::Compressor).to receive(:compress).with( + job, Sidekiq.dump_json(args) + ).and_return('a' * 60) + + expect do + validate.call(TestSizeLimiterWorker, job) + end.to raise_error(Gitlab::SidekiqMiddleware::SizeLimiter::ExceedLimitError) end end end @@ -218,6 +286,7 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do before do stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_MODE', mode) stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_LIMIT_BYTES', size_limit) + stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_COMPRESSION_THRESHOLD_BYTES', compression_threshold) end it_behaves_like 'validate limit job payload size' @@ -226,14 +295,14 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do context 'when creating an instance with the related ENV variables' do let(:validate) do ->(worker_clas, job) do - validator = described_class.new(worker_class, job, mode: mode, size_limit: size_limit) - validator.validate! + described_class.new(worker_class, job).validate! end end before do stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_MODE', mode) stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_LIMIT_BYTES', size_limit) + stub_env('GITLAB_SIDEKIQ_SIZE_LIMITER_COMPRESSION_THRESHOLD_BYTES', compression_threshold) end it_behaves_like 'validate limit job payload size' @@ -242,7 +311,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::SizeLimiter::Validator do context 'when creating an instance with mode and size limit' do let(:validate) do ->(worker_clas, job) do - validator = described_class.new(worker_class, job, mode: mode, size_limit: size_limit) + validator = described_class.new( + worker_class, job, + mode: mode, size_limit: size_limit, compression_threshold: compression_threshold + ) validator.validate! end end |