Merge branch '35552-improve-logging-of-sync-services-alt' into 'master'

Geo: Improve logging of sync services - include host

See merge request gitlab-org/gitlab!22203

changelogs/unreleased/35552-improve-logging-of-sync-services-alt.yml

+---
+title: 'Geo: Include host when logging'
+merge_request: 22203
+author:
+type: other

ee/app/services/geo/base_file_service.rb

@@ -44,13 +44,14 @@ module Geo
       object_type == :lfs
     end
 
-    def base_log_data(message)
+    # This is called by LogHelpers to build json log with context info
+    #
+    # @see ::Gitlab::Geo::LogHelpers
+    def extra_log_data
       {
-        class: self.class.name,
         object_type: object_type,
-        object_db_id: object_db_id,
-        message: message
-      }
+        object_db_id: object_db_id
+      }.compact
     end
   end
 end

ee/app/services/geo/cache_invalidation_event_store.rb

@@ -16,15 +16,13 @@ module Geo
       Geo::CacheInvalidationEvent.new(key: key)
     end
 
-    # This is called by ProjectLogHelpers to build json log with context info
+    # This is called by LogHelpers to build json log with context info
     #
-    # @see ::Gitlab::Geo::ProjectLogHelpers
-    def base_log_data(message)
+    # @see ::Gitlab::Geo::LogHelpers
+    def extra_log_data
       {
-        class: self.class.name,
         cache_key: key.to_s,
-        job_id: get_sidekiq_job_id,
-        message: message
+        job_id: get_sidekiq_job_id
       }.compact
     end
   end

ee/app/services/geo/container_repository_updated_event_store.rb

@@ -18,14 +18,12 @@ module Geo
       )
     end
 
-    # This is called by ProjectLogHelpers to build json log with context info
+    # This is called by LogHelpers to build json log with context info
     #
-    # @see ::Gitlab::Geo::ProjectLogHelpers
-    def base_log_data(message)
+    # @see ::Gitlab::Geo::LogHelpers
+    def extra_log_data
       {
-        class: self.class.name,
-        container_repository_id: repository.try(:id),
-        message: message
+        container_repository_id: repository.try(:id)
       }.compact
     end
   end

ee/app/services/geo/files_expire_service.rb

@@ -66,14 +66,12 @@ module Geo
     # This is called by LogHelpers to build json log with context info
     #
     # @see ::Gitlab::Geo::LogHelpers
-    def base_log_data(message)
+    def extra_log_data
       {
-        class: self.class.name,
         project_id: project.id,
         project_path: project.full_path,
-        project_old_path: old_full_path,
-        message: message
-      }
+        project_old_path: old_full_path
+      }.compact
     end
   end
 end

ee/app/services/geo/job_artifact_deleted_event_store.rb

@@ -25,16 +25,18 @@ module Geo
       job_artifact.file.relative_path if job_artifact.file.present?
     end
 
-    # This is called by ProjectLogHelpers to build json log with context info
+    def project
+      job_artifact.project
+    end
+
+    # This is called by LogHelpers to build json log with context info
     #
-    # @see ::Gitlab::Geo::ProjectLogHelpers
-    def base_log_data(message)
+    # @see ::Gitlab::Geo::LogHelpers
+    def extra_log_data
       {
-        class: self.class.name,
         job_artifact_id: job_artifact.id,
-        file_path: job_artifact.file.path,
-        message: message
-      }
+        file_path: job_artifact.file.path
+      }.compact
     end
   end
 end

ee/app/services/geo/lfs_object_deleted_event_store.rb

@@ -26,16 +26,14 @@ module Geo
       lfs_object.file.relative_path if lfs_object.file.present?
     end
 
-    # This is called by ProjectLogHelpers to build json log with context info
+    # This is called by LogHelpers to build json log with context info
     #
-    # @see ::Gitlab::Geo::ProjectLogHelpers
-    def base_log_data(message)
+    # @see ::Gitlab::Geo::LogHelpers
+    def extra_log_data
       {
-        class: self.class.name,
         lfs_object_id: lfs_object.id,
-        file_path: lfs_object.file.path,
-        message: message
-      }
+        file_path: lfs_object.file.path
+      }.compact
     end
   end
 end

ee/app/services/geo/upload_deleted_event_store.rb

@@ -24,19 +24,17 @@ module Geo
       )
     end
 
-    # This is called by ProjectLogHelpers to build json log with context info
+    # This is called by LogHelpers to build json log with context info
     #
-    # @see ::Gitlab::Geo::ProjectLogHelpers
-    def base_log_data(message)
+    # @see ::Gitlab::Geo::LogHelpers
+    def extra_log_data
       {
-        class: self.class.name,
         upload_id: upload.id,
         file_path: upload.path,
         model_id: upload.model_id,
         model_type: upload.model_type,
-        uploader: upload.uploader,
-        message: message
-      }
+        uploader: upload.uploader
+      }.compact
     end
   end
 end

ee/lib/gitlab/geo/container_repository_log_helpers.rb

@@ -5,14 +5,14 @@ module Gitlab
     module ContainerRepositoryLogHelpers
       include LogHelpers
 
-      def base_log_data(message)
+      # This is called by LogHelpers to build json log with context info
+      #
+      # @see ::Gitlab::Geo::LogHelpers
+      def extra_log_data
         {
-          class: self.class.name,
           project_id: container_repository.project.id,
           project_path: container_repository.project.full_path,
-          container_repository_name: container_repository.name,
-          message: message,
-          job_id: get_sidekiq_job_id
+          container_repository_name: container_repository.name
         }.compact
       end
     end

ee/lib/gitlab/geo/log_cursor/logger.rb

@@ -52,6 +52,7 @@ module Gitlab
         def base_log_data(message, params = {})
           {
             pid: PID,
+            host: Gitlab.config.gitlab.host,
             class: caller_name,
             message: message
           }.merge(params)

ee/lib/gitlab/geo/log_helpers.rb

@@ -23,9 +23,15 @@ module Gitlab
       def base_log_data(message)
         {
           class: self.class.name,
+          host: Gitlab.config.gitlab.host,
           message: message,
           job_id: get_sidekiq_job_id
-        }.compact
+        }.merge(extra_log_data).compact
+      end
+
+      # Intended to be overidden elsewhere
+      def extra_log_data
+        {}
       end
 
       def geo_logger
@@ -34,11 +40,9 @@ module Gitlab
 
       def get_sidekiq_job_id
         context_data = Thread.current[:sidekiq_context]&.first
-
         return unless context_data
 
         index = context_data.index('JID-')
-
         return unless index
 
         context_data[index + 4..-1]

ee/lib/gitlab/geo/project_log_helpers.rb

@@ -6,14 +6,11 @@ module Gitlab
       include LogHelpers
 
       def base_log_data(message)
-        {
-          class: self.class.name,
-          project_id: project.id,
-          project_path: project.full_path,
-          storage_version: project.storage_version,
-          message: message,
-          job_id: get_sidekiq_job_id
-        }.compact
+        super.merge({
+          project_id: project.try(:id),
+          project_path: project.try(:full_path),
+          storage_version: project.try(:storage_version)
+        }).compact
       end
     end
   end

ee/spec/lib/gitlab/geo/log_cursor/lease_spec.rb

@@ -29,6 +29,7 @@ describe Gitlab::Geo::LogCursor::Lease, :clean_gitlab_redis_shared_state do
 
       expect(::Gitlab::Logger).to receive(:debug).with(pid: 111,
                                                        class: 'Gitlab::Geo::LogCursor::Lease',
+                                                       host: "localhost",
                                                        message: 'Lease renewed.')
 
       described_class.renew!

ee/spec/lib/gitlab/geo/log_cursor/logger_spec.rb

@@ -7,7 +7,7 @@ describe Gitlab::Geo::LogCursor::Logger, :geo do
 
   subject(:logger) { described_class.new(LoggerSpec) }
 
-  let(:data) { { pid: 111, class: 'LoggerSpec', message: 'Test' } }
+  let(:data) { { pid: 111, class: 'LoggerSpec', host: 'localhost', message: 'Test' } }
 
   before do
     stub_const("#{described_class.name}::PID", 111)
@@ -35,6 +35,7 @@ describe Gitlab::Geo::LogCursor::Logger, :geo do
     it 'logs an info event' do
       expect(::Gitlab::Logger).to receive(:info).with(pid: 111,
                                                       class: "LoggerSpec",
+                                                      host: 'localhost',
                                                       message: 'Test',
                                                       cursor_delay_s: 0.0)
 

ee/spec/lib/gitlab/geo/log_helpers_spec.rb

@@ -23,6 +23,7 @@ describe Gitlab::Geo::LogHelpers do
   context 'Sidekiq context' do
     it 'does not log empty job_id when running outside of job' do
       expect(Gitlab::Geo::Logger).to receive(:error).with({ class: 'FakeLogHelpersConsumer',
+                                                            host: 'localhost',
                                                             message: 'Test message' })
 
       FakeLogHelpersConsumer.new.execute
@@ -30,6 +31,7 @@ describe Gitlab::Geo::LogHelpers do
 
     it 'logs sidekiq_context' do
       expect(Gitlab::Geo::Logger).to receive(:error).with({ class: 'FakeLogHelpersConsumer',
+                                                            host: 'localhost',
                                                             message: 'Test message',
                                                             job_id: '5b9b108c7558fe3c32cc61a5' })
 

ee/spec/services/geo/cache_invalidation_event_store_spec.rb

@@ -30,6 +30,7 @@ describe Geo::CacheInvalidationEventStore do
 
         expected_message = {
           class: described_class.name,
+          host: "localhost",
           cache_key: '',
           message: 'Cache invalidation event could not be created',
           error: "Validation failed: Key can't be blank"

ee/spec/services/geo/container_repository_updated_event_store_spec.rb

@@ -29,9 +29,10 @@ describe Geo::ContainerRepositoryUpdatedEventStore do
         subject = described_class.new(nil)
 
         expected_message = {
-            class: described_class.name,
-            message: 'Container repository updated event could not be created',
-            error: "Validation failed: Container repository can't be blank"
+          class: described_class.name,
+          host: 'localhost',
+          message: 'Container repository updated event could not be created',
+          error: "Validation failed: Container repository can't be blank"
         }
 
         expect(Gitlab::Geo::Logger).to receive(:error).with(expected_message).and_call_original

ee/spec/services/geo/job_artifact_deleted_event_store_spec.rb

@@ -32,12 +32,16 @@ describe Geo::JobArtifactDeletedEventStore do
 
       it 'logs an error message when event creation fail' do
         invalid_job_artifact = create(:ci_job_artifact)
+        project = invalid_job_artifact.project
         subject = described_class.new(invalid_job_artifact)
 
         expected_message = {
           class: "Geo::JobArtifactDeletedEventStore",
+          host: "localhost",
           job_artifact_id: invalid_job_artifact.id,
-          file_path: nil,
+          project_id: project.id,
+          project_path: project.full_path,
+          storage_version: project.storage_version,
           message: "Job artifact deleted event could not be created",
           error: "Validation failed: File path can't be blank"
         }

ee/spec/services/geo/lfs_object_deleted_event_store_spec.rb

@@ -37,8 +37,8 @@ describe Geo::LfsObjectDeletedEventStore do
 
         expected_message = {
           class: "Geo::LfsObjectDeletedEventStore",
+          host: "localhost",
           lfs_object_id: invalid_lfs_object.id,
-          file_path: nil,
           message: "Lfs object deleted event could not be created",
           error: "Validation failed: File path can't be blank"
         }