From 3b482c16c8e18f16a591c1463274b5f7972beba1 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 26 Jul 2022 09:52:53 +0200 Subject: [PATCH 1/2] Move probe gauge_delta method to helper This helper can then be more easily used by other probes. I removed the call to the `gauge` method because I'm not moving that to the helper right now. It's a tiny wrapper around `Appsignal.set_gauge`. [skip changeset] --- lib/appsignal/probes.rb | 1 + lib/appsignal/probes/helpers.rb | 29 +++++++++++++++++++++++++++++ lib/appsignal/probes/sidekiq.rb | 33 ++++++++++----------------------- 3 files changed, 40 insertions(+), 23 deletions(-) create mode 100644 lib/appsignal/probes/helpers.rb diff --git a/lib/appsignal/probes.rb b/lib/appsignal/probes.rb index cdc360ffc..0f14956f3 100644 --- a/lib/appsignal/probes.rb +++ b/lib/appsignal/probes.rb @@ -3,5 +3,6 @@ module Probes end end +require "appsignal/probes/helpers" require "appsignal/probes/mri" require "appsignal/probes/sidekiq" diff --git a/lib/appsignal/probes/helpers.rb b/lib/appsignal/probes/helpers.rb new file mode 100644 index 000000000..7edbe4fa4 --- /dev/null +++ b/lib/appsignal/probes/helpers.rb @@ -0,0 +1,29 @@ +module Appsignal + module Probes + module Helpers + private + + def gauge_delta_cache + @gauge_delta_cache ||= {} + end + + # Calculate the delta of two values for a gauge metric + # + # First call will store the data for the metric in the cache and the + # second call will return the delta of the gauge metric. This is used for + # absolute counter values which we want to track as gauges. + # + # @example + # gauge_delta :my_cache_key, 10 + # gauge_delta :my_cache_key, 15 + # # Returns a value of `5` + def gauge_delta(cache_key, value) + previous_value = gauge_delta_cache[cache_key] + gauge_delta_cache[cache_key] = value + return unless previous_value + + value - previous_value + end + end + end +end diff --git a/lib/appsignal/probes/sidekiq.rb b/lib/appsignal/probes/sidekiq.rb index d4d09fde5..ea27290ac 100644 --- a/lib/appsignal/probes/sidekiq.rb +++ b/lib/appsignal/probes/sidekiq.rb @@ -1,6 +1,8 @@ module Appsignal module Probes class SidekiqProbe + include Helpers + # @api private attr_reader :config @@ -42,11 +44,15 @@ def track_stats gauge "worker_count", stats.workers_size gauge "process_count", stats.processes_size - gauge_delta :jobs_processed, "job_count", stats.processed, - :status => :processed - gauge_delta :jobs_failed, "job_count", stats.failed, :status => :failed + jobs_processed = gauge_delta :jobs_processed, stats.processed + if jobs_processed + gauge "job_count", jobs_processed, :status => :processed + end + jobs_failed = gauge_delta :jobs_failed, stats.failed + gauge "job_count", jobs_failed, :status => :failed if jobs_failed gauge "job_count", stats.retry_size, :status => :retry_queue - gauge_delta :jobs_dead, "job_count", stats.dead_size, :status => :died + jobs_dead = gauge_delta :jobs_dead, stats.dead_size + gauge "job_count", jobs_dead, :status => :died if jobs_dead gauge "job_count", stats.scheduled_size, :status => :scheduled gauge "job_count", stats.enqueued, :status => :enqueued end @@ -65,25 +71,6 @@ def gauge(key, value, tags = {}) Appsignal.set_gauge "sidekiq_#{key}", value, tags end - # Track the delta of two values for a gauge metric - # - # First call will store the data for the metric and the second call will - # set a gauge metric with the difference. This is used for absolute - # counter values which we want to track as gauges. - # - # @example - # gauge_delta :my_cache_key, "my_gauge", 10 - # gauge_delta :my_cache_key, "my_gauge", 15 - # # Creates a gauge with the value `5` - # @see #gauge - def gauge_delta(cache_key, key, value, tags = {}) - previous_value = cache[cache_key] - cache[cache_key] = value - return unless previous_value - new_value = value - previous_value - gauge key, new_value, tags - end - def hostname return @hostname if defined?(@hostname) if config.key?(:hostname) From dd803449bd3990ba020c0bec4429166977071c02 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 26 Jul 2022 09:54:14 +0200 Subject: [PATCH 2/2] Store delta gauge of allocated objects in Ruby The total_allocated_objects metric will continuously report an growing value, which resets on every restart or new deploy. Creating a lot of highs and lows. This makes it difficult to track if the app is allocating more or less objects between deploys, other than tracking how steep the increase is. The delta of this metric reports the difference between probe runs, which should result in a more stable graph in apps with stable memory allocation. --- ...gauge-delta-value-for-allocated-objects.md | 6 ++++ lib/appsignal/probes/mri.rb | 11 ++++++- spec/lib/appsignal/probes/mri_spec.rb | 29 +++++++++++++------ 3 files changed, 36 insertions(+), 10 deletions(-) create mode 100644 .changesets/report-gauge-delta-value-for-allocated-objects.md diff --git a/.changesets/report-gauge-delta-value-for-allocated-objects.md b/.changesets/report-gauge-delta-value-for-allocated-objects.md new file mode 100644 index 000000000..ace01b8b0 --- /dev/null +++ b/.changesets/report-gauge-delta-value-for-allocated-objects.md @@ -0,0 +1,6 @@ +--- +bump: "patch" +type: "change" +--- + +Report gauge delta value for allocated objects. This reports a more user friendly metric we can graph with a more stable continuous value in apps with stable memory allocation. diff --git a/lib/appsignal/probes/mri.rb b/lib/appsignal/probes/mri.rb index 4643c3502..9dac7f765 100644 --- a/lib/appsignal/probes/mri.rb +++ b/lib/appsignal/probes/mri.rb @@ -1,6 +1,8 @@ module Appsignal module Probes class MriProbe + include Helpers + # @api private def self.dependencies_present? defined?(::RubyVM) && ::RubyVM.respond_to?(:stat) @@ -35,7 +37,14 @@ def call @appsignal.set_gauge("gc_total_time", MriProbe.garbage_collection_profiler.total_time) gc_stats = GC.stat - @appsignal.set_gauge("total_allocated_objects", gc_stats[:total_allocated_objects] || gc_stats[:total_allocated_object]) + allocated_objects = + gauge_delta( + :allocated_objects, + gc_stats[:total_allocated_objects] || gc_stats[:total_allocated_object] + ) + if allocated_objects + @appsignal.set_gauge("allocated_objects", allocated_objects) + end @appsignal.add_distribution_value("gc_count", GC.count, :metric => :gc_count) @appsignal.add_distribution_value("gc_count", gc_stats[:minor_gc_count], :metric => :minor_gc_count) diff --git a/spec/lib/appsignal/probes/mri_spec.rb b/spec/lib/appsignal/probes/mri_spec.rb index ea7707e06..76e2b32d6 100644 --- a/spec/lib/appsignal/probes/mri_spec.rb +++ b/spec/lib/appsignal/probes/mri_spec.rb @@ -33,31 +33,42 @@ def set_gauge(*args) # rubocop:disable Naming/AccessorMethodName unless DependencyHelper.running_jruby? || DependencyHelper.running_ruby_2_0? describe "#call" do - before do - probe.call - end - it "should track vm metrics" do + probe.call expect_distribution_value("ruby_vm", :class_serial) expect_distribution_value("ruby_vm", :global_constant_state) end it "tracks thread counts" do + probe.call expect_gauge_value("thread_count") end it "tracks GC total time" do + probe.call expect_gauge_value("gc_total_time") end it "tracks GC runs" do + probe.call expect_distribution_value("gc_count", :gc_count) expect_distribution_value("gc_count", :major_gc_count) expect_distribution_value("gc_count", :minor_gc_count) end - it "tracks GC stats" do - expect_gauge_value("total_allocated_objects") + it "tracks object allocation" do + expect(GC).to receive(:stat).and_return( + { :total_allocated_objects => 10 }, + :total_allocated_objects => 15 + ) + # Only tracks delta value so the needs to be called twice + probe.call + probe.call + expect_gauge_value("allocated_objects", 5) + end + + it "tracks heap slots" do + probe.call expect_distribution_value("heap_slots", :heap_live) expect_distribution_value("heap_slots", :heap_free) end @@ -73,10 +84,10 @@ def expect_distribution_value(expected_key, metric) end end - def expect_gauge_value(key) + def expect_gauge_value(expected_key, expected_value = nil) expect(appsignal_mock.gauges).to satisfy do |gauges| - gauges.any? do |gauge| - gauge.first == key && !gauge.last.nil? + gauges.any? do |(key, value)| + expected_key == key && expected_value ? expected_value == value : !value.nil? end end end