Call `GC::Profiler.clear` only in one place

Previously, both InfluxSampler and RubySampler were relying on the
`GC::Profiler.total_time` data which is the sum over the list
of captured GC events. Also, both samplers asynchronously called
`GC::Profiler.clear` which led to incorrect metric data because
each sampler has the wrong assumption it is the only object who calls
`GC::Profiler.clear` and thus could rely on the gathered results between
such calls.

We should ensure that `GC::Profiler.total_time` is called only in one
place making it possible to rely on accumulated data between such wipes.

Also, we need to track the amount of profiler reports we lost.

2 files changed, 16 insertions, 24 deletions

diff --git a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb
index 81954fcf8c5..2923048f742 100644
--- a/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb
+++ b/spec/lib/gitlab/metrics/samplers/influx_sampler_spec.rb
@@ -17,18 +17,10 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do
     it 'samples various statistics' do
       expect(sampler).to receive(:sample_memory_usage)
       expect(sampler).to receive(:sample_file_descriptors)
-      expect(sampler).to receive(:sample_gc)
       expect(sampler).to receive(:flush)
 
       sampler.sample
     end
-
-    it 'clears any GC profiles' do
-      expect(sampler).to receive(:flush)
-      expect(GC::Profiler).to receive(:clear)
-
-      sampler.sample
-    end
   end
 
   describe '#flush' do
@@ -67,18 +59,6 @@ describe Gitlab::Metrics::Samplers::InfluxSampler do
     end
   end
 
-  describe '#sample_gc' do
-    it 'adds a metric containing garbage collection statistics' do
-      expect(GC::Profiler).to receive(:total_time).and_return(0.24)
-
-      expect(sampler).to receive(:add_metric)
-        .with(/gc_statistics/, an_instance_of(Hash))
-        .and_call_original
-
-      sampler.sample_gc
-    end
-  end
-
   describe '#add_metric' do
     it 'prefixes the series name for a Rails process' do
       expect(sampler).to receive(:sidekiq?).and_return(false)
diff --git a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
index 4d93b70e6e3..5005a5d9ebc 100644
--- a/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
+++ b/spec/lib/gitlab/metrics/samplers/ruby_sampler_spec.rb
@@ -59,17 +59,29 @@ describe Gitlab::Metrics::Samplers::RubySampler do
     end
 
     it 'clears any GC profiles' do
-      expect(GC::Profiler).to receive(:clear)
+      expect(GC::Profiler).to receive(:clear).at_least(:once)
 
       sampler.sample
     end
   end
 
   describe '#sample_gc' do
-    it 'adds a metric containing garbage collection time statistics' do
-      expect(GC::Profiler).to receive(:total_time).and_return(0.24)
+    let!(:sampler) { described_class.new(5) }
 
-      expect(sampler.metrics[:total_time]).to receive(:increment).with({}, 0.24)
+    let(:gc_reports) { [{ GC_TIME: 0.1 }, { GC_TIME: 0.2 }, { GC_TIME: 0.3 }] }
+
+    it 're-enables GC::Profiler if needed' do
+      expect(GC::Profiler).to receive(:enable)
+
+      sampler.sample
+    end
+
+    it 'observes GC cycles time' do
+      expect(sampler).to receive(:sample_gc_reports).and_return(gc_reports)
+
+      expect(sampler.metrics[:gc_duration_seconds]).to receive(:observe).with({}, 0.1).ordered
+      expect(sampler.metrics[:gc_duration_seconds]).to receive(:observe).with({}, 0.2).ordered
+      expect(sampler.metrics[:gc_duration_seconds]).to receive(:observe).with({}, 0.3).ordered
 
       sampler.sample
     end