-
Notifications
You must be signed in to change notification settings - Fork 224
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
chore: instrument smithy-client v2 #3526
Conversation
@@ -37,7 +37,7 @@ const nodeHasInstrumentableFetch = typeof (global.fetch) === 'function' | |||
|
|||
var MODULES = [ | |||
'@apollo/server', | |||
['@aws-sdk/smithy-client', '@smithy/smithy-client'], // Instrument the base client which all AWS-SDK v3 clients extends. | |||
['@smithy/smithy-client', '@aws-sdk/smithy-client'], // Instrument the base client which all AWS-SDK v3 clients extends. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to reviewer: moved the smithy client file so path represents properly the namespace of the package on its recent version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/test tav @aws-sdk/client-s3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/test tav @aws-sdk/client-s3 16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to fix the package-lock file.
package-lock.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This accidentally changed to lockfileVersion: 1:
diff --git a/package-lock.json b/package-lock.json
index 51f534bf..bc50cde7 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,17985 +1,8 @@
{
"name": "elastic-apm-node",
"version": "3.48.0",
- "lockfileVersion": 2,
+ "lockfileVersion": 1,
...
I think you may have npm install
ed with an older version of Node than v16.
The huge change to
A lot of the diff is because these devDeps are at different versions:
Leading to a loss of a lot of possible dedupes for common transitive deps -- now at different versions. I'll upgrade the latter and see if that helps. |
Yes, that helps a lot. For example, instead of >30 installations of
And the diff is reduced:
and with the default algorithm as well:
@david-luna I think as a rule, if we update one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, assuming tests pass again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/test tav @aws-sdk/client-s3
`@smithy/client@2` is used in `@aws-sdk/*` v3.378.0 and later. Closes: elastic#3523
Add support for
@smithy/smithy-client
v2+. This version is introduced by@aws-sdk/client-s3
and others in version v3.378.0Looking at the major release of
smithy-client
there are breaking changes in its types but these are not affecting the middlewares of the AWS clients so its safe to add support for it.Closes #3523
Checklist