From 283279bc9c1da831e2110ad38eef9c3403bf3bba Mon Sep 17 00:00:00 2001 From: hramadan Date: Tue, 25 Jul 2023 14:00:50 -0700 Subject: [PATCH 01/31] Initial commit Roda instrumentation Adds prepend and chaining options for Roda v3.19.0+ --- .../agent/configuration/default_source.rb | 16 ++++++ .../controller_instrumentation.rb | 1 + lib/new_relic/agent/instrumentation/roda.rb | 40 ++++++++++++++ .../agent/instrumentation/roda/chain.rb | 45 ++++++++++++++++ .../instrumentation/roda/instrumentation.rb | 54 +++++++++++++++++++ .../agent/instrumentation/roda/prepend.rb | 24 +++++++++ .../roda/roda_transaction_namer.rb | 38 +++++++++++++ lib/new_relic/agent/transaction.rb | 3 +- lib/new_relic/control/frameworks/roda.rb | 20 +++++++ newrelic.yml | 4 ++ test/multiverse/suites/roda/Envfile | 9 ++++ .../suites/roda/config/newrelic.yml | 19 +++++++ .../suites/roda/roda_instrumentation_test.rb | 15 ++++++ 13 files changed, 287 insertions(+), 1 deletion(-) create mode 100644 lib/new_relic/agent/instrumentation/roda.rb create mode 100644 lib/new_relic/agent/instrumentation/roda/chain.rb create mode 100644 lib/new_relic/agent/instrumentation/roda/instrumentation.rb create mode 100644 lib/new_relic/agent/instrumentation/roda/prepend.rb create mode 100644 lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb create mode 100644 lib/new_relic/control/frameworks/roda.rb create mode 100644 test/multiverse/suites/roda/Envfile create mode 100644 test/multiverse/suites/roda/config/newrelic.yml create mode 100644 test/multiverse/suites/roda/roda_instrumentation_test.rb diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index a8d67d7fb8..ef20c701b7 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -115,6 +115,7 @@ def self.framework :rails_notifications end when defined?(::Sinatra) && defined?(::Sinatra::Base) then :sinatra + when defined?(::Roda) then :roda when defined?(::NewRelic::IA) then :external else :ruby end @@ -1227,6 +1228,13 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'If `true`, disables [Sidekiq instrumentation](/docs/agents/ruby-agent/background-jobs/sidekiq-instrumentation).' }, + :disable_roda_auto_middleware => { + :default => false, + :public => true, + :type => Boolean, + :allowed_from_server => false, + :description => 'If `true`, disables agent middleware for Roda. This middleware is responsible for advanced feature support such as [page load timing](/docs/browser/new-relic-browser/getting-started/new-relic-browser) and [error collection](/docs/apm/applications-menu/events/view-apm-error-analytics).' + }, :disable_sinatra_auto_middleware => { :default => false, :public => true, @@ -1564,6 +1572,14 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'Controls auto-instrumentation of resque at start up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' }, + :'instrumentation.roda' => { + :default => 'auto', + :public => true, + :type => String, + :dynamic_name => true, + :allowed_from_server => false, + :description => 'Controls auto-instrumentation of Roda at start up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' + }, :'instrumentation.sinatra' => { :default => 'auto', :public => true, diff --git a/lib/new_relic/agent/instrumentation/controller_instrumentation.rb b/lib/new_relic/agent/instrumentation/controller_instrumentation.rb index 90e77bdc5d..db3a6a634d 100644 --- a/lib/new_relic/agent/instrumentation/controller_instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/controller_instrumentation.rb @@ -245,6 +245,7 @@ def self.prefix_for_category(txn, category = nil) when :background then ::NewRelic::Agent::Transaction::TASK_PREFIX when :rack then ::NewRelic::Agent::Transaction::RACK_PREFIX when :uri then ::NewRelic::Agent::Transaction::CONTROLLER_PREFIX + when :roda then ::NewRelic::Agent::Transaction::RODA_PREFIX when :sinatra then ::NewRelic::Agent::Transaction::SINATRA_PREFIX when :middleware then ::NewRelic::Agent::Transaction::MIDDLEWARE_PREFIX when :grape then ::NewRelic::Agent::Transaction::GRAPE_PREFIX diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb new file mode 100644 index 0000000000..0b46763fbe --- /dev/null +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -0,0 +1,40 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +require_relative 'roda/instrumentation' +require_relative 'roda/chain' +require_relative 'roda/prepend' + +DependencyDetection.defer do + named :roda + + depends_on do + defined?(Roda) && + Roda::RodaVersion >= '3.19.0' && + Roda::RodaPlugins::Base::ClassMethods.private_method_defined?(:build_rack_app) && + Roda::RodaPlugins::Base::InstanceMethods.method_defined?(:_roda_handle_main_route) + end + + executes do + # These requires are inside an executes block because they require rack, and + # we can't be sure that rack is available when this file is first required. + require 'new_relic/rack/agent_hooks' + require 'new_relic/rack/browser_monitoring' + if use_prepend? + prepend_instrument Roda.singleton_class, NewRelic::Agent::Instrumentation::Roda::Build::Prepend + else + chain_instrument NewRelic::Agent::Instrumentation::Roda::Build::Chain + end + end + + executes do + NewRelic::Agent.logger.info('Installing roda instrumentation') + + if use_prepend? + prepend_instrument Roda, NewRelic::Agent::Instrumentation::Roda::Prepend + else + chain_instrument NewRelic::Agent::Instrumentation::Roda::Chain + end + end +end diff --git a/lib/new_relic/agent/instrumentation/roda/chain.rb b/lib/new_relic/agent/instrumentation/roda/chain.rb new file mode 100644 index 0000000000..e0af65910f --- /dev/null +++ b/lib/new_relic/agent/instrumentation/roda/chain.rb @@ -0,0 +1,45 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic::Agent::Instrumentation + module Roda + module Chain + def self.instrument! + ::Roda.class_eval do + include ::NewRelic::Agent::Instrumentation::Roda::Tracer + + alias_method(:_roda_handle_main_route_without_tracing, :_roda_handle_main_route) + alias_method(:_roda_handle_main_route, :_roda_handle_main_route_with_tracing) + + def _roda_handle_main_route(*args) + _roda_handle_main_route_with_tracing(*args) do + _roda_handle_main_route_without_tracing(*args) + end + end + end + end + end + + module Build + module Chain + def self.instrument! + ::Roda.class_eval do + include ::NewRelic::Agent::Instrumentation::Roda::Tracer + + class << self + alias_method(:build_rack_app_without_tracing, :build_rack_app) + alias_method(:build_rack_app, :build_rack_app_with_tracing) + + def build_rack_app + build_rack_app_with_tracing do + build_rack_app_without_tracing + end + end + end + end + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb new file mode 100644 index 0000000000..25b033feb4 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -0,0 +1,54 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic::Agent::Instrumentation + module Roda + module Tracer + include ::NewRelic::Agent::Instrumentation::ControllerInstrumentation + + def self.included(clazz) + clazz.extend(self) + end + + def newrelic_middlewares + middlewares = [NewRelic::Rack::BrowserMonitoring] + if NewRelic::Rack::AgentHooks.needed? + middlewares << NewRelic::Rack::AgentHooks + end + middlewares + end + + def build_rack_app_with_tracing + unless NewRelic::Agent.config[:disable_roda_auto_middleware] + newrelic_middlewares.each do |middleware_class| + self.use middleware_class + end + end + yield + end + + # Roda makes use of Rack, so we can get params from the request object + def rack_request_params + begin + @_request.params + rescue => e + NewRelic::Agent.logger.debug('Failed to get params from Rack request.', e) + nil + end + end + + def _roda_handle_main_route_with_tracing(*args) + request_params = rack_request_params + filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params || {}) + name = TransactionNamer.initial_transaction_name(request) + + perform_action_with_newrelic_trace(:category => :roda, + :name => name, + :params => filtered_params) do + yield + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/roda/prepend.rb b/lib/new_relic/agent/instrumentation/roda/prepend.rb new file mode 100644 index 0000000000..0da5f44af2 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/roda/prepend.rb @@ -0,0 +1,24 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic::Agent::Instrumentation + module Roda + module Prepend + include ::NewRelic::Agent::Instrumentation::Roda::Tracer + + def _roda_handle_main_route(*args) + _roda_handle_main_route_with_tracing(*args) { super } + end + end + + module Build + module Prepend + include ::NewRelic::Agent::Instrumentation::Roda::Tracer + def build_rack_app + build_rack_app_with_tracing { super } + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb new file mode 100644 index 0000000000..e59c42092c --- /dev/null +++ b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb @@ -0,0 +1,38 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +module NewRelic + module Agent + module Instrumentation + module Roda + module TransactionNamer + extend self + + def initial_transaction_name(request) + transaction_name(::NewRelic::Agent::UNKNOWN_METRIC, request) + end + + ROOT = '/'.freeze + + def transaction_name(path, request) + verb = http_verb(request) + path = request.path if request.path + name = path.gsub(%r{^[/^\\A]*(.*?)[/\$\?\\z]*$}, '\1') # remove any rouge slashes + name = ROOT if name.empty? + name = "#{verb} #{name}" unless verb.nil? + + name + rescue => e + ::NewRelic::Agent.logger.debug("#{e.class} : #{e.message} - Error encountered trying to identify Roda transaction name") + ::NewRelic::Agent::UNKNOWN_METRIC + end + + def http_verb(request) + request.request_method if request.respond_to?(:request_method) + end + end + end + end + end +end diff --git a/lib/new_relic/agent/transaction.rb b/lib/new_relic/agent/transaction.rb index 2d6d8ce370..314a0b1076 100644 --- a/lib/new_relic/agent/transaction.rb +++ b/lib/new_relic/agent/transaction.rb @@ -31,11 +31,12 @@ class Transaction RAKE_PREFIX = "#{OTHER_TRANSACTION_PREFIX}Rake/" MESSAGE_PREFIX = "#{OTHER_TRANSACTION_PREFIX}Message/" RACK_PREFIX = "#{CONTROLLER_PREFIX}Rack/" + RODA_PREFIX = "#{CONTROLLER_PREFIX}Roda/" SINATRA_PREFIX = "#{CONTROLLER_PREFIX}Sinatra/" GRAPE_PREFIX = "#{CONTROLLER_PREFIX}Grape/" ACTION_CABLE_PREFIX = "#{CONTROLLER_PREFIX}ActionCable/" - WEB_TRANSACTION_CATEGORIES = [:web, :controller, :uri, :rack, :sinatra, :grape, :middleware, :action_cable].freeze + WEB_TRANSACTION_CATEGORIES = [:web, :controller, :uri, :rack, :sinatra, :grape, :middleware, :action_cable, :roda].freeze MIDDLEWARE_SUMMARY_METRICS = ['Middleware/all'].freeze WEB_SUMMARY_METRIC = 'HttpDispatcher' diff --git a/lib/new_relic/control/frameworks/roda.rb b/lib/new_relic/control/frameworks/roda.rb new file mode 100644 index 0000000000..09a6010786 --- /dev/null +++ b/lib/new_relic/control/frameworks/roda.rb @@ -0,0 +1,20 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +require 'new_relic/control/frameworks/ruby' +module NewRelic + class Control + module Frameworks + # Contains basic control logic for Roda + class Roda < NewRelic::Control::Frameworks::Ruby + protected + + def install_shim + super + ::Roda.class_eval { include NewRelic::Agent::Instrumentation::ControllerInstrumentation::Shim } + end + end + end + end +end diff --git a/newrelic.yml b/newrelic.yml index 1403250495..278d45bfd3 100644 --- a/newrelic.yml +++ b/newrelic.yml @@ -372,6 +372,10 @@ common: &default_settings # prepend, chain, disabled. # instrumentation.bunny: auto + # Controls auto-instrumentation of roda at start up. + # May be one of [auto|prepend|chain|disabled] + # instrumentation.roda: auto + # Controls auto-instrumentation of the concurrent-ruby library at start up. May be # one of: auto, prepend, chain, disabled. # instrumentation.concurrent_ruby: auto diff --git a/test/multiverse/suites/roda/Envfile b/test/multiverse/suites/roda/Envfile new file mode 100644 index 0000000000..644cd8746d --- /dev/null +++ b/test/multiverse/suites/roda/Envfile @@ -0,0 +1,9 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +instrumentation_methods :chain, :prepend + +gemfile <<~RB + gem 'roda' +RB diff --git a/test/multiverse/suites/roda/config/newrelic.yml b/test/multiverse/suites/roda/config/newrelic.yml new file mode 100644 index 0000000000..df405d6a86 --- /dev/null +++ b/test/multiverse/suites/roda/config/newrelic.yml @@ -0,0 +1,19 @@ +--- +development: + error_collector: + enabled: true + apdex_t: 0.5 + monitor_mode: true + license_key: bootstrap_newrelic_admin_license_key_000 + instrumentation: + roda: <%= $instrumentation_method %> + app_name: test + log_level: debug + host: 127.0.0.1 + api_host: 127.0.0.1 + transaction_trace: + record_sql: obfuscated + enabled: true + stack_trace_threshold: 0.5 + transaction_threshold: 1.0 + capture_params: false diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb new file mode 100644 index 0000000000..65725eb37c --- /dev/null +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -0,0 +1,15 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +class RodaInstrumentationTest < Minitest::Test + def setup + @stats_engine = NewRelic::Agent.instance.stats_engine + end + + def teardown + NewRelic::Agent.instance.stats_engine.clear_stats + end + + # Add tests here +end From 64b6ac9d049072266d57446458b21dd294d49e3c Mon Sep 17 00:00:00 2001 From: hramadan Date: Tue, 25 Jul 2023 15:45:57 -0700 Subject: [PATCH 02/31] disable Lint/DuplicateMethods --- lib/new_relic/agent/instrumentation/roda/chain.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/new_relic/agent/instrumentation/roda/chain.rb b/lib/new_relic/agent/instrumentation/roda/chain.rb index e0af65910f..c27a7c46fd 100644 --- a/lib/new_relic/agent/instrumentation/roda/chain.rb +++ b/lib/new_relic/agent/instrumentation/roda/chain.rb @@ -2,6 +2,7 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true +# rubocop:disable Lint/DuplicateMethods module NewRelic::Agent::Instrumentation module Roda module Chain @@ -43,3 +44,4 @@ def build_rack_app end end end +# rubocop:enable Lint/DuplicateMethods From 31a1ee2fe5cde45d56839b736eb986254426feff Mon Sep 17 00:00:00 2001 From: hramadan Date: Tue, 25 Jul 2023 21:01:32 -0700 Subject: [PATCH 03/31] Address feedback --- lib/new_relic/agent/instrumentation/roda.rb | 2 +- lib/new_relic/agent/instrumentation/roda/instrumentation.rb | 3 ++- .../agent/instrumentation/roda/roda_transaction_namer.rb | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 0b46763fbe..6c01cffe10 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -11,7 +11,7 @@ depends_on do defined?(Roda) && - Roda::RodaVersion >= '3.19.0' && + Gem::Version.new(Roda::RodaVersion) >= '3.19.0' && Roda::RodaPlugins::Base::ClassMethods.private_method_defined?(:build_rack_app) && Roda::RodaPlugins::Base::InstanceMethods.method_defined?(:_roda_handle_main_route) end diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index 25b033feb4..bec15ac30a 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -40,7 +40,8 @@ def rack_request_params def _roda_handle_main_route_with_tracing(*args) request_params = rack_request_params - filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params || {}) + filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params || + NewRelic::EMPTY_HASH) name = TransactionNamer.initial_transaction_name(request) perform_action_with_newrelic_trace(:category => :roda, diff --git a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb index e59c42092c..f733be0bf3 100644 --- a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb +++ b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb @@ -14,11 +14,12 @@ def initial_transaction_name(request) end ROOT = '/'.freeze + REGEX_MUTIPLE_SLASHES = %r{^[/^\\A]*(.*?)[/\$\?\\z]*$}.freeze def transaction_name(path, request) verb = http_verb(request) path = request.path if request.path - name = path.gsub(%r{^[/^\\A]*(.*?)[/\$\?\\z]*$}, '\1') # remove any rouge slashes + name = path.gsub(REGEX_MUTIPLE_SLASHES, '\1') # remove any rogue slashes name = ROOT if name.empty? name = "#{verb} #{name}" unless verb.nil? From 4612fe854111661393c738e0c8ca45dd0b1f2333 Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 26 Jul 2023 11:05:32 -0700 Subject: [PATCH 04/31] Remove uncessary alias --- .../agent/instrumentation/roda/chain.rb | 4 ---- test/multiverse/suites/roda/Envfile | 9 -------- test/multiverse/suites/roda/Envfile.rb | 21 +++++++++++++++++++ 3 files changed, 21 insertions(+), 13 deletions(-) delete mode 100644 test/multiverse/suites/roda/Envfile create mode 100644 test/multiverse/suites/roda/Envfile.rb diff --git a/lib/new_relic/agent/instrumentation/roda/chain.rb b/lib/new_relic/agent/instrumentation/roda/chain.rb index c27a7c46fd..e96b28a079 100644 --- a/lib/new_relic/agent/instrumentation/roda/chain.rb +++ b/lib/new_relic/agent/instrumentation/roda/chain.rb @@ -2,7 +2,6 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -# rubocop:disable Lint/DuplicateMethods module NewRelic::Agent::Instrumentation module Roda module Chain @@ -11,7 +10,6 @@ def self.instrument! include ::NewRelic::Agent::Instrumentation::Roda::Tracer alias_method(:_roda_handle_main_route_without_tracing, :_roda_handle_main_route) - alias_method(:_roda_handle_main_route, :_roda_handle_main_route_with_tracing) def _roda_handle_main_route(*args) _roda_handle_main_route_with_tracing(*args) do @@ -30,7 +28,6 @@ def self.instrument! class << self alias_method(:build_rack_app_without_tracing, :build_rack_app) - alias_method(:build_rack_app, :build_rack_app_with_tracing) def build_rack_app build_rack_app_with_tracing do @@ -44,4 +41,3 @@ def build_rack_app end end end -# rubocop:enable Lint/DuplicateMethods diff --git a/test/multiverse/suites/roda/Envfile b/test/multiverse/suites/roda/Envfile deleted file mode 100644 index 644cd8746d..0000000000 --- a/test/multiverse/suites/roda/Envfile +++ /dev/null @@ -1,9 +0,0 @@ -# This file is distributed under New Relic's license terms. -# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. -# frozen_string_literal: true - -instrumentation_methods :chain, :prepend - -gemfile <<~RB - gem 'roda' -RB diff --git a/test/multiverse/suites/roda/Envfile.rb b/test/multiverse/suites/roda/Envfile.rb new file mode 100644 index 0000000000..c5c3983b1b --- /dev/null +++ b/test/multiverse/suites/roda/Envfile.rb @@ -0,0 +1,21 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +instrumentation_methods :chain, :prepend + +RODA_VERSIONS = [ + [nil, 2.4], + ['3.19.0', 2.4] +] + +def gem_list(roda_version = nil) + <<~RB + gem 'roda'#{roda_version} + gem 'rack', '~> 2.2' + gem 'rack-test', '>= 0.8.0', :require => 'rack/test' + + RB +end + +create_gemfiles(RODA_VERSIONS) From 6ee5ef9e2bfb60dc2f0a85ce6440df5fe7af550a Mon Sep 17 00:00:00 2001 From: hramadan Date: Tue, 1 Aug 2023 09:26:13 -0700 Subject: [PATCH 05/31] Refactor naming method --- .../agent/instrumentation/roda/instrumentation.rb | 2 +- .../agent/instrumentation/roda/roda_transaction_namer.rb | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index bec15ac30a..05b9981b2a 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -42,7 +42,7 @@ def _roda_handle_main_route_with_tracing(*args) request_params = rack_request_params filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params || NewRelic::EMPTY_HASH) - name = TransactionNamer.initial_transaction_name(request) + name = TransactionNamer.transaction_name(request) perform_action_with_newrelic_trace(:category => :roda, :name => name, diff --git a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb index f733be0bf3..56568b9816 100644 --- a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb +++ b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb @@ -9,16 +9,12 @@ module Roda module TransactionNamer extend self - def initial_transaction_name(request) - transaction_name(::NewRelic::Agent::UNKNOWN_METRIC, request) - end - ROOT = '/'.freeze REGEX_MUTIPLE_SLASHES = %r{^[/^\\A]*(.*?)[/\$\?\\z]*$}.freeze - def transaction_name(path, request) + def transaction_name(request) verb = http_verb(request) - path = request.path if request.path + path = request.path || ::NewRelic::Agent::UNKNOWN_METRIC name = path.gsub(REGEX_MUTIPLE_SLASHES, '\1') # remove any rogue slashes name = ROOT if name.empty? name = "#{verb} #{name}" unless verb.nil? From 05cc30fcd30879550fc8096e3825203bef891b37 Mon Sep 17 00:00:00 2001 From: hramadan Date: Tue, 1 Aug 2023 09:26:38 -0700 Subject: [PATCH 06/31] Roda test files --- test/multiverse/lib/multiverse/runner.rb | 2 +- test/multiverse/suites/roda/Envfile.rb | 35 ++++--- .../suites/roda/roda_instrumentation_test.rb | 92 ++++++++++++++++++- 3 files changed, 109 insertions(+), 20 deletions(-) diff --git a/test/multiverse/lib/multiverse/runner.rb b/test/multiverse/lib/multiverse/runner.rb index d67b079c86..ffa96e1055 100644 --- a/test/multiverse/lib/multiverse/runner.rb +++ b/test/multiverse/lib/multiverse/runner.rb @@ -103,7 +103,7 @@ def execute_suites(filter, opts) 'background_2' => ['rake'], 'database' => %w[elasticsearch mongo redis sequel], 'rails' => %w[active_record active_record_pg rails rails_prepend activemerchant], - 'frameworks' => %w[sinatra padrino grape], + 'frameworks' => %w[sinatra padrino grape roda], 'httpclients' => %w[curb excon httpclient], 'httpclients_2' => %w[typhoeus net_http httprb], 'infinite_tracing' => ['infinite_tracing'], diff --git a/test/multiverse/suites/roda/Envfile.rb b/test/multiverse/suites/roda/Envfile.rb index c5c3983b1b..4b6fcbab67 100644 --- a/test/multiverse/suites/roda/Envfile.rb +++ b/test/multiverse/suites/roda/Envfile.rb @@ -2,20 +2,27 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -instrumentation_methods :chain, :prepend +# instrumentation_methods :chain, :prepend -RODA_VERSIONS = [ - [nil, 2.4], - ['3.19.0', 2.4] -] +# RODA_VERSIONS = [ +# [nil, 2.4], +# ['3.19.0', 2.4] +# ] -def gem_list(roda_version = nil) - <<~RB - gem 'roda'#{roda_version} - gem 'rack', '~> 2.2' - gem 'rack-test', '>= 0.8.0', :require => 'rack/test' - - RB -end +# def gem_list(roda_version = nil) +# <<~RB +# gem 'roda'#{roda_version} +# gem 'rack', '~> 2.2' +# gem 'rack-test', '>= 0.8.0', :require => 'rack/test' +# gem 'minitest', '~> 5.18.0' +# RB +# end -create_gemfiles(RODA_VERSIONS) +# create_gemfiles(RODA_VERSIONS) + +gemfile <<~RB + gem 'roda' + gem 'rack', '~> 2.2' + gem 'rack-test', '>= 0.8.0', :require => 'rack/test' + gem 'minitest', '~> 5.18.0' +RB diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 65725eb37c..5f016879b7 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -2,14 +2,96 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true +require 'minitest/autorun' +require_relative '../../../../lib/new_relic/agent/instrumentation/roda/instrumentation' + +# require 'lib/new_relic/agent/instrumentation/roda/roda_transaction_namer' +# require 'roda' + +# class RodaTestApp < Roda +# post '/test' do +# 'test' +# end +# end + class RodaInstrumentationTest < Minitest::Test - def setup - @stats_engine = NewRelic::Agent.instance.stats_engine + # include MultiverseHelpers + + def test_roda_defined + assert_equal 1, 1 + end + + def test_roda_undefined + end + + def test_roda_version_supported + end + + def test_roda_version_unspoorted + end + + def test_build_rack_app_defined + end + + def test_build_rack_app_undefined + end + + def test_roda_handle_main_route_defined + end + + def test_roda_handle_main_route_undefined + end + + # patched methods + def test_roda_handle_main_route + end + + def test_build_rack_app + end + + # instrumentation file + def test_newrelic_middlewares_agenthook_inserted + end + + def test_newrelic_middlewares_agenthook_not_inserted + end + + def test_newrelic_middlewares_all_inserted + # should have a helper method out there - last_transaction_trace // event? last_response + # get last t + end + + def test_build_rack_app_with_tracing_unless_middleware_disabled + end + + def test_rack_request_params_returns_rack_params end - def teardown - NewRelic::Agent.instance.stats_engine.clear_stats + def test_rack_request_params_fails end - # Add tests here + def test_roda_handle_main_route_with_tracing + # should have a helper method out there - last_transaction_trace // event? last_response + # get last txn, is the name correct? are other things correct about that txn + end + + # Transaction File + + def test_transaction_name_standard_request + end + + def test_transaction_no_request_path + end + + def test_transaction_name_regex_clears_extra_backslashes + end + + def test_transaction_name_path_name_empty + end + + def test_transaction_name_verb_nil + end + + def test_http_verb_does_not_respond_to_request_method + end end From 67cacf6a246eeb7294c99669a3389c5224942acc Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 2 Aug 2023 16:10:51 -0700 Subject: [PATCH 07/31] Add some tests --- test/multiverse/suites/roda/Envfile | 21 +++ test/multiverse/suites/roda/Envfile.rb | 28 ---- .../suites/roda/roda_instrumentation_test.rb | 128 +++++++++--------- 3 files changed, 85 insertions(+), 92 deletions(-) create mode 100644 test/multiverse/suites/roda/Envfile delete mode 100644 test/multiverse/suites/roda/Envfile.rb diff --git a/test/multiverse/suites/roda/Envfile b/test/multiverse/suites/roda/Envfile new file mode 100644 index 0000000000..1da1e7808c --- /dev/null +++ b/test/multiverse/suites/roda/Envfile @@ -0,0 +1,21 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +instrumentation_methods :chain, :prepend + +RODA_VERSIONS = [ + [nil, 2.4], + ['3.19.0', 2.4] +] + +def gem_list(roda_version = nil) + <<~RB + gem 'roda'#{roda_version} + gem 'rack', '~> 2.2' + gem 'rack-test', '>= 0.8.0', :require => 'rack/test' + gem 'minitest', '~> 5.18.0' + RB +end + +create_gemfiles(RODA_VERSIONS) diff --git a/test/multiverse/suites/roda/Envfile.rb b/test/multiverse/suites/roda/Envfile.rb deleted file mode 100644 index 4b6fcbab67..0000000000 --- a/test/multiverse/suites/roda/Envfile.rb +++ /dev/null @@ -1,28 +0,0 @@ -# This file is distributed under New Relic's license terms. -# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. -# frozen_string_literal: true - -# instrumentation_methods :chain, :prepend - -# RODA_VERSIONS = [ -# [nil, 2.4], -# ['3.19.0', 2.4] -# ] - -# def gem_list(roda_version = nil) -# <<~RB -# gem 'roda'#{roda_version} -# gem 'rack', '~> 2.2' -# gem 'rack-test', '>= 0.8.0', :require => 'rack/test' -# gem 'minitest', '~> 5.18.0' -# RB -# end - -# create_gemfiles(RODA_VERSIONS) - -gemfile <<~RB - gem 'roda' - gem 'rack', '~> 2.2' - gem 'rack-test', '>= 0.8.0', :require => 'rack/test' - gem 'minitest', '~> 5.18.0' -RB diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 5f016879b7..18305488aa 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -2,96 +2,96 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -require 'minitest/autorun' require_relative '../../../../lib/new_relic/agent/instrumentation/roda/instrumentation' +require_relative '../../../../lib/new_relic/agent/instrumentation/roda/roda_transaction_namer' -# require 'lib/new_relic/agent/instrumentation/roda/roda_transaction_namer' -# require 'roda' - -# class RodaTestApp < Roda -# post '/test' do -# 'test' -# end -# end - -class RodaInstrumentationTest < Minitest::Test - # include MultiverseHelpers - - def test_roda_defined - assert_equal 1, 1 +class RodaTestApp < Roda + plugin :error_handler do |e| + 'Oh No!' end - def test_roda_undefined - end - - def test_roda_version_supported - end - - def test_roda_version_unspoorted - end - - def test_build_rack_app_defined - end - - def test_build_rack_app_undefined - end + route do |r| + # GET / request + r.root do + r.redirect('home') + end - def test_roda_handle_main_route_defined - end + r.on('home') do + 'home page' + end - def test_roda_handle_main_route_undefined - end + # /hello branch + r.on('hello') do + # Set variable for all routes in /hello branch + @greeting = 'Hello' - # patched methods - def test_roda_handle_main_route - end + # GET /hello/world request + r.get('world') do + "#{@greeting} world!" + end + end - def test_build_rack_app - end + r.on('error') do + raise 'boom' + end - # instrumentation file - def test_newrelic_middlewares_agenthook_inserted + r.on('slow') do + sleep(3) + 'I slept for 3 seconds!' + end end +end - def test_newrelic_middlewares_agenthook_not_inserted - end +class RodaInstrumentationTest < Minitest::Test + include Rack::Test::Methods + include MultiverseHelpers - def test_newrelic_middlewares_all_inserted - # should have a helper method out there - last_transaction_trace // event? last_response - # get last t + def app + RodaTestApp end - def test_build_rack_app_with_tracing_unless_middleware_disabled - end + def test_request_is_recorded + get('/home') + txn = harvest_transaction_events![1][0] - def test_rack_request_params_returns_rack_params + assert_equal 'Controller/Roda/RodaTestApp/GET home', txn[0]['name'] + assert_equal 200, txn[2][:'http.statusCode'] end - def test_rack_request_params_fails - end + def test_500_response_status + get('/error') + errors = harvest_error_traces! + txn = harvest_transaction_events! - def test_roda_handle_main_route_with_tracing - # should have a helper method out there - last_transaction_trace // event? last_response - # get last txn, is the name correct? are other things correct about that txn + assert_equal 500, txn[1][0][2][:"http.statusCode"] + assert_equal 'Oh No!', last_response.body + assert_equal 1, errors.size end - # Transaction File - - def test_transaction_name_standard_request - end + def test_404_response_status + get('/nothing') + errors = harvest_error_traces! + txn = harvest_transaction_events! - def test_transaction_no_request_path + assert_equal 404, txn[1][0][2][:"http.statusCode"] + assert_equal 0, errors.size end - def test_transaction_name_regex_clears_extra_backslashes - end + def test_empty_route_name_and_response_status + get('') + errors = harvest_error_traces! + txn = harvest_transaction_events![1][0] - def test_transaction_name_path_name_empty + assert_equal 'Controller/Roda/RodaTestApp/GET /', txn[0]['name'] + assert_equal 302, txn[2][:'http.statusCode'] end - def test_transaction_name_verb_nil - end + def test_roda_middleware_disabled + with_config(:disable_roda_auto_middleware => true) do + get('/home') + end + txn = harvest_transaction_events![1][0] - def test_http_verb_does_not_respond_to_request_method + assert_equal 200, txn[2][:"http.statusCode"] end end From 1852fb93096fc8e2bdb1d68afbc034f578c18671 Mon Sep 17 00:00:00 2001 From: hramadan Date: Fri, 4 Aug 2023 10:26:19 -0700 Subject: [PATCH 08/31] Add test --- .../suites/roda/roda_instrumentation_test.rb | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 18305488aa..924da4c4d8 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -22,23 +22,15 @@ class RodaTestApp < Roda # /hello branch r.on('hello') do - # Set variable for all routes in /hello branch - @greeting = 'Hello' - - # GET /hello/world request - r.get('world') do - "#{@greeting} world!" + # GET /hello/:name request + r.get(':name') do |name| + "Hello #{name}!" end end r.on('error') do raise 'boom' end - - r.on('slow') do - sleep(3) - 'I slept for 3 seconds!' - end end end @@ -50,6 +42,16 @@ def app RodaTestApp end + def test_nil_verb + NewRelic::Agent::Instrumentation::Roda::TransactionNamer.stub(:http_verb, nil) do + get('/home') + txn = harvest_transaction_events![1][0] + + assert_equal 'Controller/Roda/RodaTestApp/home', txn[0]['name'] + assert_equal 200, txn[2][:'http.statusCode'] + end + end + def test_request_is_recorded get('/home') txn = harvest_transaction_events![1][0] From 09267754cf5649ff2aa6ccd9c063bef3129464ce Mon Sep 17 00:00:00 2001 From: hramadan Date: Fri, 4 Aug 2023 10:47:25 -0700 Subject: [PATCH 09/31] test: remove minitest from env --- newrelic.yml | 4 ---- test/multiverse/suites/roda/Envfile | 1 - 2 files changed, 5 deletions(-) diff --git a/newrelic.yml b/newrelic.yml index 278d45bfd3..1403250495 100644 --- a/newrelic.yml +++ b/newrelic.yml @@ -372,10 +372,6 @@ common: &default_settings # prepend, chain, disabled. # instrumentation.bunny: auto - # Controls auto-instrumentation of roda at start up. - # May be one of [auto|prepend|chain|disabled] - # instrumentation.roda: auto - # Controls auto-instrumentation of the concurrent-ruby library at start up. May be # one of: auto, prepend, chain, disabled. # instrumentation.concurrent_ruby: auto diff --git a/test/multiverse/suites/roda/Envfile b/test/multiverse/suites/roda/Envfile index 1da1e7808c..c656d08209 100644 --- a/test/multiverse/suites/roda/Envfile +++ b/test/multiverse/suites/roda/Envfile @@ -14,7 +14,6 @@ def gem_list(roda_version = nil) gem 'roda'#{roda_version} gem 'rack', '~> 2.2' gem 'rack-test', '>= 0.8.0', :require => 'rack/test' - gem 'minitest', '~> 5.18.0' RB end From db7734de25bd3ad159f6f2e675fa338dd0710f8e Mon Sep 17 00:00:00 2001 From: hramadan Date: Fri, 4 Aug 2023 12:03:51 -0700 Subject: [PATCH 10/31] debug txn --- test/multiverse/suites/roda/roda_instrumentation_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 924da4c4d8..4ccf54fcc4 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -64,6 +64,7 @@ def test_500_response_status get('/error') errors = harvest_error_traces! txn = harvest_transaction_events! + puts "Here is the transaction <<<<<<<<< #{txn} <<<<<<<<<<<<" assert_equal 500, txn[1][0][2][:"http.statusCode"] assert_equal 'Oh No!', last_response.body From 8fbbe4e9ae1eb3f727241a9a7f3e00ab36eb3e46 Mon Sep 17 00:00:00 2001 From: hramadan Date: Fri, 4 Aug 2023 12:22:03 -0700 Subject: [PATCH 11/31] Ruby 2.4 fix --- lib/new_relic/agent/instrumentation/roda.rb | 2 +- test/multiverse/suites/roda/roda_instrumentation_test.rb | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 6c01cffe10..e4286c0189 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -11,7 +11,7 @@ depends_on do defined?(Roda) && - Gem::Version.new(Roda::RodaVersion) >= '3.19.0' && + Gem::Version.new(Roda::RodaVersion) >= Gem::Version.new('3.19.0') && Roda::RodaPlugins::Base::ClassMethods.private_method_defined?(:build_rack_app) && Roda::RodaPlugins::Base::InstanceMethods.method_defined?(:_roda_handle_main_route) end diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 4ccf54fcc4..924da4c4d8 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -64,7 +64,6 @@ def test_500_response_status get('/error') errors = harvest_error_traces! txn = harvest_transaction_events! - puts "Here is the transaction <<<<<<<<< #{txn} <<<<<<<<<<<<" assert_equal 500, txn[1][0][2][:"http.statusCode"] assert_equal 'Oh No!', last_response.body From ca48d74754fa5e930a350b348007b89d4bbe08f3 Mon Sep 17 00:00:00 2001 From: hramadan Date: Fri, 4 Aug 2023 14:51:46 -0700 Subject: [PATCH 12/31] test: and another one --- test/multiverse/suites/roda/roda_instrumentation_test.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 924da4c4d8..022fb050b2 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -52,6 +52,13 @@ def test_nil_verb end end + def test_http_verb_request_no_request_method + fake_request = Struct.new('FakeRequest', :path).new + name = NewRelic::Agent::Instrumentation::Roda::TransactionNamer.transaction_name(fake_request) + + assert_equal ::NewRelic::Agent::UNKNOWN_METRIC, name + end + def test_request_is_recorded get('/home') txn = harvest_transaction_events![1][0] From cc43194f1f73b61014499fe3ae60c535b17c62da Mon Sep 17 00:00:00 2001 From: hramadan Date: Mon, 7 Aug 2023 09:03:03 -0700 Subject: [PATCH 13/31] test: middleware disabling --- .../suites/roda/roda_instrumentation_test.rb | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 022fb050b2..7d259c5cba 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -38,6 +38,8 @@ class RodaInstrumentationTest < Minitest::Test include Rack::Test::Methods include MultiverseHelpers + setup_and_teardown_agent + def app RodaTestApp end @@ -95,12 +97,21 @@ def test_empty_route_name_and_response_status assert_equal 302, txn[2][:'http.statusCode'] end - def test_roda_middleware_disabled + def test_roda_auto_middleware_disabled with_config(:disable_roda_auto_middleware => true) do get('/home') + txn = harvest_transaction_events![1][0] + + assert_equal 'Controller/Roda/RodaTestApp/GET home', txn[0]['name'] end - txn = harvest_transaction_events![1][0] + end - assert_equal 200, txn[2][:"http.statusCode"] + def test_roda_instrumentation_works_if_middleware_disabled + with_config(:disable_middleware_instrumentation => true) do + get('/home') + txn = harvest_transaction_events![1][0] + + assert_equal 'Controller/Roda/RodaTestApp/GET home', txn[0]['name'] + end end end From 13ca3067bcb9dfc42aabc4ea90c566ec750ef924 Mon Sep 17 00:00:00 2001 From: hramadan Date: Mon, 7 Aug 2023 14:43:29 -0700 Subject: [PATCH 14/31] test: middleware fix --- .../suites/roda/roda_instrumentation_test.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 7d259c5cba..d387984b0a 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -34,6 +34,8 @@ class RodaTestApp < Roda end end +class RodaNoMiddleware < Roda; end + class RodaInstrumentationTest < Minitest::Test include Rack::Test::Methods include MultiverseHelpers @@ -44,6 +46,10 @@ def app RodaTestApp end + def app2 + RodaNoMiddleware + end + def test_nil_verb NewRelic::Agent::Instrumentation::Roda::TransactionNamer.stub(:http_verb, nil) do get('/home') @@ -99,10 +105,9 @@ def test_empty_route_name_and_response_status def test_roda_auto_middleware_disabled with_config(:disable_roda_auto_middleware => true) do - get('/home') - txn = harvest_transaction_events![1][0] + RodaNoMiddleware.build_rack_app_with_tracing {} - assert_equal 'Controller/Roda/RodaTestApp/GET home', txn[0]['name'] + assert_truthy NewRelic::Agent::Agent::config[:disable_roda_auto_middleware] end end From 10d8b0ed1998643cc4d137f8b0bacd75ee054f61 Mon Sep 17 00:00:00 2001 From: hramadan Date: Mon, 7 Aug 2023 15:26:19 -0700 Subject: [PATCH 15/31] Add CHANGELOG --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5222112df3..0651524f2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## dev -Version of the agent introduces improved error tracking functionality by associating a transaction id with each error, and uses more reliable network timeout logic. +Version of the agent introduces improved error tracking functionality by associating a transaction id with each error, uses more reliable network timeout logic, and adds Roda instrumentation. - **Feature: Improved error tracking transaction linking** @@ -12,6 +12,10 @@ Version of the agent introduces improved error tracking functionality by a In line with current Ruby best practices, make use of Net::HTTP's own timeout logic and avoid the use of `Timeout.timeout()` when possible. The agent's data transmissions and cloud provider detection routines have been updated accordingly. [PR#2147](https://github.com/newrelic/newrelic-ruby-agent/pull/2147) +- **Feature: Add Roda instrumentation** + + Roda is a now an instrumented framework. The agent currently supports Roda versions 3.19.0+. [PR#2144](https://github.com/newrelic/newrelic-ruby-agent/pull/2144) + ## v9.3.1 Version 9.3.1 of the agent fixes `NewRelic::Agent.require_test_helper`. From 6ec648c304ff50db9fad33adf6068344211b9c2e Mon Sep 17 00:00:00 2001 From: hramadan Date: Mon, 7 Aug 2023 17:27:07 -0700 Subject: [PATCH 16/31] Add tests --- .../suites/roda/roda_instrumentation_test.rb | 34 +++++++++++++++++++ .../suites/roda_agent_disabled/Envfile | 20 +++++++++++ .../roda_agent_disabled/config/newrelic.yml | 20 +++++++++++ .../suites/roda_agent_disabled/shim_test.rb | 24 +++++++++++++ 4 files changed, 98 insertions(+) create mode 100644 test/multiverse/suites/roda_agent_disabled/Envfile create mode 100644 test/multiverse/suites/roda_agent_disabled/config/newrelic.yml create mode 100644 test/multiverse/suites/roda_agent_disabled/shim_test.rb diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index d387984b0a..b3fcc8e5ee 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -119,4 +119,38 @@ def test_roda_instrumentation_works_if_middleware_disabled assert_equal 'Controller/Roda/RodaTestApp/GET home', txn[0]['name'] end end + + def test_transaction_name_error + NewRelic::Agent.stub(:logger, NewRelic::Agent::MemoryLogger.new) do + begin + NewRelic::Agent::Instrumentation::Roda::TransactionNamer.transaction_name({}) + rescue + # NOOP - Allow error to be raised + ensure + assert_logged(/Error encountered trying to identify Roda transaction name/) + end + end + end + + def test_rack_request_params_error + NewRelic::Agent.stub(:logger, NewRelic::Agent::MemoryLogger.new) do + begin + RodaTestApp::RodaRequest.any_instance + .stubs(:params).raises(StandardError.new) + get('/home?') + rescue + # NOOP - Allow error to be raised + ensure + assert_logged(/Failed to get params from Rack request./) + end + end + end + + def assert_logged(expected) + logger = NewRelic::Agent.logger + flattened = logger.messages.flatten + found = flattened.any? { |msg| msg.to_s.match?(expected) } + + assert(found, "Didn't see message '#{expected}'") + end end diff --git a/test/multiverse/suites/roda_agent_disabled/Envfile b/test/multiverse/suites/roda_agent_disabled/Envfile new file mode 100644 index 0000000000..c656d08209 --- /dev/null +++ b/test/multiverse/suites/roda_agent_disabled/Envfile @@ -0,0 +1,20 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +instrumentation_methods :chain, :prepend + +RODA_VERSIONS = [ + [nil, 2.4], + ['3.19.0', 2.4] +] + +def gem_list(roda_version = nil) + <<~RB + gem 'roda'#{roda_version} + gem 'rack', '~> 2.2' + gem 'rack-test', '>= 0.8.0', :require => 'rack/test' + RB +end + +create_gemfiles(RODA_VERSIONS) diff --git a/test/multiverse/suites/roda_agent_disabled/config/newrelic.yml b/test/multiverse/suites/roda_agent_disabled/config/newrelic.yml new file mode 100644 index 0000000000..477bb05fe6 --- /dev/null +++ b/test/multiverse/suites/roda_agent_disabled/config/newrelic.yml @@ -0,0 +1,20 @@ +--- +development: + error_collector: + enabled: true + apdex_t: 0.5 + agent_enabled: false + monitor_mode: false + license_key: bootstrap_newrelic_admin_license_key_000 + ca_bundle_path: ../../../config/test.cert.crt + app_name: test + host: localhost + api_host: localhost + port: <%= $collector && $collector.port %> + transaction_tracer: + record_sql: obfuscated + enabled: true + stack_trace_threshold: 0.5 + transaction_threshold: 1.0 + capture_params: false + disable_serialization: false diff --git a/test/multiverse/suites/roda_agent_disabled/shim_test.rb b/test/multiverse/suites/roda_agent_disabled/shim_test.rb new file mode 100644 index 0000000000..765521e27e --- /dev/null +++ b/test/multiverse/suites/roda_agent_disabled/shim_test.rb @@ -0,0 +1,24 @@ +# This file is distributed under New Relic's license terms. +# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. +# frozen_string_literal: true + +require 'roda' + +class TestRodaApp < Roda; end + +class RodaAgentDisabledTestCase < Minitest::Test + def assert_shims_defined + # class method shim + assert_respond_to TestRodaApp, :newrelic_ignore, 'Class method newrelic_ignore not defined' + assert_respond_to TestRodaApp, :newrelic_ignore_apdex, 'Class method newrelic_ignore_apdex not defined' + assert_respond_to TestRodaApp, :newrelic_ignore_enduser, 'Class method newrelic_ignore_enduser not defined' + + # instance method shims + assert_includes(TestRodaApp.instance_methods, :perform_action_with_newrelic_trace, 'Instance method perform_action_with_newrelic_trace not defined') + end + + # Agent disabled via config/newrelic.yml + def test_shims_exist_when_agent_enabled_false + assert_shims_defined + end +end From 160a64b6fbbc3f6dfb13af9c2770c2f76287a29b Mon Sep 17 00:00:00 2001 From: hramadan Date: Mon, 7 Aug 2023 17:32:19 -0700 Subject: [PATCH 17/31] Refactor --- lib/new_relic/agent/instrumentation/roda/instrumentation.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index 05b9981b2a..14e7311c37 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -34,14 +34,13 @@ def rack_request_params @_request.params rescue => e NewRelic::Agent.logger.debug('Failed to get params from Rack request.', e) - nil + NewRelic::EMPTY_HASH end end def _roda_handle_main_route_with_tracing(*args) request_params = rack_request_params - filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params || - NewRelic::EMPTY_HASH) + filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params) name = TransactionNamer.transaction_name(request) perform_action_with_newrelic_trace(:category => :roda, From 72010af2a2ebe05e7e5113e94d85740a415b4404 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Tue, 8 Aug 2023 09:12:28 -0700 Subject: [PATCH 18/31] Update CHANGELOG.md Co-authored-by: James Bunch --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0651524f2d..f4f3e28073 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## dev -Version of the agent introduces improved error tracking functionality by associating a transaction id with each error, uses more reliable network timeout logic, and adds Roda instrumentation. +Version of the agent introduces improved error tracking functionality by associating a transaction id with each error, uses more reliable network timeout logic, and adds [Roda](https://roda.jeremyevans.net/) instrumentation. - **Feature: Improved error tracking transaction linking** From 8a5739428bb6b6b5cbb34680b4a5bc470f7e06cc Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Tue, 8 Aug 2023 09:13:47 -0700 Subject: [PATCH 19/31] Apply suggestions from code review Co-authored-by: James Bunch Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 2 +- lib/new_relic/agent/instrumentation/roda.rb | 2 +- .../suites/roda/roda_instrumentation_test.rb | 13 ++++++------- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f4f3e28073..bac17fa342 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ Version of the agent introduces improved error tracking functionality by a - **Feature: Add Roda instrumentation** - Roda is a now an instrumented framework. The agent currently supports Roda versions 3.19.0+. [PR#2144](https://github.com/newrelic/newrelic-ruby-agent/pull/2144) + [Roda](https://roda.jeremyevans.net/) is a now an instrumented framework. The agent currently supports Roda versions 3.19.0+. [PR#2144](https://github.com/newrelic/newrelic-ruby-agent/pull/2144) ## v9.3.1 diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index e4286c0189..6894e5a4b2 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -29,7 +29,7 @@ end executes do - NewRelic::Agent.logger.info('Installing roda instrumentation') + NewRelic::Agent.logger.info('Installing Roda instrumentation') if use_prepend? prepend_instrument Roda, NewRelic::Agent::Instrumentation::Roda::Prepend diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index b3fcc8e5ee..1f9fdb0413 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -122,13 +122,12 @@ def test_roda_instrumentation_works_if_middleware_disabled def test_transaction_name_error NewRelic::Agent.stub(:logger, NewRelic::Agent::MemoryLogger.new) do - begin - NewRelic::Agent::Instrumentation::Roda::TransactionNamer.transaction_name({}) - rescue - # NOOP - Allow error to be raised - ensure - assert_logged(/Error encountered trying to identify Roda transaction name/) - end + # pass in {} to produce an error, because {} doesn't support #path and + # confirm that the desired error handling took place + result = NewRelic::Agent::Instrumentation::Roda::TransactionNamer.transaction_name({}) + + assert_equal NewRelic::Agent::UNKNOWN_METRIC, result + assert_logged(/NoMethodError.*Error encountered trying to identify Roda transaction name/) end end From 283463885fa4a6dfcce478738e51257534825c4e Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Tue, 8 Aug 2023 09:45:17 -0700 Subject: [PATCH 20/31] Update test/multiverse/suites/roda/roda_instrumentation_test.rb Co-authored-by: James Bunch --- .../suites/roda/roda_instrumentation_test.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 1f9fdb0413..4a2315886c 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -133,15 +133,12 @@ def test_transaction_name_error def test_rack_request_params_error NewRelic::Agent.stub(:logger, NewRelic::Agent::MemoryLogger.new) do - begin - RodaTestApp::RodaRequest.any_instance - .stubs(:params).raises(StandardError.new) - get('/home?') - rescue - # NOOP - Allow error to be raised - ensure - assert_logged(/Failed to get params from Rack request./) - end + # Have the #params call made on the request raise an exception to test + # the error handling + RodaTestApp::RodaRequest.any_instance.stubs(:params).raises(StandardError.new) + get('/home?') + + assert_logged(/Failed to get params from Rack request./) end end From 545c12662d0b20f7bc7e97f321266c16e73dd32c Mon Sep 17 00:00:00 2001 From: hramadan Date: Tue, 8 Aug 2023 09:58:10 -0700 Subject: [PATCH 21/31] Updates --- CHANGELOG.md | 10 +++++----- .../agent/instrumentation/roda/instrumentation.rb | 6 ++++-- .../suites/roda/roda_instrumentation_test.rb | 4 ---- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bac17fa342..c21eaef636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ ## dev -Version of the agent introduces improved error tracking functionality by associating a transaction id with each error, uses more reliable network timeout logic, and adds [Roda](https://roda.jeremyevans.net/) instrumentation. +Version of the agent adds [Roda](https://roda.jeremyevans.net/) instrumentation, introduces improved error tracking functionality by associating a transaction id with each error, and uses more reliable network timeout logic. + +- **Feature: Add Roda instrumentation** + + [Roda](https://roda.jeremyevans.net/) is a now an instrumented framework. The agent currently supports Roda versions 3.19.0+. [PR#2144](https://github.com/newrelic/newrelic-ruby-agent/pull/2144) - **Feature: Improved error tracking transaction linking** @@ -12,10 +16,6 @@ Version of the agent introduces improved error tracking functionality by a In line with current Ruby best practices, make use of Net::HTTP's own timeout logic and avoid the use of `Timeout.timeout()` when possible. The agent's data transmissions and cloud provider detection routines have been updated accordingly. [PR#2147](https://github.com/newrelic/newrelic-ruby-agent/pull/2147) -- **Feature: Add Roda instrumentation** - - [Roda](https://roda.jeremyevans.net/) is a now an instrumented framework. The agent currently supports Roda versions 3.19.0+. [PR#2144](https://github.com/newrelic/newrelic-ruby-agent/pull/2144) - ## v9.3.1 Version 9.3.1 of the agent fixes `NewRelic::Agent.require_test_helper`. diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index 14e7311c37..c73d0705f9 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -43,9 +43,11 @@ def _roda_handle_main_route_with_tracing(*args) filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params) name = TransactionNamer.transaction_name(request) - perform_action_with_newrelic_trace(:category => :roda, + perform_action_with_newrelic_trace( + :category => :roda, :name => name, - :params => filtered_params) do + :params => filtered_params + ) do yield end end diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 4a2315886c..90970ff1a6 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -46,10 +46,6 @@ def app RodaTestApp end - def app2 - RodaNoMiddleware - end - def test_nil_verb NewRelic::Agent::Instrumentation::Roda::TransactionNamer.stub(:http_verb, nil) do get('/home') From 57ca2275420f317f64289ed6ff3b66856cec0435 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Tue, 8 Aug 2023 14:35:40 -0700 Subject: [PATCH 22/31] Apply suggestions from code review Co-authored-by: James Bunch --- .../agent/instrumentation/roda/instrumentation.rb | 9 ++++----- lib/new_relic/agent/transaction.rb | 2 +- test/multiverse/lib/multiverse/runner.rb | 2 +- test/multiverse/suites/roda/roda_instrumentation_test.rb | 6 ++---- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index c73d0705f9..1f61c7b263 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -39,14 +39,13 @@ def rack_request_params end def _roda_handle_main_route_with_tracing(*args) - request_params = rack_request_params - filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, request_params) + filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, rack_request_params) name = TransactionNamer.transaction_name(request) perform_action_with_newrelic_trace( - :category => :roda, - :name => name, - :params => filtered_params + category: :roda, + name: name, + params: filtered_params ) do yield end diff --git a/lib/new_relic/agent/transaction.rb b/lib/new_relic/agent/transaction.rb index 1656815172..7c4654c550 100644 --- a/lib/new_relic/agent/transaction.rb +++ b/lib/new_relic/agent/transaction.rb @@ -36,7 +36,7 @@ class Transaction GRAPE_PREFIX = "#{CONTROLLER_PREFIX}Grape/" ACTION_CABLE_PREFIX = "#{CONTROLLER_PREFIX}ActionCable/" - WEB_TRANSACTION_CATEGORIES = [:web, :controller, :uri, :rack, :sinatra, :grape, :middleware, :action_cable, :roda].freeze + WEB_TRANSACTION_CATEGORIES = %i[action_cable controller grape middleware rack roda sinatra web uri].freeze MIDDLEWARE_SUMMARY_METRICS = ['Middleware/all'].freeze WEB_SUMMARY_METRIC = 'HttpDispatcher' diff --git a/test/multiverse/lib/multiverse/runner.rb b/test/multiverse/lib/multiverse/runner.rb index ffa96e1055..b4b1b1be56 100644 --- a/test/multiverse/lib/multiverse/runner.rb +++ b/test/multiverse/lib/multiverse/runner.rb @@ -103,7 +103,7 @@ def execute_suites(filter, opts) 'background_2' => ['rake'], 'database' => %w[elasticsearch mongo redis sequel], 'rails' => %w[active_record active_record_pg rails rails_prepend activemerchant], - 'frameworks' => %w[sinatra padrino grape roda], + 'frameworks' => %w[grape padrino roda sinatra], 'httpclients' => %w[curb excon httpclient], 'httpclients_2' => %w[typhoeus net_http httprb], 'infinite_tracing' => ['infinite_tracing'], diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 90970ff1a6..5be81f867e 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -139,10 +139,8 @@ def test_rack_request_params_error end def assert_logged(expected) - logger = NewRelic::Agent.logger - flattened = logger.messages.flatten - found = flattened.any? { |msg| msg.to_s.match?(expected) } + found = NewRelic::Agent.logger.messages.flatten.any? { |m| m.match?(expected) } - assert(found, "Didn't see message '#{expected}'") + assert(found, "Didn't see log message: '#{expected}'") end end From c79d97894104b7318e715972449333c7c0a5c6bc Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 09:01:24 -0700 Subject: [PATCH 23/31] Code feedback --- lib/new_relic/agent/instrumentation/roda.rb | 6 ++++-- .../instrumentation/roda/roda_transaction_namer.rb | 10 +++------- test/multiverse/suites/roda/Envfile | 2 +- .../suites/roda/roda_instrumentation_test.rb | 10 ---------- test/multiverse/suites/roda_agent_disabled/Envfile | 2 +- 5 files changed, 9 insertions(+), 21 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 6894e5a4b2..0cae399e8a 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -3,8 +3,6 @@ # frozen_string_literal: true require_relative 'roda/instrumentation' -require_relative 'roda/chain' -require_relative 'roda/prepend' DependencyDetection.defer do named :roda @@ -22,8 +20,10 @@ require 'new_relic/rack/agent_hooks' require 'new_relic/rack/browser_monitoring' if use_prepend? + require_relative 'roda/prepend' prepend_instrument Roda.singleton_class, NewRelic::Agent::Instrumentation::Roda::Build::Prepend else + require_relative 'roda/chain' chain_instrument NewRelic::Agent::Instrumentation::Roda::Build::Chain end end @@ -32,8 +32,10 @@ NewRelic::Agent.logger.info('Installing Roda instrumentation') if use_prepend? + require_relative 'roda/prepend' prepend_instrument Roda, NewRelic::Agent::Instrumentation::Roda::Prepend else + require_relative 'roda/chain' chain_instrument NewRelic::Agent::Instrumentation::Roda::Chain end end diff --git a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb index 56568b9816..0d28eba7e4 100644 --- a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb +++ b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb @@ -10,12 +10,12 @@ module TransactionNamer extend self ROOT = '/'.freeze - REGEX_MUTIPLE_SLASHES = %r{^[/^\\A]*(.*?)[/\$\?\\z]*$}.freeze + REGEX_MULTIPLE_SLASHES = %r{^[/^\A]*(.*?)[/$?\z]*$}.freeze def transaction_name(request) - verb = http_verb(request) + verb = request.request_method if request.respond_to?(:request_method) path = request.path || ::NewRelic::Agent::UNKNOWN_METRIC - name = path.gsub(REGEX_MUTIPLE_SLASHES, '\1') # remove any rogue slashes + name = path.gsub(REGEX_MULTIPLE_SLASHES, '\1') # remove any rogue slashes name = ROOT if name.empty? name = "#{verb} #{name}" unless verb.nil? @@ -24,10 +24,6 @@ def transaction_name(request) ::NewRelic::Agent.logger.debug("#{e.class} : #{e.message} - Error encountered trying to identify Roda transaction name") ::NewRelic::Agent::UNKNOWN_METRIC end - - def http_verb(request) - request.request_method if request.respond_to?(:request_method) - end end end end diff --git a/test/multiverse/suites/roda/Envfile b/test/multiverse/suites/roda/Envfile index c656d08209..5feccbaecb 100644 --- a/test/multiverse/suites/roda/Envfile +++ b/test/multiverse/suites/roda/Envfile @@ -12,7 +12,7 @@ RODA_VERSIONS = [ def gem_list(roda_version = nil) <<~RB gem 'roda'#{roda_version} - gem 'rack', '~> 2.2' + gem 'rack' gem 'rack-test', '>= 0.8.0', :require => 'rack/test' RB end diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index 5be81f867e..d5fb4cc15b 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -46,16 +46,6 @@ def app RodaTestApp end - def test_nil_verb - NewRelic::Agent::Instrumentation::Roda::TransactionNamer.stub(:http_verb, nil) do - get('/home') - txn = harvest_transaction_events![1][0] - - assert_equal 'Controller/Roda/RodaTestApp/home', txn[0]['name'] - assert_equal 200, txn[2][:'http.statusCode'] - end - end - def test_http_verb_request_no_request_method fake_request = Struct.new('FakeRequest', :path).new name = NewRelic::Agent::Instrumentation::Roda::TransactionNamer.transaction_name(fake_request) diff --git a/test/multiverse/suites/roda_agent_disabled/Envfile b/test/multiverse/suites/roda_agent_disabled/Envfile index c656d08209..5feccbaecb 100644 --- a/test/multiverse/suites/roda_agent_disabled/Envfile +++ b/test/multiverse/suites/roda_agent_disabled/Envfile @@ -12,7 +12,7 @@ RODA_VERSIONS = [ def gem_list(roda_version = nil) <<~RB gem 'roda'#{roda_version} - gem 'rack', '~> 2.2' + gem 'rack' gem 'rack-test', '>= 0.8.0', :require => 'rack/test' RB end From e4ca1e00535731899c3c4e41eeaaa9bb64399290 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Wed, 9 Aug 2023 09:02:02 -0700 Subject: [PATCH 24/31] Update lib/new_relic/agent/instrumentation/roda/instrumentation.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../agent/instrumentation/roda/instrumentation.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index 1f61c7b263..07adb10236 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -39,13 +39,10 @@ def rack_request_params end def _roda_handle_main_route_with_tracing(*args) - filtered_params = ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, rack_request_params) - name = TransactionNamer.transaction_name(request) - perform_action_with_newrelic_trace( category: :roda, - name: name, - params: filtered_params + name: TransactionNamer.transaction_name(request), + params: ::NewRelic::Agent::ParameterFiltering::apply_filters(request.env, rack_request_params) ) do yield end From 8123d11f5f6b42b32c77a3779cd647e099480379 Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 09:55:07 -0700 Subject: [PATCH 25/31] Don't stub any_instance --- test/multiverse/suites/roda/roda_instrumentation_test.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/multiverse/suites/roda/roda_instrumentation_test.rb b/test/multiverse/suites/roda/roda_instrumentation_test.rb index d5fb4cc15b..8bc078d467 100644 --- a/test/multiverse/suites/roda/roda_instrumentation_test.rb +++ b/test/multiverse/suites/roda/roda_instrumentation_test.rb @@ -119,10 +119,9 @@ def test_transaction_name_error def test_rack_request_params_error NewRelic::Agent.stub(:logger, NewRelic::Agent::MemoryLogger.new) do - # Have the #params call made on the request raise an exception to test - # the error handling - RodaTestApp::RodaRequest.any_instance.stubs(:params).raises(StandardError.new) - get('/home?') + # Unit-syle test calling rack_request_params directly. No Rack request exists, + # so @_request.params should fail. + app.rack_request_params assert_logged(/Failed to get params from Rack request./) end From c08cf6b18a3bd6371a33907a9a5c216746f47f71 Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 10:28:19 -0700 Subject: [PATCH 26/31] Move require to top of file --- lib/new_relic/agent/instrumentation/roda.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 0cae399e8a..29cd55dc8d 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -3,6 +3,8 @@ # frozen_string_literal: true require_relative 'roda/instrumentation' +require_relative '../../rack/agent_hooks' +require_relative '../../rack/browser_monitoring' DependencyDetection.defer do named :roda @@ -15,10 +17,6 @@ end executes do - # These requires are inside an executes block because they require rack, and - # we can't be sure that rack is available when this file is first required. - require 'new_relic/rack/agent_hooks' - require 'new_relic/rack/browser_monitoring' if use_prepend? require_relative 'roda/prepend' prepend_instrument Roda.singleton_class, NewRelic::Agent::Instrumentation::Roda::Build::Prepend From 9b89d729a24782d62825dacc302f4c7a1ec25a7c Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 10:32:48 -0700 Subject: [PATCH 27/31] Relocate rack/browser requires --- lib/new_relic/agent/instrumentation/roda.rb | 2 -- lib/new_relic/agent/instrumentation/roda/instrumentation.rb | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 29cd55dc8d..209dbe3e50 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -3,8 +3,6 @@ # frozen_string_literal: true require_relative 'roda/instrumentation' -require_relative '../../rack/agent_hooks' -require_relative '../../rack/browser_monitoring' DependencyDetection.defer do named :roda diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index 07adb10236..5e89e71f3c 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -2,6 +2,9 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true +require_relative '../../../rack/agent_hooks' +require_relative '../../../rack/browser_monitoring' + module NewRelic::Agent::Instrumentation module Roda module Tracer From ed60437d6fcb9e2e045863c619323f3537ebc6a1 Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 10:41:44 -0700 Subject: [PATCH 28/31] Refactor --- lib/new_relic/agent/instrumentation/roda.rb | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 209dbe3e50..7cc4f1495a 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -3,6 +3,8 @@ # frozen_string_literal: true require_relative 'roda/instrumentation' +require_relative '../../rack/agent_hooks' +require_relative '../../rack/browser_monitoring' DependencyDetection.defer do named :roda @@ -14,24 +16,16 @@ Roda::RodaPlugins::Base::InstanceMethods.method_defined?(:_roda_handle_main_route) end - executes do - if use_prepend? - require_relative 'roda/prepend' - prepend_instrument Roda.singleton_class, NewRelic::Agent::Instrumentation::Roda::Build::Prepend - else - require_relative 'roda/chain' - chain_instrument NewRelic::Agent::Instrumentation::Roda::Build::Chain - end - end - executes do NewRelic::Agent.logger.info('Installing Roda instrumentation') if use_prepend? require_relative 'roda/prepend' + prepend_instrument Roda.singleton_class, NewRelic::Agent::Instrumentation::Roda::Build::Prepend prepend_instrument Roda, NewRelic::Agent::Instrumentation::Roda::Prepend else require_relative 'roda/chain' + chain_instrument NewRelic::Agent::Instrumentation::Roda::Build::Chain chain_instrument NewRelic::Agent::Instrumentation::Roda::Chain end end From 8e90f4fd1c4e4bc641048c253fe37091846935a8 Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 10:45:44 -0700 Subject: [PATCH 29/31] put requires back --- lib/new_relic/agent/instrumentation/roda.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda.rb b/lib/new_relic/agent/instrumentation/roda.rb index 7cc4f1495a..f55553572d 100644 --- a/lib/new_relic/agent/instrumentation/roda.rb +++ b/lib/new_relic/agent/instrumentation/roda.rb @@ -3,8 +3,6 @@ # frozen_string_literal: true require_relative 'roda/instrumentation' -require_relative '../../rack/agent_hooks' -require_relative '../../rack/browser_monitoring' DependencyDetection.defer do named :roda @@ -17,6 +15,9 @@ end executes do + require_relative '../../rack/agent_hooks' + require_relative '../../rack/browser_monitoring' + NewRelic::Agent.logger.info('Installing Roda instrumentation') if use_prepend? From 6b1fcc176911a8521ebf5efab42466dbc44dcffe Mon Sep 17 00:00:00 2001 From: hramadan Date: Wed, 9 Aug 2023 10:53:48 -0700 Subject: [PATCH 30/31] remove from instrum class --- lib/new_relic/agent/instrumentation/roda/instrumentation.rb | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb index 5e89e71f3c..07adb10236 100644 --- a/lib/new_relic/agent/instrumentation/roda/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/roda/instrumentation.rb @@ -2,9 +2,6 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -require_relative '../../../rack/agent_hooks' -require_relative '../../../rack/browser_monitoring' - module NewRelic::Agent::Instrumentation module Roda module Tracer From 138176bd77a0ebbf2aab72484697a6820b1ce3d6 Mon Sep 17 00:00:00 2001 From: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> Date: Wed, 9 Aug 2023 11:57:27 -0700 Subject: [PATCH 31/31] Update lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb Co-authored-by: James Bunch --- .../agent/instrumentation/roda/roda_transaction_namer.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb index 0d28eba7e4..2985a2a6f0 100644 --- a/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb +++ b/lib/new_relic/agent/instrumentation/roda/roda_transaction_namer.rb @@ -13,11 +13,10 @@ module TransactionNamer REGEX_MULTIPLE_SLASHES = %r{^[/^\A]*(.*?)[/$?\z]*$}.freeze def transaction_name(request) - verb = request.request_method if request.respond_to?(:request_method) path = request.path || ::NewRelic::Agent::UNKNOWN_METRIC name = path.gsub(REGEX_MULTIPLE_SLASHES, '\1') # remove any rogue slashes name = ROOT if name.empty? - name = "#{verb} #{name}" unless verb.nil? + name = "#{request.request_method} #{name}" if request.respond_to?(:request_method) name rescue => e