aws / aws-xray-sdk-node

The official AWS X-Ray SDK for Node.js.
Apache License 2.0
271 stars 155 forks source link

Improve support for AWS SDK v3 #547

Closed mattfysh closed 1 year ago

mattfysh commented 1 year ago

In the recent release of the Node.js v18 runtime on Lambda, the embedded SDK has been upgraded to v3: https://aws.amazon.com/blogs/compute/node-js-18-x-runtime-now-available-in-aws-lambda/

The current state of v3 support in aws-xray-sdk-node appears to be rather patchy. e.g.

  1. Much of the command data no longer appears in the trace
  2. Wrapping most clients with captureAWSv3Client throws TypeScript errors. In fact, the only client I've found that works without a TS error is AWSXRay.captureAWSv3Client(new S3Client({}))

I was wondering if the team were aware of the shift to v3 in the latest lambda runtime environmnent, and whether this may translate to improved support for v3 by the Xray SDK? Thanks for reading!

adambiggs commented 1 year ago

We really need better X-Ray support for SDK v3. Especially since ADOT is currently borderline unusable, despite it being recommended at the top of README.md...

willarmiros commented 1 year ago

Hello,

Thanks for raising this issue, I'll try to address both issues:

Much of the command data no longer appears in the trace

This is unfortunately a limitation of the AWS SDK v3 itself. The V3 middleware exposes much less data than v2 did, and as such our instrumentation can only read the basic attributes you see today. This limitation exists in ADOT as well, and I would suggest opening an issue against the aws-sdk-js-v3 repo to suggest exposing this info more readily.

Wrapping most clients with captureAWSv3Client throws TypeScript errors

This is a known issue and #449 was an attempt to fix it, but unfortunately there were issues with the fix and we were not able to prioritize it since. Once we are able to take another look we can track it here.

bilalq commented 1 year ago

@RanVaknin commented

Hi @bilalq ,

JS SDK v3 exposes the same exact parameters that v2 does, the only difference is that those parameters are accessible via the middleware stack, which will require the Xray team to make adjustments on how the Xray clients accesses the data.

Im sorry to ping pong you around back to the Xray team, but I did have a conversation with their PM @willarmiros (who now is no longer at Amazon) on March 2023, about this issue and had proven that all the relevant info is available via middleware stack.

Since there's no action needed from the JS SDK team so Im going to close this. I encourage you to open the same feature request in their repo, so it can get traction and prioritized.

All the best, Ran~

bilalq commented 1 year ago

Hey, @carolabadeer. Given the performance issues with OpenTelemetry noted in this comment, would you consider making the changes to capture additional metadata in the traces here?

bilalq commented 1 year ago

This would also improve the usability of the Powertools library maintained by AWS, as that is built on top of the X-Ray Node SDK.

bilalq commented 1 year ago

Copying the comment left on the sdk issue here since that was closed:

Hi @RanVaknin and @carolabadeer,

Unfortunately, the OpenTelemetry JS SDK is not suitable in Lambda for most production use-cases. It comes with a massive performance and latency penalty. It's a complete non-starter to add tracing through it when the overhead is so large.

Related issues showing that this is still a problem today:

https://github.com/aws-observability/aws-otel-lambda/issues/228 https://github.com/open-telemetry/opentelemetry-lambda/issues/263

This is even highlighted in the official service docs for X-Ray:

Note AWS Distro for OpenTelemetry offers a simpler getting started experience for instrumenting your Lambda functions. However, due to the flexibility OpenTelemetry offers, your Lambda function will require additional memory and invocations may experience cold start latency increases, which can lead to additional charges. If you're optimizing for low-latency and do not require OpenTelemetry's advanced capabilities such as dynamically configurable backend destinations, you may want to use the AWS X-Ray SDK to instrument your application.

Source: https://docs.aws.amazon.com/xray/latest/devguide/xray-instrumenting-your-app.html#xray-instrumenting-choosing

SchollSimon commented 1 year ago

@RanVaknin commented

Hi @bilalq , JS SDK v3 exposes the same exact parameters that v2 does, the only difference is that those parameters are accessible via the middleware stack, which will require the Xray team to make adjustments on how the Xray clients accesses the data. Im sorry to ping pong you around back to the Xray team, but I did have a conversation with their PM @willarmiros (who now is no longer at Amazon) on March 2023, about this issue and had proven that all the relevant info is available via middleware stack. Since there's no action needed from the JS SDK team so Im going to close this. I encourage you to open the same feature request in their repo, so it can get traction and prioritized. All the best, Ran~

@carolabadeer @willarmiros so what's the status here? We really need a better support on AWS SDKv3 for Xray it is nearly unusable in its current state. It would help our company a lot if we get a solution soon.

vinicius91carvalho commented 1 year ago

For me here, I included it wrapping all of aws-sdk v3 clients, but the main issue is that it requires "aws-sdk" in the runtime, forcing me to add it on the build only for this dep.

SchollSimon commented 1 year ago

For me here, I included it wrapping all of aws-sdk v3 clients, but the main issue is that it requires "aws-sdk" in the runtime, forcing me to add it on the build only for this dep.

How is this related to the issue aka access of data from the middleware stack?

carolabadeer commented 1 year ago

Hi all - happy to share with all of you that support for additional attributes in the AWS SDK v3 instrumentation has been released in v3.5.2! The newest version also includes the fix for the TS errors that would occur when using captureAWSv3Client (released in 3.5.0)

Please check out the newest version and let us know if you are still running into any problems. Since the two issues reported initially have been addressed, I'll close out this issue as resolved!

mnwk commented 6 months ago
  1. Much of the command data no longer appears in the trace

I think this still isn't fixed.

Using: "@aws-sdk/client-dynamodb": "^3.568.0", "aws-xray-sdk": "^3.6.0" ... const client = AWSXRay.captureAWSv3Client(new DynamoDBClient({}));

I still don't get any data besides operation and tablename for a DynamoDB call.