aws / aws-xray-sdk-go

AWS X-Ray SDK for the Go programming language.
Apache License 2.0
278 stars 118 forks source link

Support AWS SDK for Go v2 #277

Closed johanneswuerbach closed 3 years ago

johanneswuerbach commented 3 years ago

The AWS SDK for Go v2 v1.0.0 has been released a few days ago. It would be great if the new SDK version would also be instrumentable like the v1 SDK.

Currently instrumenting a v2 SDK clients like:

s3Client := s3.NewFromConfig(cfg)
xray.AWS(s3Client)

fails with: cannot use s3Client (variable of type *s3.Client) as *client.Client value in argument to xray.AWS

awssandra commented 3 years ago

Hi johanneswuerbach,

Unfortunately because the AWS SDK team has done foundational changes to the core APIs and request handling in the V2 rework, well need to dive into the new stack for AWS SDK V2 support.

https://aws.github.io/aws-sdk-go-v2/docs/middleware/

We will add this into our feature backlog, and we are happy to work with the community on PRs.

lukestoward commented 3 years ago

I also am keen to see this support added. Right now, this is a barrier for adoption of the Go SDK v2 as we don't want to lose tracing.

wangzlei commented 3 years ago

There is solution for aws go sdk v2 in OpenTelemetry by middleware, it would be good if someone can help to migrate to here. https://github.com/open-telemetry/opentelemetry-go-contrib/pull/621

johanneswuerbach commented 3 years ago

@wangzlei looks like this has been closed by accident or is the opentelemetry based instrumentation supposed to replace xray? If yes, is there any documentation how to configure it?

stevefchambers commented 3 years ago

@awssandra @wangzlei -- Any thoughts on posting some documentation here on how to use the opentelemetry solution?

The middleware examples are fairly clear. It is just not clear where which particularly piece of info needs to be passed. I assume that the "x-amzn-trace-id" needs to be add somewhere to something...

kevinmcconnell commented 3 years ago

@awssandra could you provide an update on whether v2 support is still on the backlog?

The fact that there is a 3rd-party project that supports it does not seem like it addresses the original issue here. Or does the introduction of the v2 SDK effectively mean the end of any official AWS support for using X-Ray with Go?

ferradario commented 3 years ago

@awssandra, as @kevinmcconnell said it would be nice to understand the timeline for this. Anywhere we can refer to?

bhautikpip commented 3 years ago

@ferradario currently we don't have a timeline for this work. I will try to look at this PR - https://github.com/aws/aws-xray-sdk-go/pull/297 on what work needs to be done in order to add this support. However, we are tracking ToDos in the PR itself.

adamantike commented 3 years ago

Will this be solved after a release is tagged, including merged PR #309?

bhautikpip commented 3 years ago

X-Ray Go SDK v1.6.0 is now released which includes aws sdk v2 support.

DanielBauman88 commented 2 years ago

The AWS SDK for Go v2 v1.0.0 has been released a few days ago. It would be great if the new SDK version would also be instrumentable like the v1 SDK.

Currently instrumenting a v2 SDK clients like:

s3Client := s3.NewFromConfig(cfg)
xray.AWS(s3Client)

fails with: cannot use s3Client (variable of type *s3.Client) as *client.Client value in argument to xray.AWS

Quoting the initial body of this issue for context. Looking at the README under "https://github.com/aws/aws-xray-sdk-go/#readme" it seems like to use xray with sdkv2 you need to perform two operations

  1. awsv2.AWSV2Instrumentor(&cfg.APIOptions)
  2. manually call xray.BeginSegment and defer its closure for each API call made with that client

This is a regression in the xray integration from v1.

Does this package provide a way to configure a client so that all calls are instrumented without requiring the consumer to do it each time?

This should be doable with middleware on the sdkv2 client. Would adding a function to this package that adds xray instrumenting middleware to an aws client solve this problem?

edit: There already is middleware which creates subsegments. What is different about sdkv1 and sdkv2 that the decision was made to require an explicit segment to be created for v2 instead of just handling this automatically in the middleware similarly to v1?