Enable Redis tracing in Sentry APM

This change causes `db.redis.command` spans to be recorded by Sentry when
APM is enabled. It also makes slight improvements to the Sentry
initializer.

flag=none
closes DE-1055

test plan:
- verify that Sentry and Canvas initialize OK, and both errors and APM
  transactions are recorded OK
- verify that controllers which interface with Redis record spans in
  their APM transactions
- verify that Redis commands are not recorded as breadcrumbs (we did not
  enable this feature)

Change-Id: I7955b3ad5d6b59e6fd96c801881671814064d622
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/285255
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Aaron Ogata <aogata@instructure.com>
QA-Review: Isaac Moore <isaac.moore@instructure.com>
Product-Review: Isaac Moore <isaac.moore@instructure.com>

config/initializers/sentry.rb

@@ -27,49 +27,49 @@
 Rails.configuration.to_prepare do
   settings = Rails.env.test? ? {} : SentryExtensions::Settings.settings
 
-  Sentry.init do |config|
-    config.dsn = settings[:dsn]
-    config.environment = Canvas.environment
-    config.release = Canvas.revision
-    config.sample_rate = SentryExtensions::Settings.get("sentry_backend_errors_sample_rate", "1.0").to_f
+  unless Sentry.initialized?
+    Sentry.init do |config|
+      config.dsn = settings[:dsn]
+      config.environment = Canvas.environment
+      config.release = Canvas.revision
+      config.sample_rate = SentryExtensions::Settings.get("sentry_backend_errors_sample_rate", "1.0").to_f
 
-    config.traces_sampler = lambda do |sampling_context|
-      unless sampling_context[:parent_sampled].nil?
-        # If there is a parent transaction, abide by its sampling decision
-        next sampling_context[:parent_sampled]
+      config.traces_sampler = lambda do |sampling_context|
+        unless sampling_context[:parent_sampled].nil?
+          # If there is a parent transaction, abide by its sampling decision
+          next sampling_context[:parent_sampled]
+        end
+
+        SentryExtensions::Settings.get("sentry_backend_traces_sample_rate", "0.0").to_f
       end
 
-      SentryExtensions::Settings.get("sentry_backend_traces_sample_rate", "0.0").to_f
+      # Override the Sentry-provided ActiveRecord subscriber with our own (to normalize SQL queries)
+      config.rails.tracing_subscribers.delete(Sentry::Rails::Tracing::ActiveRecordSubscriber)
+      config.rails.tracing_subscribers.add(SentryExtensions::Tracing::ActiveRecordSubscriber)
+
+      # sentry_logger would be nice here (it records log messages), but it currently includes raw SQL logs
+      config.breadcrumbs_logger = [:http_logger] if Canvas::Plugin.value_to_boolean(SentryExtensions::Settings.get("sentry_backend_breadcrumbs_enabled", "false"))
+
+      filter = ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters)
+      config.before_send = lambda do |event, _|
+        filter.filter(event.to_hash)
+      end
+
+      # this array should only contain exceptions that are intentionally
+      # thrown to drive client facing behavior.  A good example
+      # are login/auth exceptions.  Exceptions that are simply noisy/inconvenient
+      # should probably be caught and solved...
+      config.excluded_exceptions += %w[
+        AuthenticationMethods::AccessTokenError
+        AuthenticationMethods::AccessTokenScopeError
+        AuthenticationMethods::LoggedOutError
+        ActionController::InvalidAuthenticityToken
+        Folio::InvalidPage
+        Turnitin::Errors::SubmissionNotScoredError
+        Rack::QueryParser::InvalidParameterError
+        PG::UnableToSend
+      ]
     end
-    config.rails.tracing_subscribers = [
-      Sentry::Rails::Tracing::ActionControllerSubscriber,
-      Sentry::Rails::Tracing::ActionViewSubscriber,
-      Sentry::Rails::Tracing::ActiveStorageSubscriber,
-      SentryExtensions::Tracing::ActiveRecordSubscriber # overridden from the Sentry-provided one
-    ]
-
-    # sentry_logger would be nice here (it records log messages), but it currently includes raw SQL logs
-    config.breadcrumbs_logger = [:http_logger] if Canvas::Plugin.value_to_boolean(SentryExtensions::Settings.get("sentry_backend_breadcrumbs_enabled", "false"))
-
-    filter = ActiveSupport::ParameterFilter.new(Rails.application.config.filter_parameters)
-    config.before_send = lambda do |event, _|
-      filter.filter(event.to_hash)
-    end
-
-    # this array should only contain exceptions that are intentionally
-    # thrown to drive client facing behavior.  A good example
-    # are login/auth exceptions.  Exceptions that are simply noisy/inconvenient
-    # should probably be caught and solved...
-    config.excluded_exceptions += %w[
-      AuthenticationMethods::AccessTokenError
-      AuthenticationMethods::AccessTokenScopeError
-      AuthenticationMethods::LoggedOutError
-      ActionController::InvalidAuthenticityToken
-      Folio::InvalidPage
-      Turnitin::Errors::SubmissionNotScoredError
-      Rack::QueryParser::InvalidParameterError
-      PG::UnableToSend
-    ]
   end
 
   Sentry.set_tags(settings.fetch(:tags, {}))

gems/canvas_cache/canvas_cache.gemspec

@@ -22,6 +22,7 @@ Gem::Specification.new do |spec|
 
   spec.add_dependency "guardrail", ">= 2.0.0"
   spec.add_dependency "inst_statsd", ">= 2.1.0"
+  spec.add_dependency "sentry-ruby", "~> 5.1.0"
 
   spec.add_development_dependency "bundler"
   spec.add_development_dependency "byebug"

gems/canvas_cache/lib/canvas_cache/redis.rb

@@ -391,6 +391,12 @@ module CanvasCache
     def self.patch
       Bundler.require "redis"
       require "redis/distributed"
+      require "sentry-ruby"
+
+      ::Sentry.register_patch do
+        patch = ::Sentry::Redis::Client
+        ::Redis::Client.prepend(patch) unless ::Redis::Client <= patch
+      end
 
       ::Redis::Client.prepend(::CanvasCache::Redis::Client)
       ::Redis::Distributed.prepend(::CanvasCache::Redis::Distributed)

lib/sentry_extensions/settings.rb

@@ -17,21 +17,23 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 
 module SentryExtensions
-  class Settings
-    def self.settings
-      @sentry_settings ||= ConfigFile.load("sentry")
+  module Settings
+    class << self
+      def settings
+        @sentry_settings ||= ConfigFile.load("sentry")
 
-      @sentry_settings.presence || {}
-    end
+        @sentry_settings.presence || {}
+      end
 
-    def self.get(name, default = nil)
-      settings[name.to_sym] || Setting.get(name, default)
-    rescue PG::ConnectionBad
-      default
-    end
+      def get(name, default = nil)
+        settings[name.to_sym] || Setting.get(name, default)
+      rescue PG::ConnectionBad
+        default
+      end
 
-    def self.reset_settings
-      @sentry_settings = nil
+      def reset_settings
+        @sentry_settings = nil
+      end
     end
   end
 end