From 29970d93a63aa174fbc4a41b29eff996ef0ede5e Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Tue, 3 Oct 2023 09:18:32 +0200 Subject: [PATCH] Add support for dry-monitor events (#996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New hook that adds transaction events every time a dry-monitor notifications instrumentation is called. There's also specific support for `rom-sql` events emitted through `dry-monitor`. Co-authored-by: Luismi Ramírez --- .../add-support-for-dry-monitor-events.md | 6 + .semaphore/semaphore.yml | 54 +++++++++ README.md | 1 + build_matrix.yml | 6 + gemfiles/dry-monitor.gemfile | 5 + .../event_formatter/rom/sql_formatter.rb | 18 +++ lib/appsignal/hooks.rb | 1 + lib/appsignal/hooks/dry_monitor.rb | 20 ++++ lib/appsignal/integrations/dry_monitor.rb | 22 ++++ .../event_formatter/rom/sql_formatter_spec.rb | 22 ++++ spec/lib/appsignal/hooks/dry_monitor_spec.rb | 104 ++++++++++++++++++ spec/support/helpers/dependency_helper.rb | 4 + 12 files changed, 263 insertions(+) create mode 100644 .changesets/add-support-for-dry-monitor-events.md create mode 100644 gemfiles/dry-monitor.gemfile create mode 100644 lib/appsignal/event_formatter/rom/sql_formatter.rb create mode 100644 lib/appsignal/hooks/dry_monitor.rb create mode 100644 lib/appsignal/integrations/dry_monitor.rb create mode 100644 spec/lib/appsignal/event_formatter/rom/sql_formatter_spec.rb create mode 100644 spec/lib/appsignal/hooks/dry_monitor_spec.rb diff --git a/.changesets/add-support-for-dry-monitor-events.md b/.changesets/add-support-for-dry-monitor-events.md new file mode 100644 index 000000000..46015c7c6 --- /dev/null +++ b/.changesets/add-support-for-dry-monitor-events.md @@ -0,0 +1,6 @@ +--- +bump: "patch" +type: "add" +--- + +Events from `dry-monitor` are now supported. There's also native support for `rom-sql` instrumentation events if they're configured. diff --git a/.semaphore/semaphore.yml b/.semaphore/semaphore.yml index a69b4220a..8f8e5a437 100644 --- a/.semaphore/semaphore.yml +++ b/.semaphore/semaphore.yml @@ -583,6 +583,24 @@ blocks: value: latest commands: - "./support/bundler_wrapper exec rake test" + - name: Ruby 3.0.5 for dry-monitor + env_vars: + - *2 + - *3 + - *4 + - *5 + - name: RUBY_VERSION + value: 3.0.5 + - name: GEMSET + value: dry-monitor + - name: BUNDLE_GEMFILE + value: gemfiles/dry-monitor.gemfile + - name: _RUBYGEMS_VERSION + value: latest + - name: _BUNDLER_VERSION + value: latest + commands: + - "./support/bundler_wrapper exec rake test" - name: Ruby 3.0.5 for grape env_vars: - *2 @@ -940,6 +958,24 @@ blocks: value: latest commands: - "./support/bundler_wrapper exec rake test" + - name: Ruby 3.1.3 for dry-monitor + env_vars: + - *2 + - *3 + - *4 + - *5 + - name: RUBY_VERSION + value: 3.1.3 + - name: GEMSET + value: dry-monitor + - name: BUNDLE_GEMFILE + value: gemfiles/dry-monitor.gemfile + - name: _RUBYGEMS_VERSION + value: latest + - name: _BUNDLER_VERSION + value: latest + commands: + - "./support/bundler_wrapper exec rake test" - name: Ruby 3.1.3 for grape env_vars: - *2 @@ -1279,6 +1315,24 @@ blocks: value: latest commands: - "./support/bundler_wrapper exec rake test" + - name: Ruby 3.2.1 for dry-monitor + env_vars: + - *2 + - *3 + - *4 + - *5 + - name: RUBY_VERSION + value: 3.2.1 + - name: GEMSET + value: dry-monitor + - name: BUNDLE_GEMFILE + value: gemfiles/dry-monitor.gemfile + - name: _RUBYGEMS_VERSION + value: latest + - name: _BUNDLER_VERSION + value: latest + commands: + - "./support/bundler_wrapper exec rake test" - name: Ruby 3.2.1 for grape env_vars: - *2 diff --git a/README.md b/README.md index 7858d3df4..7a5cf13dc 100644 --- a/README.md +++ b/README.md @@ -231,6 +231,7 @@ configurations you need to run the spec suite with a specific Gemfile. ``` BUNDLE_GEMFILE=gemfiles/capistrano2.gemfile bundle exec rspec BUNDLE_GEMFILE=gemfiles/capistrano3.gemfile bundle exec rspec +BUNDLE_GEMFILE=gemfiles/dry-monitor.gemfile bundle exec rspec BUNDLE_GEMFILE=gemfiles/grape.gemfile bundle exec rspec BUNDLE_GEMFILE=gemfiles/hanami.gemfile bundle exec rspec BUNDLE_GEMFILE=gemfiles/http5.gemfile bundle exec rspec diff --git a/build_matrix.yml b/build_matrix.yml index 8f0f1cc23..5235d2719 100644 --- a/build_matrix.yml +++ b/build_matrix.yml @@ -182,6 +182,12 @@ matrix: - gem: "no_dependencies" - gem: "capistrano2" - gem: "capistrano3" + - gem: "dry-monitor" + only: + ruby: + - "3.0.5" + - "3.1.3" + - "3.2.1" - gem: "grape" - gem: "hanami" only: diff --git a/gemfiles/dry-monitor.gemfile b/gemfiles/dry-monitor.gemfile new file mode 100644 index 000000000..c7768fac9 --- /dev/null +++ b/gemfiles/dry-monitor.gemfile @@ -0,0 +1,5 @@ +source 'https://rubygems.org' + +gem "dry-monitor", "~> 1.0.1" + +gemspec :path => '../' diff --git a/lib/appsignal/event_formatter/rom/sql_formatter.rb b/lib/appsignal/event_formatter/rom/sql_formatter.rb new file mode 100644 index 000000000..7fd0d21e2 --- /dev/null +++ b/lib/appsignal/event_formatter/rom/sql_formatter.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Appsignal + class EventFormatter + module Rom + class SqlFormatter + def format(payload) + ["query.#{payload[:name]}", payload[:query], SQL_BODY_FORMAT] + end + end + end + end +end + +Appsignal::EventFormatter.register( + "sql.dry", + Appsignal::EventFormatter::Rom::SqlFormatter +) diff --git a/lib/appsignal/hooks.rb b/lib/appsignal/hooks.rb index 53e1665dc..1397668f5 100644 --- a/lib/appsignal/hooks.rb +++ b/lib/appsignal/hooks.rb @@ -95,6 +95,7 @@ def self.const_missing(name) require "appsignal/hooks/celluloid" require "appsignal/hooks/delayed_job" require "appsignal/hooks/gvl" +require "appsignal/hooks/dry_monitor" require "appsignal/hooks/http" require "appsignal/hooks/mri" require "appsignal/hooks/net_http" diff --git a/lib/appsignal/hooks/dry_monitor.rb b/lib/appsignal/hooks/dry_monitor.rb new file mode 100644 index 000000000..8fb551663 --- /dev/null +++ b/lib/appsignal/hooks/dry_monitor.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +module Appsignal + class Hooks + # @api private + class DryMonitorHook < Appsignal::Hooks::Hook + register :dry_monitor + + def dependencies_present? + defined?(::Dry::Monitor::Notifications) + end + + def install + require "appsignal/integrations/dry_monitor" + + ::Dry::Monitor::Notifications.prepend(Appsignal::Integrations::DryMonitorIntegration) + end + end + end +end diff --git a/lib/appsignal/integrations/dry_monitor.rb b/lib/appsignal/integrations/dry_monitor.rb new file mode 100644 index 000000000..3ec7ff247 --- /dev/null +++ b/lib/appsignal/integrations/dry_monitor.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Appsignal + module Integrations + module DryMonitorIntegration + def instrument(event_id, payload = {}, &block) + Appsignal::Transaction.current.start_event + + super + ensure + title, body, body_format = Appsignal::EventFormatter.format("#{event_id}.dry", payload) + + Appsignal::Transaction.current.finish_event( + title || event_id.to_s, + title, + body, + body_format + ) + end + end + end +end diff --git a/spec/lib/appsignal/event_formatter/rom/sql_formatter_spec.rb b/spec/lib/appsignal/event_formatter/rom/sql_formatter_spec.rb new file mode 100644 index 000000000..7fde94c10 --- /dev/null +++ b/spec/lib/appsignal/event_formatter/rom/sql_formatter_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +describe Appsignal::EventFormatter::Rom::SqlFormatter do + let(:klass) { described_class } + let(:formatter) { klass.new } + + it "registers the sql event formatter" do + expect(Appsignal::EventFormatter.registered?("sql.dry", klass)).to be_truthy + end + + describe "#format" do + let(:payload) do + { + :name => "postgres", + :query => "SELECT * FROM users" + } + end + subject { formatter.format(payload) } + + it { is_expected.to eq ["query.postgres", "SELECT * FROM users", 1] } + end +end diff --git a/spec/lib/appsignal/hooks/dry_monitor_spec.rb b/spec/lib/appsignal/hooks/dry_monitor_spec.rb new file mode 100644 index 000000000..5077f7493 --- /dev/null +++ b/spec/lib/appsignal/hooks/dry_monitor_spec.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +if DependencyHelper.dry_monitor_present? + require "dry-monitor" + + describe Appsignal::Hooks::DryMonitorHook do + describe "#dependencies_present?" do + subject { described_class.new.dependencies_present? } + + context "when Dry::Monitor::Notifications constant is found" do + before { stub_const "Dry::Monitor::Notifications", Class.new } + + it { is_expected.to be_truthy } + end + + context "when Dry::Monitor::Notifications constant is not found" do + before { hide_const "Dry::Monitor::Notifications" } + + it { is_expected.to be_falsy } + end + end + end + + describe "#install" do + it "installs the dry-monitor hook" do + start_agent + + expect(Dry::Monitor::Notifications.included_modules).to include( + Appsignal::Integrations::DryMonitorIntegration + ) + end + end + + describe "Dry Monitor Integration" do + before :context do + start_agent + end + + let!(:transaction) do + Appsignal::Transaction.create("uuid", Appsignal::Transaction::HTTP_REQUEST, "test") + end + + let(:notifications) { Dry::Monitor::Notifications.new(:test) } + + context "when is a dry-sql event" do + let(:event_id) { :sql } + let(:payload) do + { + :name => "postgres", + :query => "SELECT * FROM users" + } + end + + it "creates an sql event" do + notifications.instrument(event_id, payload) + expect(transaction.to_h["events"]).to match([ + { + "allocation_count" => kind_of(Integer), + "body" => "SELECT * FROM users", + "body_format" => Appsignal::EventFormatter::SQL_BODY_FORMAT, + "child_allocation_count" => kind_of(Integer), + "child_duration" => kind_of(Float), + "child_gc_duration" => kind_of(Float), + "count" => 1, + "duration" => kind_of(Float), + "gc_duration" => kind_of(Float), + "name" => "query.postgres", + "start" => kind_of(Float), + "title" => "query.postgres" + } + ]) + end + end + + context "when is an unregistered formatter event" do + let(:event_id) { :foo } + let(:payload) do + { + :name => "foo" + } + end + + it "creates a generic event" do + notifications.instrument(event_id, payload) + expect(transaction.to_h["events"]).to match([ + { + "allocation_count" => kind_of(Integer), + "body" => "", + "body_format" => Appsignal::EventFormatter::DEFAULT, + "child_allocation_count" => kind_of(Integer), + "child_duration" => kind_of(Float), + "child_gc_duration" => kind_of(Float), + "count" => 1, + "duration" => kind_of(Float), + "gc_duration" => kind_of(Float), + "name" => "foo", + "start" => kind_of(Float), + "title" => "" + } + ]) + end + end + end +end diff --git a/spec/support/helpers/dependency_helper.rb b/spec/support/helpers/dependency_helper.rb index e0e4abc12..012d3cfac 100644 --- a/spec/support/helpers/dependency_helper.rb +++ b/spec/support/helpers/dependency_helper.rb @@ -115,6 +115,10 @@ def hanami_present? dependency_present? "hanami" end + def dry_monitor_present? + dependency_present? "dry-monitor" + end + def hanami2_present? hanami_present? && Gem.loaded_specs["hanami"].version >= Gem::Version.new("2.0") end