Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveSupport notifications: allowlist event names #2610

Merged
merged 6 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/new_relic/agent/configuration/default_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

require 'forwardable'
require_relative '../../constants'
require_relative '../instrumentation/active_support_subscriber'

module NewRelic
module Agent
Expand Down Expand Up @@ -1415,6 +1416,19 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
:description => 'Configures the TCP/IP port for the trace observer Host'
},
# Instrumentation
:'instrumentation.active_support_notifications.active_support_events' => {
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
:default => NewRelic::Agent::Instrumentation::ActiveSupportSubscriber::EVENT_NAME_TO_METHOD_NAME.keys,
:public => true,
:type => Array,
:allowed_from_server => false,
:description => <<~ACTIVE_SUPPORT_EVENTS.chomp.tr("\n", ' ')
An allowlist array of Active Support notifications events specific to the Active Support library
itself that the agent should subscribe to. The Active Support library specific events focus primarily
on caching. Any event name not included in this list will be ignored by the agent. Provide complete event
names such as 'cache_fetch_hit.active_support'. Do not provide asterisks or regex patterns, and do not
escape any characters with backslashes.
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
ACTIVE_SUPPORT_EVENTS
},
:'instrumentation.active_support_broadcast_logger' => {
:default => instrumentation_value_from_boolean(:'application_logging.enabled'),
:documentation_default => 'auto',
Expand Down
9 changes: 8 additions & 1 deletion lib/new_relic/agent/instrumentation/active_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
DependencyDetection.defer do
named :active_support

EVENT_NAMES_PARAMETER = :'instrumentation.active_support_notifications.active_support_events'

depends_on do
!NewRelic::Agent.config[:disable_active_support]
end
Expand All @@ -16,12 +18,17 @@
!NewRelic::Agent::Instrumentation::ActiveSupportSubscriber.subscribed?
end

depends_on do
!NewRelic::Agent.config[EVENT_NAMES_PARAMETER].empty?
end

executes do
NewRelic::Agent.logger.info('Installing ActiveSupport instrumentation')
end

executes do
ActiveSupport::Notifications.subscribe(/\.active_support$/,
event_names = NewRelic::Agent.config[EVENT_NAMES_PARAMETER].map { |n| Regexp.escape(n) }.join('|')
fallwith marked this conversation as resolved.
Show resolved Hide resolved
ActiveSupport::Notifications.subscribe(/\A(?:#{event_names})\z/,
NewRelic::Agent::Instrumentation::ActiveSupportSubscriber.new)
end
end
31 changes: 15 additions & 16 deletions lib/new_relic/agent/instrumentation/active_support_subscriber.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,25 @@
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

require 'new_relic/agent/instrumentation/notifications_subscriber'
require_relative 'notifications_subscriber'

module NewRelic
module Agent
module Instrumentation
class ActiveSupportSubscriber < NotificationsSubscriber
EVENT_NAME_TO_METHOD_NAME = {
'cache_fetch_hit.active_support' => 'fetch_hit',
'cache_generate.active_support' => 'generate',
'cache_read.active_support' => 'read',
'cache_write.active_support' => 'write',
'cache_delete.active_support' => 'delete',
'cache_exist?.active_support' => 'exist?',
'cache_read_multi.active_support' => 'read_multi',
'cache_write_multi.active_support' => 'write_multi',
'cache_delete_multi.active_support' => 'delete_multi',
'message_serializer_fallback.active_support' => 'message_serializer_fallback'
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
}.freeze

def add_segment_params(segment, payload)
segment.params[:key] = payload[:key]
segment.params[:store] = payload[:store]
Expand All @@ -18,23 +31,9 @@ def add_segment_params(segment, payload)

def metric_name(name, payload)
store = payload[:store]
method = method_from_name(name)
method = EVENT_NAME_TO_METHOD_NAME.fetch(name, name.delete_prefix('cache_').delete_suffix('.active_support'))
fallwith marked this conversation as resolved.
Show resolved Hide resolved
"Ruby/ActiveSupport#{"/#{store}" if store}/#{method}"
end

PATTERN = /\Acache_([^\.]*)\.active_support\z/

METHOD_NAME_MAPPING = Hash.new do |h, k|
if PATTERN =~ k
h[k] = $1
else
h[k] = NewRelic::UNKNOWN
end
end

def method_from_name(name)
METHOD_NAME_MAPPING[name]
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,12 @@ def test_metric_recorded_for_new_event_names
end

def test_failsafe_if_event_does_not_match_expected_pattern
name = 'charcuterie_build_a_board_workshop'
in_transaction('test') do
generate_event('charcuterie_build_a_board_workshop')
generate_event(name)
end

assert_metrics_recorded "#{METRIC_PREFIX}#{DEFAULT_STORE}/Unknown"
assert_metrics_recorded "#{METRIC_PREFIX}#{DEFAULT_STORE}/#{name}"
end

def test_key_recorded_as_attribute_on_traces
Expand Down
Loading