From ab243e50e18188327f4d105bea528f493748b3df Mon Sep 17 00:00:00 2001
From: Matthias Kaeppler <mkaeppler@gitlab.com>
Date: Wed, 19 Jan 2022 14:48:38 +0100
Subject: [PATCH] RubySampler: allow specifying metrics prefix

This prevents metric names from clashing in cases
where multiple processes emit the same metrics
to the same scraper job and which do not have
a pid label.
---
 lib/gitlab/metrics/samplers/ruby_sampler.rb   | 36 ++++++++++---------
 metrics_server/metrics_server.rb              |  2 +-
 .../metrics/samplers/ruby_sampler_spec.rb     | 14 ++++++++
 3 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/lib/gitlab/metrics/samplers/ruby_sampler.rb b/lib/gitlab/metrics/samplers/ruby_sampler.rb
index d71ee671b8d42..4a3ef3711a508 100644
--- a/lib/gitlab/metrics/samplers/ruby_sampler.rb
+++ b/lib/gitlab/metrics/samplers/ruby_sampler.rb
@@ -7,22 +7,20 @@ class RubySampler < BaseSampler
         DEFAULT_SAMPLING_INTERVAL_SECONDS = 60
         GC_REPORT_BUCKETS = [0.01, 0.05, 0.1, 0.2, 0.3, 0.5, 1].freeze
 
-        def initialize(...)
+        def initialize(prefix: nil, **options)
+          @prefix = prefix
+
           GC::Profiler.clear
 
           metrics[:process_start_time_seconds].set(labels, Time.now.to_i)
 
-          super(...)
+          super(**options)
         end
 
         def metrics
           @metrics ||= init_metrics
         end
 
-        def with_prefix(prefix, name)
-          "ruby_#{prefix}_#{name}".to_sym
-        end
-
         def to_doc_string(name)
           name.to_s.humanize
         end
@@ -33,19 +31,19 @@ def labels
 
         def init_metrics
           metrics = {
-            file_descriptors:                  ::Gitlab::Metrics.gauge(with_prefix(:file, :descriptors), 'File descriptors used', labels),
-            process_cpu_seconds_total:         ::Gitlab::Metrics.gauge(with_prefix(:process, :cpu_seconds_total), 'Process CPU seconds total'),
-            process_max_fds:                   ::Gitlab::Metrics.gauge(with_prefix(:process, :max_fds), 'Process max fds'),
-            process_resident_memory_bytes:     ::Gitlab::Metrics.gauge(with_prefix(:process, :resident_memory_bytes), 'Memory used (RSS)', labels),
-            process_unique_memory_bytes:       ::Gitlab::Metrics.gauge(with_prefix(:process, :unique_memory_bytes), 'Memory used (USS)', labels),
-            process_proportional_memory_bytes: ::Gitlab::Metrics.gauge(with_prefix(:process, :proportional_memory_bytes), 'Memory used (PSS)', labels),
-            process_start_time_seconds:        ::Gitlab::Metrics.gauge(with_prefix(:process, :start_time_seconds), 'Process start time seconds'),
-            sampler_duration:                  ::Gitlab::Metrics.counter(with_prefix(:sampler, :duration_seconds_total), 'Sampler time', labels),
-            gc_duration_seconds:               ::Gitlab::Metrics.histogram(with_prefix(:gc, :duration_seconds), 'GC time', labels, GC_REPORT_BUCKETS)
+            file_descriptors:                  ::Gitlab::Metrics.gauge(metric_name(:file, :descriptors), 'File descriptors used', labels),
+            process_cpu_seconds_total:         ::Gitlab::Metrics.gauge(metric_name(:process, :cpu_seconds_total), 'Process CPU seconds total'),
+            process_max_fds:                   ::Gitlab::Metrics.gauge(metric_name(:process, :max_fds), 'Process max fds'),
+            process_resident_memory_bytes:     ::Gitlab::Metrics.gauge(metric_name(:process, :resident_memory_bytes), 'Memory used (RSS)', labels),
+            process_unique_memory_bytes:       ::Gitlab::Metrics.gauge(metric_name(:process, :unique_memory_bytes), 'Memory used (USS)', labels),
+            process_proportional_memory_bytes: ::Gitlab::Metrics.gauge(metric_name(:process, :proportional_memory_bytes), 'Memory used (PSS)', labels),
+            process_start_time_seconds:        ::Gitlab::Metrics.gauge(metric_name(:process, :start_time_seconds), 'Process start time seconds'),
+            sampler_duration:                  ::Gitlab::Metrics.counter(metric_name(:sampler, :duration_seconds_total), 'Sampler time', labels),
+            gc_duration_seconds:               ::Gitlab::Metrics.histogram(metric_name(:gc, :duration_seconds), 'GC time', labels, GC_REPORT_BUCKETS)
           }
 
           GC.stat.keys.each do |key|
-            metrics[key] = ::Gitlab::Metrics.gauge(with_prefix(:gc_stat, key), to_doc_string(key), labels)
+            metrics[key] = ::Gitlab::Metrics.gauge(metric_name(:gc_stat, key), to_doc_string(key), labels)
           end
 
           metrics
@@ -65,6 +63,12 @@ def sample
 
         private
 
+        def metric_name(group, metric)
+          name = "ruby_#{group}_#{metric}"
+          name = "#{@prefix}_#{name}" if @prefix.present?
+          name.to_sym
+        end
+
         def sample_gc
           # Observe all GC samples
           sample_gc_reports.each do |report|
diff --git a/metrics_server/metrics_server.rb b/metrics_server/metrics_server.rb
index 122a4e4fc1e78..68d39eaf77c2d 100644
--- a/metrics_server/metrics_server.rb
+++ b/metrics_server/metrics_server.rb
@@ -50,7 +50,7 @@ def start
     # a race where not all Prometheus db files will be visible to the exporter, resulting
     # in missing metrics.
     # Warming up ensures that these files exist prior to the exporter starting up.
-    Gitlab::Metrics::Samplers::RubySampler.initialize_instance(warmup: true).start
+    Gitlab::Metrics::Samplers::RubySampler.initialize_instance(prefix: name, warmup: true).start
 
     exporter_class = "Gitlab::Metrics::Exporter::#{@target.camelize}Exporter".constantize
     settings = Settings.new(Settings.monitoring[name])
diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
index a4877208bcf22..dfae5aa678450 100644
--- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
+++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
@@ -18,6 +18,20 @@
         expect(sampler.metrics[:process_start_time_seconds].get).to eq(Time.now.to_i)
       end
     end
+
+    context 'when not setting a prefix' do
+      it 'does not prepend metrics with that prefix' do
+        expect(sampler.metrics[:process_start_time_seconds].name).to eq(:ruby_process_start_time_seconds)
+      end
+    end
+
+    context 'when using custom prefix' do
+      let(:sampler) { described_class.new(prefix: 'custom') }
+
+      it 'prepends metrics with that prefix' do
+        expect(sampler.metrics[:process_start_time_seconds].name).to eq(:custom_ruby_process_start_time_seconds)
+      end
+    end
   end
 
   describe '#sample' do
-- 
GitLab