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

Using captureAWSClient over directly referenced dynamodb client fails #23

Open
nolde opened this issue Feb 13, 2018 · 27 comments
Open

Using captureAWSClient over directly referenced dynamodb client fails #23

nolde opened this issue Feb 13, 2018 · 27 comments
Assignees

Comments

@nolde
Copy link

nolde commented Feb 13, 2018

The following code does not work:

const DynamoDB = require('aws-sdk/clients/dynamodb')
const xray = require('aws-xray-sdk-core')
const ddb = xray.captureAWSClient(new DynamoDB())

In theory, it should be just the same as encapsulating new AWS.DynamoDB() coming straight from aws-sdk, but it does not work.

This is specially problematic in AWS Lambda environment, where loading more than you need makes you use more memory and more time.

@awssandra
Copy link
Contributor

awssandra commented Feb 13, 2018

Hi nolde,

I haven't been able to reproduce your issue.
Here is the code I used to test.

var xray = require('aws-xray-sdk-core');   // v1.2.0

const DynamoDB = require('aws-sdk/clients/dynamodb') // aws-sdk v2.193.0
const dynamodb = xray.captureAWSClient(new DynamoDB())

exports.handler = (event, context, callback) => {
  // params = { params here };

  dynamodb.batchGetItem(params, function (err, data) {
    if (err) console.log(err, err.stack); // an error occurred
    else     console.log(data);           // successful response
  });

  callback(null, 'Hello from Lambda');
};

And the resultant trace:

    "Duration": 0.054,
    "Id": "xxxxxxxxxxxx",
    "Segments": [
        {
            "Document": {
                "id": "5fb2ad3a78c0c7a8",
                "name": "myDynamoTest",
                "start_time": 1518547704.129,
                "end_time": 1518547704.176,
                "parent_id": "5adfc1d316a9ed6d",
                "aws": {
                    "function_arn": "arn:aws:xxxxxxxx",
                    "resource_names": [
                        "myDynamoTest"
                    ],
                    "account_id": "243559052476"
                },
                "trace_id": "xxxxxxxx",
                "origin": "AWS::Lambda::Function",
                "subsegments": [
                    {
                        "id": "ba73dac2d7ac91ee",
                        "name": "DynamoDB",
                        "start_time": 1518547704.155,
                        "end_time": 1518547704.156,
                        ...
                        "http": {
                            "response": {}
                        },
                        "aws": {
                            "operation": "BatchGetItem",
                            "region": "us-east-1",
                            "request_id": "",
                            "retries": 0
                        },
                        "namespace": "aws"
                    }
                ]
            },
            "Id": "5fb2ad3a78c0c7a8"
        }
...

Can you post a code snippet to reproduce the issue? Are there any errors in the logs?

Thanks,
Sandra

@nolde
Copy link
Author

nolde commented Feb 19, 2018

After reviewing the issue, I found out that it was actually my mistake.

Thanks for the quick reply, though.

@nolde nolde closed this as completed Feb 19, 2018
@nolde nolde reopened this Feb 22, 2018
@nolde nolde closed this as completed Feb 22, 2018
@nolde
Copy link
Author

nolde commented Feb 22, 2018

And should the usage below work?

var xray = require('aws-xray-sdk-core')

const AWS = require('aws-sdk')
const docclient = xray.captureAWSClient(new AWS.DynamoDB.DocumentClient())

@nolde nolde reopened this Feb 22, 2018
@awssandra
Copy link
Contributor

awssandra commented Feb 22, 2018

The forums post here covers this: https://forums.aws.amazon.com/thread.jspa?messageID=821049&#821049

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.

We are currently investigating a work around for this issue.

Hope this helps,
Sandra

@nolde
Copy link
Author

nolde commented Feb 23, 2018

Ok!
Thanks for the quick answers!

@nolde nolde closed this as completed Feb 23, 2018
@serverlesspolska
Copy link

Is this issue resolved now?

@leonardovillela
Copy link

Any progress?

@haotianw465 haotianw465 reopened this Jun 4, 2018
@haotianw465
Copy link
Contributor

Re-open due to popular requests. We need to work with AWS SDK team so there is a proper public interface to plugin the X-Ray intercepter on the Document client. This is on our backlog and we already brought this issue to AWS SDK team's attention.

We will post an update as soon as we have further information. Please stay tuned and thank you for your patience.

@monken
Copy link
Contributor

monken commented Sep 7, 2018

Here is a workaround (from aws/aws-sdk-js#1846)

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

const client = new AWS.DynamoDB.DocumentClient({
  service: new AWS.DynamoDB({...})
});

AWSXray.captureAWSClient(client.service);

@heitorlessa
Copy link

I'm using Typescript and Webpack - here's how I'm instrumenting with the thin dynamoDB client, if that's helpful:

const AWSXRay = require('aws-xray-sdk-core')
import { DocumentClient } from 'aws-sdk/clients/dynamodb';

let client: DocumentClient;
client = new DocumentClient();

AWSXRay.captureAWSClient((client as any).service);

@phstc
Copy link

phstc commented Aug 14, 2019

Hi @heitorlessa

Thanks for sharing how you are instrumenting DDB. It worked with DocumentClient, but when I try to use it with the lambda and S3 clients, I get:

fails "errorMessage": "Cannot read property 'customizeRequests' of undefined"

const AWSXRay = require("aws-xray-sdk");
const lambda = new Lambda();
AWSXRay.captureAWSClient((lambda as any).service); 

Have you tried to use captureAWSClient with other clients other than DocumentClient?

@moransk8
Copy link

Hi @phstc
I got the same error instrumenting the Lambda, and I resolved by doing this:

const AWSXRay = require("aws-xray-sdk");
const lambda = new Lambda();
AWSXRay.captureAWSClient(lambda); 

That's because the class Lambda extends an aws Service.

@willarmiros
Copy link
Contributor

Hi @phstc,
I hope your issue has been resolved by the follow up. If it hasn't please open a new issue to avoid too much clutter on this one, and we will prioritize following up further. Please note that the X-Ray SDK for node.js does not support typescript at the moment and using typescript notation with our APIs can cause issues.

@alfaproject
Copy link

I'm not sure why this has been closed. It's still not possible to patch a DynamoDB Client class directly.

@willarmiros can you reopen, please?

@willarmiros
Copy link
Contributor

Sure. This issue was just getting some clutter but the bug does still exist. Unfortunately this and #184 are reliant on the AWS SDK team to unify their client classes. Reopening to continue to track that progress.

@willarmiros willarmiros reopened this Oct 21, 2019
@vladholubiev
Copy link
Contributor

FYI, I've created npm package which encapsulates the workaround described in the comments: https://github.com/shelfio/aws-ddb-with-xray

@rpstreef
Copy link

rpstreef commented Dec 2, 2020

this is getting closed for no good reason, the problem still exists and no proper solutions has been presented here yet.

@willarmiros
Copy link
Contributor

@rpstreef have you tried the workaround suggested on this thread? As we've mentioned this is a problem with how the Document Client is modeled in the AWS SDK itself. Given that the AWS SDK for JS team is soon to release V3 of their SDK, it's unlikely they'll make this change to the DynamoDB client in V2. Furthermore, this is not a problem in V3 of the AWS SDK since the DynamoDB document client is implemented in the same way as other clients and will be able to be traced appropriately. Please keep an eye on #298 for our support of V3 of the AWS SDK.

@paul-uz
Copy link

paul-uz commented Dec 15, 2020

Here is a workaround (from aws/aws-sdk-js#1846)

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

const client = new AWS.DynamoDB.DocumentClient({
  service: new AWS.DynamoDB({...})
});

AWSXray.captureAWSClient(client.service);

How exactly do I use this workaround? there is something missing here service: new AWS.DynamoDB({...}) - what should it be?

@willarmiros
Copy link
Contributor

@paul-uz If you're asking about the {...} section, that is the options you can construct the DynamoDB client with, which are all optional and documented here

If you're asking how this client is actually traced, you'll notice after creating the DocumentClient, they use:

AWSXray.captureAWSClient(client.service);

To enable X-Ray tracing the underlying DynamoDB client.

@mkapiczy
Copy link

But this way the return type is DynamoDB not DocumentClient which in my case is no good as it requires me to rewrite my whole service to use the DynamoDB client API instead of DocumentClient one. So there is currently no way to directly instrument DocumentClient?

@willarmiros
Copy link
Contributor

@mkapiczy in this solution:

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

const client = new AWS.DynamoDB.DocumentClient({
  service: new AWS.DynamoDB({...})
});

AWSXray.captureAWSClient(client.service);

client is still of type DocumentClient and allows the usage of DocumentClient APIs, does it not? The captureAWSClient captures the underlying DynamoDB client. So you should be able to use the client normally and see its requests traced, or am I missing anything?

@mkapiczy
Copy link

My bad, I thought it doesn't modify given object but returns new instrumented instance. Works! Thanks!

@revmischa
Copy link

I am using this:

import AWS from "aws-sdk"
import AWSXRay from "aws-xray-sdk"

AWSXRay.setContextMissingStrategy(() => {})

// clients
const isTest = process.env.JEST_WORKER_ID
const config: AWS.DynamoDB.ClientConfiguration = {
  ...(isTest && {
    endpoint: "localhost:8000",
    sslEnabled: false,
    region: "local-env",
  }),
}

// instantiate xray-instrumented dynamoDB document client
export const documentClient = new AWS.DynamoDB.DocumentClient({
  ...config,
})

// this adds x-ray instrumentation to dynamo's DocumentClient
// but also adds 5s of startup time to the lambda???
AWSXRay.captureAWSClient((documentClient as any).service)

The problem is if I comment out the captureAWSClient call then I don't have DynamoDB instrumented. But if I uncomment it, it reliably adds five seconds of execution time to my lambda. I'm using https://docs.aws.amazon.com/cdk/api/latest/docs/aws-lambda-nodejs-readme.html

@monken
Copy link
Contributor

monken commented Jul 1, 2021

Are you using a bundler like webpack to bundle the lambda and/or source maps by any chance?

@revmischa
Copy link

Are you using a bundler like webpack to bundle the lambda and/or source maps by any chance?

I am using the bundler that https://docs.aws.amazon.com/cdk/api/latest/docs/aws-lambda-nodejs-readme.html#local-bundling uses - esbuild
It doesn't support tree shaking very well yet which could certainly be an issue.

@willarmiros
Copy link
Contributor

@revmischa the performance issue described is definitely new - does it impact all clients or just the document client? Since this is a workaround though it's not surprising there's some odd behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests