feat(instrumentation-aws-sdk): upstream aws-sdk instrumentation from ext-js

[blumamir]

This PR is for upstreaming the aws-sdk instrumentation from this repo.

I copied the code here and made the required modification to match the contrib repo's tooling and conventions.

This plugin has been around for about a year and a half and has proven stable in production. it has >5K weekly downloads in npm and was actively maintained in the original repo. All issues and bug fixes along this period had been addressed and fixed.
The aws-sdk V2 and V3 codebase is kind of complex and was designed in a way that is not very instrumentation-friendly. I had to implement very complicated and non-trivial patches to be able to extract the needed attributes, as well as normalize the data across V2 and V3 - which can be quite hard to review and understand I suppose. Please share ideas for improvements if you have any or open discussions on things that require additional clarifications.

I am open to discuss any issue regarding the porting, in praticular:

• code and instrumentation conventions
• specification compliance and known gaps
• documentation
• testing strategy for this codebase
• breaking changes from the current published version (as this is a good chance to fix those with the new npm package name)

[codecov]

Codecov Report

Merging #678 (a57cd4a) into main (314c890) will decrease coverage by 0.95%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #678      +/-   ##
==========================================
- Coverage   96.31%   95.35%   -0.96%     
==========================================
  Files           5       27      +22     
  Lines         515     2197    +1682     
  Branches       97      302     +205     
==========================================
+ Hits          496     2095    +1599     
- Misses         19      102      +83     

Impacted Files	Coverage Δ
...metry-instrumentation-mongodb/test/mongodb.test.ts	95.58% <0.00%> (ø)
...trumentation-document-load/src/enums/EventNames.ts	100.00% <0.00%> (ø)
...try-instrumentation-mongodb/src/instrumentation.ts	87.36% <0.00%> (ø)
...strumentation-document-load/src/instrumentation.ts	98.79% <0.00%> (ø)
...ry-instrumentation-aws-sdk/test/aws-sdk-v3.test.ts	100.00% <0.00%> (ø)
...y-instrumentation-aws-sdk/src/services/dynamodb.ts	100.00% <0.00%> (ø)
...etry-instrumentation-aws-sdk/test/dynamodb.test.ts	100.00% <0.00%> (ø)
...tapackages/auto-instrumentations-node/src/utils.ts	96.77% <0.00%> (ø)
...telemetry-instrumentation-aws-sdk/test/sqs.test.ts	88.17% <0.00%> (ø)
...emetry-instrumentation-aws-sdk/src/services/sqs.ts	87.83% <0.00%> (ø)
... and 12 more

[Flarna]

Should we move this package also?

[blumamir]

Yes you are right, I need to move it as well.

[willarmiros]

Code LGTM, apart from breaking config changes discussed offline. Looks like yargs-parser dropped Node 6 and 8 support a while ago in v19.0.0: https://github.com/yargs/yargs-parser/releases/tag/v19.0.0

So if we can force the package to adopt a version before then (18.x) it should fix the build issues. In the future we can hopefully drop support for Node 8 altogether.

[blumamir]

The only thing blocking from merging is adding the new packages to the release-please config

Added

[willarmiros]

LGTM, thanks again for all the hard work @blumamir!

[blumamir]

Thanks to everyone who reviewed and helped fix the issues.
As far as I can tell, everything has been addressed and this one is finally ready to be merged.

[mattfysh]

Hey @blumamir - thanks for this great contrib!

I had a question about using the table name as "DB_NAME" attribute, I wonder if this should be "DynamoDB" as shown in this screenshot: https://docs.aws.amazon.com/xray/latest/devguide/images/scorekeep-PUTrules-timeline.png

The table name would then be stored alongside the operation name, and rendered by the Xray UI in the segment description as "OperationName: TableName"

On a loosely related note, do you know if there is any documentation covering how Xray decides what to render as the grey description text (to the right of the coloured bar in the timeline)? Thanks!

[blumamir]

I had a question about using the table name as "DB_NAME" attribute, I wonder if this should be "DynamoDB" as shown in this screenshot: https://docs.aws.amazon.com/xray/latest/devguide/images/scorekeep-PUTrules-timeline.png

Currently, db.name is filled with the table name, and db.system contains the "dynamodb" value. I agree that maybe the table name is more appropriate in an attribute like db.table, but the spec only has db.sql.table but dyanmodb is not an sql database.

The table name would then be stored alongside the operation name, and rendered by the Xray UI in the segment description as "OperationName: TableName"

Not sure what you are suggesting here. Are you suggesting to add the table name in the span name? or to concat the table name into the db.operation attribute?

Regarding Xray, I have no experience with it. maybe @willarmiros or @NathanielRN can help

[willarmiros]

On a loosely related note, do you know if there is any documentation covering how Xray decides what to render as the grey description text (to the right of the coloured bar in the timeline)? Thanks!

I don't believe this is documented since it's more of an internal implementation detail - the closest documentation we have is this on viewing traces. However I can say that basically the gray text attempts to represent the operation performed, and depends on the namespace of a subsegment, and the attributes of that subsegment. For instance, if the namespace is aws, then the text is aws.operation, and for some cases it will also include a resource like for DynamoDB it will be aws.operation: aws.table_name. If the namespace is remote, the gray text will be something like Remote: http.request.method.