From 19d6396b36dcb5b42dadc39da4afa1c473b2b0c5 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 09:41:01 -0500 Subject: [PATCH 01/16] add instrumentation files for sqs --- .../agent/configuration/default_source.rb | 8 ++++ .../agent/instrumentation/aws_sqs.rb | 25 +++++++++++++ .../agent/instrumentation/aws_sqs/chain.rb | 37 +++++++++++++++++++ .../aws_sqs/instrumentation.rb | 23 ++++++++++++ .../agent/instrumentation/aws_sqs/prepend.rb | 21 +++++++++++ test/multiverse/suites/awssqs/Envfile | 9 +++++ .../awssqs/awssqs_instrumentation_test.rb | 15 ++++++++ .../suites/awssqs/config/newrelic.yml | 19 ++++++++++ 8 files changed, 157 insertions(+) create mode 100644 lib/new_relic/agent/instrumentation/aws_sqs.rb create mode 100644 lib/new_relic/agent/instrumentation/aws_sqs/chain.rb create mode 100644 lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb create mode 100644 lib/new_relic/agent/instrumentation/aws_sqs/prepend.rb create mode 100644 test/multiverse/suites/awssqs/Envfile create mode 100644 test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb create mode 100644 test/multiverse/suites/awssqs/config/newrelic.yml diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 75f7bbe8b3..0d351c97f6 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -1450,6 +1450,14 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'Controls auto-instrumentation of bunny at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' }, + :'instrumentation.aws_sqs' => { + :default => 'auto', + :public => true, + :type => String, + :dynamic_name => true, + :allowed_from_server => false, + :description => 'Controls auto-instrumentation of the aws-sdk-sqs library at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.' + }, :'instrumentation.dynamodb' => { :default => 'auto', :public => true, diff --git a/lib/new_relic/agent/instrumentation/aws_sqs.rb b/lib/new_relic/agent/instrumentation/aws_sqs.rb new file mode 100644 index 0000000000..40b7a5db2b --- /dev/null +++ b/lib/new_relic/agent/instrumentation/aws_sqs.rb @@ -0,0 +1,25 @@ +# 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 'awssqs/instrumentation' +require_relative 'awssqs/chain' +require_relative 'awssqs/prepend' + +DependencyDetection.defer do + named :aws_sqs + + depends_on do + defined?(::Aws::SQS::Client) + end + + executes do + ::NewRelic::Agent.logger.info('Installing aws-sdk-sqs instrumentation') + + if use_prepend? + prepend_instrument ::Aws::SQS::Client, NewRelic::Agent::Instrumentation::AwsSqs::Prepend + else + chain_instrument NewRelic::Agent::Instrumentation::AwsSqs::Chain + end + end +end diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/chain.rb b/lib/new_relic/agent/instrumentation/aws_sqs/chain.rb new file mode 100644 index 0000000000..e739b37e09 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/aws_sqs/chain.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 + +module NewRelic::Agent::Instrumentation + module AwsSqs::Chain + def self.instrument! + ::Aws::SQS::Client.class_eval do + include NewRelic::Agent::Instrumentation::AwsSqs + + alias_method(:send_message_without_new_relic, :send_message) + + def send_message(*args) + send_message_with_new_relic(*args) do + send_message_without_new_relic(*args) + end + end + + alias_method(:send_message_batch_without_new_relic, :send_message_batch) + + def send_message_batch(*args) + send_message_batch_with_new_relic(*args) do + send_message_batch_without_new_relic(*args) + end + end + + alias_method(:receive_message_without_new_relic, :receive_message) + + def receive_message(*args) + receive_message_with_new_relic(*args) do + receive_message_without_new_relic(*args) + end + end + end + end + end +end diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb new file mode 100644 index 0000000000..a4fcfea381 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.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 + +module NewRelic::Agent::Instrumentation + module AwsSqs + + def send_message_with_new_relic(*args) + # add instrumentation content here + yield + end + + def send_message_batch_with_new_relic(*args) + # add instrumentation content here + yield + end + + def receive_message_with_new_relic(*args) + # add instrumentation content here + yield + end + end +end diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/prepend.rb b/lib/new_relic/agent/instrumentation/aws_sqs/prepend.rb new file mode 100644 index 0000000000..fff44e1fb0 --- /dev/null +++ b/lib/new_relic/agent/instrumentation/aws_sqs/prepend.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 + +module NewRelic::Agent::Instrumentation + module AwsSqs::Prepend + include NewRelic::Agent::Instrumentation::AwsSqs + + def send_message(*args) + send_message_with_new_relic(*args) { super } + end + + def send_message_batch(*args) + send_message_batch_with_new_relic(*args) { super } + end + + def receive_message(*args) + receive_message_with_new_relic(*args) { super } + end + end +end diff --git a/test/multiverse/suites/awssqs/Envfile b/test/multiverse/suites/awssqs/Envfile new file mode 100644 index 0000000000..e801276c4c --- /dev/null +++ b/test/multiverse/suites/awssqs/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 'aws-sdk-sqs' +RB diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb new file mode 100644 index 0000000000..0023204e45 --- /dev/null +++ b/test/multiverse/suites/awssqs/awssqs_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 AwssqsInstrumentationTest < 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 diff --git a/test/multiverse/suites/awssqs/config/newrelic.yml b/test/multiverse/suites/awssqs/config/newrelic.yml new file mode 100644 index 0000000000..613e0eacec --- /dev/null +++ b/test/multiverse/suites/awssqs/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: + aws_sqs: <%= $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 From 8a90bda88381a7ceac6deed3edc34c7842ae11c5 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 10:46:35 -0500 Subject: [PATCH 02/16] add instrumentation --- .../agent/instrumentation/aws_sqs.rb | 6 +- .../aws_sqs/instrumentation.rb | 62 ++++++++++++++++--- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs.rb b/lib/new_relic/agent/instrumentation/aws_sqs.rb index 40b7a5db2b..cc60c42b6c 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs.rb @@ -2,9 +2,9 @@ # See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details. # frozen_string_literal: true -require_relative 'awssqs/instrumentation' -require_relative 'awssqs/chain' -require_relative 'awssqs/prepend' +require_relative 'aws_sqs/instrumentation' +require_relative 'aws_sqs/chain' +require_relative 'aws_sqs/prepend' DependencyDetection.defer do named :aws_sqs diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index a4fcfea381..8568071bce 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -4,20 +4,68 @@ module NewRelic::Agent::Instrumentation module AwsSqs - def send_message_with_new_relic(*args) - # add instrumentation content here - yield + segment = nil + begin + queue_name = get_queue_name(args[0]) + segment = NewRelic::Agent::Tracer.start_message_broker_segment( + action: 'Produce', + library: 'SQS', + destination_type: 'Queue', + destination_name: queue_name + ) + rescue => e + NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) + end + NewRelic::Agent::Tracer.capture_segment_error(segment) do + yield + end + ensure + segment&.finish end def send_message_batch_with_new_relic(*args) - # add instrumentation content here - yield + segment = nil + begin + queue_name = get_queue_name(args[0]) + segment = NewRelic::Agent::Tracer.start_message_broker_segment( + action: 'Produce', + library: 'SQS', + destination_type: 'Queue', + destination_name: queue_name + ) + rescue => e + NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) + end + NewRelic::Agent::Tracer.capture_segment_error(segment) do + yield + end + ensure + segment&.finish end def receive_message_with_new_relic(*args) - # add instrumentation content here - yield + segment = nil + begin + queue_name = get_queue_name(args[0]) + segment = NewRelic::Agent::Tracer.start_message_broker_segment( + action: 'Consume', + library: 'SQS', + destination_type: 'Queue', + destination_name: queue_name + ) + rescue => e + NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) + end + NewRelic::Agent::Tracer.capture_segment_error(segment) do + yield + end + ensure + segment&.finish + end + + def get_queue_name(params) + params[:queue_url].split('/').last end end end From a3b5d8923b83daf3678267c1a59d7a0c414f237c Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 11:02:12 -0500 Subject: [PATCH 03/16] add entity attributes --- .../instrumentation/aws_sqs/instrumentation.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index 8568071bce..3cf6c2073c 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -14,6 +14,10 @@ def send_message_with_new_relic(*args) destination_type: 'Queue', destination_name: queue_name ) + segment&.add_agent_attribute('messaging.system', 'aws_sqs') + segment&.add_agent_attribute('cloud.region', config&.region) + segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) + # segment&.add_agent_attribute('messaging.destination.name', queue_name) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -34,6 +38,9 @@ def send_message_batch_with_new_relic(*args) destination_type: 'Queue', destination_name: queue_name ) + segment&.add_agent_attribute('messaging.system', 'aws_sqs') + segment&.add_agent_attribute('cloud.region', config&.region) + segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -54,6 +61,9 @@ def receive_message_with_new_relic(*args) destination_type: 'Queue', destination_name: queue_name ) + segment&.add_agent_attribute('messaging.system', 'aws_sqs') + segment&.add_agent_attribute('cloud.region', config&.region) + segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -67,5 +77,9 @@ def receive_message_with_new_relic(*args) def get_queue_name(params) params[:queue_url].split('/').last end + + def get_account_id(params) + params[:queue_url].split('/')[-2] + end end end From b3005733eb9c120e2e9a189fc66c69a6e73cc691 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 11:37:46 -0500 Subject: [PATCH 04/16] rubocop --- lib/new_relic/agent/instrumentation/aws_sqs.rb | 6 +++--- .../instrumentation/aws_sqs/instrumentation.rb | 16 +++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs.rb b/lib/new_relic/agent/instrumentation/aws_sqs.rb index cc60c42b6c..7f5acb82c3 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs.rb @@ -10,14 +10,14 @@ named :aws_sqs depends_on do - defined?(::Aws::SQS::Client) + defined?(Aws::SQS::Client) end executes do - ::NewRelic::Agent.logger.info('Installing aws-sdk-sqs instrumentation') + NewRelic::Agent.logger.info('Installing aws-sdk-sqs instrumentation') if use_prepend? - prepend_instrument ::Aws::SQS::Client, NewRelic::Agent::Instrumentation::AwsSqs::Prepend + prepend_instrument Aws::SQS::Client, NewRelic::Agent::Instrumentation::AwsSqs::Prepend else chain_instrument NewRelic::Agent::Instrumentation::AwsSqs::Chain end diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index 3cf6c2073c..d42467f1da 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -9,15 +9,15 @@ def send_message_with_new_relic(*args) begin queue_name = get_queue_name(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( - action: 'Produce', + action: :produce, library: 'SQS', - destination_type: 'Queue', + destination_type: :queue, destination_name: queue_name ) segment&.add_agent_attribute('messaging.system', 'aws_sqs') segment&.add_agent_attribute('cloud.region', config&.region) segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) - # segment&.add_agent_attribute('messaging.destination.name', queue_name) + segment&.add_agent_attribute('messaging.destination.name', queue_name) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -33,14 +33,15 @@ def send_message_batch_with_new_relic(*args) begin queue_name = get_queue_name(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( - action: 'Produce', + action: :produce, library: 'SQS', - destination_type: 'Queue', + destination_type: :queue, destination_name: queue_name ) segment&.add_agent_attribute('messaging.system', 'aws_sqs') segment&.add_agent_attribute('cloud.region', config&.region) segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) + segment&.add_agent_attribute('messaging.destination.name', queue_name) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -56,14 +57,15 @@ def receive_message_with_new_relic(*args) begin queue_name = get_queue_name(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( - action: 'Consume', + action: :consume, library: 'SQS', - destination_type: 'Queue', + destination_type: :queue, destination_name: queue_name ) segment&.add_agent_attribute('messaging.system', 'aws_sqs') segment&.add_agent_attribute('cloud.region', config&.region) segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) + segment&.add_agent_attribute('messaging.destination.name', queue_name) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end From ddf210de752360391b73811cf4be526a67863f83 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 11:42:17 -0500 Subject: [PATCH 05/16] add tests --- test/multiverse/suites/awssqs/Envfile | 1 + .../awssqs/awssqs_instrumentation_test.rb | 76 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/test/multiverse/suites/awssqs/Envfile b/test/multiverse/suites/awssqs/Envfile index e801276c4c..97608c2ab0 100644 --- a/test/multiverse/suites/awssqs/Envfile +++ b/test/multiverse/suites/awssqs/Envfile @@ -6,4 +6,5 @@ instrumentation_methods :chain, :prepend gemfile <<~RB gem 'aws-sdk-sqs' + gem 'nokogiri' RB diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index 0023204e45..f3798e8a60 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -4,12 +4,88 @@ class AwssqsInstrumentationTest < Minitest::Test def setup + Aws.config.update(stub_responses: true) @stats_engine = NewRelic::Agent.instance.stats_engine end def teardown + mocha_teardown NewRelic::Agent.instance.stats_engine.clear_stats end + def create_client + Aws::SQS::Client.new(region: 'us-east-2') + end + + def test_all_attributes_added_to_segment_send_message + client = create_client + + in_transaction do |txn| + client.send_message({ + queue_url: 'https://sqs.us-east-2.amazonaws.com/123456789/itsatestqueuewow', + message_body: 'wow, its a message' + }) + end + + spans = harvest_span_events! + span = spans[1][0] + + assert_equal 'MessageBroker/SQS/Queue/Produce/Named/itsatestqueuewow', span[0]['name'] + + assert_equal 'aws_sqs', span[2]['messaging.system'] + assert_equal 'us-east-2', span[2]['cloud.region'] + assert_equal '123456789', span[2]['cloud.account.id'] + assert_equal 'itsatestqueuewow', span[2]['messaging.destination.name'] + end + + def test_all_attributes_added_to_segment_send_message_batch + client = create_client + + in_transaction do |txn| + client.send_message_batch({ + queue_url: 'https://sqs.us-east-2.amazonaws.com/123456789/itsatestqueuewow', + entries: [ + { + id: "msq1", + message_body: "wow 1", + }, + { + id: "msq2", + message_body: "wow 2", + } + ] + }) + end + + spans = harvest_span_events! + span = spans[1][0] + + assert_equal 'MessageBroker/SQS/Queue/Produce/Named/itsatestqueuewow', span[0]['name'] + + assert_equal 'aws_sqs', span[2]['messaging.system'] + assert_equal 'us-east-2', span[2]['cloud.region'] + assert_equal '123456789', span[2]['cloud.account.id'] + assert_equal 'itsatestqueuewow', span[2]['messaging.destination.name'] + end + def test_all_attributes_added_to_segment_receive_message + client = create_client + + in_transaction do |txn| + client.receive_message({ + queue_url: 'https://sqs.us-east-2.amazonaws.com/123456789/itsatestqueuewow' + }) + end + + spans = harvest_span_events! + span = spans[1][0] + + assert_equal 'MessageBroker/SQS/Queue/Consume/Named/itsatestqueuewow', span[0]['name'] + + assert_equal 'aws_sqs', span[2]['messaging.system'] + assert_equal 'us-east-2', span[2]['cloud.region'] + assert_equal '123456789', span[2]['cloud.account.id'] + assert_equal 'itsatestqueuewow', span[2]['messaging.destination.name'] + end + # Add tests here end From 81c84538feb3babcd12f7f15e0f45b7d0b91c845 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 11:42:43 -0500 Subject: [PATCH 06/16] rubocop --- .../suites/awssqs/awssqs_instrumentation_test.rb | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index f3798e8a60..59731e397a 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -46,12 +46,12 @@ def test_all_attributes_added_to_segment_send_message_batch queue_url: 'https://sqs.us-east-2.amazonaws.com/123456789/itsatestqueuewow', entries: [ { - id: "msq1", - message_body: "wow 1", + id: 'msq1', + message_body: 'wow 1' }, { - id: "msq2", - message_body: "wow 2", + id: 'msq2', + message_body: 'wow 2' } ] }) @@ -67,6 +67,7 @@ def test_all_attributes_added_to_segment_send_message_batch assert_equal '123456789', span[2]['cloud.account.id'] assert_equal 'itsatestqueuewow', span[2]['messaging.destination.name'] end + def test_all_attributes_added_to_segment_receive_message client = create_client From d9c6f553fb5d5c91d63915e7b4d8576793221325 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 11:48:27 -0500 Subject: [PATCH 07/16] move attributes to shared location --- .../aws_sqs/instrumentation.rb | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index d42467f1da..303d21fbda 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -4,20 +4,19 @@ module NewRelic::Agent::Instrumentation module AwsSqs + MESSAGING_LIBRARY = 'SQS' + def send_message_with_new_relic(*args) segment = nil begin queue_name = get_queue_name(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: :produce, - library: 'SQS', + library: MESSAGING_LIBRARY, destination_type: :queue, destination_name: queue_name ) - segment&.add_agent_attribute('messaging.system', 'aws_sqs') - segment&.add_agent_attribute('cloud.region', config&.region) - segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) - segment&.add_agent_attribute('messaging.destination.name', queue_name) + add_aws_attributes(segment, queue_name, args[0]) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -34,16 +33,13 @@ def send_message_batch_with_new_relic(*args) queue_name = get_queue_name(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: :produce, - library: 'SQS', + library: MESSAGING_LIBRARY, destination_type: :queue, destination_name: queue_name ) - segment&.add_agent_attribute('messaging.system', 'aws_sqs') - segment&.add_agent_attribute('cloud.region', config&.region) - segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) - segment&.add_agent_attribute('messaging.destination.name', queue_name) + add_aws_attributes(segment, queue_name, args[0]) rescue => e - NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) + NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message_batch ', e) end NewRelic::Agent::Tracer.capture_segment_error(segment) do yield @@ -58,16 +54,13 @@ def receive_message_with_new_relic(*args) queue_name = get_queue_name(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: :consume, - library: 'SQS', + library: MESSAGING_LIBRARY, destination_type: :queue, destination_name: queue_name ) - segment&.add_agent_attribute('messaging.system', 'aws_sqs') - segment&.add_agent_attribute('cloud.region', config&.region) - segment&.add_agent_attribute('cloud.account.id', get_account_id(args[0])) - segment&.add_agent_attribute('messaging.destination.name', queue_name) + add_aws_attributes(segment, queue_name, args[0]) rescue => e - NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) + NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#receive_message ', e) end NewRelic::Agent::Tracer.capture_segment_error(segment) do yield @@ -76,6 +69,13 @@ def receive_message_with_new_relic(*args) segment&.finish end + def add_aws_attributes(segment, queue_name, params) + segment&.add_agent_attribute('messaging.system', 'aws_sqs') + segment&.add_agent_attribute('cloud.region', config&.region) + segment&.add_agent_attribute('cloud.account.id', get_account_id(params)) + segment&.add_agent_attribute('messaging.destination.name', queue_name) + end + def get_queue_name(params) params[:queue_url].split('/').last end From 2178e903f2faf759935a3c58f457a2b19ddd3f7d Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 12:07:34 -0500 Subject: [PATCH 08/16] add changelog entry --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c8eb0adb3..ab03a81c9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ ## dev -Version fixes a bug related to expected errors not bearing a "true" value for the "expected" attribute if expected as a result of an HTTP status code match and changes the way Stripe instrumentation metrics are named to prevent high-cardinality issues. +Version introduces instrumentation for the aws-sdk-sqs gem, fixes a bug related to expected errors not bearing a "true" value for the "expected" attribute if expected as a result of an HTTP status code match and changes the way Stripe instrumentation metrics are named to prevent high-cardinality issues. + +- **Feature: Add instrumentation for SQS** + + The agent has added instrumentation for the aws-sdk-sqs gem. The agent will now record message broker spans for SQS client calls made with the aws-sdk-sqs gem. [PR#2679](https://github.com/newrelic/newrelic-ruby-agent/pull/2679) - **Bugfix: HTTP status code based expected errors will now have an "expected" value of "true"** From 21ee22bfb31f829fe6fe04e410dff129712c1974 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 12:42:39 -0500 Subject: [PATCH 09/16] remove comment --- test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index 59731e397a..b496d800de 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -87,6 +87,4 @@ def test_all_attributes_added_to_segment_receive_message assert_equal '123456789', span[2]['cloud.account.id'] assert_equal 'itsatestqueuewow', span[2]['messaging.destination.name'] end - - # Add tests here end From fc7f03780c7567ab2bb77b19f6ce9ca8e6e619ac Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 14:12:35 -0500 Subject: [PATCH 10/16] add segment guard --- .../agent/instrumentation/aws_sqs/instrumentation.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index 303d21fbda..aae96965eb 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -70,10 +70,12 @@ def receive_message_with_new_relic(*args) end def add_aws_attributes(segment, queue_name, params) - segment&.add_agent_attribute('messaging.system', 'aws_sqs') - segment&.add_agent_attribute('cloud.region', config&.region) - segment&.add_agent_attribute('cloud.account.id', get_account_id(params)) - segment&.add_agent_attribute('messaging.destination.name', queue_name) + return unless segment + + segment.add_agent_attribute('messaging.system', 'aws_sqs') + segment.add_agent_attribute('cloud.region', config&.region) + segment.add_agent_attribute('cloud.account.id', get_account_id(params)) + segment.add_agent_attribute('messaging.destination.name', queue_name) end def get_queue_name(params) From 6e1c09820228c80800eb79021d4a1c79aaa72b95 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 14:23:21 -0500 Subject: [PATCH 11/16] only split once --- .../aws_sqs/instrumentation.rb | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index aae96965eb..f18d94d577 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -9,14 +9,14 @@ module AwsSqs def send_message_with_new_relic(*args) segment = nil begin - queue_name = get_queue_name(args[0]) + info = get_url_info(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: :produce, library: MESSAGING_LIBRARY, destination_type: :queue, - destination_name: queue_name + destination_name: info[:queue_name] ) - add_aws_attributes(segment, queue_name, args[0]) + add_aws_attributes(segment, info) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) end @@ -30,14 +30,14 @@ def send_message_with_new_relic(*args) def send_message_batch_with_new_relic(*args) segment = nil begin - queue_name = get_queue_name(args[0]) + info = get_url_info(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: :produce, library: MESSAGING_LIBRARY, destination_type: :queue, - destination_name: queue_name + destination_name: info[:queue_name] ) - add_aws_attributes(segment, queue_name, args[0]) + add_aws_attributes(segment, info) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message_batch ', e) end @@ -51,14 +51,14 @@ def send_message_batch_with_new_relic(*args) def receive_message_with_new_relic(*args) segment = nil begin - queue_name = get_queue_name(args[0]) + info = get_url_info(args[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( action: :consume, library: MESSAGING_LIBRARY, destination_type: :queue, - destination_name: queue_name + destination_name: info[:queue_name] ) - add_aws_attributes(segment, queue_name, args[0]) + add_aws_attributes(segment, info) rescue => e NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#receive_message ', e) end @@ -69,21 +69,21 @@ def receive_message_with_new_relic(*args) segment&.finish end - def add_aws_attributes(segment, queue_name, params) + def add_aws_attributes(segment, info) return unless segment segment.add_agent_attribute('messaging.system', 'aws_sqs') segment.add_agent_attribute('cloud.region', config&.region) - segment.add_agent_attribute('cloud.account.id', get_account_id(params)) - segment.add_agent_attribute('messaging.destination.name', queue_name) + segment.add_agent_attribute('cloud.account.id', info[:account_id]) + segment.add_agent_attribute('messaging.destination.name', info[:queue_name]) end - def get_queue_name(params) - params[:queue_url].split('/').last - end - - def get_account_id(params) - params[:queue_url].split('/')[-2] + def get_url_info(params) + split = params[:queue_url].split('/') + { + queue_name: split.last, + account_id: split[-2] + } end end end From f251b1498325f0b1ff4e7073227890e03e985b11 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 14:36:57 -0500 Subject: [PATCH 12/16] add sad tests --- .../awssqs/awssqs_instrumentation_test.rb | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index b496d800de..38de645761 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -9,6 +9,7 @@ def setup end def teardown + harvest_span_events! mocha_teardown NewRelic::Agent.instance.stats_engine.clear_stats end @@ -87,4 +88,58 @@ def test_all_attributes_added_to_segment_receive_message assert_equal '123456789', span[2]['cloud.account.id'] assert_equal 'itsatestqueuewow', span[2]['messaging.destination.name'] end + + def test_error_send_message + client = create_client + + log = with_array_logger(:info) do + in_transaction do |txn| + begin + client.send_message({ + queue_url: 42 + }) + rescue + # will cause an error in the instrumentation, but also will make the sdk raise an error + end + end + end + + assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client#send_message') + end + + def test_error_send_message_batch + client = create_client + + log = with_array_logger(:info) do + in_transaction do |txn| + begin + client.send_message_batch({ + queue_url: 42 + }) + rescue + # will cause an error in the instrumentation, but also will make the sdk raise an error + end + end + end + + assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client#send_message_batch') + end + + def test_error_receive_message + client = create_client + + log = with_array_logger(:info) do + in_transaction do |txn| + begin + client.receive_message({ + queue_url: 42 + }) + rescue + # will cause an error in the instrumentation, but also will make the sdk raise an error + end + end + end + + assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client#receive_message') + end end From e5f533cd9b5e72ac73f4749d6f5fb50d10e1cd9a Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 14:39:22 -0500 Subject: [PATCH 13/16] rubocop --- test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index 38de645761..20e0ae006e 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -96,7 +96,7 @@ def test_error_send_message in_transaction do |txn| begin client.send_message({ - queue_url: 42 + queue_url: 42 }) rescue # will cause an error in the instrumentation, but also will make the sdk raise an error From 6f51feb02eb50e8e847761d56154b0db3fc23ecc Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 18 Jun 2024 14:51:24 -0500 Subject: [PATCH 14/16] Update CHANGELOG.md Co-authored-by: Hannah Ramadan <76922290+hannahramadan@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab03a81c9e..f1c72a0bc7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Version introduces instrumentation for the aws-sdk-sqs gem, fixes a bug re - **Feature: Add instrumentation for SQS** - The agent has added instrumentation for the aws-sdk-sqs gem. The agent will now record message broker spans for SQS client calls made with the aws-sdk-sqs gem. [PR#2679](https://github.com/newrelic/newrelic-ruby-agent/pull/2679) + The agent has added instrumentation for the [aws-sdk-sqs gem](https://rubygems.org/gems/aws-sdk-sqs). The agent will now record message broker spans for SQS client calls made with the aws-sdk-sqs gem. [PR#2679](https://github.com/newrelic/newrelic-ruby-agent/pull/2679) - **Bugfix: HTTP status code based expected errors will now have an "expected" value of "true"** From 278a3b509968facd0aa481d2241325dc37d9da56 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 20 Jun 2024 10:36:18 -0500 Subject: [PATCH 15/16] remove unused stuff left from scaffold command --- test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index 20e0ae006e..0455c44730 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -5,13 +5,11 @@ class AwssqsInstrumentationTest < Minitest::Test def setup Aws.config.update(stub_responses: true) - @stats_engine = NewRelic::Agent.instance.stats_engine end def teardown harvest_span_events! mocha_teardown - NewRelic::Agent.instance.stats_engine.clear_stats end def create_client From df6d3ca0acb102e7b55e2c0b89ed32c4a46aabdc Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 20 Jun 2024 10:45:10 -0500 Subject: [PATCH 16/16] refactor --- .../aws_sqs/instrumentation.rb | 48 +++++-------------- .../awssqs/awssqs_instrumentation_test.rb | 38 +-------------- 2 files changed, 14 insertions(+), 72 deletions(-) diff --git a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb index f18d94d577..e78bfcb9d1 100644 --- a/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/aws_sqs/instrumentation.rb @@ -7,60 +7,36 @@ module AwsSqs MESSAGING_LIBRARY = 'SQS' def send_message_with_new_relic(*args) - segment = nil - begin - info = get_url_info(args[0]) - segment = NewRelic::Agent::Tracer.start_message_broker_segment( - action: :produce, - library: MESSAGING_LIBRARY, - destination_type: :queue, - destination_name: info[:queue_name] - ) - add_aws_attributes(segment, info) - rescue => e - NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message ', e) - end - NewRelic::Agent::Tracer.capture_segment_error(segment) do + with_tracing(:produce, args) do yield end - ensure - segment&.finish end def send_message_batch_with_new_relic(*args) - segment = nil - begin - info = get_url_info(args[0]) - segment = NewRelic::Agent::Tracer.start_message_broker_segment( - action: :produce, - library: MESSAGING_LIBRARY, - destination_type: :queue, - destination_name: info[:queue_name] - ) - add_aws_attributes(segment, info) - rescue => e - NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#send_message_batch ', e) - end - NewRelic::Agent::Tracer.capture_segment_error(segment) do + with_tracing(:produce, args) do yield end - ensure - segment&.finish end def receive_message_with_new_relic(*args) + with_tracing(:consume, args) do + yield + end + end + + def with_tracing(action, params) segment = nil begin - info = get_url_info(args[0]) + info = get_url_info(params[0]) segment = NewRelic::Agent::Tracer.start_message_broker_segment( - action: :consume, + action: action, library: MESSAGING_LIBRARY, destination_type: :queue, destination_name: info[:queue_name] ) add_aws_attributes(segment, info) rescue => e - NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client#receive_message ', e) + NewRelic::Agent.logger.error('Error starting message broker segment in Aws::SQS::Client', e) end NewRelic::Agent::Tracer.capture_segment_error(segment) do yield @@ -69,6 +45,8 @@ def receive_message_with_new_relic(*args) segment&.finish end + private + def add_aws_attributes(segment, info) return unless segment diff --git a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb index 0455c44730..03e30ea5be 100644 --- a/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb +++ b/test/multiverse/suites/awssqs/awssqs_instrumentation_test.rb @@ -102,42 +102,6 @@ def test_error_send_message end end - assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client#send_message') - end - - def test_error_send_message_batch - client = create_client - - log = with_array_logger(:info) do - in_transaction do |txn| - begin - client.send_message_batch({ - queue_url: 42 - }) - rescue - # will cause an error in the instrumentation, but also will make the sdk raise an error - end - end - end - - assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client#send_message_batch') - end - - def test_error_receive_message - client = create_client - - log = with_array_logger(:info) do - in_transaction do |txn| - begin - client.receive_message({ - queue_url: 42 - }) - rescue - # will cause an error in the instrumentation, but also will make the sdk raise an error - end - end - end - - assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client#receive_message') + assert_log_contains(log, 'Error starting message broker segment in Aws::SQS::Client') end end