Merge branch 'sh-instrument-geo-file-downloads' into 'master'

Log Geo file download attempts and status

See merge request !2722

app/services/geo/file_download_service.rb

@@ -4,8 +4,13 @@ module Geo
 
     def execute
       try_obtain_lease do |lease|
+        start_time = Time.now
         bytes_downloaded = downloader.execute
         success = bytes_downloaded && bytes_downloaded >= 0
+        log_info("File download",
+                 success: success,
+                 bytes_downloaded: bytes_downloaded,
+                 download_time_s: (Time.now - start_time).to_f.round(3))
         update_registry(bytes_downloaded) if success
       end
     end

app/services/geo/file_service.rb

@@ -27,8 +27,9 @@ module Geo
       klass_name.camelize
     end
 
-    def log_info(message)
+    def log_info(message, details = {})
       data = log_base_data(message)
+      data.merge!(details) if details
       Gitlab::Geo::Logger.info(data)
     end
 

spec/services/geo/file_download_service_spec.rb

@@ -98,14 +98,22 @@ describe Geo::FileDownloadService do
 
       subject { described_class.new(:lfs, lfs_object.id) }
 
-      it 'downloads an LFS object' do
+      before do
         allow_any_instance_of(Gitlab::ExclusiveLease)
           .to receive(:try_obtain).and_return(true)
         allow_any_instance_of(Gitlab::Geo::LfsTransfer)
           .to receive(:download_from_primary).and_return(100)
+      end
 
+      it 'downloads an LFS object' do
         expect { subject.execute }.to change { Geo::FileRegistry.count }.by(1)
       end
+
+      it 'logs a message' do
+        expect(Gitlab::Geo::Logger).to receive(:info).with(hash_including(:message, :download_time_s, success: true, bytes_downloaded: 100)).and_call_original
+
+        subject.execute
+      end
     end
 
     context 'bad object type' do