Commit 46fd463c authored by George Koltsov's avatar George Koltsov

Replace use of Rails.logger with Structured logger

- Rails.logger usage is not allowed and should be replaced
  with json structured logging. This changes replaces any use of
  Rails.logger
- Introduces new Gitlab::Export::Logger 'exporter.log' for
  export related logging, similar to Gitlab::Import::Logger
parent 8f56f4b6
...@@ -84,7 +84,11 @@ class ProjectImportState < ApplicationRecord ...@@ -84,7 +84,11 @@ class ProjectImportState < ApplicationRecord
update_column(:last_error, sanitized_message) update_column(:last_error, sanitized_message)
rescue ActiveRecord::ActiveRecordError => e rescue ActiveRecord::ActiveRecordError => e
Gitlab::AppLogger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}") Gitlab::Import::Logger.error(
message: 'Error setting import status to failed',
error: e.message,
original_error: sanitized_message
)
ensure ensure
@errors = original_errors @errors = original_errors
end end
......
...@@ -4,10 +4,11 @@ module Groups ...@@ -4,10 +4,11 @@ module Groups
module ImportExport module ImportExport
class ExportService class ExportService
def initialize(group:, user:, params: {}) def initialize(group:, user:, params: {})
@group = group @group = group
@current_user = user @current_user = user
@params = params @params = params
@shared = @params[:shared] || Gitlab::ImportExport::Shared.new(@group) @shared = @params[:shared] || Gitlab::ImportExport::Shared.new(@group)
@logger = Gitlab::Export::Logger.build
end end
def async_execute def async_execute
...@@ -91,21 +92,21 @@ module Groups ...@@ -91,21 +92,21 @@ module Groups
end end
def notify_success def notify_success
@shared.logger.info( @logger.info(
group_id: @group.id, message: 'Group Export succeeded',
group_name: @group.name, group_id: @group.id,
message: 'Group Import/Export: Export succeeded' group_name: @group.name
) )
notification_service.group_was_exported(@group, @current_user) notification_service.group_was_exported(@group, @current_user)
end end
def notify_error def notify_error
@shared.logger.error( @logger.error(
group_id: @group.id, message: 'Group Export failed',
group_id: @group.id,
group_name: @group.name, group_name: @group.name,
error: @shared.errors.join(', '), errors: @shared.errors.join(', ')
message: 'Group Import/Export: Export failed'
) )
notification_service.group_was_not_exported(@group, @current_user, @shared.errors) notification_service.group_was_not_exported(@group, @current_user, @shared.errors)
......
...@@ -9,6 +9,7 @@ module Groups ...@@ -9,6 +9,7 @@ module Groups
@group = group @group = group
@current_user = user @current_user = user
@shared = Gitlab::ImportExport::Shared.new(@group) @shared = Gitlab::ImportExport::Shared.new(@group)
@logger = Gitlab::Import::Logger.build
end end
def async_execute def async_execute
...@@ -81,7 +82,7 @@ module Groups ...@@ -81,7 +82,7 @@ module Groups
end end
def notify_success def notify_success
@shared.logger.info( @logger.info(
group_id: @group.id, group_id: @group.id,
group_name: @group.name, group_name: @group.name,
message: 'Group Import/Export: Import succeeded' message: 'Group Import/Export: Import succeeded'
...@@ -89,7 +90,7 @@ module Groups ...@@ -89,7 +90,7 @@ module Groups
end end
def notify_error def notify_error
@shared.logger.error( @logger.error(
group_id: @group.id, group_id: @group.id,
group_name: @group.name, group_name: @group.name,
message: "Group Import/Export: Errors occurred, see '#{Gitlab::ErrorTracking::Logger.file_name}' for details" message: "Group Import/Export: Errors occurred, see '#{Gitlab::ErrorTracking::Logger.file_name}' for details"
......
...@@ -22,8 +22,12 @@ module Projects ...@@ -22,8 +22,12 @@ module Projects
# causing GC to run every time. # causing GC to run every time.
service.increment! service.increment!
rescue Projects::HousekeepingService::LeaseTaken => e rescue Projects::HousekeepingService::LeaseTaken => e
Gitlab::AppLogger.info( Gitlab::Import::Logger.info(
"Could not perform housekeeping for project #{@project.full_path} (#{@project.id}): #{e}") message: 'Project housekeeping failed',
project_full_path: @project.full_path,
project_id: @project.id,
error: e.message
)
end end
private private
......
...@@ -9,6 +9,7 @@ module Projects ...@@ -9,6 +9,7 @@ module Projects
super super
@shared = project.import_export_shared @shared = project.import_export_shared
@logger = Gitlab::Export::Logger.build
end end
def execute(after_export_strategy = nil) def execute(after_export_strategy = nil)
...@@ -115,11 +116,20 @@ module Projects ...@@ -115,11 +116,20 @@ module Projects
end end
def notify_success def notify_success
Gitlab::AppLogger.info("Import/Export - Project #{project.name} with ID: #{project.id} successfully exported") @logger.info(
message: 'Project successfully exported',
project_name: project.name,
project_id: project.id
)
end end
def notify_error def notify_error
Gitlab::AppLogger.error("Import/Export - Project #{project.name} with ID: #{project.id} export error - #{shared.errors.join(', ')}") @logger.error(
message: 'Project export error',
export_errors: shared.errors.join(', '),
project_name: project.name,
project_id: project.id
)
notification_service.project_not_exported(project, current_user, shared.errors) notification_service.project_not_exported(project, current_user, shared.errors)
end end
......
...@@ -43,7 +43,12 @@ class RepositoryImportWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -43,7 +43,12 @@ class RepositoryImportWorker # rubocop:disable Scalability/IdempotentWorker
def start_import def start_import
return true if start(project.import_state) return true if start(project.import_state)
Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.") # rubocop:disable Gitlab/RailsLogger Gitlab::Import::Logger.info(
message: 'Project was in inconsistent state while importing',
project_full_path: project.full_path,
project_import_status: project.import_status
)
false false
end end
......
...@@ -45,7 +45,11 @@ class StuckImportJobsWorker # rubocop:disable Scalability/IdempotentWorker ...@@ -45,7 +45,11 @@ class StuckImportJobsWorker # rubocop:disable Scalability/IdempotentWorker
completed_import_states = enqueued_import_states_with_jid.where(id: completed_import_state_ids) completed_import_states = enqueued_import_states_with_jid.where(id: completed_import_state_ids)
completed_import_state_jids = completed_import_states.map { |import_state| import_state.jid }.join(', ') completed_import_state_jids = completed_import_states.map { |import_state| import_state.jid }.join(', ')
Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_import_state_jids}") # rubocop:disable Gitlab/RailsLogger
Gitlab::Import::Logger.info(
message: 'Marked stuck import jobs as failed',
job_ids: completed_import_state_jids
)
completed_import_states.each do |import_state| completed_import_states.each do |import_state|
import_state.mark_as_failed(error_message) import_state.mark_as_failed(error_message)
......
...@@ -597,6 +597,16 @@ installations from source. ...@@ -597,6 +597,16 @@ installations from source.
It logs the progress of the import process. It logs the progress of the import process.
## `exporter.log`
> Introduced in GitLab 13.1.
This file lives in `/var/log/gitlab/gitlab-rails/exporter.log` for
Omnibus GitLab packages or in `/home/git/gitlab/log/exporter.log` for
installations from source.
It logs the progress of the export process.
## `auth.log` ## `auth.log`
> Introduced in GitLab 12.0. > Introduced in GitLab 12.0.
......
# frozen_string_literal: true
module Gitlab
module Export
class Logger < ::Gitlab::JsonLogger
def self.file_name_noext
'exporter'
end
end
end
end
...@@ -29,7 +29,10 @@ module Gitlab ...@@ -29,7 +29,10 @@ module Gitlab
yield object yield object
end end
rescue StandardError => e rescue StandardError => e
Rails.logger.error("The Lfs import process failed. #{e.message}") # rubocop:disable Gitlab/RailsLogger Gitlab::Import::Logger.error(
message: 'The Lfs import process failed',
error: e.message
)
end end
end end
end end
......
...@@ -40,8 +40,10 @@ module Gitlab ...@@ -40,8 +40,10 @@ module Gitlab
pname = project.path_with_namespace pname = project.path_with_namespace
Rails.logger # rubocop:disable Gitlab/RailsLogger Gitlab::Import::Logger.info(
.info("GitHub importer finished updating repository for #{pname}") message: 'GitHub importer finished updating repository',
project_name: pname
)
repository_updates_counter.increment repository_updates_counter.increment
end end
......
...@@ -41,7 +41,13 @@ module Gitlab ...@@ -41,7 +41,13 @@ module Gitlab
def create_source_branch def create_source_branch
@project.repository.create_branch(@merge_request.source_branch, @diff_head_sha) @project.repository.create_branch(@merge_request.source_branch, @diff_head_sha)
rescue => err rescue => err
Rails.logger.warn("Import/Export warning: Failed to create source branch #{@merge_request.source_branch} => #{@diff_head_sha} for MR #{@merge_request.iid}: #{err}") # rubocop:disable Gitlab/RailsLogger Gitlab::Import::Logger.warn(
message: 'Import warning: Failed to create source branch',
source_branch: @merge_request.source_branch,
diff_head_sha: @diff_head_sha,
merge_request_iid: @merge_request.iid,
error: err.message
)
end end
def create_target_branch def create_target_branch
......
...@@ -11,14 +11,18 @@ module Gitlab ...@@ -11,14 +11,18 @@ module Gitlab
def initialize(exportable:, shared:) def initialize(exportable:, shared:)
@exportable = exportable @exportable = exportable
@shared = shared @shared = shared
end end
def save def save
if compress_and_save if compress_and_save
remove_export_path remove_export_path
Rails.logger.info("Saved #{@exportable.class} export #{archive_file}") # rubocop:disable Gitlab/RailsLogger Gitlab::Export::Logger.info(
message: 'Export archive saved',
exportable_class: @exportable.class.to_s,
archive_file: archive_file
)
save_upload save_upload
else else
......
...@@ -36,7 +36,11 @@ module Gitlab ...@@ -36,7 +36,11 @@ module Gitlab
def different_version?(version) def different_version?(version)
Gem::Version.new(version) != Gem::Version.new(Gitlab::ImportExport.version) Gem::Version.new(version) != Gem::Version.new(Gitlab::ImportExport.version)
rescue => e rescue => e
Rails.logger.error("Import/Export error: #{e.message}") # rubocop:disable Gitlab/RailsLogger Gitlab::Import::Logger.error(
message: 'Import error',
error: e.message
)
raise Gitlab::ImportExport::Error.new('Incorrect VERSION format') raise Gitlab::ImportExport::Error.new('Incorrect VERSION format')
end end
end end
......
...@@ -154,9 +154,11 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do ...@@ -154,9 +154,11 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
.to receive(:fetch_remote) .to receive(:fetch_remote)
.with('github', forced: false) .with('github', forced: false)
expect(Rails.logger) expect_next_instance_of(Gitlab::Import::Logger) do |logger|
.to receive(:info) expect(logger)
.with(an_instance_of(String)) .to receive(:info)
.with(an_instance_of(Hash))
end
expect(importer.repository_updates_counter) expect(importer.repository_updates_counter)
.to receive(:increment) .to receive(:increment)
......
...@@ -62,11 +62,16 @@ describe ProjectImportState, type: :model do ...@@ -62,11 +62,16 @@ describe ProjectImportState, type: :model do
it 'logs error when update column fails' do it 'logs error when update column fails' do
allow(import_state).to receive(:update_column).and_raise(ActiveRecord::ActiveRecordError) allow(import_state).to receive(:update_column).and_raise(ActiveRecord::ActiveRecordError)
allow(Gitlab::AppLogger).to receive(:error)
import_state.mark_as_failed(error_message) expect_next_instance_of(Gitlab::Import::Logger) do |logger|
expect(logger).to receive(:error).with(
error: 'ActiveRecord::ActiveRecordError',
message: 'Error setting import status to failed',
original_error: error_message
)
end
expect(Gitlab::AppLogger).to have_received(:error) import_state.mark_as_failed(error_message)
end end
it 'updates last_error with error message' do it 'updates last_error with error message' do
......
...@@ -103,12 +103,14 @@ describe Groups::ImportExport::ExportService do ...@@ -103,12 +103,14 @@ describe Groups::ImportExport::ExportService do
end end
it 'logs the error' do it 'logs the error' do
expect(shared.logger).to receive(:error).with( expect_next_instance_of(Gitlab::Export::Logger) do |logger|
group_id: group.id, expect(logger).to receive(:error).with(
group_name: group.name, group_id: group.id,
error: expected_message, group_name: group.name,
message: 'Group Import/Export: Export failed' errors: expected_message,
) message: 'Group Export failed'
)
end
expect { service.execute }.to raise_error(Gitlab::ImportExport::Error) expect { service.execute }.to raise_error(Gitlab::ImportExport::Error)
end end
...@@ -162,7 +164,8 @@ describe Groups::ImportExport::ExportService do ...@@ -162,7 +164,8 @@ describe Groups::ImportExport::ExportService do
it 'notifies logger' do it 'notifies logger' do
allow(service).to receive_message_chain(:tree_exporter, :save).and_return(false) allow(service).to receive_message_chain(:tree_exporter, :save).and_return(false)
expect(shared.logger).to receive(:error)
expect(service.instance_variable_get(:@logger)).to receive(:error)
expect { service.execute }.to raise_error(Gitlab::ImportExport::Error) expect { service.execute }.to raise_error(Gitlab::ImportExport::Error)
end end
......
...@@ -119,9 +119,7 @@ describe Projects::ImportExport::ExportService do ...@@ -119,9 +119,7 @@ describe Projects::ImportExport::ExportService do
end end
it 'notifies logger' do it 'notifies logger' do
allow(Gitlab::AppLogger).to receive(:error) expect(service.instance_variable_get(:@logger)).to receive(:error)
expect(Gitlab::AppLogger).to receive(:error)
end end
end end
end end
...@@ -149,7 +147,7 @@ describe Projects::ImportExport::ExportService do ...@@ -149,7 +147,7 @@ describe Projects::ImportExport::ExportService do
end end
it 'notifies logger' do it 'notifies logger' do
expect(Gitlab::AppLogger).to receive(:error) expect(service.instance_variable_get(:@logger)).to receive(:error)
end end
it 'does not call the export strategy' do it 'does not call the export strategy' 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