Commit 39348adb authored by Stan Hu's avatar Stan Hu

Merge branch '56547-limit-sidekiq-logging-based-on-argument-size' into 'master'

Resolve "Limit sidekiq logging based on argument size"

Closes #56547

See merge request gitlab-org/gitlab-ce!24493
parents a5fb45ad 9d2be756
---
title: Prevent Sidekiq arguments over 10 KB in size from being logged to JSON
merge_request: 24493
author:
type: changed
...@@ -14,3 +14,8 @@ gitlab_rails['env'] = {"SIDEKIQ_LOG_ARGUMENTS" => "1"} ...@@ -14,3 +14,8 @@ gitlab_rails['env'] = {"SIDEKIQ_LOG_ARGUMENTS" => "1"}
Please note: It is not recommend to enable this setting in production because some Please note: It is not recommend to enable this setting in production because some
Sidekiq jobs (such as sending a password reset email) take secret arguments (for Sidekiq jobs (such as sending a password reset email) take secret arguments (for
example the password reset token). example the password reset token).
When using [Sidekiq JSON logging](../administration/logs.md#sidekiqlog),
arguments logs are limited to a maximum size of 10 kilobytes of text;
any arguments after this limit will be discarded and replaced with a
single argument containing the string `"..."`.
...@@ -5,6 +5,7 @@ module Gitlab ...@@ -5,6 +5,7 @@ module Gitlab
class StructuredLogger class StructuredLogger
START_TIMESTAMP_FIELDS = %w[created_at enqueued_at].freeze START_TIMESTAMP_FIELDS = %w[created_at enqueued_at].freeze
DONE_TIMESTAMP_FIELDS = %w[started_at retried_at failed_at completed_at].freeze DONE_TIMESTAMP_FIELDS = %w[started_at retried_at failed_at completed_at].freeze
MAXIMUM_JOB_ARGUMENTS_LENGTH = 10.kilobytes
def call(job, queue) def call(job, queue)
started_at = current_time started_at = current_time
...@@ -64,6 +65,7 @@ module Gitlab ...@@ -64,6 +65,7 @@ module Gitlab
job['pid'] = ::Process.pid job['pid'] = ::Process.pid
job.delete('args') unless ENV['SIDEKIQ_LOG_ARGUMENTS'] job.delete('args') unless ENV['SIDEKIQ_LOG_ARGUMENTS']
job['args'] = limited_job_args(job['args']) if job['args']
convert_to_iso8601(job, START_TIMESTAMP_FIELDS) convert_to_iso8601(job, START_TIMESTAMP_FIELDS)
...@@ -93,6 +95,21 @@ module Gitlab ...@@ -93,6 +95,21 @@ module Gitlab
Time.at(timestamp).utc.iso8601(3) Time.at(timestamp).utc.iso8601(3)
end end
def limited_job_args(args)
return unless args.is_a?(Array)
total_length = 0
limited_args = args.take_while do |arg|
total_length += arg.to_json.length
total_length <= MAXIMUM_JOB_ARGUMENTS_LENGTH
end
limited_args.push('...') if total_length > MAXIMUM_JOB_ARGUMENTS_LENGTH
limited_args
end
end end
end end
end end
...@@ -82,15 +82,36 @@ describe Gitlab::SidekiqLogging::StructuredLogger do ...@@ -82,15 +82,36 @@ describe Gitlab::SidekiqLogging::StructuredLogger do
end.to raise_error(ArgumentError) end.to raise_error(ArgumentError)
end end
end end
context 'when the job args are bigger than the maximum allowed' do
it 'keeps args from the front until they exceed the limit' do
Timecop.freeze(timestamp) do
job['args'] = [
1,
2,
'a' * (described_class::MAXIMUM_JOB_ARGUMENTS_LENGTH / 2),
'b' * (described_class::MAXIMUM_JOB_ARGUMENTS_LENGTH / 2),
3
]
expected_args = job['args'].take(3) + ['...']
expect(logger).to receive(:info).with(start_payload.merge('args' => expected_args)).ordered
expect(logger).to receive(:info).with(end_payload.merge('args' => expected_args)).ordered
expect(subject).to receive(:log_job_start).and_call_original
expect(subject).to receive(:log_job_done).and_call_original
subject.call(job, 'test_queue') { }
end
end
end
end end
context 'with SIDEKIQ_LOG_ARGUMENTS disabled' do context 'with SIDEKIQ_LOG_ARGUMENTS disabled' do
it 'logs start and end of job' do it 'logs start and end of job without args' do
Timecop.freeze(timestamp) do Timecop.freeze(timestamp) do
start_payload.delete('args') expect(logger).to receive(:info).with(start_payload.except('args')).ordered
expect(logger).to receive(:info).with(end_payload.except('args')).ordered
expect(logger).to receive(:info).with(start_payload).ordered
expect(logger).to receive(:info).with(end_payload).ordered
expect(subject).to receive(:log_job_start).and_call_original expect(subject).to receive(:log_job_start).and_call_original
expect(subject).to receive(:log_job_done).and_call_original expect(subject).to receive(:log_job_done).and_call_original
......
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