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

captureAWSv3Client does not support SQS or SSM clients #439

Closed
thesuavehog opened this issue Jun 11, 2021 · 18 comments
Closed

captureAWSv3Client does not support SQS or SSM clients #439

thesuavehog opened this issue Jun 11, 2021 · 18 comments
Assignees

Comments

@thesuavehog
Copy link

The captureAWSv3Client documentation states:

/**
 * Instruments AWS SDK V3 clients with X-Ray via middleware.
 *
 * @param client - AWS SDK V3 client to instrument
 * @param manualSegment - Parent segment or subsegment that is passed in for manual mode users
 * @returns - the client with the X-Ray instrumentation middleware added to its middleware stack
 */

However if you try and use SSMClient or SQSClient in a TypeScript context you will get an error:

Argument of type 'SSMClient' is not assignable to parameter of type 'Client<any, any, any>'.
  The types of 'middlewareStack.concat' are incompatible between these types.
     ... and many other lines...

Presumably this applies to any AWS v3 Client which extends
Client<HandlerOptions, ClientInput extends object, ClientOutput extends MetadataBearer, ResolvedClientConfiguration extends SmithyResolvedConfiguration<HandlerOptions>>
(in @aws-sdk/smithy-client/dist/types/client.d.ts).

import { SSMClient } from '@aws-sdk/client-ssm';
import { captureAWSv3Client } from 'aws-xray-sdk';
let ssm: SSMClient = new SSMClient({});
ssm = captureAWSv3Client(ssm);
Argument of type 'SSMClient' is not assignable to parameter of type 'Client<any, any, any>'.
  The types of 'middlewareStack.concat' are incompatible between these types.
    Type '<InputType extends ServiceInputTypes, OutputType extends ServiceOutputTypes>(from: MiddlewareStack<InputType, OutputType>) => MiddlewareStack<InputType, OutputType>' is not assignable to type '<InputType extends any, OutputType extends any>(from: MiddlewareStack<InputType, OutputType>) => MiddlewareStack<InputType, OutputType>'.
      Types of parameters 'from' and 'from' are incompatible.
        Type 'MiddlewareStack<InputType, OutputType>' is not assignable to type 'MiddlewareStack<InputType, ServiceOutputTypes>'.
          Types of property 'addRelativeTo' are incompatible.
            Type '(middleware: MiddlewareType<InputType, OutputType>, options: RelativeMiddlewareOptions) => void' is not assignable to type '(middleware: MiddlewareType<InputType, ServiceOutputTypes>, options: RelativeMiddlewareOptions) => void'.
              Types of parameters 'middleware' and 'middleware' are incompatible.
                Type 'MiddlewareType<InputType, ServiceOutputTypes>' is not assignable to type 'MiddlewareType<InputType, OutputType>'.
                  Type 'InitializeMiddleware<InputType, ServiceOutputTypes>' is not assignable to type 'MiddlewareType<InputType, OutputType>'.
                    Type 'InitializeMiddleware<InputType, ServiceOutputTypes>' is not assignable to type 'InitializeMiddleware<InputType, OutputType>'.
                      Call signature return types 'InitializeHandler<InputType, ServiceOutputTypes>' and 'InitializeHandler<InputType, OutputType>' are incompatible.
                        Type 'Promise<InitializeHandlerOutput<ServiceOutputTypes>>' is not assignable to type 'Promise<InitializeHandlerOutput<OutputType>>'.
                          Type 'InitializeHandlerOutput<ServiceOutputTypes>' is not assignable to type 'InitializeHandlerOutput<OutputType>'.
                            Types of property 'output' are incompatible.
                              Type 'ServiceOutputTypes' is not assignable to type 'OutputType'.
                                'ServiceOutputTypes' is assignable to the constraint of type 'OutputType', but 'OutputType' could be instantiated with a different subtype of constraint 'any'.
                                  Type 'AddTagsToResourceCommandOutput' is not assignable to type 'OutputType'.
                                    'AddTagsToResourceCommandOutput' is assignable to the constraint of type 'OutputType', but 'OutputType' could be instantiated with a different subtype of constraint 'any'.ts(2345)

If captureAWSClient should be used for these type of clients, then the documentation is very confusing since client-ssm and client-sqs are explicitly v3 clients and captureAWSv3Client does not define any specific subset of v3 clients that it supports, nor does it indicate the user should fallback to captureAWSClient (I'm not sure if that's actually what should be done, just saying it doesn't state to do so therefore the assumption for a reader of the documentation should be that you cannot fallback due to the explicit documentation on captureAWSv3Client that it is for v3 clients).

(I'm not sure if this a code bug, a documentation bug, or if someone is going to reply "you didn't read...")

@willarmiros
Copy link
Contributor

Hi @thesuavehog,

Thanks for raising this. This is definitely not the expected behavior, and you should not try to use captureAWSClient since that is only for AWS SDK V2 clients. This seems like an inconsistency with the AWS SDK's typing strategy, similar to aws/aws-sdk-js-v3#1803. You may want to try some of the suggestions on that issue, or open a new issue on their repo.

For now, you can guarantee a workaround with:

import { SSMClient } from '@aws-sdk/client-ssm';
import { captureAWSv3Client } from 'aws-xray-sdk';
let ssm: SSMClient = new SSMClient({});
ssm = captureAWSv3Client(ssm as any);

@thesuavehog
Copy link
Author

@willarmiros Thanks. I will use that workaround for now.

I agree that it's an inconsistency in the AWS SDK's typing for their clients, however in the vein of "try and make things clearer even if it's an issue in our dependencies", it would be great to update the documentation on that captureAWSv3Client given that it expressly requires the Client<any, any, any> type.

Currently the 3 different capture functions for client (captureAWS, captureAWSClient, captureAWSv3Client) and 3 (at least?) different client types (v2, v3<any, any, any> and v3 smithy-client<any, any, any, any>) do result in a pretty confusing grid of choices during this "phase" (hopefully...) of moving everyone to v3 - especially when the main AWS X-Ray docs discussing instrumentation with Node.js don't even mention the v3 SDK.

Anyway, thanks for the workaround!

@willarmiros
Copy link
Contributor

Totally fair! I'll include the workaround in our README for now and also kick off a documentation update for our public docs to include the v3 instrumentation.

@m-radzikowski
Copy link

Hello,
I believe the root cause of the The types of 'middlewareStack.concat' are incompatible between these types... type errors is setting the @aws-sdk/types in the dependencies section instead of peerDependencies.

See those PRs for the similar change:

@willarmiros
Copy link
Contributor

@m-radzikowski thanks for that insight! Makes sense since it seemed like the incompatibility issue stemmed from different versions of the AWS SDK between what we vs. the customer uses. I'll get a PR out to address this.

@m-radzikowski
Copy link

@m-radzikowski thanks for that insight! Makes sense since it seemed like the incompatibility issue stemmed from different versions of the AWS SDK between what we vs. the customer uses. I'll get a PR out to address this.

No problem, did the same mistake in my lib and was helped with that, so I'm sharing it further :D

@willarmiros
Copy link
Contributor

@m-radzikowski Please see my latest comment on #449. I believe I cannot apply the same fix because there are users of this X-Ray SDK which do not also consume the AWS SDK V3, unlike your library (which is specifically intended for AWS SDK v3 users). Therefore, I do not think I can safely move @aws-sdk/types to a peer dependency since some users may not be able to provide it. Please correct me if I'm misunderstanding.

@thesuavehog
Copy link
Author

Would it be an acceptable change to allow for _Client<any, any, any, any> as a type for the AWS v3 patcher?

e.g. in https://github.com/aws/aws-xray-sdk-node/blob/a9d0cf9cbd0328e40f30554f61b4bd5fac08bafc/packages/core/lib/patchers/aws3_p.ts

import { Client as __Client } from '@aws-sdk/smithy-client';
...
type AWSClientV3 = Client<any, any, any> | __Client<any, any, any, any>;
export function captureAWSClient<T extends AWSClientV3>(client: T, manualSegment?: SegmentLike): T {
...

This does not add another dependency because smithy-client is already used to import SdkError.

I'm not sure if there is yet another v3 client pattern that is required but at least those two cover off most I think.

@stale
Copy link

stale bot commented Jan 8, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs in next 7 days. Thank you for your contributions.

@stale stale bot added the stale label Jan 8, 2022
@thesuavehog
Copy link
Author

note: I did a quick test in the code and my suggestion of a quick type alias to encapsulate the two Client type formats does not work as there is a type incompatibility in the middleware stacks of the clients (see below).

I could probably as any stuff to get it working, but not sure that any better than just passing client as any to the function when using it in the first place.

If someone else see a "oh, just do this..." change I can make it and submit the PR but guess I'll live with as any in using the AWS XRay SDK for now... seems like a bit of a core typing issue to deal with!

const getXRayPlugin: (config: RegionResolvedConfig, manualSegment?: SegmentLike | undefined) => Pluggable<any, any>
Argument of type 'Pluggable<any, any>' is not assignable to parameter of type 'Pluggable<any, any> & Pluggable<any, any>'.
  Type 'import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/packages/core/node_modules/@aws-sdk/types/types/middleware").Pluggable<any, any>' is not assignable to type 'import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/node_modules/@aws-sdk/smithy-client/node_modules/@aws-sdk/types/dist/types/middleware").Pluggable<any, any>'.
    Types of property 'applyToStack' are incompatible.
      Type '(stack: import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/packages/core/node_modules/@aws-sdk/types/types/middleware").MiddlewareStack<any, any>) => void' is not assignable to type '(stack: import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/node_modules/@aws-sdk/smithy-client/node_modules/@aws-sdk/types/dist/types/middleware").MiddlewareStack<any, any>) => void'.
        Types of parameters 'stack' and 'stack' are incompatible.
          Type 'import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/node_modules/@aws-sdk/smithy-client/node_modules/@aws-sdk/types/dist/types/middleware").MiddlewareStack<any, any>' is not assignable to type 'import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/packages/core/node_modules/@aws-sdk/types/types/middleware").MiddlewareStack<any, any>'.
            Types of property 'concat' are incompatible.
              Type '<InputType extends any, OutputType extends any>(from: import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/node_modules/@aws-sdk/smithy-client/node_modules/@aws-sdk/types/dist/types/middleware").MiddlewareStack<InputType, OutputType>) => import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/node_modules/@aws-sdk/sm...' is not assignable to type '<InputType extends any, OutputType extends any>(from: import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/packages/core/node_modules/@aws-sdk/types/types/middleware").MiddlewareStack<InputType, OutputType>) => import("C:/Repos/github/thesuavehog/aws-xray-sdk-node/packages/core/node_modules/@aws-sdk/types/types/mid...'.
                Types of parameters 'from' and 'from' are incompatible.
                  Type 'MiddlewareStack<InputType, OutputType>' is not assignable to type 'MiddlewareStack<InputType, OutputType | undefined>'.
                    Types of property 'addRelativeTo' are incompatible.
                      Type '(middleware: MiddlewareType<InputType, OutputType>, options: RelativeMiddlewareOptions) => void' is not assignable to type '(middleware: MiddlewareType<InputType, OutputType | undefined>, options: RelativeMiddlewareOptions) => void'.
                        Types of parameters 'middleware' and 'middleware' are incompatible.
                          Type 'MiddlewareType<InputType, OutputType | undefined>' is not assignable to type 'MiddlewareType<InputType, OutputType>'.
                            Type 'InitializeMiddleware<InputType, OutputType | undefined>' is not assignable to type 'MiddlewareType<InputType, OutputType>'.
                              Type 'InitializeMiddleware<InputType, OutputType | undefined>' is not assignable to type 'InitializeMiddleware<InputType, OutputType>'.
                                Call signature return types 'InitializeHandler<InputType, OutputType | undefined>' and 'InitializeHandler<InputType, OutputType>' are incompatible.
                                  Type 'Promise<InitializeHandlerOutput<OutputType | undefined>>' is not assignable to type 'Promise<InitializeHandlerOutput<OutputType>>'.
                                    Type 'InitializeHandlerOutput<OutputType | undefined>' is not assignable to type 'InitializeHandlerOutput<OutputType>'.
                                      Types of property 'output' are incompatible.
                                        Type 'OutputType | undefined' is not assignable to type 'OutputType'.
                                          'OutputType | undefined' is assignable to the constraint of type 'OutputType', but 'OutputType' could be instantiated with a different subtype of constraint 'any'.
                                            Type 'undefined' is not assignable to type 'OutputType'.
                                              'undefined' is assignable to the constraint of type 'OutputType', but 'OutputType' could be instantiated with a different subtype of constraint 'any'.ts(2345)

@stale stale bot removed the stale label Jan 9, 2022
@nilsstre
Copy link

nilsstre commented Feb 2, 2022

I'm having the same issue with the SNS client using @aws-sdk/client-sns (v3.49.0) and aws-xray-sdk-core (v3.3.4)

Example:

import { SNS } from '@aws-sdk/client-sns'
import AWSXRay from 'aws-xray-sdk-core'

AWSXRay.captureAWSv3Client(new SNS({}))

@paul-uz
Copy link

paul-uz commented May 5, 2022

Same issue with @aws-sdk/client-lambda v3.51.0+

Please can you fix this, rather than us needing to use the workaround?

client-dynamodb works fine.

@paul-uz
Copy link

paul-uz commented May 5, 2022

I'm having the same issue with the SNS client using @aws-sdk/client-sns (v3.49.0) and aws-xray-sdk-core (v3.3.4)

Example:

import { SNS } from '@aws-sdk/client-sns'
import AWSXRay from 'aws-xray-sdk-core'

AWSXRay.captureAWSv3Client(new SNS({}))

You need to use AWSXRay.captureAWSv3Client(new SNS({}) as any)

@MElkady
Copy link

MElkady commented Oct 28, 2022

Any plans to fix this issue soon?

@willarmiros willarmiros added the bug label Nov 1, 2022
@willarmiros
Copy link
Contributor

Hi folks - no ETA on a fix for this, we continue to recommend the workaround of using as any. Would also suggest checking AWS Distro for OpenTelemetry JS, which has 1st class support for X-Ray & more robust instrumentation for AWS SDK v3 clients

@bilalq
Copy link

bilalq commented Jul 1, 2023

AWS Distro for OpenTelemetry JS comes with a significant performance penalty and is not at all ready for prime-time. This should be an entirely solvable problem, though it may require collaboration between the XRay team and the JS SDK team to agree on interface adjustments.

Telling people to cast to any is more than a little ridiculous of a workaround. As a customer, I really do wish this would be addressed.

@mward-LT
Copy link

I only came across this Issue by chance, but this seems to be working perfectly fine for me:

const client = AWSXRay.captureAWSv3Client(new SSMClient({"region": process.env.AWS_REGION}));

"aws-xray-sdk-core": "^3.5.1"
"@aws-sdk/client-ssm": "^3.408.0"

Was this shadow fixed or something? Or am I missing something (and inadvertantly hurting myself by using this)?

@carolabadeer
Copy link
Contributor

Hi @mward-LT, the TS error reported in this issue was fixed in #575! I'll close out this issue since the fix has been released

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

9 participants