Add a rubocop for Rails.logger

Suggests to use a JSON structured log instead

Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/54102

app/models/chat_team.rb

@@ -12,6 +12,6 @@ class ChatTeam < ApplicationRecord
     # Either the group is not found, or the user doesn't have the proper
     # access on the mattermost instance. In the first case, we're done either way
     # in the latter case, we can't recover by retrying, so we just log what happened
-    Rails.logger.error("Mattermost team deletion failed: #{e}")
+    Rails.logger.error("Mattermost team deletion failed: #{e}") # rubocop:disable Gitlab/RailsLogger
   end
 end

app/models/ci/build.rb

@@ -266,7 +266,7 @@ module Ci
           begin
             Ci::Build.retry(build, build.user)
           rescue Gitlab::Access::AccessDeniedError => ex
-            Rails.logger.error "Unable to auto-retry job #{build.id}: #{ex}"
+            Rails.logger.error "Unable to auto-retry job #{build.id}: #{ex}" # rubocop:disable Gitlab/RailsLogger
           end
         end
       end

app/models/concerns/cacheable_attributes.rb

@@ -49,7 +49,7 @@ module CacheableAttributes
       current_without_cache.tap { |current_record| current_record&.cache! }
     rescue => e
       if Rails.env.production?
-        Rails.logger.warn("Cached record for #{name} couldn't be loaded, falling back to uncached record: #{e}")
+        Rails.logger.warn("Cached record for #{name} couldn't be loaded, falling back to uncached record: #{e}") # rubocop:disable Gitlab/RailsLogger
       else
         raise e
       end

app/models/concerns/storage/legacy_namespace.rb

@@ -64,7 +64,7 @@ module Storage
         unless gitlab_shell.mv_namespace(repository_storage, full_path_before_last_save, full_path)
-          Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_before_last_save} to #{full_path}"
+          Rails.logger.error "Exception moving path #{repository_storage} from #{full_path_before_last_save} to #{full_path}" # rubocop:disable Gitlab/RailsLogger
           # if we cannot move namespace directory we should rollback
           # db changes in order to prevent out of sync between db and fs

app/models/project.rb

@@ -784,6 +784,7 @@ class Project < ApplicationRecord
     job_id
   end
+  # rubocop:disable Gitlab/RailsLogger
   def log_import_activity(job_id, type: :import)
     job_type = type.to_s.capitalize
@@ -793,6 +794,7 @@ class Project < ApplicationRecord
       Rails.logger.error("#{job_type} job failed to create for #{full_path}.")
     end
   end
+  # rubocop:enable Gitlab/RailsLogger
   def reset_cache_and_import_attrs
     run_after_commit do
@@ -1665,6 +1667,7 @@ class Project < ApplicationRecord
   end
   # rubocop: enable CodeReuse/ServiceClass
+  # rubocop:disable Gitlab/RailsLogger
   def write_repository_config(gl_full_path: full_path)
     # We'd need to keep track of project full path otherwise directory tree
     # created with hashed storage enabled cannot be usefully imported using
@@ -1674,6 +1677,7 @@ class Project < ApplicationRecord
     Rails.logger.error("Error writing to .git/config for project #{full_path} (#{id}): #{e.message}.")
     nil
   end
+  # rubocop:enable Gitlab/RailsLogger
   def after_import
     repository.after_import
@@ -1715,6 +1719,7 @@ class Project < ApplicationRecord
     @pipeline_status ||= Gitlab::Cache::Ci::ProjectPipelineStatus.load_for_project(self)
   end
+  # rubocop:disable Gitlab/RailsLogger
   def add_export_job(current_user:, after_export_strategy: nil, params: {})
     job_id = ProjectExportWorker.perform_async(current_user.id, self.id, after_export_strategy, params)
@@ -1724,6 +1729,7 @@ class Project < ApplicationRecord
       Rails.logger.error "Export job failed to start for project ID #{self.id}"
     end
   end
+  # rubocop:enable Gitlab/RailsLogger
   def import_export_shared
     @import_export_shared ||= Gitlab::ImportExport::Shared.new(self)

app/models/project_import_state.rb

@@ -65,7 +65,7 @@ class ProjectImportState < ApplicationRecord
     update_column(:last_error, sanitized_message)
   rescue ActiveRecord::ActiveRecordError => e
-    Rails.logger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}")
+    Rails.logger.error("Error setting import status to failed: #{e.message}. Original error: #{sanitized_message}") # rubocop:disable Gitlab/RailsLogger
   ensure
     @errors = original_errors
   end

app/models/repository.rb

@@ -273,7 +273,7 @@ class Repository
       # This will still fail if the file is corrupted (e.g. 0 bytes)
       raw_repository.write_ref(keep_around_ref_name(sha), sha)
     rescue Gitlab::Git::CommandError => ex
-      Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}"
+      Rails.logger.error "Unable to create keep-around reference for repository #{disk_path}: #{ex}" # rubocop:disable Gitlab/RailsLogger
     end
   end
@@ -934,6 +934,7 @@ class Repository
     async_remove_remote(remote_name) if tmp_remote_name
   end
+  # rubocop:disable Gitlab/RailsLogger
   def async_remove_remote(remote_name)
     return unless remote_name
@@ -947,6 +948,7 @@ class Repository
     job_id
   end
+  # rubocop:enable Gitlab/RailsLogger
   def fetch_source_branch!(source_repository, source_branch, local_ref)
     raw_repository.fetch_source_branch!(source_repository.raw_repository, source_branch, local_ref)

app/models/ssh_host_key.rb

@@ -106,7 +106,7 @@ class SshHostKey
     if status.success? && !errors.present?
       { known_hosts: known_hosts }
     else
-      Rails.logger.debug("Failed to detect SSH host keys for #{id}: #{errors}")
+      Rails.logger.debug("Failed to detect SSH host keys for #{id}: #{errors}") # rubocop:disable Gitlab/RailsLogger
       { error: 'Failed to detect SSH host keys' }
     end

app/models/storage/legacy_project.rb

@@ -41,7 +41,7 @@ module Storage
           gitlab_shell.mv_repository(repository_storage, "#{old_full_path}.wiki", "#{new_full_path}.wiki")
           return true
         rescue => e
-          Rails.logger.error "Exception renaming #{old_full_path} -> #{new_full_path}: #{e}"
+          Rails.logger.error "Exception renaming #{old_full_path} -> #{new_full_path}: #{e}" # rubocop:disable Gitlab/RailsLogger
           # Returning false does not rollback after_* transaction but gives
           # us information about failing some of tasks
           return false

app/models/uploads/base.rb

@@ -7,7 +7,7 @@ module Uploads
     attr_reader :logger
     def initialize(logger: nil)
-      @logger ||= Rails.logger
+      @logger ||= Rails.logger # rubocop:disable Gitlab/RailsLogger
     end
     def delete_keys_async(keys_to_delete)

app/services/akismet_service.rb

@@ -25,7 +25,7 @@ class AkismetService
       is_spam, is_blatant = akismet_client.check(options[:ip_address], options[:user_agent], params)
       is_spam || is_blatant
     rescue => e
-      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check")
+      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check") # rubocop:disable Gitlab/RailsLogger
       false
     end
   end
@@ -63,7 +63,7 @@ class AkismetService
       akismet_client.public_send(type, options[:ip_address], options[:user_agent], params) # rubocop:disable GitlabSecurity/PublicSend
       true
     rescue => e
-      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!")
+      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") # rubocop:disable Gitlab/RailsLogger
       false
     end
   end

app/services/ci/archive_trace_service.rb

@@ -24,11 +24,11 @@ module Ci
     def archive_error(error, job)
       failed_archive_counter.increment
-      Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}"
+      Rails.logger.error "Failed to archive trace. id: #{job.id} message: #{error.message}" # rubocop:disable Gitlab/RailsLogger
       Gitlab::Sentry
         .track_exception(error,
-                         issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
+                          issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/51502',
                          extra: { job_id: job.id })
     end
   end

app/services/concerns/exclusive_lease_guard.rb

@@ -58,6 +58,6 @@ module ExclusiveLeaseGuard
   end
   def log_error(message, extra_args = {})
-    Rails.logger.error(message)
+    Rails.logger.error(message) # rubocop:disable Gitlab/RailsLogger
   end
 end

app/services/groups/destroy_service.rb

@@ -6,7 +6,7 @@ module Groups
     def async_execute
       job_id = GroupDestroyWorker.perform_async(group.id, current_user.id)
-      Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}")
+      Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") # rubocop:disable Gitlab/RailsLogger
     end
     # rubocop: disable CodeReuse/ActiveRecord

app/services/labels/create_service.rb

@@ -20,7 +20,7 @@ module Labels
         label.save
         label
       else
-        Rails.logger.warn("target_params should contain :project or :group or :template, actual value: #{target_params}")
+        Rails.logger.warn("target_params should contain :project or :group or :template, actual value: #{target_params}") # rubocop:disable Gitlab/RailsLogger
       end
     end
   end

app/services/merge_requests/merge_service.rb

@@ -113,12 +113,12 @@ module MergeRequests
     end
     def handle_merge_error(log_message:, save_message_on_model: false)
-      Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}")
+      Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}") # rubocop:disable Gitlab/RailsLogger
       @merge_request.update(merge_error: log_message) if save_message_on_model
     end
     def log_info(message)
-      @logger ||= Rails.logger
+      @logger ||= Rails.logger # rubocop:disable Gitlab/RailsLogger
       @logger.info("#{merge_request_info} - #{message}")
     end

app/services/projects/after_import_service.rb

@@ -13,7 +13,7 @@ module Projects
         repository.delete_all_refs_except(RESERVED_REF_PREFIXES)
       end
     rescue Projects::HousekeepingService::LeaseTaken => e
-      Rails.logger.info(
+      Rails.logger.info( # rubocop:disable Gitlab/RailsLogger
         "Could not perform housekeeping for project #{@project.full_path} (#{@project.id}): #{e}")
     end

app/services/projects/create_service.rb

@@ -151,7 +151,7 @@ module Projects
       log_message = message.dup
       log_message << " Project ID: #{@project.id}" if @project&.id
-      Rails.logger.error(log_message)
+      Rails.logger.error(log_message) # rubocop:disable Gitlab/RailsLogger
       if @project && @project.persisted? && @project.import_state
         @project.import_state.mark_as_failed(message)

app/services/projects/destroy_service.rb

@@ -18,7 +18,7 @@ module Projects
       schedule_stale_repos_removal
       job_id = ProjectDestroyWorker.perform_async(project.id, current_user.id, params)
-      Rails.logger.info("User #{current_user.id} scheduled destruction of project #{project.full_path} with job ID #{job_id}")
+      Rails.logger.info("User #{current_user.id} scheduled destruction of project #{project.full_path} with job ID #{job_id}") # rubocop:disable Gitlab/RailsLogger
     end
     def execute

app/services/projects/hashed_storage/migrate_attachments_service.rb

@@ -5,7 +5,7 @@ module Projects
     class MigrateAttachmentsService < BaseAttachmentService
       def initialize(project, old_disk_path, logger: nil)
         @project = project
-        @logger = logger || Rails.logger
+        @logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger
         @old_disk_path = old_disk_path
         @skipped = false
       end