From 89a7b364476f4715471626ea3d8ace0d31d41b7e Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 15:28:10 -0500 Subject: [PATCH 1/8] not all credentials are just credentials, but they do all seem to include credentials --- lib/new_relic/agent/aws.rb | 5 ++++- test/new_relic/agent/aws_test.rb | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/new_relic/agent/aws.rb b/lib/new_relic/agent/aws.rb index 5791a6b723..248b4ae7c7 100644 --- a/lib/new_relic/agent/aws.rb +++ b/lib/new_relic/agent/aws.rb @@ -9,8 +9,11 @@ module Aws HEX_MASK = '7fffffffff80' def self.create_arn(service, resource, config) + access_key_id = config&.credentials&.credentials&.access_key_id if config&.credentials&.credentials&.respond_to?(:access_key_id) + return unless access_key_id + region = config.region - account_id = NewRelic::Agent::Aws.convert_access_key_to_account_id(config.credentials.access_key_id) + account_id = NewRelic::Agent::Aws.convert_access_key_to_account_id(access_key_id) "arn:aws:#{service}:#{region}:#{account_id}:#{resource}" rescue => e diff --git a/test/new_relic/agent/aws_test.rb b/test/new_relic/agent/aws_test.rb index 660ad24e6e..ddaf0b99e9 100644 --- a/test/new_relic/agent/aws_test.rb +++ b/test/new_relic/agent/aws_test.rb @@ -9,6 +9,7 @@ def test_create_arn config = mock config.stubs(:region).returns('us-test-region-1') mock_credentials = mock + mock_credentials.stubs(:credentials).returns(mock_credentials) mock_credentials.stubs(:access_key_id).returns('AKIAIOSFODNN7EXAMPLE') # this is a fake access key id from aws docs config.stubs(:credentials).returns(mock_credentials) From d13670f9eb487c8a2f68f3e8faac0a11e74f8c7a Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 15:51:18 -0500 Subject: [PATCH 2/8] make aws account_id more performant --- lib/new_relic/agent/aws.rb | 15 ++++++------- .../dynamodb/instrumentation.rb | 8 +++++-- test/new_relic/agent/aws_test.rb | 21 ++++++++++++------- 3 files changed, 28 insertions(+), 16 deletions(-) diff --git a/lib/new_relic/agent/aws.rb b/lib/new_relic/agent/aws.rb index 248b4ae7c7..273b003cd9 100644 --- a/lib/new_relic/agent/aws.rb +++ b/lib/new_relic/agent/aws.rb @@ -8,18 +8,19 @@ module Aws CHARACTERS = %w[A B C D E F G H I J K L M N O P Q R S T U V W X Y Z 2 3 4 5 6 7].freeze HEX_MASK = '7fffffffff80' - def self.create_arn(service, resource, config) - access_key_id = config&.credentials&.credentials&.access_key_id if config&.credentials&.credentials&.respond_to?(:access_key_id) - return unless access_key_id - - region = config.region - account_id = NewRelic::Agent::Aws.convert_access_key_to_account_id(access_key_id) - + def self.create_arn(service, resource, region, account_id) "arn:aws:#{service}:#{region}:#{account_id}:#{resource}" rescue => e NewRelic::Agent.logger.warn("Failed to create ARN: #{e}") end + def self.get_account_id(config) + access_key_id = config&.credentials&.credentials&.access_key_id if config&.credentials&.credentials&.respond_to?(:access_key_id) + return unless access_key_id + + NewRelic::Agent::Aws.convert_access_key_to_account_id(access_key_id) + end + def self.convert_access_key_to_account_id(access_key) decoded_key = Integer(decode_to_hex(access_key[4..-1]), 16) mask = Integer(HEX_MASK, 16) diff --git a/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb b/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb index 12ddd9c2aa..a14868158e 100644 --- a/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb @@ -49,10 +49,14 @@ def build_request_with_new_relic(*args) @nr_captured_request = yield end + def nr_account_id + @nr_account_id ||= NewRelic::Agent::Aws.get_account_id(config) + end + def get_arn(params) - return unless params[:table_name] + return unless params[:table_name] && nr_account_id - NewRelic::Agent::Aws.create_arn(PRODUCT.downcase, "table/#{params[:table_name]}", config) + NewRelic::Agent::Aws.create_arn(PRODUCT.downcase, "table/#{params[:table_name]}", config&.region, nr_account_id) end end end diff --git a/test/new_relic/agent/aws_test.rb b/test/new_relic/agent/aws_test.rb index ddaf0b99e9..61f32c81d0 100644 --- a/test/new_relic/agent/aws_test.rb +++ b/test/new_relic/agent/aws_test.rb @@ -6,19 +6,26 @@ class AwsTest < Minitest::Test def test_create_arn + service = 'test-service' + region = 'us-test-region-1' + account_id = '123456789' + resource = 'test/test-resource' + arn = NewRelic::Agent::Aws.create_arn(service, resource, region, account_id) + + expected = 'arn:aws:test-service:us-test-region-1:123456789:test/test-resource' + + assert_equal expected, arn + end + + def test_get_account_id config = mock - config.stubs(:region).returns('us-test-region-1') mock_credentials = mock mock_credentials.stubs(:credentials).returns(mock_credentials) mock_credentials.stubs(:access_key_id).returns('AKIAIOSFODNN7EXAMPLE') # this is a fake access key id from aws docs config.stubs(:credentials).returns(mock_credentials) - service = 'test-service' - resource = 'test/test-resource' - arn = NewRelic::Agent::Aws.create_arn(service, resource, config) + account_id = NewRelic::Agent::Aws.get_account_id(config) - expected = 'arn:aws:test-service:us-test-region-1:36315003739:test/test-resource' - - assert_equal expected, arn + assert_equal 36315003739, account_id end end From 5ef7b6853c16e6ff84c7eba46038fcec695c395f Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 15:55:06 -0500 Subject: [PATCH 3/8] update tests --- .../suites/dynamodb/dynamodb_instrumentation_test.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb b/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb index 4b347028ff..72dfbc7ae0 100644 --- a/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb +++ b/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb @@ -7,6 +7,8 @@ class DynamodbInstrumentationTest < Minitest::Test def setup Aws.config.update(stub_responses: true) + NewRelic::Agent::Aws.stubs(:create_arn).returns('test-arn') + NewRelic::Agent::Aws.stubs(:get_account_id).returns('123456789') @stats_engine = NewRelic::Agent.instance.stats_engine end @@ -22,7 +24,6 @@ def create_client def test_all_attributes_added_to_segment client = create_client Seahorse::Client::Http::Response.any_instance.stubs(:headers).returns({'x-amzn-requestid' => '1234321'}) - NewRelic::Agent::Aws.stubs(:create_arn).returns('test-arn') in_transaction do |txn| client.query({ From 251914068c617e03f8c36e4e2b3a59001391db35 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 3 Jun 2024 16:07:15 -0500 Subject: [PATCH 4/8] remove unecessary &. --- lib/new_relic/agent/aws.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/new_relic/agent/aws.rb b/lib/new_relic/agent/aws.rb index 273b003cd9..9cd8ed5c5a 100644 --- a/lib/new_relic/agent/aws.rb +++ b/lib/new_relic/agent/aws.rb @@ -15,7 +15,7 @@ def self.create_arn(service, resource, region, account_id) end def self.get_account_id(config) - access_key_id = config&.credentials&.credentials&.access_key_id if config&.credentials&.credentials&.respond_to?(:access_key_id) + access_key_id = config.credentials.credentials.access_key_id if config&.credentials&.credentials&.respond_to?(:access_key_id) return unless access_key_id NewRelic::Agent::Aws.convert_access_key_to_account_id(access_key_id) From a58d6cba23522f1a43b212f380839c198d62f5ea Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Wed, 5 Jun 2024 12:36:46 -0500 Subject: [PATCH 5/8] do memoization different --- .../agent/instrumentation/dynamodb/instrumentation.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb b/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb index a14868158e..b2584aa22c 100644 --- a/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb @@ -50,7 +50,9 @@ def build_request_with_new_relic(*args) end def nr_account_id - @nr_account_id ||= NewRelic::Agent::Aws.get_account_id(config) + return @nr_account_id if defined?(@nr_account_id) + + @nr_account_id = NewRelic::Agent::Aws.get_account_id(config) end def get_arn(params) From 0e13cde86bbd4b5b4b1e793e68d9046e414e8d1c Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Wed, 5 Jun 2024 16:00:40 -0500 Subject: [PATCH 6/8] rescue account id --- lib/new_relic/agent/aws.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/new_relic/agent/aws.rb b/lib/new_relic/agent/aws.rb index 9cd8ed5c5a..81dcd9e862 100644 --- a/lib/new_relic/agent/aws.rb +++ b/lib/new_relic/agent/aws.rb @@ -19,6 +19,8 @@ def self.get_account_id(config) return unless access_key_id NewRelic::Agent::Aws.convert_access_key_to_account_id(access_key_id) + rescue => e + NewRelic::Agent.logger.debug("Failed to create account id: #{e}") end def self.convert_access_key_to_account_id(access_key) From ca4b4d8e1d1a34959d268e5ce7744ee542460708 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 6 Jun 2024 10:41:23 -0500 Subject: [PATCH 7/8] add changelog entry --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 592fb888a2..91a24f6349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # New Relic Ruby Agent Release Notes +## dev + +Version fixes a bug related to the new DynamoDB instrumentation. + +- **Bugfix: DynamoDB instrumentation logging errors when trying to get account_id** + When trying to access data needed to add the account_id to the dynamodb span, the agent was encountering an error when certain credentials classes were in use. This has been fixed. Thanks to [@kichik](https://github.com/kichik) for bringing this to our attention. [PR#2767](https://github.com/newrelic/newrelic-ruby-agent/pull/2681) + + ## v9.10.1 - **Bugfix: Incompatibility with Bootstrap** From f8add64aa284fd136002b111989932314272d5bb Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 6 Jun 2024 13:06:42 -0500 Subject: [PATCH 8/8] Update CHANGELOG.md Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91a24f6349..1e80ee8165 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ Version fixes a bug related to the new DynamoDB instrumentation. - **Bugfix: DynamoDB instrumentation logging errors when trying to get account_id** - When trying to access data needed to add the account_id to the dynamodb span, the agent was encountering an error when certain credentials classes were in use. This has been fixed. Thanks to [@kichik](https://github.com/kichik) for bringing this to our attention. [PR#2767](https://github.com/newrelic/newrelic-ruby-agent/pull/2681) + + When trying to access data needed to add the `account_id` to the DynamoDB span, the agent encountered an error when certain credentials classes were used. This has been fixed. Thanks to [@kichik](https://github.com/kichik) for bringing this to our attention. [PR#2864](https://github.com/newrelic/newrelic-ruby-agent/pull/2684) ## v9.10.1