Merge branch 'tc-geo-fix-daemon-log-class' into 'master'

Refactor the Geo LogCursor Logger to make class more descriptive

Closes #5632

See merge request gitlab-org/gitlab-ee!5483

ee/changelogs/unreleased/tc-geo-fix-daemon-log-class.yml

+---
+title: Refactor the Geo LogCursor Logger to make class more descriptive
+merge_request: 5483
+author:
+type: fixed

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

@@ -2,6 +2,8 @@ module Gitlab
   module Geo
     module LogCursor
       class Daemon
+        include Utils::StrongMemoize
+
         VERSION = '0.2.0'.freeze
         BATCH_SIZE = 250
         SECONDARY_CHECK_INTERVAL = 1.minute
@@ -11,7 +13,6 @@ module Gitlab
         def initialize(options = {})
           @options = options
           @exit = false
-          logger.geo_logger.build.level = options[:debug] ? :debug : Rails.logger.level
         end
 
         def run!
@@ -301,7 +302,13 @@ module Gitlab
         end
 
         def logger
-          Gitlab::Geo::LogCursor::Logger
+          strong_memoize(:logger) do
+            Gitlab::Geo::LogCursor::Logger.new(self.class, log_level)
+          end
+        end
+
+        def log_level
+          options[:debug] ? :debug : Rails.logger.level
         end
       end
     end

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

@@ -50,7 +50,7 @@ module Gitlab
         end
 
         def self.logger
-          Gitlab::Geo::LogCursor::Logger
+          @logger ||= Gitlab::Geo::LogCursor::Logger.new(self)
         end
 
         private_class_method :exclusive_lease, :lease_taken_message, :logger

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

 module Gitlab
   module Geo
     module LogCursor
-      module Logger
+      class Logger
+        attr_accessor :klass
+
         PID = Process.pid.freeze
 
-        def self.event_info(created_at, message, params = {})
-          args = { pid: PID,
-                   class: caller_name,
-                   message: message,
-                   cursor_delay_s: cursor_delay(created_at) }.merge(params)
+        def initialize(klass, level = nil)
+          @klass = klass
+          geo_logger.build.level = level unless level.nil?
+        end
+
+        def event_info(created_at, message, params = {})
+          params = params.merge(cursor_delay_s: cursor_delay(created_at))
 
-          geo_logger.info(args)
+          info(message, params)
         end
 
-        def self.info(message, params = {})
-          geo_logger.info({ pid: PID, class: caller_name, message: message }.merge(params))
+        def info(message, params = {})
+          geo_logger.info(base_log_data(message, params))
         end
 
-        def self.error(message, params = {})
-          geo_logger.error({ pid: PID, class: caller_name, message: message }.merge(params))
+        def error(message, params = {})
+          geo_logger.error(base_log_data(message, params))
         end
 
-        def self.debug(message, params = {})
-          geo_logger.debug({ pid: PID, class: caller_name, message: message }.merge(params))
+        def debug(message, params = {})
+          geo_logger.debug(base_log_data(message, params))
         end
 
-        def self.geo_logger
+        private
+
+        def geo_logger
           Gitlab::Geo::Logger
         end
 
-        def self.caller_name
-          caller_locations[1].to_s.rpartition('/').last[/[a-z_]*/]&.classify
+        def caller_name
+          klass.name
         end
 
-        def self.cursor_delay(created_at)
+        def cursor_delay(created_at)
           (Time.now - created_at).to_f.round(3)
         end
 
-        private_class_method :caller_name, :cursor_delay
+        def base_log_data(message, params = {})
+          {
+            pid: PID,
+            class: caller_name,
+            message: message
+          }.merge(params)
+        end
       end
     end
   end

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

@@ -7,15 +7,26 @@ describe Gitlab::Geo::LogCursor::Lease, :clean_gitlab_redis_shared_state do
     end
   end
 
-  describe '.renew_lease!' do
+  describe '.renew!' do
     it 'returns an exclusive lease instance' do
       expect_any_instance_of(Gitlab::ExclusiveLease).to receive(:renew)
 
       described_class.renew!
     end
+
+    it 'logs with the correct caller class' do
+      stub_const("Gitlab::Geo::LogCursor::Logger::PID", 111)
+      allow_any_instance_of(Gitlab::ExclusiveLease).to receive(:renew).and_return(true)
+
+      expect(::Gitlab::Logger).to receive(:debug).with(pid: 111,
+                                                       class: 'Gitlab::Geo::LogCursor::Lease',
+                                                       message: 'Lease renewed.')
+
+      described_class.renew!
+    end
   end
 
-  describe '.try_obtain_lease' do
+  describe '.try_obtain_lease_with_ttl' do
     it 'returns zero when there is no lease' do
       result = described_class.try_obtain_with_ttl {}
 

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

 require 'spec_helper'
 
 describe Gitlab::Geo::LogCursor::Logger do
+  class LoggerSpec; end
+
+  subject(:logger) { described_class.new(LoggerSpec) }
+
   before do
     stub_const("#{described_class.name}::PID", 111)
   end
@@ -10,7 +14,7 @@ describe Gitlab::Geo::LogCursor::Logger do
                                                     class: "LoggerSpec",
                                                     message: 'Test')
 
-    described_class.info('Test')
+    logger.info('Test')
   end
 
   it 'logs an error event' do
@@ -18,7 +22,7 @@ describe Gitlab::Geo::LogCursor::Logger do
                                                      class: "LoggerSpec",
                                                      message: 'Test')
 
-    described_class.error('Test')
+    logger.error('Test')
   end
 
   describe '.event_info' do
@@ -28,7 +32,7 @@ describe Gitlab::Geo::LogCursor::Logger do
                                                       message: 'Test',
                                                       cursor_delay_s: 0.0)
 
-      described_class.event_info(Time.now, 'Test')
+      logger.event_info(Time.now, 'Test')
     end
   end
 end