diff --git a/CHANGELOG.md b/CHANGELOG.md index e850f2f9e5..89a65541f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,15 @@ ## dev -Version adds instrumentation for Async::HTTP, gleans Docker container IDs from cgroups v2-based containers, records additional synthetics attributes, fixes an issue with Rails 7.1 that could cause duplicate log records to be sent to New Relic, and fixes a deprecation warning for the Sidekiq error handler. +Version adds instrumentation for Async::HTTP and Ethon, gleans Docker container IDs from cgroups v2-based containers, records additional synthetics attributes, fixes an issue with Rails 7.1 that could cause duplicate log records to be sent to New Relic, and fixes a deprecation warning for the Sidekiq error handler. - **Feature: Add instrumentation for Async::HTTP** The agent will now record spans for Async::HTTP requests. Versions 0.59.0 and above of the async-http gem are supported. [PR#2272](https://github.com/newrelic/newrelic-ruby-agent/pull/2272) +- **Feature: Add instrumentation for Ethon** + + Instrumentation has been added for the [Ethon](https://github.com/typhoeus/ethon) HTTP client gem. Versions 0.12.0 and above are supported. The agent will now record external request segments for invocations of `Ethon::Easy#perform` and `Ethon::Multi#perform`. NOTE: The [Typhoeus](https://github.com/typhoeus/typhoeus) gem is maintained by the same team that maintains Ethon and depends on Ethon for its functionality. To prevent duplicate reporting for each HTTP request, the Ethon instrumentation will be disabled when Typhoeus is detected. [PR#2260](https://github.com/newrelic/newrelic-ruby-agent/pull/2260) - **Feature: Prevent the agent from starting in rails commands in Rails 7** diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 7f297cf086..54ebf932e5 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1437,6 +1437,14 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'Controls auto-instrumentation of the elasticsearch library at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' }, + :'instrumentation.ethon' => { + :default => 'auto', + :public => true, + :type => String, + :dynamic_name => true, + :allowed_from_server => false, + :description => 'Controls auto-instrumentation of ethon at start up. May be one of [auto|prepend|chain|disabled]' + }, :'instrumentation.excon' => { :default => 'enabled', :documentation_default => 'enabled', diff --git a/lib/new_relic/agent/http_clients/ethon_wrappers.rb b/lib/new_relic/agent/http_clients/ethon_wrappers.rb new file mode 100644 index 0000000000..190a2d4c4d --- /dev/null +++ b/lib/new_relic/agent/http_clients/ethon_wrappers.rb @@ -0,0 +1,111 @@ +# 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 'uri' +require_relative 'abstract' + +module NewRelic + module Agent + module HTTPClients + class EthonHTTPResponse < AbstractResponse + def initialize(easy) + @easy = easy + end + + def status_code + @easy.response_code + end + + def [](key) + headers[format_key(key)] + end + + def headers + # Ethon::Easy#response_headers will return '' if headers are unset + @easy.response_headers.scan(/\n([^:]+?): ([^:\n]+?)\r/).each_with_object({}) do |pair, hash| + hash[format_key(pair[0])] = pair[1] + end + end + alias to_hash headers + + private + + def format_key(key) + key.tr('-', '_').downcase + end + end + + class EthonHTTPRequest < AbstractRequest + attr_reader :uri + + DEFAULT_ACTION = 'GET' + DEFAULT_HOST = 'UNKNOWN_HOST' + ETHON = 'Ethon' + LHOST = 'host'.freeze + UHOST = 'Host'.freeze + + def initialize(easy) + @easy = easy + @uri = uri_from_easy + end + + def type + ETHON + end + + def host_from_header + self[LHOST] || self[UHOST] + end + + def uri_from_easy + # anticipate `Ethon::Easy#url` being `example.com` without a protocol + # defined and use an 'http' protocol prefix for `URI.parse` to work + # with the URL as desired + url_str = @easy.url.match?(':') ? @easy.url : "http://#{@easy.url}" + begin + URI.parse(url_str) + rescue URI::InvalidURIError => e + NewRelic::Agent.logger.debug("Failed to parse URI '#{url_str}': #{e.class} - #{e.message}") + URI.parse(NewRelic::EMPTY_STR) + end + end + + def host + host_from_header || uri.host&.downcase || DEFAULT_HOST + end + + def method + return DEFAULT_ACTION unless @easy.instance_variable_defined?(action_instance_var) + + @easy.instance_variable_get(action_instance_var) + end + + def action_instance_var + NewRelic::Agent::Instrumentation::Ethon::Easy::ACTION_INSTANCE_VAR + end + + def []=(key, value) + headers[key] = value + @easy.headers = headers + end + + def headers + @headers ||= if @easy.instance_variable_defined?(headers_instance_var) + @easy.instance_variable_get(headers_instance_var) + else + {} + end + end + + def headers_instance_var + NewRelic::Agent::Instrumentation::Ethon::Easy::HEADERS_INSTANCE_VAR + end + + def [](key) + headers[key] + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/ethon.rb b/lib/new_relic/agent/instrumentation/ethon.rb new file mode 100644 index 0000000000..20daa492d5 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/ethon.rb @@ -0,0 +1,39 @@ +# 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 'ethon/instrumentation' +require_relative 'ethon/chain' +require_relative 'ethon/prepend' + +DependencyDetection.defer do + named :ethon + + # If Ethon is being used as a dependency of Typhoeus, allow the Typhoeus + # instrumentation to handle everything. Otherwise each external network call + # will confusingly result in "Ethon" segments duplicating the information + # already provided by "Typhoeus" segments. + depends_on do + !defined?(Typhoeus) + end + + depends_on do + defined?(Ethon) && Gem::Version.new(Ethon::VERSION) >= Gem::Version.new('0.12.0') + end + + executes do + NewRelic::Agent.logger.info('Installing ethon instrumentation') + end + + executes do + if use_prepend? + # NOTE: by default prepend_instrument will go with the module name that + # precedes 'Prepend' (so 'Easy' and 'Multi'), but we want to use + # 'Ethon::Easy' and 'Ethon::Multi' so 3rd argument is supplied + prepend_instrument Ethon::Easy, NewRelic::Agent::Instrumentation::Ethon::Easy::Prepend, Ethon::Easy.name + prepend_instrument Ethon::Multi, NewRelic::Agent::Instrumentation::Ethon::Multi::Prepend, Ethon::Multi.name + else + chain_instrument NewRelic::Agent::Instrumentation::Ethon::Chain + end + end +end diff --git a/lib/new_relic/agent/instrumentation/ethon/chain.rb b/lib/new_relic/agent/instrumentation/ethon/chain.rb new file mode 100644 index 0000000000..809a62b4f1 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/ethon/chain.rb @@ -0,0 +1,39 @@ +# 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 Ethon + module Chain + def self.instrument! + ::Ethon::Easy.class_eval do + include NewRelic::Agent::Instrumentation::Ethon::Easy + + alias_method(:fabricate_without_tracing, :fabricate) + def fabricate(url, action_name, options) + fabricate_with_tracing(url, action_name, options) { fabricate_without_tracing(url, action_name, options) } + end + + alias_method(:headers_equals_without_tracing, :headers=) + def headers=(headers) + headers_equals_with_tracing(headers) { headers_equals_without_tracing(headers) } + end + + alias_method(:perform_without_tracing, :perform) + def perform(*args) + perform_with_tracing(*args) { perform_without_tracing(*args) } + end + end + + ::Ethon::Multi.class_eval do + include NewRelic::Agent::Instrumentation::Ethon::Multi + + alias_method(:perform_without_tracing, :perform) + def perform(*args) + perform_with_tracing(*args) { perform_without_tracing(*args) } + end + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/ethon/instrumentation.rb b/lib/new_relic/agent/instrumentation/ethon/instrumentation.rb new file mode 100644 index 0000000000..a40f60db46 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/ethon/instrumentation.rb @@ -0,0 +1,105 @@ +# 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/agent/http_clients/ethon_wrappers' + +module NewRelic::Agent::Instrumentation + module Ethon + module NRShared + INSTRUMENTATION_NAME = 'Ethon' + NOTICEABLE_ERROR_CLASS = 'Ethon::Errors::EthonError' + + def prep_easy(easy, parent = nil) + wrapped_request = NewRelic::Agent::HTTPClients::EthonHTTPRequest.new(easy) + segment = NewRelic::Agent::Tracer.start_external_request_segment( + library: wrapped_request.type, + uri: wrapped_request.uri, + procedure: wrapped_request.method, + parent: parent + ) + segment.add_request_headers(wrapped_request) + + callback = proc do + wrapped_response = NewRelic::Agent::HTTPClients::EthonHTTPResponse.new(easy) + segment.process_response_headers(wrapped_response) + + if easy.return_code != :ok + e = NewRelic::Agent::NoticeableError.new(NOTICEABLE_ERROR_CLASS, + "return_code: >>#{easy.return_code}<<, response_code: >>#{easy.response_code}<<") + segment.notice_error(e) + end + + ::NewRelic::Agent::Transaction::Segment.finish(segment) + end + + easy.on_complete { callback.call } + + segment + end + + def wrap_with_tracing(segment, &block) + NewRelic::Agent.record_instrumentation_invocation(INSTRUMENTATION_NAME) + + NewRelic::Agent::Tracer.capture_segment_error(segment) do + yield + end + ensure + NewRelic::Agent::Transaction::Segment.finish(segment) + end + end + + module Easy + include NRShared + + ACTION_INSTANCE_VAR = :@nr_action + HEADERS_INSTANCE_VAR = :@nr_headers + + # `Ethon::Easy` doesn't expose the "action name" ('GET', 'POST', etc.) + # and Ethon's fabrication of HTTP classes uses + # `Ethon::Easy::Http::Custom` for non-standard actions. To be able to + # know the action name at `#perform` time, we set a new instance variable + # on the `Ethon::Easy` instance with the base name of the fabricated + # class, respecting the 'Custom' name where appropriate. + def fabricate_with_tracing(_url, action_name, _options) + fabbed = yield + instance_variable_set(ACTION_INSTANCE_VAR, NewRelic::Agent.base_name(fabbed.class.name).upcase) + fabbed + end + + # `Ethon::Easy` uses `Ethon::Easy::Header` to set request headers on + # libcurl with `#headers=`. After they are set, they aren't easy to get + # at again except via FFI so set a new instance variable on the + # `Ethon::Easy` instance to store them in Ruby hash format. + def headers_equals_with_tracing(headers) + instance_variable_set(HEADERS_INSTANCE_VAR, headers) + yield + end + + def perform_with_tracing(*args) + return unless NewRelic::Agent::Tracer.state.is_execution_traced? + + segment = prep_easy(self) + wrap_with_tracing(segment) { yield } + end + end + + module Multi + include NRShared + + MULTI_SEGMENT_NAME = 'External/Multiple/Ethon::Multi/perform' + + def perform_with_tracing(*args) + return unless NewRelic::Agent::Tracer.state.is_execution_traced? + + segment = NewRelic::Agent::Tracer.start_segment(name: MULTI_SEGMENT_NAME) + + wrap_with_tracing(segment) do + easy_handles.each { |easy| prep_easy(easy, segment) } + + yield + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/ethon/prepend.rb b/lib/new_relic/agent/instrumentation/ethon/prepend.rb new file mode 100644 index 0000000000..8a892afb4d --- /dev/null +++ b/lib/new_relic/agent/instrumentation/ethon/prepend.rb @@ -0,0 +1,35 @@ +# 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 Ethon + module Easy + module Prepend + include NewRelic::Agent::Instrumentation::Ethon::Easy + + def fabricate(url, action_name, options) + fabricate_with_tracing(url, action_name, options) { super } + end + + def headers=(headers) + headers_equals_with_tracing(headers) { super } + end + + def perform(*args) + perform_with_tracing(*args) { super } + end + end + end + + module Multi + module Prepend + include NewRelic::Agent::Instrumentation::Ethon::Multi + + def perform(*args) + perform_with_tracing(*args) { super } + end + end + end + end +end diff --git a/lib/new_relic/agent/tracer.rb b/lib/new_relic/agent/tracer.rb index 34a879f8ad..6313638c80 100644 --- a/lib/new_relic/agent/tracer.rb +++ b/lib/new_relic/agent/tracer.rb @@ -357,9 +357,7 @@ def capture_segment_error(segment) yield rescue => exception # needs else branch coverage - if segment && segment.is_a?(Transaction::AbstractSegment) # rubocop:disable Style/SafeNavigation - segment.notice_error(exception) - end + segment.notice_error(exception) if segment&.is_a?(Transaction::AbstractSegment) raise end diff --git a/newrelic.yml b/newrelic.yml index 095419ac69..d99e45f620 100644 --- a/newrelic.yml +++ b/newrelic.yml @@ -102,7 +102,7 @@ common: &default_settings # Specify a list of constants that should prevent the agent from starting # automatically. Separate individual constants with a comma ,. For example, # "Rails::Console,UninstrumentedBackgroundJob". - # autostart.denylisted_constants: Rails::Console + # autostart.denylisted_constants: Rails::Command::ConsoleCommand,Rails::Command::CredentialsCommand,Rails::Command::Db::System::ChangeCommand,Rails::Command::DbConsoleCommand,Rails::Command::DestroyCommand,Rails::Command::DevCommand,Rails::Command::EncryptedCommand,Rails::Command::GenerateCommand,Rails::Command::InitializersCommand,Rails::Command::NotesCommand,Rails::Command::RoutesCommand,Rails::Command::SecretsCommand,Rails::Console,Rails::DBConsole # Defines a comma-delimited list of executables that the agent should not # instrument. For example, "rake,my_ruby_script.rb". @@ -374,8 +374,12 @@ common: &default_settings # Configures the TCP/IP port for the trace observer Host # infinite_tracing.trace_observer.port: 443 - # Controls auto-instrumentation of ActiveSupport::Logger at start-up. May be one - # of: auto, prepend, chain, disabled. + # Controls auto-instrumentation of ActiveSupport::BroadcastLogger at start up. May + # be one of: auto, prepend, chain, disabled. Used in Rails versions >= 7.1. + # instrumentation.active_support_broadcast_logger: auto + + # Controls auto-instrumentation of ActiveSupport::Logger at start up. May be one + # of: auto, prepend, chain, disabled. Used in Rails versions below 7.1. # instrumentation.active_support_logger: auto # Controls auto-instrumentation of bunny at start-up. May be one of: auto, @@ -402,6 +406,10 @@ common: &default_settings # one of: auto, prepend, chain, disabled. # instrumentation.elasticsearch: auto + # Controls auto-instrumentation of ethon at start up. May be one of + # [auto|prepend|chain|disabled] + # instrumentation.ethon: auto + # Controls auto-instrumentation of Excon at start-up. May be one of: enabled, # disabled. # instrumentation.excon: enabled @@ -515,8 +523,8 @@ common: &default_settings # add tracing to all Threads created in the application. # instrumentation.thread.tracing: true - # Controls auto-instrumentation of the Tilt template rendering library at start - # up. May be one of: auto, prepend, chain, disabled. + # Controls auto-instrumentation of the Tilt template rendering library at + # start-up. May be one of: auto, prepend, chain, disabled. # instrumentation.tilt: auto # Controls auto-instrumentation of Typhoeus at start-up. May be one of: auto, @@ -672,7 +680,7 @@ common: &default_settings # Regexp.new to permit advanced matching. For each hash pair, if either the key or # value is matched the # pair will not be reported. By default, no user_data is reported, so this option - # should only be used if + # should only be used if # the stripe.user_data.include option is being used. # stripe.user_data.exclude: [] diff --git a/test/multiverse/lib/multiverse/runner.rb b/test/multiverse/lib/multiverse/runner.rb index 44e98313f5..ac8189bde7 100644 --- a/test/multiverse/lib/multiverse/runner.rb +++ b/test/multiverse/lib/multiverse/runner.rb @@ -105,7 +105,7 @@ def execute_suites(filter, opts) 'rails' => %w[active_record active_record_pg active_support_broadcast_logger active_support_logger rails rails_prepend activemerchant], 'frameworks' => %w[grape padrino roda sinatra], 'httpclients' => %w[async_http curb excon httpclient], - 'httpclients_2' => %w[typhoeus net_http httprb], + 'httpclients_2' => %w[typhoeus net_http httprb ethon], 'infinite_tracing' => ['infinite_tracing'], 'rest' => [] # Specially handled below diff --git a/test/multiverse/suites/ethon/Envfile b/test/multiverse/suites/ethon/Envfile new file mode 100644 index 0000000000..9b246a0ca2 --- /dev/null +++ b/test/multiverse/suites/ethon/Envfile @@ -0,0 +1,19 @@ +# 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 + +ETHON_VERSIONS = [ + nil, + '0.12.0' +] + +def gem_list(ethon_version = nil) + <<~GEM_LIST + gem 'ethon'#{ethon_version} + gem 'rack' + GEM_LIST +end + +create_gemfiles(ETHON_VERSIONS) diff --git a/test/multiverse/suites/ethon/config/newrelic.yml b/test/multiverse/suites/ethon/config/newrelic.yml new file mode 100644 index 0000000000..9016427c50 --- /dev/null +++ b/test/multiverse/suites/ethon/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: + ethon: <%= $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/ethon/ethon_instrumentation_test.rb b/test/multiverse/suites/ethon/ethon_instrumentation_test.rb new file mode 100644 index 0000000000..ec03c23555 --- /dev/null +++ b/test/multiverse/suites/ethon/ethon_instrumentation_test.rb @@ -0,0 +1,144 @@ +# 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 'ethon' +require 'newrelic_rpm' +require 'http_client_test_cases' +require_relative '../../../../lib/new_relic/agent/http_clients/ethon_wrappers' +require_relative '../../../test_helper' + +class EthonInstrumentationTest < Minitest::Test + include HttpClientTestCases + + # Ethon::Easy#perform doesn't return a response object. Our Ethon + # instrumentation knows that and works fine. But the shared HTTP + # client test cases expect one, so we'll fake one. + DummyResponse = Struct.new(:body, :response_headers) + + def test_ethon_multi + easies = [] + count = 2 + in_transaction do + multi = Ethon::Multi.new + count.times do + easy = Ethon::Easy.new + easy.http_request(default_url, :get, {}) + easies << easy + multi.add(easy) + end + multi.perform + end + + multi_node_name = NewRelic::Agent::Instrumentation::Ethon::Multi::MULTI_SEGMENT_NAME + node = find_node_with_name(last_transaction_trace, multi_node_name) + + assert node, "Unable to locate a node named '#{multi_node_name}'" + assert_equal count, node.children.size, + "Expected '#{multi_node_name}' node to have #{count} children, found #{node.children.size}" + node.children.each { |child| assert_equal 'External/localhost/Ethon/GET', child.metric_name } + easies.each do |easy| + assert_match(//, easy.response_body) + assert_match(%r{^HTTP/1.1 200 OK}, easy.response_headers) + end + end + + def test_host_is_host_from_uri + skip_unless_minitest5_or_above + + host = 'silverpumpin.com' + easy = Ethon::Easy.new(url: host) + wrapped = NewRelic::Agent::HTTPClients::EthonHTTPRequest.new(easy) + + assert_equal host, wrapped.host + end + + def test_host_is_default_host + skip_unless_minitest5_or_above + + url = 'foxs' + mock_uri = Minitest::Mock.new + mock_uri.expect :host, nil, [] + URI.stub :parse, mock_uri, [url] do + easy = Ethon::Easy.new(url: url) + wrapped = NewRelic::Agent::HTTPClients::EthonHTTPRequest.new(easy) + + assert_equal NewRelic::Agent::HTTPClients::EthonHTTPRequest::DEFAULT_HOST, wrapped.host + end + end + + private + + def perform_easy_request(url, action, headers = nil) + e = Ethon::Easy.new + e.http_request(url, action, {}) + e.headers = headers if headers + e.perform + DummyResponse.new(e.response_body, e.response_headers) + end + + # HttpClientTestCases required method + def client_name + NewRelic::Agent::HTTPClients::EthonHTTPRequest::ETHON + end + + # HttpClientTestCases required method + def get_response(url = default_url, headers = nil) + perform_easy_request(url, :get, headers) + end + + # HttpClientTestCases required method + def post_response + perform_easy_request(default_url, :post) + end + + # HttpClientTestCases required method + # NOTE that the request won't actually be performed; simply inspected + def request_instance + NewRelic::Agent::HTTPClients::EthonHTTPRequest.new(Ethon::Easy.new(url: 'not a real URL')) + end + + # HttpClientTestCases required method + def test_delete + perform_easy_request(default_url, :delete) + end + + # HttpClientTestCases required method + def test_head + perform_easy_request(default_url, :head) + end + + # HttpClientTestCases required method + def test_put + perform_easy_request(default_url, :put) + end + + # HttpClientTestCases required method + def timeout_error_class + ::NewRelic::LanguageSupport.constantize(NewRelic::Agent::Instrumentation::Ethon::Easy::NOTICEABLE_ERROR_CLASS) + end + + # HttpClientTestCases required method + def simulate_error_response + e = Ethon::Easy.new + e.http_request(default_url, :get, {}) + e.stub :headers, -> { raise timeout_error_class.new('timeout') } do + e.perform + end + DummyResponse.new(e.response_body, e.response_headers) + end + + # HttpClientTestCases required method + def get_wrapped_response(url) + e = Ethon::Easy.new + e.http_request(url, :get, {}) + e.perform + NewRelic::Agent::HTTPClients::EthonHTTPResponse.new(e) + end + + # HttpClientTestCases required method + def response_instance(headers = {}) + response = DummyResponse.new('', headers.inject(+"200\r\n") { |s, (k, v)| s += "#{k}: #{v}\r\n" }) + NewRelic::Agent::HTTPClients::EthonHTTPResponse.new(response) + end +end diff --git a/test/new_relic/http_client_test_cases.rb b/test/new_relic/http_client_test_cases.rb index ea3bc675da..d86108756d 100644 --- a/test/new_relic/http_client_test_cases.rb +++ b/test/new_relic/http_client_test_cases.rb @@ -60,13 +60,6 @@ def body(res) res.body end - # TODO: remove method and its callers once Excon version is at or above 0.20.0 - def jruby_excon_skip? - defined?(JRUBY_VERSION) && - defined?(::Excon::VERSION) && - Gem::Version.new(::Excon::VERSION) < Gem::Version.new('0.20.0') - end - # Tests def test_validate_request_wrapper @@ -223,14 +216,10 @@ def test_transactional_traces_nodes get_response end - last_node = find_last_transaction_node() - - assert_equal "External/localhost/#{client_name}/GET", last_node.metric_name + perform_last_node_assertions end def test_ignore - skip "Don't test JRuby with old Excon." if jruby_excon_skip? - in_transaction do NewRelic::Agent.disable_all_tracing do post_response @@ -247,16 +236,12 @@ def test_head end def test_post - skip "Don't test JRuby with old Excon." if jruby_excon_skip? - in_transaction { post_response } assert_externals_recorded_for('localhost', 'POST') end def test_put - skip "Don't test JRuby with old Excon." if jruby_excon_skip? - in_transaction { put_response } assert_externals_recorded_for('localhost', 'PUT') @@ -391,12 +376,7 @@ def test_instrumentation_with_crossapp_enabled_records_crossapp_metrics_if_heade end end - last_node = find_last_transaction_node() - - assert_includes last_node.params.keys, :transaction_guid - assert_equal TRANSACTION_GUID, last_node.params[:transaction_guid] - - assert_metrics_recorded([ + perform_last_node_error_assertions([ 'External/all', 'External/allOther', 'ExternalApp/localhost/18#1884/all', @@ -416,12 +396,7 @@ def test_crossapp_metrics_allow_valid_utf8_characters end end - last_node = find_last_transaction_node() - - assert_includes last_node.params.keys, :transaction_guid - assert_equal TRANSACTION_GUID, last_node.params[:transaction_guid] - - assert_metrics_recorded([ + perform_last_node_error_assertions([ 'External/all', 'External/allOther', 'ExternalApp/localhost/12#1114/all', @@ -531,27 +506,38 @@ def test_still_records_tt_node_when_request_fails # transaction in which the error occurs. That, coupled with the fact that # fixing it for old versions of Typhoeus would require large changes to # the instrumentation, makes us say 'meh'. - is_typhoeus = (client_name == 'Typhoeus') - if !is_typhoeus || (is_typhoeus && Typhoeus::VERSION >= '0.5.4') - evil_server = NewRelic::EvilServer.new - evil_server.start + skip 'Not tested with Typhoeus < 0.5.4' if client_name == 'Typhoeus' && Typhoeus::VERSION < '0.5.4' - in_transaction do - begin - get_response("http://localhost:#{evil_server.port}") - rescue - # it's expected that this will raise for some HTTP libraries (e.g. - # Net::HTTP). we unfortunately don't know the exact exception class - # across all libraries - end + evil_server = NewRelic::EvilServer.new + evil_server.start + + in_transaction do + begin + get_response("http://localhost:#{evil_server.port}") + rescue + # it's expected that this will raise for some HTTP libraries (e.g. + # Net::HTTP). we unfortunately don't know the exact exception class + # across all libraries end + end - last_node = find_last_transaction_node() + perform_last_node_assertions - assert_equal("External/localhost/#{client_name}/GET", last_node.metric_name) + evil_server.stop + end - evil_server.stop - end + def perform_last_node_assertions + last_node = find_last_transaction_node() + + assert_equal("External/localhost/#{client_name}/GET", last_node.metric_name) + end + + def perform_last_node_error_assertions(metrics) + last_node = find_last_transaction_node() + + assert_includes last_node.params.keys, :transaction_guid + assert_equal TRANSACTION_GUID, last_node.params[:transaction_guid] + assert_metrics_recorded(metrics) end def test_raw_synthetics_header_is_passed_along_if_present @@ -730,7 +716,8 @@ def test_noticed_error_at_segment_and_txn_on_error # NOP -- allowing span and transaction to notice error end - assert_segment_noticed_error txn, /GET$/, timeout_error_class.name, /timeout|couldn't connect/i + # allow "timeout", "couldn't connect", or "couldnt_connect" + assert_segment_noticed_error txn, /GET$/, timeout_error_class.name, /timeout|couldn'?t(?:\s|_)connect/i assert_transaction_noticed_error txn, timeout_error_class.name end @@ -745,7 +732,8 @@ def test_noticed_error_only_at_segment_on_error end end - assert_segment_noticed_error txn, /GET$/, timeout_error_class.name, /timeout|couldn't connect/i + # allow "timeout", "couldn't connect", or "couldnt_connect" + assert_segment_noticed_error txn, /GET$/, timeout_error_class.name, /timeout|couldn'?t(?:\s|_)connect/i refute_transaction_noticed_error txn, timeout_error_class.name end