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

Bugfix: Dynamodb instrumentation access_key_id error on some credential classes #2684

Merged
merged 10 commits into from
Jun 6, 2024
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# New Relic Ruby Agent Release Notes

## dev

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

- **Bugfix: Incompatibility with Bootstrap**
Expand Down
14 changes: 10 additions & 4 deletions lib/new_relic/agent/aws.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,21 @@ 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)
region = config.region
account_id = NewRelic::Agent::Aws.convert_access_key_to_account_id(config.credentials.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)
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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,16 @@ 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]
return unless params[:table_name] && nr_account_id
Copy link
Contributor Author

Choose a reason for hiding this comment

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

table name though could change every time the client makes a call jsyk


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
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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({
Expand Down
22 changes: 15 additions & 7 deletions test/new_relic/agent/aws_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,26 @@

class AwsTest < Minitest::Test
def test_create_arn
kaylareopelle marked this conversation as resolved.
Show resolved Hide resolved
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
Loading