diff --git a/.changesets/listen-if-garbage-collection-is-enabled.md b/.changesets/listen-if-garbage-collection-is-enabled.md new file mode 100644 index 000000000..63979999a --- /dev/null +++ b/.changesets/listen-if-garbage-collection-is-enabled.md @@ -0,0 +1,6 @@ +--- +bump: "patch" +type: "change" +--- + +Listen if the Ruby Garbage Collection profiler is enabled and collect how long the GC is running for the Ruby VM magic dashboard. An app will need to call `GC::Profiler.enable` to enable the GC profiler. Do not enable this in production environments, or at least not for long, because this can negatively impact performance of apps. diff --git a/lib/appsignal/garbage_collection.rb b/lib/appsignal/garbage_collection.rb index a0d98c0e5..127ae4ed4 100644 --- a/lib/appsignal/garbage_collection.rb +++ b/lib/appsignal/garbage_collection.rb @@ -5,19 +5,34 @@ module Appsignal module GarbageCollection # Return the GC profiler wrapper. # - # Temporarily always returns the {NilProfiler}. + # Returns {Profiler} if the Ruby Garbage Collection profiler is enabled. + # This is checked by calling `GC::Profiler.enabled?`. # # GC profiling is disabled by default due to the overhead it causes. Do not # enable this in production for long periods of time. def self.profiler - @profiler ||= NilProfiler.new + # Cached instances so it doesn't create a new object every time this + # method is called. Especially necessary for the {Profiler} because a new + # instance will have a new internal time counter. + @real_profiler ||= Profiler.new + @nil_profiler ||= NilProfiler.new + + enabled? ? @real_profiler : @nil_profiler + end + + # Check if Garbage Collection is enabled at the moment. + # + # @return [Boolean] + def self.enabled? + GC::Profiler.enabled? end - # Unset the currently cached profiler + # Unset the currently cached profilers. # # @return [void] def self.clear_profiler! - @profiler = nil + @real_profiler = nil + @nil_profiler = nil end # A wrapper around Ruby's `GC::Profiler` that tracks garbage collection diff --git a/lib/appsignal/probes/mri.rb b/lib/appsignal/probes/mri.rb index f98419d35..7858b1d6c 100644 --- a/lib/appsignal/probes/mri.rb +++ b/lib/appsignal/probes/mri.rb @@ -31,7 +31,7 @@ def call ) set_gauge("thread_count", Thread.list.size) - if GC::Profiler.enabled? + if Appsignal::GarbageCollection.enabled? gauge_delta(:gc_time, @gc_profiler.total_time) do |gc_time| set_gauge("gc_time", gc_time) if gc_time > 0 end diff --git a/spec/lib/appsignal/garbage_collection_spec.rb b/spec/lib/appsignal/garbage_collection_spec.rb index 0ea10fa04..9e1d7ca48 100644 --- a/spec/lib/appsignal/garbage_collection_spec.rb +++ b/spec/lib/appsignal/garbage_collection_spec.rb @@ -12,8 +12,11 @@ end context "when GC profiling is enabled" do + before { GC::Profiler.enable } + after { GC::Profiler.disable } + it "returns the Profiler" do - expect(described_class.profiler).to be_a(Appsignal::GarbageCollection::NilProfiler) + expect(described_class.profiler).to be_a(Appsignal::GarbageCollection::Profiler) end end end diff --git a/spec/lib/appsignal/probes/mri_spec.rb b/spec/lib/appsignal/probes/mri_spec.rb index 255a36322..78c8ed8fa 100644 --- a/spec/lib/appsignal/probes/mri_spec.rb +++ b/spec/lib/appsignal/probes/mri_spec.rb @@ -81,7 +81,7 @@ def set_gauge(*args) # rubocop:disable Naming/AccessorMethodName expect(metrics).to_not include("gc_time") end - it "does not report a gc_time metric while disable" do + it "does not report a gc_time metric while temporarily disabled" do # While enabled allow(GC::Profiler).to receive(:enabled?).and_return(true) expect(gc_profiler_mock).to receive(:total_time).and_return(10, 15)