diff --git a/CHANGELOG.md b/CHANGELOG.md index e0f4f87f3b..89a65541f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,11 +2,15 @@ ## dev -Version adds instrumentation for 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. +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. 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) + 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.rb b/lib/new_relic/agent.rb index dcf33aed8d..2edaacf191 100644 --- a/lib/new_relic/agent.rb +++ b/lib/new_relic/agent.rb @@ -633,7 +633,9 @@ def add_custom_attributes(params) # THREAD_LOCAL_ACCESS def add_new_segment_attributes(params, segment) # Make sure not to override existing segment-level custom attributes segment_custom_keys = segment.attributes.custom_attributes.keys.map(&:to_sym) - segment.add_custom_attributes(params.reject { |k, _v| segment_custom_keys.include?(k.to_sym) }) + segment.add_custom_attributes(params.reject do |k, _v| + segment_custom_keys.include?(k.to_sym) if k.respond_to?(:to_sym) # param keys can be integers + end) end # Add custom attributes to the span event for the current span. Attributes will be visible on spans in the diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 81f8814814..54ebf932e5 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1379,6 +1379,14 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => '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.async_http' => { + :default => 'auto', + :public => true, + :type => String, + :dynamic_name => true, + :allowed_from_server => false, + :description => 'Controls auto-instrumentation of Async::HTTP at start up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' + }, :'instrumentation.bunny' => { :default => 'auto', :public => true, diff --git a/lib/new_relic/agent/http_clients/async_http_wrappers.rb b/lib/new_relic/agent/http_clients/async_http_wrappers.rb new file mode 100644 index 0000000000..394d8b7f48 --- /dev/null +++ b/lib/new_relic/agent/http_clients/async_http_wrappers.rb @@ -0,0 +1,83 @@ +# 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 'abstract' +require 'resolv' + +module NewRelic + module Agent + module HTTPClients + class AsyncHTTPResponse < AbstractResponse + def get_status_code + get_status_code_using(:status) + end + + def [](key) + to_hash[key.downcase]&.first + end + + def to_hash + @wrapped_response.headers.to_h + end + end + + class AsyncHTTPRequest < AbstractRequest + def initialize(connection, method, url, headers) + @connection = connection + @method = method + @url = ::NewRelic::Agent::HTTPClients::URIUtil.parse_and_normalize_url(url) + @headers = headers + end + + ASYNC_HTTP = 'Async::HTTP' + LHOST = 'host' + UHOST = 'Host' + COLON = ':' + + def type + ASYNC_HTTP + end + + def host_from_header + if hostname = (self[LHOST] || self[UHOST]) + hostname.split(COLON).first + end + end + + def host + host_from_header || uri.host.to_s + end + + def [](key) + return headers[key] unless headers.is_a?(Array) + + headers.each do |header| + return header[1] if header[0].casecmp?(key) + end + nil + end + + def []=(key, value) + if headers.is_a?(Array) + headers << [key, value] + else + headers[key] = value + end + end + + def uri + @url + end + + def headers + @headers + end + + def method + @method + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/async_http.rb b/lib/new_relic/agent/instrumentation/async_http.rb new file mode 100644 index 0000000000..9192ac5ca6 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/async_http.rb @@ -0,0 +1,26 @@ +# 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 'async_http/instrumentation' +require_relative 'async_http/chain' +require_relative 'async_http/prepend' + +DependencyDetection.defer do + named :async_http + + depends_on do + defined?(Async::HTTP) && Gem::Version.new(Async::HTTP::VERSION) >= Gem::Version.new('0.59.0') + end + + executes do + NewRelic::Agent.logger.info('Installing async_http instrumentation') + + require 'async/http/internet' + if use_prepend? + prepend_instrument Async::HTTP::Internet, NewRelic::Agent::Instrumentation::AsyncHttp::Prepend + else + chain_instrument NewRelic::Agent::Instrumentation::AsyncHttp::Chain + end + end +end diff --git a/lib/new_relic/agent/instrumentation/async_http/chain.rb b/lib/new_relic/agent/instrumentation/async_http/chain.rb new file mode 100644 index 0000000000..ad4109f042 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/async_http/chain.rb @@ -0,0 +1,23 @@ +# 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 'instrumentation' + +module NewRelic::Agent::Instrumentation + module AsyncHttp::Chain + def self.instrument! + ::Async::HTTP::Internet.class_eval do + include NewRelic::Agent::Instrumentation::AsyncHttp + + alias_method(:call_without_new_relic, :call) + + def call(method, url, headers = nil, body = nil) + call_with_new_relic(method, url, headers, body) do |hdr| + call_without_new_relic(method, url, hdr, body) + end + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/async_http/instrumentation.rb b/lib/new_relic/agent/instrumentation/async_http/instrumentation.rb new file mode 100644 index 0000000000..ceab6de327 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/async_http/instrumentation.rb @@ -0,0 +1,37 @@ +# 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/async_http_wrappers' + +module NewRelic::Agent::Instrumentation + module AsyncHttp + def call_with_new_relic(method, url, headers = nil, body = nil) + headers ||= {} # if it is nil, we need to make it a hash so we can insert headers + wrapped_request = NewRelic::Agent::HTTPClients::AsyncHTTPRequest.new(self, method, url, headers) + + segment = NewRelic::Agent::Tracer.start_external_request_segment( + library: wrapped_request.type, + uri: wrapped_request.uri, + procedure: wrapped_request.method + ) + + begin + response = nil + segment.add_request_headers(wrapped_request) + + NewRelic::Agent.disable_all_tracing do + response = NewRelic::Agent::Tracer.capture_segment_error(segment) do + yield(headers) + end + end + + wrapped_response = NewRelic::Agent::HTTPClients::AsyncHTTPResponse.new(response) + segment.process_response_headers(wrapped_response) + response + ensure + segment&.finish + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/async_http/prepend.rb b/lib/new_relic/agent/instrumentation/async_http/prepend.rb new file mode 100644 index 0000000000..b3bb3884ab --- /dev/null +++ b/lib/new_relic/agent/instrumentation/async_http/prepend.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 + +require_relative 'instrumentation' + +module NewRelic::Agent::Instrumentation + module AsyncHttp::Prepend + include NewRelic::Agent::Instrumentation::AsyncHttp + + def call(method, url, headers = nil, body = nil) + call_with_new_relic(method, url, headers, body) { |hdr| super(method, url, hdr, body) } + end + end +end diff --git a/lib/new_relic/agent/instrumentation/rails_notifications/action_controller.rb b/lib/new_relic/agent/instrumentation/rails_notifications/action_controller.rb index aaf1b90960..dd0285e248 100644 --- a/lib/new_relic/agent/instrumentation/rails_notifications/action_controller.rb +++ b/lib/new_relic/agent/instrumentation/rails_notifications/action_controller.rb @@ -34,6 +34,7 @@ subs = %w[send_file send_data + send_stream redirect_to halted_callback unpermitted_parameters] diff --git a/lib/new_relic/agent/transaction.rb b/lib/new_relic/agent/transaction.rb index cb3af595c4..7ca7ec8152 100644 --- a/lib/new_relic/agent/transaction.rb +++ b/lib/new_relic/agent/transaction.rb @@ -520,7 +520,7 @@ def needs_middleware_summary_metrics?(name) end def finish - return unless state.is_execution_traced? + return unless state.is_execution_traced? && initial_segment @end_time = Process.clock_gettime(Process::CLOCK_REALTIME) @duration = @end_time - @start_time diff --git a/newrelic.yml b/newrelic.yml index 4b2326ba80..d99e45f620 100644 --- a/newrelic.yml +++ b/newrelic.yml @@ -386,7 +386,11 @@ common: &default_settings # prepend, chain, disabled. # instrumentation.bunny: auto - # Controls auto-instrumentation of the concurrent-ruby library at start-up. May be + # Controls auto-instrumentation of Async::HTTP at start up. + # May be one of [auto|prepend|chain|disabled] + # instrumentation.async_http: 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/lib/multiverse/runner.rb b/test/multiverse/lib/multiverse/runner.rb index af61d3358c..ac8189bde7 100644 --- a/test/multiverse/lib/multiverse/runner.rb +++ b/test/multiverse/lib/multiverse/runner.rb @@ -104,7 +104,7 @@ def execute_suites(filter, opts) 'database' => %w[elasticsearch mongo redis sequel], '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[curb excon httpclient], + 'httpclients' => %w[async_http curb excon httpclient], 'httpclients_2' => %w[typhoeus net_http httprb ethon], 'infinite_tracing' => ['infinite_tracing'], diff --git a/test/multiverse/suites/async_http/Envfile b/test/multiverse/suites/async_http/Envfile new file mode 100644 index 0000000000..4488de786d --- /dev/null +++ b/test/multiverse/suites/async_http/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 + +ASYNC_HTTP_VERSIONS = [ + [nil, 2.5], + ['0.59.0', 2.5] +] + +def gem_list(async_http_version = nil) + <<~GEM_LIST + gem 'async-http'#{async_http_version} + gem 'rack' + GEM_LIST +end + +create_gemfiles(ASYNC_HTTP_VERSIONS) diff --git a/test/multiverse/suites/async_http/async_http_instrumentation_test.rb b/test/multiverse/suites/async_http/async_http_instrumentation_test.rb new file mode 100644 index 0000000000..3ea376388f --- /dev/null +++ b/test/multiverse/suites/async_http/async_http_instrumentation_test.rb @@ -0,0 +1,131 @@ +# 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 'http_client_test_cases' + +class AsyncHttpInstrumentationTest < Minitest::Test + include HttpClientTestCases + + def client_name + 'Async::HTTP' + end + + def timeout_error_class + Async::TimeoutError + end + + def simulate_error_response + Async::HTTP::Client.any_instance.stubs(:call).raises(timeout_error_class.new('read timeout reached')) + get_response + end + + def get_response(url = nil, headers = nil) + request_and_wait(:get, url || default_url, headers) + end + + def request_and_wait(method, url, headers = nil, body = nil) + resp = nil + Async do + begin + internet = Async::HTTP::Internet.new + resp = internet.send(method, url, headers) + @read_resp = resp&.read + ensure + internet&.close + end + end + resp + end + + def get_wrapped_response(url) + NewRelic::Agent::HTTPClients::AsyncHTTPResponse.new(get_response(url)) + end + + def head_response + request_and_wait(:head, default_url) + end + + def post_response + request_and_wait(:post, default_url, nil, '') + end + + def put_response + request_and_wait(:put, default_url, nil, '') + end + + def delete_response + request_and_wait(:delete, default_url, nil, '') + end + + def request_instance + NewRelic::Agent::HTTPClients::AsyncHTTPRequest.new(Async::HTTP::Internet.new, 'GET', default_url, {}) + end + + def response_instance(headers = {}) + resp = get_response(default_url, headers) + headers.each do |k, v| + resp.headers[k] = v + end + + NewRelic::Agent::HTTPClients::AsyncHTTPResponse.new(resp) + end + + def body(res) + @read_resp + end + + def test_noticed_error_at_segment_and_txn_on_error + # skipping this test + # Async gem does not allow the errors to escape the async block + # so the errors will never end up on the transaction, only ever the async http segment + end + + def test_raw_synthetics_header_is_passed_along_if_present_array + with_config(:"cross_application_tracer.enabled" => true) do + in_transaction do + NewRelic::Agent::Tracer.current_transaction.raw_synthetics_header = 'boo' + + get_response(default_url, [%w[itsaheader itsavalue]]) + + assert_equal 'boo', server.requests.last['HTTP_X_NEWRELIC_SYNTHETICS'] + end + end + end + + def test_raw_synthetics_header_is_passed_along_if_present_hash + with_config(:"cross_application_tracer.enabled" => true) do + in_transaction do + NewRelic::Agent::Tracer.current_transaction.raw_synthetics_header = 'boo' + + get_response(default_url, {'itsaheader' => 'itsavalue'}) + + assert_equal 'boo', server.requests.last['HTTP_X_NEWRELIC_SYNTHETICS'] + end + end + end + + def test_raw_synthetics_header_is_passed_along_if_present_protocol_header_hash + with_config(:"cross_application_tracer.enabled" => true) do + in_transaction do + NewRelic::Agent::Tracer.current_transaction.raw_synthetics_header = 'boo' + + get_response(default_url, ::Protocol::HTTP::Headers[{'itsaheader' => 'itsavalue'}]) + + assert_equal 'boo', server.requests.last['HTTP_X_NEWRELIC_SYNTHETICS'] + end + end + end + + def test_raw_synthetics_header_is_passed_along_if_present_protocol_header_array + with_config(:"cross_application_tracer.enabled" => true) do + in_transaction do + NewRelic::Agent::Tracer.current_transaction.raw_synthetics_header = 'boo' + + get_response(default_url, ::Protocol::HTTP::Headers[%w[itsaheader itsavalue]]) + + assert_equal 'boo', server.requests.last['HTTP_X_NEWRELIC_SYNTHETICS'] + end + end + end +end diff --git a/test/multiverse/suites/async_http/config/newrelic.yml b/test/multiverse/suites/async_http/config/newrelic.yml new file mode 100644 index 0000000000..51fa3fc536 --- /dev/null +++ b/test/multiverse/suites/async_http/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: + async_http: <%= $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/rails/action_controller_live_rum_test.rb b/test/multiverse/suites/rails/action_controller_live_rum_test.rb index 32b30ec04a..ed21ace9d8 100644 --- a/test/multiverse/suites/rails/action_controller_live_rum_test.rb +++ b/test/multiverse/suites/rails/action_controller_live_rum_test.rb @@ -13,6 +13,7 @@ def brains end def brain_stream + headers['last-modified'] = Time.now # tell Rack not to call #to_ary on the response object render(:inline => RESPONSE_BODY, :stream => true) end end diff --git a/test/multiverse/suites/rails/action_controller_other_test.rb b/test/multiverse/suites/rails/action_controller_other_test.rb index aec7091b8f..672469d6b8 100644 --- a/test/multiverse/suites/rails/action_controller_other_test.rb +++ b/test/multiverse/suites/rails/action_controller_other_test.rb @@ -17,6 +17,11 @@ def send_test_data send_data('wow its a adata') end + # send_stream + def send_test_stream + send_stream(filename: 'dinosaurs.html') + end + # halted_callback before_action :do_a_redirect, only: :halt_my_callback def halt_my_callback; end @@ -51,6 +56,13 @@ def test_send_data assert_metrics_recorded(['Controller/data/send_test_data', 'Ruby/ActionController/send_data']) end + def test_send_stream + skip if Gem::Version.new(Rails::VERSION::STRING) < Gem::Version.new('7.2.0') + get('/data/send_test_stream') + + assert_metrics_recorded(['Controller/data/send_test_stream', 'Ruby/ActionController/send_stream']) + end + def test_halted_callback get('/data/halt_my_callback') diff --git a/test/multiverse/suites/rails/activejob_test.rb b/test/multiverse/suites/rails/activejob_test.rb index 37d0e72e4d..5ecc6a769d 100644 --- a/test/multiverse/suites/rails/activejob_test.rb +++ b/test/multiverse/suites/rails/activejob_test.rb @@ -104,7 +104,7 @@ def test_code_information_recorded_with_new_transaction (NewRelic::Agent::Instrumentation::ActiveJobSubscriber::PAYLOAD_KEYS.size + 1).times do segment.expect(:params, {}, []) end - 3.times do + 4.times do segment.expect(:finish, []) end segment.expect(:record_scoped_metric=, nil, [false]) diff --git a/test/multiverse/suites/rails/before_suite.rb b/test/multiverse/suites/rails/before_suite.rb deleted file mode 100644 index 6ba45b158a..0000000000 --- a/test/multiverse/suites/rails/before_suite.rb +++ /dev/null @@ -1,23 +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 - -# These are hacks to make the 'rails' multiverse test suite compatible with -# Rails v7.1 released on 2023-10-05. -# -# TODO: refactor these out with non-hack replacements as time permits - -if Gem::Version.new(Rails.version) >= Gem::Version.new('7.1.0') - # NoMethodError (undefined method `to_ary' for an instance of ActionController::Streaming::Body): - # actionpack (7.1.0) lib/action_dispatch/http/response.rb:107:in `to_ary' - # actionpack (7.1.0) lib/action_dispatch/http/response.rb:509:in `to_ary' - # rack (3.0.8) lib/rack/body_proxy.rb:41:in `method_missing' - # rack (3.0.8) lib/rack/etag.rb:32:in `call' - # newrelic-ruby-agent/lib/new_relic/agent/instrumentation/middleware_tracing.rb:99:in `call' - require 'action_controller/railtie' - class ActionController::Streaming::Body - def to_ary - self - end - end -end diff --git a/test/multiverse/suites/rails/view_instrumentation_test.rb b/test/multiverse/suites/rails/view_instrumentation_test.rb index 9a2f2c3f97..00ae52d14d 100644 --- a/test/multiverse/suites/rails/view_instrumentation_test.rb +++ b/test/multiverse/suites/rails/view_instrumentation_test.rb @@ -65,19 +65,6 @@ def collection_render render((1..3).map { |x| Foo.new }) end - # proc rendering isn't available in rails 3 but you can do nonsense like this - # and assign an enumerable object to the response body. - def proc_render - streamer = Class.new do - def each - 10_000.times do |i| - yield("This is line #{i}\n") - end - end - end - self.response_body = streamer.new - end - def raise_render raise 'this is an uncaught RuntimeError' end @@ -85,7 +72,7 @@ def raise_render class ViewInstrumentationTest < ActionDispatch::IntegrationTest include MultiverseHelpers - RENDERING_OPTIONS = [:js_render, :xml_render, :proc_render, :json_render] + RENDERING_OPTIONS = [:js_render, :xml_render, :json_render] setup_and_teardown_agent do # ActiveSupport testing keeps blowing away my subscribers on @@ -98,7 +85,7 @@ class ViewInstrumentationTest < ActionDispatch::IntegrationTest end end - (ViewsController.action_methods - %w[raise_render collection_render haml_render proc_render]).each do |method| + (ViewsController.action_methods - %w[raise_render collection_render haml_render]).each do |method| define_method("test_sanity_#{method}") do get "/views/#{method}"