Merge branch '65278-cleanup-sidekiq-metrics-dir-on-start' into 'master'

Clean Sidekiq metrics from multiproc dir on start

See merge request gitlab-org/gitlab-ce!31855

config.ru

@@ -17,24 +17,16 @@ end
 
 require ::File.expand_path('../config/environment', __FILE__)
 
-# The following is necessary to ensure stale Prometheus metrics don't accumulate over time.
-# It needs to be done as early as here to ensure metrics files aren't deleted.
-# After we hit our app in `warmup`, first metrics and corresponding files already being created,
-# for example in `lib/gitlab/metrics/requests_rack_middleware.rb`.
-def cleanup_prometheus_multiproc_dir
-  if dir = ::Prometheus::Client.configuration.multiprocess_files_dir
-    old_metrics = Dir[File.join(dir, '*.db')]
-
-    FileUtils.rm_rf(old_metrics)
-  end
-end
-
 def master_process?
   Prometheus::PidProvider.worker_id.in? %w(unicorn_master puma_master)
 end
 
 warmup do |app|
-  cleanup_prometheus_multiproc_dir if master_process?
+  # The following is necessary to ensure stale Prometheus metrics don't accumulate over time.
+  # It needs to be done as early as here to ensure metrics files aren't deleted.
+  # After we hit our app in `warmup`, first metrics and corresponding files already being created,
+  # for example in `lib/gitlab/metrics/requests_rack_middleware.rb`.
+  Prometheus::CleanupMultiprocDirService.new.execute if master_process?
 
   client = Rack::MockRequest.new(app)
   client.get('/')

config/initializers/7_prometheus_metrics.rb

@@ -32,6 +32,9 @@ end
 
 Sidekiq.configure_server do |config|
   config.on(:startup) do
+    # webserver metrics are cleaned up in config.ru: `warmup` block
+    Prometheus::CleanupMultiprocDirService.new.execute
+
     Gitlab::Metrics::SidekiqMetricsExporter.instance.start
   end
 end

lib/prometheus/cleanup_multiproc_dir_service.rb

+# frozen_string_literal: true
+
+module Prometheus
+  class CleanupMultiprocDirService
+    include Gitlab::Utils::StrongMemoize
+
+    def execute
+      FileUtils.rm_rf(old_metrics) if old_metrics
+    end
+
+    private
+
+    def old_metrics
+      strong_memoize(:old_metrics) do
+        Dir[File.join(multiprocess_files_dir, '*.db')] if multiprocess_files_dir
+      end
+    end
+
+    def multiprocess_files_dir
+      ::Prometheus::Client.configuration.multiprocess_files_dir
+    end
+  end
+end

spec/lib/prometheus/cleanup_multiproc_dir_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Prometheus::CleanupMultiprocDirService do
+  describe '.call' do
+    subject { described_class.new.execute }
+
+    let(:metrics_multiproc_dir) { Dir.mktmpdir }
+    let(:metrics_file_path) { File.join(metrics_multiproc_dir, 'counter_puma_master-0.db') }
+
+    before do
+      FileUtils.touch(metrics_file_path)
+    end
+
+    after do
+      FileUtils.rm_r(metrics_multiproc_dir)
+    end
+
+    context 'when `multiprocess_files_dir` is defined' do
+      before do
+        expect(Prometheus::Client.configuration)
+              .to receive(:multiprocess_files_dir)
+              .and_return(metrics_multiproc_dir)
+              .at_least(:once)
+      end
+
+      it 'removes old metrics' do
+        expect { subject }
+          .to change { File.exist?(metrics_file_path) }
+          .from(true)
+          .to(false)
+      end
+    end
+
+    context 'when `multiprocess_files_dir` is not defined' do
+      before do
+        expect(Prometheus::Client.configuration)
+              .to receive(:multiprocess_files_dir)
+              .and_return(nil)
+              .at_least(:once)
+      end
+
+      it 'does not remove any files' do
+        expect { subject }
+          .not_to change { File.exist?(metrics_file_path) }
+          .from(true)
+      end
+    end
+  end
+end