From 0ab89d8e36ba58a95244b4c6dd49d53fac7a699f Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Wed, 10 Jul 2019 19:26:47 +0000 Subject: 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/workers/concerns/new_issuable.rb | 2 +- app/workers/create_gpg_signature_worker.rb | 2 +- app/workers/delete_user_worker.rb | 2 +- app/workers/email_receiver_worker.rb | 2 +- app/workers/expire_build_artifacts_worker.rb | 2 +- app/workers/expire_build_instance_artifacts_worker.rb | 2 +- app/workers/new_note_worker.rb | 2 +- app/workers/object_storage/migrate_uploads_worker.rb | 4 +++- app/workers/repository_fork_worker.rb | 2 +- app/workers/repository_import_worker.rb | 2 +- app/workers/repository_update_remote_mirror_worker.rb | 2 +- app/workers/run_pipeline_schedule_worker.rb | 2 ++ app/workers/stuck_ci_jobs_worker.rb | 4 ++-- app/workers/stuck_import_jobs_worker.rb | 2 +- app/workers/stuck_merge_jobs_worker.rb | 2 +- app/workers/trending_projects_worker.rb | 2 +- app/workers/update_merge_requests_worker.rb | 2 +- app/workers/upload_checksum_worker.rb | 2 +- 18 files changed, 22 insertions(+), 18 deletions(-) (limited to 'app/workers') diff --git a/app/workers/concerns/new_issuable.rb b/app/workers/concerns/new_issuable.rb index a89451a4475..22ba7c97309 100644 --- a/app/workers/concerns/new_issuable.rb +++ b/app/workers/concerns/new_issuable.rb @@ -27,6 +27,6 @@ module NewIssuable # rubocop: enable CodeReuse/ActiveRecord def log_error(record_class, record_id) - Rails.logger.error("#{self.class}: couldn't find #{record_class} with ID=#{record_id}, skipping job") + Rails.logger.error("#{self.class}: couldn't find #{record_class} with ID=#{record_id}, skipping job") # rubocop:disable Gitlab/RailsLogger end end diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb index 7fac7822cf7..e3fb5d479ae 100644 --- a/app/workers/create_gpg_signature_worker.rb +++ b/app/workers/create_gpg_signature_worker.rb @@ -22,7 +22,7 @@ class CreateGpgSignatureWorker commits.each do |commit| Gitlab::Gpg::Commit.new(commit).signature rescue => e - Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") + Rails.logger.error("Failed to create signature for commit #{commit.id}. Error: #{e.message}") # rubocop:disable Gitlab/RailsLogger end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/delete_user_worker.rb b/app/workers/delete_user_worker.rb index 4d0295f8d2e..efa8794b214 100644 --- a/app/workers/delete_user_worker.rb +++ b/app/workers/delete_user_worker.rb @@ -9,6 +9,6 @@ class DeleteUserWorker Users::DestroyService.new(current_user).execute(delete_user, options.symbolize_keys) rescue Gitlab::Access::AccessDeniedError => e - Rails.logger.warn("User could not be destroyed: #{e}") + Rails.logger.warn("User could not be destroyed: #{e}") # rubocop:disable Gitlab/RailsLogger end end diff --git a/app/workers/email_receiver_worker.rb b/app/workers/email_receiver_worker.rb index c4bcda2da16..e70bf17d5a9 100644 --- a/app/workers/email_receiver_worker.rb +++ b/app/workers/email_receiver_worker.rb @@ -16,7 +16,7 @@ class EmailReceiverWorker private def handle_failure(raw, error) - Rails.logger.warn("Email can not be processed: #{error}\n\n#{raw}") + Rails.logger.warn("Email can not be processed: #{error}\n\n#{raw}") # rubocop:disable Gitlab/RailsLogger return unless raw.present? diff --git a/app/workers/expire_build_artifacts_worker.rb b/app/workers/expire_build_artifacts_worker.rb index 251e95c68d5..6f0e0fd33f7 100644 --- a/app/workers/expire_build_artifacts_worker.rb +++ b/app/workers/expire_build_artifacts_worker.rb @@ -18,7 +18,7 @@ class ExpireBuildArtifactsWorker # rubocop: disable CodeReuse/ActiveRecord def perform_legacy_artifacts_removal - Rails.logger.info 'Scheduling removal of build artifacts' + Rails.logger.info 'Scheduling removal of build artifacts' # rubocop:disable Gitlab/RailsLogger build_ids = Ci::Build.with_expired_artifacts.pluck(:id) build_ids = build_ids.map { |build_id| [build_id] } diff --git a/app/workers/expire_build_instance_artifacts_worker.rb b/app/workers/expire_build_instance_artifacts_worker.rb index 94426dcf921..71e61dcb878 100644 --- a/app/workers/expire_build_instance_artifacts_worker.rb +++ b/app/workers/expire_build_instance_artifacts_worker.rb @@ -12,7 +12,7 @@ class ExpireBuildInstanceArtifactsWorker return unless build&.project && !build.project.pending_delete - Rails.logger.info "Removing artifacts for build #{build.id}..." + Rails.logger.info "Removing artifacts for build #{build.id}..." # rubocop:disable Gitlab/RailsLogger build.erase_erasable_artifacts! end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index 98f9f45e608..1d1ea926c21 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -11,7 +11,7 @@ class NewNoteWorker NotificationService.new.new_note(note) unless skip_notification?(note) Notes::PostProcessService.new(note).execute else - Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") + Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") # rubocop:disable Gitlab/RailsLogger end end diff --git a/app/workers/object_storage/migrate_uploads_worker.rb b/app/workers/object_storage/migrate_uploads_worker.rb index 12400d4e025..55ac7cd9b3c 100644 --- a/app/workers/object_storage/migrate_uploads_worker.rb +++ b/app/workers/object_storage/migrate_uploads_worker.rb @@ -37,6 +37,7 @@ module ObjectStorage end end + # rubocop:disable Gitlab/RailsLogger def report!(results) success, failures = results.partition(&:success?) @@ -45,6 +46,7 @@ module ObjectStorage raise MigrationFailures.new(failures.map(&:error)) if failures.any? end + # rubocop:enable Gitlab/RailsLogger def header(success, failures) _("Migrated %{success_count}/%{total_count} files.") % { success_count: success.count, total_count: success.count + failures.count } @@ -98,7 +100,7 @@ module ObjectStorage report!(results) rescue SanityCheckError => e # do not retry: the job is insane - Rails.logger.warn "#{self.class}: Sanity check error (#{e.message})" + Rails.logger.warn "#{self.class}: Sanity check error (#{e.message})" # rubocop:disable Gitlab/RailsLogger end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/workers/repository_fork_worker.rb b/app/workers/repository_fork_worker.rb index a9b88a133be..35e9c58eb13 100644 --- a/app/workers/repository_fork_worker.rb +++ b/app/workers/repository_fork_worker.rb @@ -35,7 +35,7 @@ class RepositoryForkWorker def start_fork(project) return true if start(project.import_state) - Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while forking.") + Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while forking.") # rubocop:disable Gitlab/RailsLogger false end end diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index 59691f48a39..dff9c8f50bf 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -36,7 +36,7 @@ class RepositoryImportWorker 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.") + Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.") # rubocop:disable Gitlab/RailsLogger false end diff --git a/app/workers/repository_update_remote_mirror_worker.rb b/app/workers/repository_update_remote_mirror_worker.rb index c0bae08ba85..03a7ff2cd7a 100644 --- a/app/workers/repository_update_remote_mirror_worker.rb +++ b/app/workers/repository_update_remote_mirror_worker.rb @@ -45,6 +45,6 @@ class RepositoryUpdateRemoteMirrorWorker def fail_remote_mirror(remote_mirror, message) remote_mirror.mark_as_failed(message) - Rails.logger.error(message) + Rails.logger.error(message) # rubocop:disable Gitlab/RailsLogger end end diff --git a/app/workers/run_pipeline_schedule_worker.rb b/app/workers/run_pipeline_schedule_worker.rb index 43e0b9db22f..351850e53cb 100644 --- a/app/workers/run_pipeline_schedule_worker.rb +++ b/app/workers/run_pipeline_schedule_worker.rb @@ -30,6 +30,7 @@ class RunPipelineScheduleWorker private + # rubocop:disable Gitlab/RailsLogger def error(schedule, error) failed_creation_counter.increment @@ -41,6 +42,7 @@ class RunPipelineScheduleWorker issue_url: 'https://gitlab.com/gitlab-org/gitlab-ce/issues/41231', extra: { schedule_id: schedule.id }) end + # rubocop:enable Gitlab/RailsLogger def failed_creation_counter @failed_creation_counter ||= diff --git a/app/workers/stuck_ci_jobs_worker.rb b/app/workers/stuck_ci_jobs_worker.rb index 25809f68080..30fba038937 100644 --- a/app/workers/stuck_ci_jobs_worker.rb +++ b/app/workers/stuck_ci_jobs_worker.rb @@ -14,7 +14,7 @@ class StuckCiJobsWorker def perform return unless try_obtain_lease - Rails.logger.info "#{self.class}: Cleaning stuck builds" + Rails.logger.info "#{self.class}: Cleaning stuck builds" # rubocop:disable Gitlab/RailsLogger drop :running, BUILD_RUNNING_OUTDATED_TIMEOUT, 'ci_builds.updated_at < ?', :stuck_or_timeout_failure drop :pending, BUILD_PENDING_OUTDATED_TIMEOUT, 'ci_builds.updated_at < ?', :stuck_or_timeout_failure @@ -66,7 +66,7 @@ class StuckCiJobsWorker # rubocop: enable CodeReuse/ActiveRecord def drop_build(type, build, status, timeout, reason) - Rails.logger.info "#{self.class}: Dropping #{type} build #{build.id} for runner #{build.runner_id} (status: #{status}, timeout: #{timeout}, reason: #{reason})" + Rails.logger.info "#{self.class}: Dropping #{type} build #{build.id} for runner #{build.runner_id} (status: #{status}, timeout: #{timeout}, reason: #{reason})" # rubocop:disable Gitlab/RailsLogger Gitlab::OptimisticLocking.retry_lock(build, 3) do |b| b.drop(reason) end diff --git a/app/workers/stuck_import_jobs_worker.rb b/app/workers/stuck_import_jobs_worker.rb index c8a186ba4ce..a9ff5b22b25 100644 --- a/app/workers/stuck_import_jobs_worker.rb +++ b/app/workers/stuck_import_jobs_worker.rb @@ -38,7 +38,7 @@ class StuckImportJobsWorker 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}") + Rails.logger.info("Marked stuck import jobs as failed. JIDs: #{completed_import_state_jids}") # rubocop:disable Gitlab/RailsLogger completed_import_states.each do |import_state| import_state.mark_as_failed(error_message) diff --git a/app/workers/stuck_merge_jobs_worker.rb b/app/workers/stuck_merge_jobs_worker.rb index f34ed6c4844..e840ae47421 100644 --- a/app/workers/stuck_merge_jobs_worker.rb +++ b/app/workers/stuck_merge_jobs_worker.rb @@ -5,7 +5,7 @@ class StuckMergeJobsWorker include CronjobQueue def self.logger - Rails.logger + Rails.logger # rubocop:disable Gitlab/RailsLogger end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/workers/trending_projects_worker.rb b/app/workers/trending_projects_worker.rb index 3297a1fe3d0..55b599ba38f 100644 --- a/app/workers/trending_projects_worker.rb +++ b/app/workers/trending_projects_worker.rb @@ -5,7 +5,7 @@ class TrendingProjectsWorker include CronjobQueue def perform - Rails.logger.info('Refreshing trending projects') + Rails.logger.info('Refreshing trending projects') # rubocop:disable Gitlab/RailsLogger TrendingProject.refresh! end diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index c7213df652a..6c0e472e05a 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -27,7 +27,7 @@ class UpdateMergeRequestsWorker "ref=#{ref}" ].join(',') - Rails.logger.info("UpdateMergeRequestsWorker#perform #{args_log}") if time.real > LOG_TIME_THRESHOLD + Rails.logger.info("UpdateMergeRequestsWorker#perform #{args_log}") if time.real > LOG_TIME_THRESHOLD # rubocop:disable Gitlab/RailsLogger end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/workers/upload_checksum_worker.rb b/app/workers/upload_checksum_worker.rb index 2a0536106d7..834dcaa435d 100644 --- a/app/workers/upload_checksum_worker.rb +++ b/app/workers/upload_checksum_worker.rb @@ -8,6 +8,6 @@ class UploadChecksumWorker upload.calculate_checksum! upload.save! rescue ActiveRecord::RecordNotFound - Rails.logger.error("UploadChecksumWorker: couldn't find upload #{upload_id}, skipping") + Rails.logger.error("UploadChecksumWorker: couldn't find upload #{upload_id}, skipping") # rubocop:disable Gitlab/RailsLogger end end -- cgit v1.2.3