Commit 1ad684ad authored by Patrick Bair's avatar Patrick Bair

Merge branch '357558-improve-logs' into 'master'

Resolve "Improve batched background migration logs"

See merge request gitlab-org/gitlab!84195
parents 4139f377 c2ec725b
...@@ -63,7 +63,13 @@ module Gitlab ...@@ -63,7 +63,13 @@ module Gitlab
job.split_and_retry! if job.can_split?(exception) job.split_and_retry! if job.can_split?(exception)
rescue SplitAndRetryError => error rescue SplitAndRetryError => error
Gitlab::AppLogger.error(message: error.message, batched_job_id: job.id) Gitlab::AppLogger.error(
message: error.message,
batched_job_id: job.id,
batched_migration_id: job.batched_migration.id,
job_class_name: job.migration_job_class_name,
job_arguments: job.migration_job_arguments
)
end end
after_transition do |job, transition| after_transition do |job, transition|
...@@ -73,13 +79,23 @@ module Gitlab ...@@ -73,13 +79,23 @@ module Gitlab
job.batched_job_transition_logs.create(previous_status: transition.from, next_status: transition.to, exception_class: exception&.class, exception_message: exception&.message) job.batched_job_transition_logs.create(previous_status: transition.from, next_status: transition.to, exception_class: exception&.class, exception_message: exception&.message)
Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id) if exception Gitlab::ErrorTracking.track_exception(exception, batched_job_id: job.id, job_class_name: job.migration_job_class_name, job_arguments: job.migration_job_arguments) if exception
Gitlab::AppLogger.info(message: 'BatchedJob transition', batched_job_id: job.id, previous_state: transition.from_name, new_state: transition.to_name) Gitlab::AppLogger.info(
message: 'BatchedJob transition',
batched_job_id: job.id,
previous_state: transition.from_name,
new_state: transition.to_name,
batched_migration_id: job.batched_migration.id,
job_class_name: job.migration_job_class_name,
job_arguments: job.migration_job_arguments,
exception_class: exception&.class,
exception_message: exception&.message
)
end end
end end
delegate :job_class, :table_name, :column_name, :job_arguments, delegate :job_class, :table_name, :column_name, :job_arguments, :job_class_name,
to: :batched_migration, prefix: :migration to: :batched_migration, prefix: :migration
attribute :pause_ms, :integer, default: 100 attribute :pause_ms, :integer, default: 100
......
...@@ -21,7 +21,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -21,7 +21,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
context 'when a job is running' do context 'when a job is running' do
it 'logs the transition' do it 'logs the transition' do
expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :running, previous_state: :failed } ) expect(Gitlab::AppLogger).to receive(:info).with(
{
batched_job_id: job.id,
batched_migration_id: job.batched_background_migration_id,
exception_class: nil,
exception_message: nil,
job_arguments: job.batched_migration.job_arguments,
job_class_name: job.batched_migration.job_class_name,
message: 'BatchedJob transition',
new_state: :running,
previous_state: :failed
}
)
expect { job.run! }.to change(job, :started_at) expect { job.run! }.to change(job, :started_at)
end end
...@@ -31,7 +43,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -31,7 +43,19 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
let(:job) { create(:batched_background_migration_job, :running) } let(:job) { create(:batched_background_migration_job, :running) }
it 'logs the transition' do it 'logs the transition' do
expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :succeeded, previous_state: :running } ) expect(Gitlab::AppLogger).to receive(:info).with(
{
batched_job_id: job.id,
batched_migration_id: job.batched_background_migration_id,
exception_class: nil,
exception_message: nil,
job_arguments: job.batched_migration.job_arguments,
job_class_name: job.batched_migration.job_class_name,
message: 'BatchedJob transition',
new_state: :succeeded,
previous_state: :running
}
)
job.succeed! job.succeed!
end end
...@@ -89,7 +113,15 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -89,7 +113,15 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
end end
it 'logs the error' do it 'logs the error' do
expect(Gitlab::AppLogger).to receive(:error).with( { message: error_message, batched_job_id: job.id } ) expect(Gitlab::AppLogger).to receive(:error).with(
{
batched_job_id: job.id,
batched_migration_id: job.batched_background_migration_id,
job_arguments: job.batched_migration.job_arguments,
job_class_name: job.batched_migration.job_class_name,
message: error_message
}
)
job.failure!(error: exception) job.failure!(error: exception)
end end
...@@ -100,13 +132,32 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -100,13 +132,32 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
let(:job) { create(:batched_background_migration_job, :running) } let(:job) { create(:batched_background_migration_job, :running) }
it 'logs the transition' do it 'logs the transition' do
expect(Gitlab::AppLogger).to receive(:info).with( { batched_job_id: job.id, message: 'BatchedJob transition', new_state: :failed, previous_state: :running } ) expect(Gitlab::AppLogger).to receive(:info).with(
{
batched_job_id: job.id,
batched_migration_id: job.batched_background_migration_id,
exception_class: RuntimeError,
exception_message: 'error',
job_arguments: job.batched_migration.job_arguments,
job_class_name: job.batched_migration.job_class_name,
message: 'BatchedJob transition',
new_state: :failed,
previous_state: :running
}
)
job.failure! job.failure!(error: RuntimeError.new('error'))
end end
it 'tracks the exception' do it 'tracks the exception' do
expect(Gitlab::ErrorTracking).to receive(:track_exception).with(RuntimeError, { batched_job_id: job.id } ) expect(Gitlab::ErrorTracking).to receive(:track_exception).with(
RuntimeError,
{
batched_job_id: job.id,
job_arguments: job.batched_migration.job_arguments,
job_class_name: job.batched_migration.job_class_name
}
)
job.failure!(error: RuntimeError.new) job.failure!(error: RuntimeError.new)
end end
...@@ -198,6 +249,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d ...@@ -198,6 +249,12 @@ RSpec.describe Gitlab::Database::BackgroundMigration::BatchedJob, type: :model d
expect(batched_job.migration_job_arguments).to eq(batched_migration.job_arguments) expect(batched_job.migration_job_arguments).to eq(batched_migration.job_arguments)
end end
end end
describe '#migration_job_class_name' do
it 'returns the migration job_class_name' do
expect(batched_job.migration_job_class_name).to eq(batched_migration.job_class_name)
end
end
end end
describe '#can_split?' do describe '#can_split?' 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