From 6439bae4ce6c611166330cb42d69c6dfd98b5cae Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Sun, 1 Oct 2023 12:16:45 +0100 Subject: [PATCH 1/2] Adopt Rails 7.1's new BroadcastLogger In https://github.com/rails/rails/pull/48615, Rails 7.1 introduced a new BroadcastLogger class that allows users to send logs to multiple loggers, which means we need to adjust the SDK's logger assignment for it. --- sentry-rails/.gitignore | 2 +- .../lib/sentry/rails/configuration.rb | 7 ++++- sentry-rails/spec/sentry/rails_spec.rb | 29 +++++++++++++++---- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/sentry-rails/.gitignore b/sentry-rails/.gitignore index d6c93fe50..e8acda211 100644 --- a/sentry-rails/.gitignore +++ b/sentry-rails/.gitignore @@ -5,7 +5,7 @@ /doc/ /pkg/ /spec/reports/ -/spec/dummy/test_rails_app/db +/spec/dummy/test_rails_app/db* /tmp/ # rspec failure tracking diff --git a/sentry-rails/lib/sentry/rails/configuration.rb b/sentry-rails/lib/sentry/rails/configuration.rb index f5ae8b116..e38d161bf 100644 --- a/sentry-rails/lib/sentry/rails/configuration.rb +++ b/sentry-rails/lib/sentry/rails/configuration.rb @@ -12,7 +12,12 @@ class Configuration @excluded_exceptions = @excluded_exceptions.concat(Sentry::Rails::IGNORE_DEFAULT) if ::Rails.logger - @logger = ::Rails.logger.dup + if ::Rails.logger.respond_to?(:broadcasts) + dupped_broadcasts = ::Rails.logger.broadcasts.map(&:dup) + @logger = ::ActiveSupport::BroadcastLogger.new(*dupped_broadcasts) + else + @logger = ::Rails.logger.dup + end else @logger.warn(Sentry::LOGGER_PROGNAME) do <<~MSG diff --git a/sentry-rails/spec/sentry/rails_spec.rb b/sentry-rails/spec/sentry/rails_spec.rb index da36a6dde..5d2e35a6e 100644 --- a/sentry-rails/spec/sentry/rails_spec.rb +++ b/sentry-rails/spec/sentry/rails_spec.rb @@ -14,6 +14,12 @@ make_basic_app end + after do + # We need to cleanup Rails.logger because after https://github.com/rails/rails/pull/49417 + # Rails.logger could get recreated with the previous logger value and become deeply nested broadcast logger + Rails.logger = nil + end + it "has version set" do expect(described_class::VERSION).to be_a(String) end @@ -36,11 +42,24 @@ describe "logger detection" do it "sets a duplicated Rails logger as the SDK's logger" do - expect(Sentry.configuration.logger).to be_a(ActiveSupport::Logger) - Sentry.configuration.logger.level = ::Logger::WARN - # Configuring the SDK's logger should not affect the Rails logger - expect(Rails.logger.level).to eq(::Logger::DEBUG) - expect(Sentry.configuration.logger.level).to eq(::Logger::WARN) + if Gem::Version.new(Rails.version) > Gem::Version.new("7.1.0.beta") + expect(Sentry.configuration.logger).to be_a(ActiveSupport::BroadcastLogger) + + Sentry.configuration.logger.level = ::Logger::WARN + + # Configuring the SDK's logger should not affect the Rails logger + expect(Rails.logger.broadcasts.first).to be_a(ActiveSupport::Logger) + expect(Rails.logger.broadcasts.first.level).to eq(::Logger::DEBUG) + expect(Sentry.configuration.logger.level).to eq(::Logger::WARN) + else + expect(Sentry.configuration.logger).to be_a(ActiveSupport::Logger) + + Sentry.configuration.logger.level = ::Logger::WARN + + # Configuring the SDK's logger should not affect the Rails logger + expect(Rails.logger.level).to eq(::Logger::DEBUG) + expect(Sentry.configuration.logger.level).to eq(::Logger::WARN) + end end it "respects the logger set by user" do From 821cb947708a2d6270e1415a1980f12577078788 Mon Sep 17 00:00:00 2001 From: Stan Lo Date: Sun, 1 Oct 2023 12:20:06 +0100 Subject: [PATCH 2/2] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e04b3d764..930c29c98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Features - Record client reports for profiles [#2107](https://github.com/getsentry/sentry-ruby/pull/2107) +- Adopt Rails 7.1's new BroadcastLogger [#2120](https://github.com/getsentry/sentry-ruby/pull/2120) ### Bug Fixes