From f01539d4443c87f97641be1db1adc19e728bb5bc Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Tue, 8 Oct 2024 13:28:34 -0500 Subject: [PATCH 1/8] add aws_account_id config --- lib/new_relic/agent/aws.rb | 51 ++----------------- .../agent/configuration/default_source.rb | 7 +++ .../dynamodb/instrumentation.rb | 15 ++---- .../dynamodb/dynamodb_instrumentation_test.rb | 4 +- test/new_relic/agent/aws_test.rb | 20 +++----- 5 files changed, 22 insertions(+), 75 deletions(-) diff --git a/lib/new_relic/agent/aws.rb b/lib/new_relic/agent/aws.rb index 81dcd9e862..79646c61d5 100644 --- a/lib/new_relic/agent/aws.rb +++ b/lib/new_relic/agent/aws.rb @@ -5,58 +5,13 @@ module NewRelic module Agent 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, region) + return unless NewRelic::Agent.config[:'aws_account_id'] - def self.create_arn(service, resource, region, account_id) - "arn:aws:#{service}:#{region}:#{account_id}:#{resource}" + "arn:aws:#{service}:#{region}:#{NewRelic::Agent.config[:'aws_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) - rescue => e - NewRelic::Agent.logger.debug("Failed to create account id: #{e}") - 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) - (decoded_key & mask) >> 7 - end - - def self.decode_to_hex(access_key) - bytes = access_key.delete('=').each_char.map { |c| CHARACTERS.index(c) } - - bytes.each_slice(8).map do |section| - convert_section(section) - end.flatten[0...6].join - end - - def self.convert_section(section) - buffer = 0 - section.each do |chunk| - buffer = (buffer << 5) + chunk - end - - chunk_count = (section.length * 5.0 / 8.0).floor - - if section.length < 8 - buffer >>= (5 - (chunk_count * 8)) % 5 - end - - decoded = [] - chunk_count.times do |i| - shift = 8 * (chunk_count - 1 - i) - decoded << ((buffer >> shift) & 255).to_s(16) - end - - decoded - end end end end diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index e091c8ecf3..08c73b1ae7 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -429,6 +429,13 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'Your New Relic . Required when using the New Relic REST API v2 to record deployments using the `newrelic deployments` command.' }, + :aws_account_id => { + :default => nil, + :public => true, + :type => String, + :allowed_from_server => false, + :description => 'The AWS account ID for the associated AWS account' + }, :backport_fast_active_record_connection_lookup => { :default => false, :public => true, diff --git a/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb b/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb index cf869660f4..721c41e582 100644 --- a/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb +++ b/lib/new_relic/agent/instrumentation/dynamodb/instrumentation.rb @@ -31,9 +31,8 @@ def instrument_method_with_new_relic(method_name, *args) collection: args[0][:table_name] ) - # TODO: Update this when it has been decided how to handle account id for ARN - # arn = get_arn(args[0]) - # segment&.add_agent_attribute('cloud.resource_id', arn) if arn + arn = get_arn(args[0]) + segment&.add_agent_attribute('cloud.resource_id', arn) if arn @nr_captured_request = nil # clear request just in case begin @@ -50,16 +49,10 @@ def build_request_with_new_relic(*args) @nr_captured_request = yield end - def nr_account_id - return @nr_account_id if defined?(@nr_account_id) - - @nr_account_id = NewRelic::Agent::Aws.get_account_id(config) - end - def get_arn(params) - return unless params[:table_name] && nr_account_id + return unless params[:table_name] - NewRelic::Agent::Aws.create_arn(PRODUCT.downcase, "table/#{params[:table_name]}", config&.region, nr_account_id) + NewRelic::Agent::Aws.create_arn(PRODUCT.downcase, "table/#{params[:table_name]}", config&.region) end end end diff --git a/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb b/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb index 9da2e08885..73c421ac4a 100644 --- a/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb +++ b/test/multiverse/suites/dynamodb/dynamodb_instrumentation_test.rb @@ -8,7 +8,6 @@ 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 @@ -42,8 +41,7 @@ def test_all_attributes_added_to_segment assert_equal 'us-east-2', span[2]['aws.region'] assert_equal 'query', span[2]['aws.operation'] assert_equal '1234321', span[2]['aws.requestId'] - # TODO: Uncomment this when the ARN is added to the segment - # assert_equal 'test-arn', span[2]['cloud.resource_id'] + assert_equal 'test-arn', span[2]['cloud.resource_id'] end def test_create_table_table_name_operation diff --git a/test/new_relic/agent/aws_test.rb b/test/new_relic/agent/aws_test.rb index 61f32c81d0..aae708b2fc 100644 --- a/test/new_relic/agent/aws_test.rb +++ b/test/new_relic/agent/aws_test.rb @@ -10,22 +10,16 @@ def test_create_arn 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 - 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) + with_config(aws_account_id: account_id) do + arn = NewRelic::Agent::Aws.create_arn(service, resource, region) - account_id = NewRelic::Agent::Aws.get_account_id(config) + assert_equal expected, arn + end + end - assert_equal 36315003739, account_id + def test_doesnt_create_arn_no_account_id + assert_nil NewRelic::Agent::Aws.create_arn('service', 'resource', 'region') end end From 62435f5490a37e77af70a7b84dc008e2901af6eb Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Wed, 9 Oct 2024 14:49:20 -0500 Subject: [PATCH 2/8] use cloud.aws.account_id for config name --- lib/new_relic/agent/aws.rb | 4 ++-- .../agent/configuration/default_source.rb | 14 +++++++------- test/new_relic/agent/aws_test.rb | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/new_relic/agent/aws.rb b/lib/new_relic/agent/aws.rb index 79646c61d5..cb1d4f53a9 100644 --- a/lib/new_relic/agent/aws.rb +++ b/lib/new_relic/agent/aws.rb @@ -6,9 +6,9 @@ module NewRelic module Agent module Aws def self.create_arn(service, resource, region) - return unless NewRelic::Agent.config[:'aws_account_id'] + return unless NewRelic::Agent.config[:'cloud.aws.account_id'] - "arn:aws:#{service}:#{region}:#{NewRelic::Agent.config[:'aws_account_id']}:#{resource}" + "arn:aws:#{service}:#{region}:#{NewRelic::Agent.config[:'cloud.aws.account_id']}:#{resource}" rescue => e NewRelic::Agent.logger.warn("Failed to create ARN: #{e}") end diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 08c73b1ae7..4d52226a90 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -429,13 +429,6 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'Your New Relic . Required when using the New Relic REST API v2 to record deployments using the `newrelic deployments` command.' }, - :aws_account_id => { - :default => nil, - :public => true, - :type => String, - :allowed_from_server => false, - :description => 'The AWS account ID for the associated AWS account' - }, :backport_fast_active_record_connection_lookup => { :default => false, :public => true, @@ -478,6 +471,13 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :allowed_from_server => false, :description => 'If `true`, the agent will clear `Tracer::State` in `Agent.drop_buffered_data`.' }, + :'cloud.aws.account_id' => { + :default => nil, + :public => true, + :type => String, + :allowed_from_server => false, + :description => 'The AWS account ID for the associated AWS account' + }, :config_path => { :default => DefaultSource.config_path, :public => true, diff --git a/test/new_relic/agent/aws_test.rb b/test/new_relic/agent/aws_test.rb index aae708b2fc..7af8b8247f 100644 --- a/test/new_relic/agent/aws_test.rb +++ b/test/new_relic/agent/aws_test.rb @@ -12,7 +12,7 @@ def test_create_arn resource = 'test/test-resource' expected = 'arn:aws:test-service:us-test-region-1:123456789:test/test-resource' - with_config(aws_account_id: account_id) do + with_config('cloud.aws.account_id': account_id) do arn = NewRelic::Agent::Aws.create_arn(service, resource, region) assert_equal expected, arn From c14fb1fac06e3f765d8fac5d9ae7d0feb5b246bc Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Wed, 9 Oct 2024 15:06:14 -0500 Subject: [PATCH 3/8] add changelog entry --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e26f1faa46..540c3bd8e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,11 @@ ## dev -Version resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem. +Version adds a configuration option to associate the AWS account ID with the DynamoDB calls from the AWS sdk, and resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem. + +- **Feature: New configuration option cloud.aws.account_id** + + A new configuration option has been added, `cloud.aws.account_id`, that will allow New Relic to provide more details about certain calls made using the AWS sdk. Currently, the DynamoDB instrumentation is the only instrumentation that will make use of this configuration option, but this will be used in future instrumentation as well. [PR#2904](https://github.com/newrelic/newrelic-ruby-agent/pull/2904) - **Bugfix: Instrumentation errors when using the karafka-rdkafka gem** From 42847f0b8dbd0d3863e9b6d791af0c23f8fef60c Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 10 Oct 2024 10:23:07 -0500 Subject: [PATCH 4/8] add allow nil to config --- lib/new_relic/agent/configuration/default_source.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 4d52226a90..38b27bfae4 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -475,6 +475,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :default => nil, :public => true, :type => String, + :allow_nil => true, :allowed_from_server => false, :description => 'The AWS account ID for the associated AWS account' }, From 086420fa0decac70c005209c75c665a6372c3926 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 10 Oct 2024 12:26:33 -0500 Subject: [PATCH 5/8] Update CHANGELOG.md Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 540c3bd8e9..d0e40a536a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## dev -Version adds a configuration option to associate the AWS account ID with the DynamoDB calls from the AWS sdk, and resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem. +Version adds a configuration option to associate the AWS account ID with the DynamoDB calls from the AWS SDK, and resolves a bug in rdkafka instrumentation when using the karafka-rdkafka gem. - **Feature: New configuration option cloud.aws.account_id** From 22b367c57481c7858d1ce500893c5350b255f1de Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 10 Oct 2024 12:26:38 -0500 Subject: [PATCH 6/8] Update CHANGELOG.md Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d0e40a536a..9f35013567 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Version adds a configuration option to associate the AWS account ID with t - **Feature: New configuration option cloud.aws.account_id** - A new configuration option has been added, `cloud.aws.account_id`, that will allow New Relic to provide more details about certain calls made using the AWS sdk. Currently, the DynamoDB instrumentation is the only instrumentation that will make use of this configuration option, but this will be used in future instrumentation as well. [PR#2904](https://github.com/newrelic/newrelic-ruby-agent/pull/2904) + A new configuration option has been added, `cloud.aws.account_id`, that will allow New Relic to provide more details about certain calls made using the AWS SDK. Currently, the DynamoDB instrumentation is the only instrumentation that will make use of this configuration option, but this will be used in future instrumentation as well. [PR#2904](https://github.com/newrelic/newrelic-ruby-agent/pull/2904) - **Bugfix: Instrumentation errors when using the karafka-rdkafka gem** From 0b65bd70f789db4a712e52735b6c5f6b56d354c3 Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Thu, 10 Oct 2024 12:26:49 -0500 Subject: [PATCH 7/8] Update lib/new_relic/agent/configuration/default_source.rb Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> --- lib/new_relic/agent/configuration/default_source.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/new_relic/agent/configuration/default_source.rb b/lib/new_relic/agent/configuration/default_source.rb index 38b27bfae4..45059d619b 100644 --- a/lib/new_relic/agent/configuration/default_source.rb +++ b/lib/new_relic/agent/configuration/default_source.rb @@ -477,7 +477,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil) :type => String, :allow_nil => true, :allowed_from_server => false, - :description => 'The AWS account ID for the associated AWS account' + :description => 'The AWS account ID for the AWS account associated with this app' }, :config_path => { :default => DefaultSource.config_path, From c8f18d44783b6486050bb87c2da2d56831d0e73e Mon Sep 17 00:00:00 2001 From: Tanna McClure Date: Mon, 21 Oct 2024 13:18:16 -0500 Subject: [PATCH 8/8] Update CHANGELOG.md Co-authored-by: Kayla Reopelle <87386821+kaylareopelle@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 615ecdf53d..deb63af0bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ Version updates View Componment instrumentation to use a default metric na - **Feature: New configuration option cloud.aws.account_id** - A new configuration option has been added, `cloud.aws.account_id`, that will allow New Relic to provide more details about certain calls made using the AWS SDK. Currently, the DynamoDB instrumentation is the only instrumentation that will make use of this configuration option, but this will be used in future instrumentation as well. [PR#2904](https://github.com/newrelic/newrelic-ruby-agent/pull/2904) + A new configuration option has been added, `cloud.aws.account_id`, that will allow New Relic to provide more details about certain calls made using the AWS SDK. One example, is that relationships between AWS services instrumented with New Relic's CloudWatch Metric Streams will have relationships formed in the service map with APM applications. Currently, the DynamoDB instrumentation is the only instrumentation that will make use of this configuration option, but this will be used in future instrumentation as well. [PR#2904](https://github.com/newrelic/newrelic-ruby-agent/pull/2904) - **Feature: Use default `View/component` metric name for unidentified View Components**