Instrument Repository.clean_old_archives

3 files changed, 30 insertions, 4 deletions

diff --git a/CHANGELOG b/CHANGELOG
index c29c207ca70..7ef374b04d8 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.7.0 (unreleased)
   - Method instrumentation now uses Module#prepend instead of aliasing methods
+  - Repository.clean_old_archives is now instrumented
   - The Projects::HousekeepingService class has extra instrumentation
   - All service classes (those residing in app/services) are now instrumented
   - Developers can now add custom tags to transactions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 308c590e3f8..589756f8531 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -12,11 +12,13 @@ class Repository
   attr_accessor :path_with_namespace, :project
 
   def self.clean_old_archives
-    repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path
+    Gitlab::Metrics.measure(:clean_old_archives) do
+      repository_downloads_path = Gitlab.config.gitlab.repository_downloads_path
 
-    return unless File.directory?(repository_downloads_path)
+      return unless File.directory?(repository_downloads_path)
 
-    Gitlab::Popen.popen(%W(find #{repository_downloads_path} -not -path #{repository_downloads_path} -mmin +120 -delete))
+      Gitlab::Popen.popen(%W(find #{repository_downloads_path} -not -path #{repository_downloads_path} -mmin +120 -delete))
+    end
   end
 
   def initialize(path_with_namespace, project)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index c163001b7c1..f30a21e79ae 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -910,9 +910,32 @@ describe Repository, models: true do
     end
   end
 
+  describe '.clean_old_archives' do
+    let(:path) { Gitlab.config.gitlab.repository_downloads_path }
+
+    context 'when the downloads directory does not exist' do
+      it 'does not remove any archives' do
+        expect(File).to receive(:directory?).with(path).and_return(false)
+
+        expect(Gitlab::Popen).not_to receive(:popen)
+
+        described_class.clean_old_archives
+      end
+    end
+
+    context 'when the downloads directory exists' do
+      it 'removes old archives' do
+        expect(File).to receive(:directory?).with(path).and_return(true)
+
+        expect(Gitlab::Popen).to receive(:popen)
+
+        described_class.clean_old_archives
+      end
+    end
+  end
+
   def create_remote_branch(remote_name, branch_name, target)
     rugged = repository.rugged
     rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", target)
   end
-
 end