Merge branch 'georgekoltsov/remove-use-of-rails-logger-import-export-sidekiq' into 'master'

Replace use of Rails.logger with json logger for Import/Export

See merge request gitlab-org/gitlab!32543

app/models/project_import_state.rb

@@ -84,7 +84,11 @@ class ProjectImportState < ApplicationRecord
 
     update_column(:last_error, sanitized_message)
   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
     @errors = original_errors
   end

app/services/groups/import_export/export_service.rb

@@ -8,6 +8,7 @@ module Groups
         @current_user = user
         @params = params
         @shared = @params[:shared] || Gitlab::ImportExport::Shared.new(@group)
+        @logger = Gitlab::Export::Logger.build
       end
 
       def async_execute
@@ -91,21 +92,21 @@ module Groups
       end
 
       def notify_success
-        @shared.logger.info(
+        @logger.info(
+          message: 'Group Export succeeded',
           group_id: @group.id,
-          group_name: @group.name,
-          message:    'Group Import/Export: Export succeeded'
+          group_name: @group.name
         )
 
         notification_service.group_was_exported(@group, @current_user)
       end
 
       def notify_error
-        @shared.logger.error(
+        @logger.error(
+          message: 'Group Export failed',
           group_id: @group.id,
           group_name: @group.name,
-          error:      @shared.errors.join(', '),
-          message:    'Group Import/Export: Export failed'
+          errors: @shared.errors.join(', ')
         )
 
         notification_service.group_was_not_exported(@group, @current_user, @shared.errors)

app/services/groups/import_export/import_service.rb

@@ -9,6 +9,7 @@ module Groups
         @group = group
         @current_user = user
         @shared = Gitlab::ImportExport::Shared.new(@group)
+        @logger = Gitlab::Import::Logger.build
       end
 
       def async_execute
@@ -81,7 +82,7 @@ module Groups
       end
 
       def notify_success
-        @shared.logger.info(
+        @logger.info(
           group_id:   @group.id,
           group_name: @group.name,
           message:    'Group Import/Export: Import succeeded'
@@ -89,7 +90,7 @@ module Groups
       end
 
       def notify_error
-        @shared.logger.error(
+        @logger.error(
           group_id:   @group.id,
           group_name: @group.name,
           message:    "Group Import/Export: Errors occurred, see '#{Gitlab::ErrorTracking::Logger.file_name}' for details"

app/services/projects/after_import_service.rb

@@ -22,8 +22,12 @@ module Projects
       # causing GC to run every time.
       service.increment!
     rescue Projects::HousekeepingService::LeaseTaken => e
-      Gitlab::AppLogger.info(
-        "Could not perform housekeeping for project #{@project.full_path} (#{@project.id}): #{e}")
+      Gitlab::Import::Logger.info(
+        message: 'Project housekeeping failed',
+        project_full_path: @project.full_path,
+        project_id: @project.id,
+        error: e.message
+      )
     end
 
     private

app/services/projects/import_export/export_service.rb

@@ -9,6 +9,7 @@ module Projects
         super
 
         @shared = project.import_export_shared
+        @logger = Gitlab::Export::Logger.build
       end
 
       def execute(after_export_strategy = nil)
@@ -115,11 +116,20 @@ module Projects
       end
 
       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
 
       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)
       end

app/workers/repository_import_worker.rb

@@ -43,7 +43,12 @@ class RepositoryImportWorker # rubocop:disable Scalability/IdempotentWorker
   def start_import
     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
   end
 

app/workers/stuck_import_jobs_worker.rb

@@ -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_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|
       import_state.mark_as_failed(error_message)

doc/administration/logs.md

@@ -597,6 +597,16 @@ installations from source.
 
 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`
 
 > Introduced in GitLab 12.0.

lib/gitlab/export/logger.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Export
+    class Logger < ::Gitlab::JsonLogger
+      def self.file_name_noext
+        'exporter'
+      end
+    end
+  end
+end

lib/gitlab/github_import/importer/lfs_objects_importer.rb

@@ -29,7 +29,10 @@ module Gitlab
             yield object
           end
         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

lib/gitlab/github_import/importer/pull_requests_importer.rb

@@ -40,8 +40,10 @@ module Gitlab
 
           pname = project.path_with_namespace
 
-          Rails.logger # rubocop:disable Gitlab/RailsLogger
-            .info("GitHub importer finished updating repository for #{pname}")
+          Gitlab::Import::Logger.info(
+            message: 'GitHub importer finished updating repository',
+            project_name: pname
+          )
 
           repository_updates_counter.increment
         end

lib/gitlab/import_export/merge_request_parser.rb

@@ -41,7 +41,13 @@ module Gitlab
       def create_source_branch
         @project.repository.create_branch(@merge_request.source_branch, @diff_head_sha)
       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
 
       def create_target_branch

lib/gitlab/import_export/saver.rb

@@ -18,7 +18,11 @@ module Gitlab
         if compress_and_save
           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
         else

lib/gitlab/import_export/version_checker.rb

@@ -36,7 +36,11 @@ module Gitlab
       def different_version?(version)
         Gem::Version.new(version) != Gem::Version.new(Gitlab::ImportExport.version)
       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')
       end
     end

spec/lib/gitlab/github_import/importer/pull_requests_importer_spec.rb

@@ -154,9 +154,11 @@ describe Gitlab::GithubImport::Importer::PullRequestsImporter do
         .to receive(:fetch_remote)
         .with('github', forced: false)
 
-      expect(Rails.logger)
+      expect_next_instance_of(Gitlab::Import::Logger) do |logger|
+        expect(logger)
           .to receive(:info)
-        .with(an_instance_of(String))
+          .with(an_instance_of(Hash))
+      end
 
       expect(importer.repository_updates_counter)
         .to receive(:increment)

spec/models/project_import_state_spec.rb

@@ -62,11 +62,16 @@ describe ProjectImportState, type: :model do
 
     it 'logs error when update column fails' do
       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
 
     it 'updates last_error with error message' do

spec/services/groups/import_export/export_service_spec.rb

@@ -103,12 +103,14 @@ describe Groups::ImportExport::ExportService do
       end
 
       it 'logs the error' do
-        expect(shared.logger).to receive(:error).with(
+        expect_next_instance_of(Gitlab::Export::Logger) do |logger|
+          expect(logger).to receive(:error).with(
             group_id: group.id,
             group_name: group.name,
-          error:      expected_message,
-          message:    'Group Import/Export: Export failed'
+            errors: expected_message,
+            message: 'Group Export failed'
           )
+        end
 
         expect { service.execute }.to raise_error(Gitlab::ImportExport::Error)
       end
@@ -162,7 +164,8 @@ describe Groups::ImportExport::ExportService do
 
         it 'notifies logger' do
           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)
         end

spec/services/projects/import_export/export_service_spec.rb

@@ -119,9 +119,7 @@ describe Projects::ImportExport::ExportService do
         end
 
         it 'notifies logger' do
-          allow(Gitlab::AppLogger).to receive(:error)
-
-          expect(Gitlab::AppLogger).to receive(:error)
+          expect(service.instance_variable_get(:@logger)).to receive(:error)
         end
       end
     end
@@ -149,7 +147,7 @@ describe Projects::ImportExport::ExportService do
       end
 
       it 'notifies logger' do
-        expect(Gitlab::AppLogger).to receive(:error)
+        expect(service.instance_variable_get(:@logger)).to receive(:error)
       end
 
       it 'does not call the export strategy' do