diff --git a/CHANGELOG.md b/CHANGELOG.md index bc58203ab..82e618efc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Returning a hash from `before_send` and `before_send_transaction` is no longer supported and will drop the event. - Remove stacktrace trimming ([#2714](https://github.com/getsentry/sentry-ruby/pull/2714)) - `config.enabled_environments` now defaults to `nil` instead of `[]` for sending to all environments ([#2716](https://github.com/getsentry/sentry-ruby/pull/2716)) +- Remove `:monotonic_active_support_logger` from `config.breadcrumbs_logger` ([#2717](https://github.com/getsentry/sentry-ruby/pull/2717)) ### Internal diff --git a/sentry-rails/lib/sentry/rails/breadcrumb/monotonic_active_support_logger.rb b/sentry-rails/lib/sentry/rails/breadcrumb/monotonic_active_support_logger.rb deleted file mode 100644 index 541f9bea3..000000000 --- a/sentry-rails/lib/sentry/rails/breadcrumb/monotonic_active_support_logger.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -require "sentry/rails/instrument_payload_cleanup_helper" - -module Sentry - module Rails - module Breadcrumb - module MonotonicActiveSupportLogger - class << self - include InstrumentPayloadCleanupHelper - - def add(name, started, _finished, _unique_id, data) - # skip Rails' internal events - return if name.start_with?("!") - - if data.is_a?(Hash) - # we should only mutate the copy of the data - data = data.dup - cleanup_data(data) - end - - crumb = Sentry::Breadcrumb.new( - data: data, - category: name, - timestamp: started.to_i - ) - Sentry.add_breadcrumb(crumb) - end - - def inject - @subscriber = ::ActiveSupport::Notifications.monotonic_subscribe(/.*/) do |name, started, finished, unique_id, data| - # we only record events that has a float as started timestamp - if started.is_a?(Float) - add(name, started, finished, unique_id, data) - end - end - end - - def detach - ::ActiveSupport::Notifications.unsubscribe(@subscriber) - end - end - end - end - end -end diff --git a/sentry-rails/lib/sentry/rails/railtie.rb b/sentry-rails/lib/sentry/rails/railtie.rb index 0691fa277..5cbd69523 100644 --- a/sentry-rails/lib/sentry/rails/railtie.rb +++ b/sentry-rails/lib/sentry/rails/railtie.rb @@ -101,17 +101,12 @@ def patch_background_worker end def inject_breadcrumbs_logger - if Sentry.configuration.breadcrumbs_logger.include?(:active_support_logger) + if Sentry.configuration.breadcrumbs_logger.include?(:active_support_logger) || + ## legacy name redirected for backwards compat + Sentry.configuration.breadcrumbs_logger.include?(:monotonic_active_support_logger) require "sentry/rails/breadcrumb/active_support_logger" Sentry::Rails::Breadcrumb::ActiveSupportLogger.inject(Sentry.configuration.rails.active_support_logger_subscription_items) end - - if Sentry.configuration.breadcrumbs_logger.include?(:monotonic_active_support_logger) - return warn "Usage of `monotonic_active_support_logger` require a version of Rails >= 6.1, please upgrade your Rails version or use another logger" if ::Rails.version.to_f < 6.1 - - require "sentry/rails/breadcrumb/monotonic_active_support_logger" - Sentry::Rails::Breadcrumb::MonotonicActiveSupportLogger.inject - end end def setup_backtrace_cleanup_callback 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 index b133d8f47..2fb493793 100644 --- a/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb +++ b/sentry-rails/spec/sentry/rails/breadcrumbs/active_support_logger_spec.rb @@ -93,6 +93,33 @@ end end + context "redirects legacy :monotonic_active_support_logger" do + before do + make_basic_app do |sentry_config| + sentry_config.breadcrumbs_logger = [:monotonic_active_support_logger] + end + end + + it "captures correct data of exception requests" do + get "/exception" + + expect(response.status).to eq(500) + breadcrumbs = event.dig("breadcrumbs", "values") + expect(breadcrumbs.count).to eq(2) + + breadcrumb = breadcrumbs.detect { |b| b["category"] == "process_action.action_controller" } + expect(breadcrumb["data"]).to include( + { + "controller" => "HelloController", + "action" => "exception", + "params" => { "controller" => "hello", "action" => "exception" }, + "format" => "html", + "method" => "GET", "path" => "/exception" + } + ) + end + end + context "with tracing" do before do make_basic_app do |sentry_config| diff --git a/sentry-rails/spec/sentry/rails/breadcrumbs/monotonic_active_support_logger_spec.rb b/sentry-rails/spec/sentry/rails/breadcrumbs/monotonic_active_support_logger_spec.rb deleted file mode 100644 index 11bb0cbaa..000000000 --- a/sentry-rails/spec/sentry/rails/breadcrumbs/monotonic_active_support_logger_spec.rb +++ /dev/null @@ -1,145 +0,0 @@ -# frozen_string_literal: true - -require "spec_helper" - - -RSpec.describe "Sentry::Breadcrumbs::MonotonicActiveSupportLogger", type: :request do - after do - # even though we cleanup breadcrumbs in the rack middleware - # Breadcrumbs::MonotonicActiveSupportLogger subscribes to "every" instrumentation - # so it'll create other instrumentations "after" the request is finished - # and we should clear those as well - Sentry.get_current_scope.clear_breadcrumbs - end - - let(:transport) do - Sentry.get_current_client.transport - end - - let(:breadcrumb_buffer) do - Sentry.get_current_scope.breadcrumbs - end - - let(:event) do - transport.events.first.to_json_compatible - end - context "without tracing" do - before do - make_basic_app do |sentry_config| - sentry_config.breadcrumbs_logger = [:monotonic_active_support_logger] - end - end - - context "given a Rails version < 6.1", skip: Rails.version.to_f >= 6.1 do - it "does not run instrumentation" do - get "/exception" - - breadcrumbs = event.dig("breadcrumbs", "values") - expect(breadcrumbs.count).to be_zero - end - end - - context "given a Rails version >= 6.1", skip: Rails.version.to_f < 6.1 do - after do - Sentry::Rails::Breadcrumb::MonotonicActiveSupportLogger.detach - end - - it "captures correct data of exception requests" do - get "/exception" - - expect(response.status).to eq(500) - breadcrumbs = event.dig("breadcrumbs", "values") - expect(breadcrumbs.count).to eq(2) - - breadcrumb = breadcrumbs.detect { |b| b["category"] == "process_action.action_controller" } - expect(breadcrumb["data"]).to include( - { - "controller" => "HelloController", - "action" => "exception", - "params" => { "controller" => "hello", "action" => "exception" }, - "format" => "html", - "method" => "GET", "path" => "/exception" - } - ) - expect(breadcrumb["data"].keys).not_to include("headers") - expect(breadcrumb["data"].keys).not_to include("request") - expect(breadcrumb["data"].keys).not_to include("response") - end - - it "ignores exception data" do - get "/view_exception" - - expect(event.dig("breadcrumbs", "values", -1, "data").keys).not_to include("exception") - expect(event.dig("breadcrumbs", "values", -1, "data").keys).not_to include("exception_object") - end - - it "ignores events that doesn't have a float as started attributes" do - expect do - ActiveSupport::Notifications.publish "foo", Time.now - end.not_to raise_error - - expect(breadcrumb_buffer.count).to be_zero - end - end - end - - context "with tracing" do - before do - make_basic_app do |sentry_config| - sentry_config.breadcrumbs_logger = [:monotonic_active_support_logger] - sentry_config.traces_sample_rate = 1.0 - end - end - - context "given a Rails version < 6.1", skip: Rails.version.to_f >= 6.1 do - it "does not run instrumentation" do - get "/exception" - - breadcrumbs = event.dig("breadcrumbs", "values") - expect(breadcrumbs.count).to be_zero - end - end - - context "given a Rails version >= 6.1", skip: Rails.version.to_f < 6.1 do - after do - Sentry::Rails::Breadcrumb::MonotonicActiveSupportLogger.detach - end - - it "captures correct request data of normal requests" do - p = Post.create! - - get "/posts/#{p.id}" - - breadcrumbs = event.dig("breadcrumbs", "values") - - breadcrumb = breadcrumbs.detect { |b| b["category"] == "process_action.action_controller" } - expect(breadcrumb["data"]).to include( - { - "controller" => "PostsController", - "action" => "show", - "params" => { "controller" => "posts", "action" => "show", "id" => p.id.to_s }, - "format" => "html", - "method" => "GET", "path" => "/posts/#{p.id}" - } - ) - expect(breadcrumb["data"].keys).not_to include("headers") - expect(breadcrumb["data"].keys).not_to include("request") - expect(breadcrumb["data"].keys).not_to include("response") - end - - it "doesn't add internal start timestamp payload to breadcrumbs data" do - p = Post.create! - - get "/posts/#{p.id}" - - expect(transport.events.count).to eq(1) - - transaction = transport.events.last.to_h - breadcrumbs = transaction[:breadcrumbs][:values] - process_action_crumb = breadcrumbs.last - expect(process_action_crumb[:category]).to eq("process_action.action_controller") - expect(process_action_crumb[:data].has_key?(Sentry::Rails::Tracing::START_TIMESTAMP_NAME)).to eq(false) - end - end - end -end diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 09b27d8d6..4bdfad6c9 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -122,7 +122,6 @@ class Configuration # # And if you also use sentry-rails: # - :active_support_logger - # - :monotonic_active_support_logger # # @return [Array] attr_reader :breadcrumbs_logger