Commit 73c9efec authored by Mayra Cabrera's avatar Mayra Cabrera Committed by Stan Hu
Browse files

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
parent c8e586b9b92e
......@@ -12,6 +12,6 @@ def remove_mattermost_team(current_user)
# 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
......@@ -266,7 +266,7 @@ def retry(build, current_user)
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
......
......@@ -49,7 +49,7 @@ def current
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
......
......@@ -64,7 +64,7 @@ def move_repositories
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
......
......@@ -784,6 +784,7 @@ def add_import_job
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 @@ def log_import_activity(job_id, type: :import)
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 @@ def remove_pages
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 @@ def write_repository_config(gl_full_path: full_path)
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 @@ def pipeline_status
@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 @@ def add_export_job(current_user:, after_export_strategy: nil, params: {})
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)
......
......@@ -65,7 +65,7 @@ def mark_as_failed(error_message)
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
......
......@@ -273,7 +273,7 @@ def keep_around(*shas)
# 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 @@ def fetch_as_mirror(url, forced: false, refmap: :all_refs, remote_name: nil, pru
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 @@ def async_remove_remote(remote_name)
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)
......
......@@ -106,7 +106,7 @@ def calculate_reactive_cache
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
......
......@@ -41,7 +41,7 @@ def rename_repo(old_full_path: nil, new_full_path: nil)
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
......
......@@ -7,7 +7,7 @@ class Base
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)
......
......@@ -25,7 +25,7 @@ def spam?
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 @@ def submit(type)
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
......
......@@ -24,11 +24,11 @@ def failed_archive_counter
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
......
......@@ -58,6 +58,6 @@ def renew_lease!
end
def log_error(message, extra_args = {})
Rails.logger.error(message)
Rails.logger.error(message) # rubocop:disable Gitlab/RailsLogger
end
end
......@@ -6,7 +6,7 @@ class DestroyService < Groups::BaseService
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
......
......@@ -20,7 +20,7 @@ def execute(target_params)
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
......
......@@ -113,12 +113,12 @@ def delete_source_branch?
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
......
......@@ -13,7 +13,7 @@ def execute
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
......
......@@ -151,7 +151,7 @@ def fail(error:)
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)
......
......@@ -18,7 +18,7 @@ def async_execute
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
......
......@@ -5,7 +5,7 @@ module HashedStorage
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
......
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