Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add async http instrumentation #2272

Merged
merged 20 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

## dev

Version <dev> 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 <dev> 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.

- **Feature: Add instrumentation for Async::HTTP**

The agent will now record spans for Async::HTTP requests. [PR#2272](https://github.com/newrelic/newrelic-ruby-agent/pull/2272)


- **Feature: Prevent the agent from starting in rails commands in Rails 7**

Expand Down
8 changes: 8 additions & 0 deletions lib/new_relic/agent/configuration/default_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
83 changes: 83 additions & 0 deletions lib/new_relic/agent/http_clients/async_http_wrappers.rb
Original file line number Diff line number Diff line change
@@ -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)
@wrapped_response.headers.to_h[key.downcase]&.first
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
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 = ':'
Comment on lines +34 to +36
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not for this PR, but maybe it's time to move these constants to a shared location.


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
31 changes: 31 additions & 0 deletions lib/new_relic/agent/instrumentation/async_http.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# 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'
tannalynn marked this conversation as resolved.
Show resolved Hide resolved

depends_on do
# The class that needs to be defined to prepend/chain onto. This can be used
# to determine whether the library is installed.
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
defined?(Async::HTTP)
# Add any additional requirements to verify whether this instrumentation
# should be installed
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
end

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

require 'async/http/internet'
require 'new_relic/agent/http_clients/async_http_wrappers'
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
if use_prepend?
prepend_instrument Async::HTTP::Internet, NewRelic::Agent::Instrumentation::AsyncHttp::Prepend
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
else
chain_instrument NewRelic::Agent::Instrumentation::AsyncHttp::Chain
end
end
end
23 changes: 23 additions & 0 deletions lib/new_relic/agent/instrumentation/async_http/chain.rb
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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 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
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
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
15 changes: 15 additions & 0 deletions lib/new_relic/agent/instrumentation/async_http/prepend.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 5 additions & 1 deletion newrelic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,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
tannalynn marked this conversation as resolved.
Show resolved Hide resolved

# Controls auto-instrumentation of the concurrent-ruby library at start up. May be
# one of: auto, prepend, chain, disabled.
# instrumentation.concurrent_ruby: auto

Expand Down
10 changes: 10 additions & 0 deletions test/multiverse/suites/async_http/Envfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# 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 'async-http'
gem 'rack'
RB
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
131 changes: 131 additions & 0 deletions test/multiverse/suites/async_http/async_http_instrumentation_test.rb
Original file line number Diff line number Diff line change
@@ -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
internet = Async::HTTP::Internet.new
resp = internet.send(method, url, headers)
@read_resp = resp&.read
rescue => e
puts "**************************ERROR: #{e}"
tannalynn marked this conversation as resolved.
Show resolved Hide resolved
ensure
internet&.close
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
19 changes: 19 additions & 0 deletions test/multiverse/suites/async_http/config/newrelic.yml
Original file line number Diff line number Diff line change
@@ -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
Loading