Commit ef67613d authored by Sean McGivern's avatar Sean McGivern

Make retry an integer in Sidekiq logs

https://github.com/mperham/sidekiq/wiki/Error-Handling#configuration
says that `retry: false` is not exactly equivalent to `retry: 0` - it's
more like `retry: 0, dead: false`.

For logging, those cases are basically the same, though, and making this
always an integer prevents mapping errors.
parent 1edb3de0
...@@ -19,6 +19,7 @@ module Gitlab ...@@ -19,6 +19,7 @@ module Gitlab
output[:message] = data output[:message] = data
when Hash when Hash
convert_to_iso8601!(data) convert_to_iso8601!(data)
convert_retry_to_integer!(data)
stringify_args!(data) stringify_args!(data)
output.merge!(data) output.merge!(data)
end end
...@@ -41,6 +42,20 @@ module Gitlab ...@@ -41,6 +42,20 @@ module Gitlab
Time.at(timestamp).utc.iso8601(3) Time.at(timestamp).utc.iso8601(3)
end end
def convert_retry_to_integer!(payload)
payload['retry'] =
case payload['retry']
when Integer
payload['retry']
when false, nil
0
when true
Sidekiq::JobRetry::DEFAULT_MAX_RETRY_ATTEMPTS
else
-1
end
end
def stringify_args!(payload) def stringify_args!(payload)
payload['args'] = Gitlab::Utils::LogLimitedArray.log_limited_array(payload['args'].map(&:to_s)) if payload['args'] payload['args'] = Gitlab::Utils::LogLimitedArray.log_limited_array(payload['args'].map(&:to_s)) if payload['args']
end end
......
...@@ -34,7 +34,8 @@ describe Gitlab::SidekiqLogging::JSONFormatter do ...@@ -34,7 +34,8 @@ describe Gitlab::SidekiqLogging::JSONFormatter do
'started_at' => timestamp_iso8601, 'started_at' => timestamp_iso8601,
'retried_at' => timestamp_iso8601, 'retried_at' => timestamp_iso8601,
'failed_at' => timestamp_iso8601, 'failed_at' => timestamp_iso8601,
'completed_at' => timestamp_iso8601 'completed_at' => timestamp_iso8601,
'retry' => 0
} }
) )
...@@ -57,6 +58,26 @@ describe Gitlab::SidekiqLogging::JSONFormatter do ...@@ -57,6 +58,26 @@ describe Gitlab::SidekiqLogging::JSONFormatter do
expect(subject['args']).to eq(["1", "test", "2", %({"test"=>1})]) expect(subject['args']).to eq(["1", "test", "2", %({"test"=>1})])
end end
context 'when the job has a non-integer value for retry' do
using RSpec::Parameterized::TableSyntax
where(:retry_in_job, :retry_in_logs) do
3 | 3
true | 25
false | 0
nil | 0
'string' | -1
end
with_them do
it 'logs as the correct integer' do
hash_input['retry'] = retry_in_job
expect(subject['retry']).to eq(retry_in_logs)
end
end
end
end end
describe 'with a String' do describe 'with a String' do
......
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