Allow customization breadcrumb data of ActiveSupportLogger (#2139)

getsentry · Oct 24, 2023 · d315a15 · d315a15
1 parent 67412c4
commit d315a15
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 65 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,15 @@
 ## Unreleased
 
+### Features
+
+- Add new `config.rails.active_support_logger_subscription_items` to allow customization breadcrumb data of active support logger [#2139](https://github.com/getsentry/sentry-ruby/pull/2139)
+
+  ```rb
+    config.rails.active_support_logger_subscription_items["sql.active_record"] << :type_casted_binds
+    config.rails.active_support_logger_subscription_items.delete("sql.active_record")
+    config.rails.active_support_logger_subscription_items["foo"] = :bar
+  ```
+
 ### Bug Fixes
 
 - Fix puma integration for versions before v5 [#2141](https://github.com/getsentry/sentry-ruby/pull/2141)

diff --git a/sentry-rails/lib/sentry/rails/breadcrumb/active_support_logger.rb b/sentry-rails/lib/sentry/rails/breadcrumb/active_support_logger.rb
@@ -2,75 +2,13 @@ module Sentry
   module Rails
     module Breadcrumb
       module ActiveSupportLogger
-        ALLOWED_LIST = {
-          # action_controller
-          "write_fragment.action_controller" => %i[key],
-          "read_fragment.action_controller" => %i[key],
-          "exist_fragment?.action_controller" => %i[key],
-          "expire_fragment.action_controller" => %i[key],
-          "start_processing.action_controller" => %i[controller action params format method path],
-          "process_action.action_controller" => %i[controller action params format method path status view_runtime db_runtime],
-          "send_file.action_controller" => %i[path],
-          "redirect_to.action_controller" => %i[status location],
-          "halted_callback.action_controller" => %i[filter],
-          # action_dispatch
-          "process_middleware.action_dispatch" => %i[middleware],
-          # action_view
-          "render_template.action_view" => %i[identifier layout],
-          "render_partial.action_view" => %i[identifier],
-          "render_collection.action_view" => %i[identifier count cache_hits],
-          "render_layout.action_view" => %i[identifier],
-          # active_record
-          "sql.active_record" => %i[sql name statement_name cached],
-          "instantiation.active_record" => %i[record_count class_name],
-          # action_mailer
-          # not including to, from, or subject..etc. because of PII concern
-          "deliver.action_mailer" => %i[mailer date perform_deliveries],
-          "process.action_mailer" => %i[mailer action params],
-          # active_support
-          "cache_read.active_support" => %i[key store hit],
-          "cache_generate.active_support" => %i[key store],
-          "cache_fetch_hit.active_support" => %i[key store],
-          "cache_write.active_support" => %i[key store],
-          "cache_delete.active_support" => %i[key store],
-          "cache_exist?.active_support" => %i[key store],
-          # active_job
-          "enqueue_at.active_job" => %i[],
-          "enqueue.active_job" => %i[],
-          "enqueue_retry.active_job" => %i[],
-          "perform_start.active_job" => %i[],
-          "perform.active_job" => %i[],
-          "retry_stopped.active_job" => %i[],
-          "discard.active_job" => %i[],
-          # action_cable
-          "perform_action.action_cable" => %i[channel_class action],
-          "transmit.action_cable" => %i[channel_class],
-          "transmit_subscription_confirmation.action_cable" => %i[channel_class],
-          "transmit_subscription_rejection.action_cable" => %i[channel_class],
-          "broadcast.action_cable" => %i[broadcasting],
-          # active_storage
-          "service_upload.active_storage" => %i[service key checksum],
-          "service_streaming_download.active_storage" => %i[service key],
-          "service_download_chunk.active_storage" => %i[service key],
-          "service_download.active_storage" => %i[service key],
-          "service_delete.active_storage" => %i[service key],
-          "service_delete_prefixed.active_storage" => %i[service prefix],
-          "service_exist.active_storage" => %i[service key exist],
-          "service_url.active_storage" => %i[service key url],
-          "service_update_metadata.active_storage" => %i[service key],
-          "preview.active_storage" => %i[key],
-          "analyze.active_storage" => %i[analyzer],
-        }.freeze
-
         class << self
           def add(name, started, _finished, _unique_id, data)
             # skip Rails' internal events
             return if name.start_with?("!")
 
-            allowed_keys = ALLOWED_LIST[name]
-
             if data.is_a?(Hash)
-              data = data.slice(*allowed_keys)
+              data = data.slice(*@allowed_keys[name])
             end
 
             crumb = Sentry::Breadcrumb.new(
@@ -81,7 +19,9 @@ def add(name, started, _finished, _unique_id, data)
             Sentry.add_breadcrumb(crumb)
           end
 
-          def inject
+          def inject(allowed_keys)
+            @allowed_keys = allowed_keys
+
             @subscriber = ::ActiveSupport::Notifications.subscribe(/.*/) do |name, started, finished, unique_id, data|
               # we only record events that has a started timestamp
               if started.is_a?(Time)

diff --git a/sentry-rails/lib/sentry/rails/configuration.rb b/sentry-rails/lib/sentry/rails/configuration.rb
@@ -46,6 +46,67 @@ module Rails
       'ActionDispatch::Http::Parameters::ParseError',
       'ActiveRecord::RecordNotFound'
     ].freeze
+
+    ACTIVE_SUPPORT_LOGGER_SUBSCRIPTION_ITEMS_DEFAULT = {
+      # action_controller
+      "write_fragment.action_controller" => %i[key],
+      "read_fragment.action_controller" => %i[key],
+      "exist_fragment?.action_controller" => %i[key],
+      "expire_fragment.action_controller" => %i[key],
+      "start_processing.action_controller" => %i[controller action params format method path],
+      "process_action.action_controller" => %i[controller action params format method path status view_runtime db_runtime],
+      "send_file.action_controller" => %i[path],
+      "redirect_to.action_controller" => %i[status location],
+      "halted_callback.action_controller" => %i[filter],
+      # action_dispatch
+      "process_middleware.action_dispatch" => %i[middleware],
+      # action_view
+      "render_template.action_view" => %i[identifier layout],
+      "render_partial.action_view" => %i[identifier],
+      "render_collection.action_view" => %i[identifier count cache_hits],
+      "render_layout.action_view" => %i[identifier],
+      # active_record
+      "sql.active_record" => %i[sql name statement_name cached],
+      "instantiation.active_record" => %i[record_count class_name],
+      # action_mailer
+      # not including to, from, or subject..etc. because of PII concern
+      "deliver.action_mailer" => %i[mailer date perform_deliveries],
+      "process.action_mailer" => %i[mailer action params],
+      # active_support
+      "cache_read.active_support" => %i[key store hit],
+      "cache_generate.active_support" => %i[key store],
+      "cache_fetch_hit.active_support" => %i[key store],
+      "cache_write.active_support" => %i[key store],
+      "cache_delete.active_support" => %i[key store],
+      "cache_exist?.active_support" => %i[key store],
+      # active_job
+      "enqueue_at.active_job" => %i[],
+      "enqueue.active_job" => %i[],
+      "enqueue_retry.active_job" => %i[],
+      "perform_start.active_job" => %i[],
+      "perform.active_job" => %i[],
+      "retry_stopped.active_job" => %i[],
+      "discard.active_job" => %i[],
+      # action_cable
+      "perform_action.action_cable" => %i[channel_class action],
+      "transmit.action_cable" => %i[channel_class],
+      "transmit_subscription_confirmation.action_cable" => %i[channel_class],
+      "transmit_subscription_rejection.action_cable" => %i[channel_class],
+      "broadcast.action_cable" => %i[broadcasting],
+      # active_storage
+      "service_upload.active_storage" => %i[service key checksum],
+      "service_streaming_download.active_storage" => %i[service key],
+      "service_download_chunk.active_storage" => %i[service key],
+      "service_download.active_storage" => %i[service key],
+      "service_delete.active_storage" => %i[service key],
+      "service_delete_prefixed.active_storage" => %i[service prefix],
+      "service_exist.active_storage" => %i[service key exist],
+      "service_url.active_storage" => %i[service key url],
+      "service_update_metadata.active_storage" => %i[service key],
+      "preview.active_storage" => %i[key],
+      "analyze.active_storage" => %i[analyzer],
+    }.freeze
+
     class Configuration
       # Rails 7.0 introduced a new error reporter feature, which the SDK once opted-in by default.
       # But after receiving multiple issue reports, the integration seemed to cause serious troubles to some users.
@@ -80,6 +141,10 @@ class Configuration
       # ```
       attr_accessor :assets_regexp
 
+      # Hash of subscription items that will be shown in breadcrumbs active support logger.
+      # @return [Hash<String, Array<Symbol>>]
+      attr_accessor :active_support_logger_subscription_items
+
       def initialize
         @register_error_subscriber = false
         @report_rescued_exceptions = true
@@ -92,6 +157,7 @@ def initialize
           Sentry::Rails::Tracing::ActiveRecordSubscriber,
           Sentry::Rails::Tracing::ActiveStorageSubscriber
         ])
+        @active_support_logger_subscription_items = Sentry::Rails::ACTIVE_SUPPORT_LOGGER_SUBSCRIPTION_ITEMS_DEFAULT.dup
       end
     end
   end

diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb
@@ -89,7 +89,7 @@ def patch_background_worker
     def inject_breadcrumbs_logger
       if Sentry.configuration.breadcrumbs_logger.include?(:active_support_logger)
         require 'sentry/rails/breadcrumb/active_support_logger'
-        Sentry::Rails::Breadcrumb::ActiveSupportLogger.inject
+        Sentry::Rails::Breadcrumb::ActiveSupportLogger.inject(Sentry.configuration.rails.active_support_logger_subscription_items)
       end
 
       if Sentry.configuration.breadcrumbs_logger.include?(:monotonic_active_support_logger)

diff --git a/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb b/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb
@@ -65,6 +65,30 @@
 
       expect(breadcrumb_buffer.count).to be_zero
     end
+
+    context "with modified items" do
+      before { Sentry.configuration.rails.active_support_logger_subscription_items["process_action.action_controller"].delete(:controller) }
+      after { Sentry.configuration.rails.active_support_logger_subscription_items["process_action.action_controller"] << :controller }
+
+      it "breadcrumb data only contains parameters setted by rails config" do
+        Sentry.configuration.rails.active_support_logger_subscription_items["process_action.action_controller"].delete(:controller)
+
+        get "/exception"
+
+        breadcrumbs = event.dig("breadcrumbs", "values")
+        breadcrumb = breadcrumbs.detect { |b| b["category"] == "process_action.action_controller" }
+
+        expect(breadcrumb["data"]).to include(
+          {
+            "action" => "exception",
+            "params" => { "controller" => "hello", "action" => "exception" },
+            "format" => "html",
+            "method" => "GET", "path" => "/exception",
+          }
+        )
+        expect(breadcrumb["data"].keys).not_to include("controller")
+      end
+    end
   end
 
   context "with tracing" do

diff --git a/sentry-rails/spec/sentry/rails/configuration_spec.rb b/sentry-rails/spec/sentry/rails/configuration_spec.rb
@@ -36,4 +36,25 @@ class MySubscriber; end
       expect(subject.tracing_subscribers.size).to eq(1)
     end
   end
+
+  describe "#active_support_logger_subscription_items" do
+    it "returns the default active support logger subscription items" do
+      expect(subject.active_support_logger_subscription_items.keys.size).to eq(47)
+    end
+
+    it "is customizable" do
+      subject.active_support_logger_subscription_items["foo"] = %i[bar]
+      expect(subject.active_support_logger_subscription_items.keys.size).to eq(48)
+
+      subject.active_support_logger_subscription_items["process_action.action_controller"] << :bar
+      expect(subject.active_support_logger_subscription_items["process_action.action_controller"]).to include(:bar)
+    end
+
+    it "is replaceable" do
+      subject.active_support_logger_subscription_items = { "foo" => %i[bar] }
+
+      expect(subject.active_support_logger_subscription_items.keys.size).to eq(1)
+      expect(subject.active_support_logger_subscription_items["foo"]).to include(:bar)
+    end
+  end
 end