Skip to content

Commit

Permalink
Merge pull request newrelic#2246 from newrelic/skip-active-support-lo…
Browse files Browse the repository at this point in the history
…gger-instrumentation-for-rails-71

Skip Active Support Logger instrumentation for Rails 7.1
  • Loading branch information
kaylareopelle authored Oct 9, 2023
2 parents 9cb982b + 257aeb3 commit 4c4cfd0
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 4 deletions.
10 changes: 9 additions & 1 deletion lib/new_relic/agent/instrumentation/active_support_logger.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@
DependencyDetection.defer do
named :active_support_logger

depends_on { defined?(ActiveSupport::Logger) }
depends_on do
defined?(ActiveSupport::Logger) &&
# TODO: Rails 7.1 - ActiveSupport::Logger#broadcast method removed
# APM logs-in-context automatic forwarding still works, but sends
# log events for each broadcasted logger, often causing duplicates
# Issue #2245
defined?(Rails::VERSION::STRING) &&
Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new('7.1.0')
end

executes do
NewRelic::Agent.logger.info('Installing ActiveSupport::Logger instrumentation')
Expand Down
14 changes: 11 additions & 3 deletions test/multiverse/suites/rails/active_support_logger_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,23 +9,31 @@ class ActiveSupportLoggerTest < Minitest::Test
include MultiverseHelpers
setup_and_teardown_agent

def rails_7_1?
Gem::Version.new(::Rails::VERSION::STRING) >= Gem::Version.new('7.1.0')
end

def setup
@output = StringIO.new
@logger = Logger.new(@output)
@broadcasted_output = StringIO.new
@broadcasted_logger = ActiveSupport::Logger.new(@broadcasted_output)
@logger.extend(ActiveSupport::Logger.broadcast(@broadcasted_logger))
@logger.extend(ActiveSupport::Logger.broadcast(@broadcasted_logger)) unless rails_7_1?

@aggregator = NewRelic::Agent.agent.log_event_aggregator
@aggregator.reset!
end

def test_broadcasted_logger_marked_skip_instrumenting
assert @broadcasted_logger.instance_variable_get(:@skip_instrumenting)
assert_nil @logger.instance_variable_get(:@skip_instrumenting)
skip 'Rails 7.1. Active Support Logger instrumentation broken, see #2245' if rails_7_1?

assert @broadcasted_logger.instance_variable_get(:@skip_instrumenting), 'Broadcasted logger not set with @skip_instrumenting'
assert_nil @logger.instance_variable_get(:@skip_instrumenting), 'Logger has @skip_instrumenting defined'
end

def test_logs_not_forwarded_by_broadcasted_logger
skip 'Rails 7.1. Active Support Logger instrumentation broken, see #2245' if rails_7_1?

message = 'Can you hear me, Major Tom?'

@logger.add(Logger::DEBUG, message)
Expand Down

0 comments on commit 4c4cfd0

Please sign in to comment.