diff options
| -rw-r--r-- | config.ru | 18 | ||||
| -rw-r--r-- | config/initializers/7_prometheus_metrics.rb | 3 | ||||
| -rw-r--r-- | lib/prometheus/cleanup_multiproc_dir_service.rb | 23 | ||||
| -rw-r--r-- | spec/lib/prometheus/cleanup_multiproc_dir_service_spec.rb | 51 |
4 files changed, 82 insertions, 13 deletions
diff --git a/config.ru b/config.ru index f6a7dca0542..750c84f7642 100644 --- a/config.ru +++ b/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('/') diff --git a/config/initializers/7_prometheus_metrics.rb b/config/initializers/7_prometheus_metrics.rb index 70e5dcd042e..143b34b5368 100644 --- a/config/initializers/7_prometheus_metrics.rb +++ b/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 diff --git a/lib/prometheus/cleanup_multiproc_dir_service.rb b/lib/prometheus/cleanup_multiproc_dir_service.rb new file mode 100644 index 00000000000..6418b4de166 --- /dev/null +++ b/lib/prometheus/cleanup_multiproc_dir_service.rb @@ -0,0 +1,23 @@ +# 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 diff --git a/spec/lib/prometheus/cleanup_multiproc_dir_service_spec.rb b/spec/lib/prometheus/cleanup_multiproc_dir_service_spec.rb new file mode 100644 index 00000000000..c7302a1a656 --- /dev/null +++ b/spec/lib/prometheus/cleanup_multiproc_dir_service_spec.rb @@ -0,0 +1,51 @@ +# 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 |