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

Bug: tracer.captureAWSClient not working consistently with DynamoDB.DocumentClient #524

Closed
dreamorosi opened this issue Jan 28, 2022 · 3 comments · Fixed by #528
Closed
Assignees
Labels
bug Something isn't working completed This item is complete and has been merged/shipped tracer This item relates to the Tracer Utility

Comments

@dreamorosi
Copy link
Contributor

Bug description

As reported by @flochaz, while using JS, after patching an AWS.DynamoDB.DocumentClient like so:

const docClient = tracer.captureAWSClient(new dynamodb.DocumentClient());

trying to use the client like docClient.put() throws an error put is not a function.

In our e2e tests we are testing for the .scan() method which works as expected so further investigation is required.

Expected Behavior

Being able to use all the methods the client has after being captured.

Current Behavior

Client throws an error when using certain methods like put.

Possible Solution

N/A

Steps to Reproduce

See above.

Environment

  • Powertools version used: >=0.5.0
  • Packaging format (Layers, npm): npm with JS
  • AWS Lambda function runtime: N/A
  • Debugging logs: N/A

Related issues, RFCs

N/A

@dreamorosi dreamorosi added bug Something isn't working tracer This item relates to the Tracer Utility triage This item has not been triaged by a maintainer, please wait labels Jan 28, 2022
@dreamorosi dreamorosi added this to the production-ready-release milestone Jan 28, 2022
@dreamorosi dreamorosi self-assigned this Jan 28, 2022
@dreamorosi
Copy link
Contributor Author

dreamorosi commented Jan 28, 2022

Was able to reproduce with aws-xray-sdk-core.

According to what described in this issue on the aws-xray-sdk-node repo:

The DocumentClient constructor is not captured in the same way the base level clients are - this is a special case of the AWS SDK not conforming to the same initialization code path that the other AWS Clients utilize and the hook they provide to us does not properly set up tracing.

The same issue above suggests a workaround which I misinterpreted while implementing PR #450. While aws-xray-sdk-node allows to still patch an AWS.DynamoDB.DocumentClient it doesn't return it like it does with the other base clients.

The AWSXray.captureAWSClient method from the sdk allows you to patch base clients in two ways:

Method 1

const client = AWS.DynamoDB({ ... });
AWSXray.captureAWSClient(client);

Method 2

const client = AWSXray.captureAWSClient(AWS.DynamoDB({ ... }));

In the first method it instruments an existing client by calling a special method existing on all base clients from the aws-sdk. In the latter it patches it and returns the instrumented client.

Method 2 doesn't work with AWS.DynamoDB.DocumentClient because the sdk returns an instance of the underlying service client instead (AWS.DynamoDB). I was able to confirm this with the example below:

DynamoDB Table Definition:

{
  "TableName": "TestTableMulti",
  "KeySchema": [
    {
      "AttributeName": "id",
      "KeyType": "HASH"
    },
    {
      "AttributeName": "sk",
      "KeyType": "RANGE"
    }
  ],
  "AttributeDefinitions": [
    {
      "AttributeName": "id",
      "AttributeType": "S"
    },
    {
      "AttributeName": "sk",
      "AttributeType": "S"
    }
  ],
  "BillingMode": "PAY_PER_REQUEST"
}

Code example:

const AWSXray = require('aws-xray-sdk-core');
const AWS = require('aws-sdk');

// If running in Docker: docker run -p 8000:8000 -d amazon/dynamodb-local -jar DynamoDBLocal.jar -sharedDb -dbPath .
const TABLE_NAME = 'TestTableMulti'; // Create aws dynamodb create-table --cli-input-json file://createTable.json --endpoint-url http://localhost:8000
const CLIENT_CONFIG = {
  region: 'us-east-1',
  endpoint: 'http://localhost:8000', // Remove this if testing against actual DynamoDB table
}

const scan = async (client) => {
  const params = {
    TableName: TABLE_NAME,
  };
  await client.scan(params).promise();
};

const put = async (client, id) => {
  const params = {
    TableName: TABLE_NAME,
    Item: {
      sk: `param-${id}`,
      value: '1',
      id: 'someValue'
    },
  };
  await client.put(params).promise();
};

const deleteOne = async (client, id) => {
  const params = {
    TableName: TABLE_NAME,
    Key: {
      id: 'someValue',
      sk: `param-${id}`,
    },
  };
  await client.delete(params).promise();
};

const get = async (client, id) => {
  const params = {
    TableName: TABLE_NAME,
    Key: {
      id: 'someValue',
      sk: `param-${id}`,
    },
  };
  await client.get(params).promise();
};

const clientOnly = async () => {
  console.log('const client = new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG });');
  const client = new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG });
  console.log('client instanceof(AWS.DynamoDB.DocumentClient) should be true, actual:', client instanceof(AWS.DynamoDB.DocumentClient));
  console.log('client instanceof(AWS.DynamoDB) should be false, actual:', client instanceof(AWS.DynamoDB));
  await scan(client);
  console.log('scan OK');
  await put(client, 1);
  console.log('put OK');
  await get(client, 1);
  console.log('get OK');
  await deleteOne(client, 1);
  console.log('delete OK');
};

const sdkTracedBroken = async () => {
  console.log('const client = AWSXray.captureAWSClient(new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG }).service);');
  const client = AWSXray.captureAWSClient(new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG }).service);
  console.log('client instanceof(AWS.DynamoDB.DocumentClient) should be true, actual:', client instanceof(AWS.DynamoDB.DocumentClient));
  console.log('client instanceof(AWS.DynamoDB) should be false, actual:', client instanceof(AWS.DynamoDB));
  await scan(client);
  console.log('scan OK');
  await put(client, 2);
  console.log('put OK');
  await get(client, 2);
  console.log('get OK');
  await deleteOne(client, 2);
  console.log('delete OK');
};

const sdkTracedOk = async () => {
  console.log('const client = new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG }); \nAWSXray.captureAWSClient(client.service);');
  const client = new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG });
  AWSXray.captureAWSClient(client.service);
  console.log('client instanceof(AWS.DynamoDB.DocumentClient) should be true, actual:', client instanceof(AWS.DynamoDB.DocumentClient));
  console.log('client instanceof(AWS.DynamoDB) should be false, actual:', client instanceof(AWS.DynamoDB));
  await scan(client);
  console.log('scan OK');
  await put(client, 2);
  console.log('put OK');
  await get(client, 2);
  console.log('get OK');
  await deleteOne(client, 2);
  console.log('delete OK');
};

module.exports = handler = async () => {
  await clientOnly();
  console.log('\n\n');
  await sdkTracedOk();
  console.log('\n\n');
  await sdkTracedBroken();
  console.log('\n\n');
};

handler();

Open below to see logs:

~/Codes/dynamodb_test
❯ node index.js
const client = new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG });
client instanceof(AWS.DynamoDB.DocumentClient) should be true, actual: true
client instanceof(AWS.DynamoDB) should be false, actual: false
scan OK
put OK
get OK
delete OK



const client = new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG }); 
 AWSXray.captureAWSClient(client.service);
client instanceof(AWS.DynamoDB.DocumentClient) should be true, actual: true
client instanceof(AWS.DynamoDB) should be false, actual: false
scan OK
put OK
get OK
delete OK



const client = AWSXray.captureAWSClient(new AWS.DynamoDB.DocumentClient({ ...CLIENT_CONFIG }).service);
client instanceof(AWS.DynamoDB.DocumentClient) should be true, actual: false
client instanceof(AWS.DynamoDB) should be false, actual: true
scan OK
/Users/aamorosi/Codes/dynamodb_test/index.js:30
  await client.put(params).promise();
               ^

TypeError: client.put is not a function
    at put (/Users/aamorosi/Codes/dynamodb_test/index.js:30:16)
    at sdkTracedBroken (/Users/aamorosi/Codes/dynamodb_test/index.js:77:9)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async handler (/Users/aamorosi/Codes/dynamodb_test/index.js:153:3)

As you can see from the logs above the third function, the one with the client patched with Method 2 returns an AWS.DynamoDB instead of an AWS.DynamoDB.DocumentClient instance.

@dreamorosi
Copy link
Contributor Author

As additional detail, this wasn't caught in the integration/e2e tests because I was using the scan method which exists and has the same exact name/signature both for AWS.DynamoDB and AWS.DynamoDB.DocumentClient.

In case of a fix I'll also update the tests to catch this.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 8, 2022

⚠️ COMMENT VISIBILITY WARNING ⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@dreamorosi dreamorosi removed the triage This item has not been triaged by a maintainer, please wait label Oct 19, 2022
@dreamorosi dreamorosi changed the title bug(tracer): tracer.captureAWSClient not working consistently with DynamoDB.DocumentClient Bug: tracer.captureAWSClient not working consistently with DynamoDB.DocumentClient Nov 14, 2022
@dreamorosi dreamorosi added the completed This item is complete and has been merged/shipped label Nov 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working completed This item is complete and has been merged/shipped tracer This item relates to the Tracer Utility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant