Make Sequel integration compatible with new gem in a basic way

Conflicts: lib/appsignal.rb lib/appsignal/config.rb spec/lib/appsignal/config_spec.rb spec/lib/appsignal_spec.rb
appsignal · Oct 30, 2015 · f0f00b4 · f0f00b4
1 parent 3c9f1db
commit f0f00b4
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 5 deletions.
diff --git a/lib/appsignal.rb b/lib/appsignal.rb
@@ -19,13 +19,13 @@ def load_integrations
       require 'appsignal/integrations/puma'
       require 'appsignal/integrations/sidekiq'
       require 'appsignal/integrations/resque'
-      require 'appsignal/integrations/sequel'
       require 'appsignal/integrations/unicorn'
     end
 
     def load_instrumentations
       require 'appsignal/instrumentations/net_http' if config[:instrument_net_http]
       require 'appsignal/instrumentations/redis' if config[:instrument_redis]
+      require 'appsignal/instrumentations/sequel' if config[:instrument_sequel]
     end
 
     def extensions

diff --git a/lib/appsignal/config.rb b/lib/appsignal/config.rb
@@ -14,6 +14,7 @@ class Config
       :endpoint                       => 'https://push.appsignal.com',
       :instrument_net_http            => true,
       :instrument_redis               => true,
+      :instrument_sequel              => true,
       :skip_session_data              => false,
       :enable_frontend_error_catching => false,
       :frontend_error_catching_path   => '/appsignal_error_catcher',
@@ -30,6 +31,7 @@ class Config
       'APPSIGNAL_DEBUG'                          => :debug,
       'APPSIGNAL_INSTRUMENT_NET_HTTP'            => :instrument_net_http,
       'APPSIGNAL_INSTRUMENT_REDIS'               => :instrument_redis,
+      'APPSIGNAL_INSTRUMENT_SEQUEL'              => :instrument_sequel,
       'APPSIGNAL_SKIP_SESSION_DATA'              => :skip_session_data,
       'APPSIGNAL_ENABLE_FRONTEND_ERROR_CATCHING' => :enable_frontend_error_catching,
       'APPSIGNAL_IGNORE_ERRORS'                  => :ignore_errors,

diff --git a/lib/appsignal/integrations/sequel.rb → lib/appsignal/instrumentations/sequel.rb b/lib/appsignal/integrations/sequel.rb → lib/appsignal/instrumentations/sequel.rb
@@ -6,10 +6,15 @@ module Integrations
       module Sequel
         # Add query instrumentation
         def log_yield(sql, args = nil)
-          name    = 'sql.sequel'
-          payload = {:sql => sql, :args => args}
 
-          ActiveSupport::Notifications.instrument(name, payload) { yield }
+          # We'd like to get full sql queries in the payloads as well. To do
+          # that we need to find out a way to ask Sequel which quoting strategy
+          # is used by the adapter. We can then do something similar to the AR
+          # formatter.
+
+          ActiveSupport::Notifications.instrument('sql.sequel') do
+            yield
+          end
         end
       end # Sequel
     end # Integrations

diff --git a/lib/sequel/extensions/appsignal_integration.rb b/lib/sequel/extensions/appsignal_integration.rb
@@ -1,3 +1,3 @@
 # This is just a placeholder file, as +Sequel.extensions+ forcefully loads it
 # instead of assuming we already did. If you're looking for the integration
-# implementation, you can find it in +lib/appsignal/integrations/sequel.rb+
+# implementation, you can find it in +lib/appsignal/instrumentations/sequel.rb+
diff --git a/spec/lib/appsignal/config_spec.rb b/spec/lib/appsignal/config_spec.rb
@@ -21,6 +21,7 @@
         :ignore_actions                 => [],
         :instrument_net_http            => true,
         :instrument_redis               => true,
+        :instrument_sequel              => true,
         :skip_session_data              => false,
         :send_params                    => true,
         :endpoint                       => 'https://push.appsignal.com',

diff --git a/...lib/appsignal/integrations/sequel_spec.rb → ...appsignal/instrumentations/sequel_spec.rb b/...lib/appsignal/integrations/sequel_spec.rb → ...appsignal/instrumentations/sequel_spec.rb
diff --git a/spec/lib/appsignal_spec.rb b/spec/lib/appsignal_spec.rb
@@ -137,6 +137,7 @@
         it "should require integrations" do
           Appsignal.should_receive(:require).with('appsignal/instrumentations/net_http').once
           Appsignal.should_receive(:require).with('appsignal/instrumentations/redis').once
+          Appsignal.should_receive(:require).with('appsignal/instrumentations/sequel').once
         end
       end
 
@@ -150,6 +151,7 @@
         it "should require integrations" do
           Appsignal.should_not_receive(:require).with('appsignal/instrumentations/net_http')
           Appsignal.should_not_receive(:require).with('appsignal/instrumentations/redis')
+          Appsignal.should_not_receive(:require).with('appsignal/instrumentations/sequel')
         end
       end