feat: add instrumentation for aws-sdk S3 client

[david-luna]

Description

This change-set add instrumentation for @aws-sdk/client-s3 which is available in v3. The approach is not to instrument the S3Client directly but the base Client class so we can add specific middleware functions based on each client without having to instrument them.

Fixes: #2955

Checklist

• Implement code
• Add tests
• Update TypeScript typings
• Update documentation
• Add CHANGELOG.asciidoc entry
• Commit message follows commit guidelines

[trentm]

This is a great start.

Eventually, another thing to add to the v3 instrumentation is span.setHttpContext(...) as is done in the v2 instrumentation for S3, which adds this to the span:

    {
        "span": {
            // ...
            "context": {
                "http": {
                    "status_code": 200,
                    "response": {
                        "encoded_body_size": 11500
                    }
                }
            },
        }
    }

[trentm]

Interesting, I got this error when running this with my current AWS config:

$ node ./test/instrumentation/modules/@aws-sdk/fixtures/use-s3-v3.js | ecslog
...
[2023-04-26T21:35:04.728Z]  INFO (elastic-apm-node): createBucket
    event.module: "app"
    bucketName: "elasticapmtest-bucket-20230426213504"
    data: null
    error: {
        "type": "S3ServiceException",
        "message":
            The async () => {
                        if (typeof region === "string") {
                            return (0, getRealRegion_1.getRealRegion)(region);
                        }
                        const providedRegion = await region();
                        return (0, getRealRegion_1.getRealRegion)(providedRegion);
                    } location constraint is incompatible for the region specific endpoint this request was sent to.,
        "stack_trace":
            IllegalLocationConstraintException: The async () => {
                        if (typeof region === "string") {
                            return (0, getRealRegion_1.getRealRegion)(region);
                        }
                        const providedRegion = await region();
                        return (0, getRealRegion_1.getRealRegion)(providedRegion);
                    } location constraint is incompatible for the region specific endpoint this request was sent to.
                at throwDefaultError (/Users/trentm/el/apm-agent-nodejs11/node_modules/@aws-sdk/smithy-client/dist-cjs/default-error-handler.js:8:22)
                at /Users/trentm/el/apm-agent-nodejs11/node_modules/@aws-sdk/smithy-client/dist-cjs/default-error-handler.js:18:39
                at de_CreateBucketCommandError (/Users/trentm/el/apm-agent-nodejs11/node_modules/@aws-sdk/client-s3/dist-cjs/protocols/Aws_restXml.js:3162:20)
                at processTicksAndRejections (node:internal/process/task_queues:96:5)
    }

I get this even when the APM agent isn't involved:

ELASTIC_APM_ACTIVE=false node ./test/instrumentation/modules/@aws-sdk/fixtures/use-s3-v3.js | ecslog

Do you know what might be causing this?

[david-luna]

Not yet. I'm going to run my own tests too and post results here

[david-luna]

This is a great start.

Eventually, another thing to add to the v3 instrumentation is span.setHttpContext(...) as is done in the v2 instrumentation for S3, which adds this to the span:

    {
        "span": {
            // ...
            "context": {
                "http": {
                    "status_code": 200,
                    "response": {
                        "encoded_body_size": 11500
                    }
                }
            },
        }
    }

For the encoded_body_size property I've took it from the response header named content-type. Would that be ok?

[trentm]

This is great.

Another couple things:

• You should add a @aws-sdk/client-s3 line to the supported modules table here: https://www.elastic.co/guide/en/apm/agent/nodejs/current/supported-technologies.html#compatibility-tracing-and-instrumentation (that is in "docs/supported-technologies.asciidoc").
• You should also add a note to the CHANGELOG.asciidoc.

[trentm]

This will avoid the failures in CI tests with node less than v14 (the min supported node version for @aws-sdk/*):

diff --git a/test/instrumentation/modules/@aws-sdk/s3-v3.test.js b/test/instrumentation/modules/@aws-sdk/s3-v3.test.js
index ecfe0dc3..453502f2 100644
--- a/test/instrumentation/modules/@aws-sdk/s3-v3.test.js
+++ b/test/instrumentation/modules/@aws-sdk/s3-v3.test.js
@@ -6,10 +6,15 @@

 'use strict'

+const semver = require('semver')
 if (process.env.GITHUB_ACTIONS === 'true' && process.platform === 'win32') {
   console.log('# SKIP: GH Actions do not support docker services on Windows')
   process.exit(0)
 }
+if (semver.lt(process.version, '14.0.0')) {
+  console.log(`# SKIP @aws-sdk min supported node is v14 (node ${process.version})`)
+  process.exit()
+}

 // Test S3 instrumentation of the '@aws-sdk/client-s3' module.
 //

[trentm]

This PR shouldn't have most (any?) of these package-lock diffs. I think some of these will be resolved by merging from recent "main" changes.

[david-luna]

The PR is adding @aws-sdk/client-s3 as a dev dependency and this is its entry in the lock file

    "node_modules/@aws-sdk/client-s3": {
      "version": "3.325.0",
      "resolved": "https://registry.npmjs.org/@aws-sdk/client-s3/-/client-s3-3.325.0.tgz",
      "integrity": "sha512-S88q84bJ1Wba3jnpIrWxMD4AbDsG7l45Ey4ZDdyriEceEgIWvBCA90ZOgSdaObhFNa5pNFZG9C6UbiaCz3F3bA==",
      "dev": true,
      "dependencies": {
        "@aws-crypto/sha1-browser": "3.0.0",
        "@aws-crypto/sha256-browser": "3.0.0",
        "@aws-crypto/sha256-js": "3.0.0",
        "@aws-sdk/client-sts": "3.325.0",
        "@aws-sdk/config-resolver": "3.310.0",
        "@aws-sdk/credential-provider-node": "3.325.0",
        "@aws-sdk/eventstream-serde-browser": "3.310.0",
        "@aws-sdk/eventstream-serde-config-resolver": "3.310.0",
        "@aws-sdk/eventstream-serde-node": "3.310.0",
        "@aws-sdk/fetch-http-handler": "3.310.0",
        "@aws-sdk/hash-blob-browser": "3.310.0",
        "@aws-sdk/hash-node": "3.310.0",
        "@aws-sdk/hash-stream-node": "3.310.0",
        "@aws-sdk/invalid-dependency": "3.310.0",
        "@aws-sdk/md5-js": "3.310.0",
        "@aws-sdk/middleware-bucket-endpoint": "3.310.0",
        "@aws-sdk/middleware-content-length": "3.325.0",
        "@aws-sdk/middleware-endpoint": "3.325.0",
        "@aws-sdk/middleware-expect-continue": "3.325.0",
        "@aws-sdk/middleware-flexible-checksums": "3.325.0",
        "@aws-sdk/middleware-host-header": "3.325.0",
        "@aws-sdk/middleware-location-constraint": "3.325.0",
        "@aws-sdk/middleware-logger": "3.325.0",
        "@aws-sdk/middleware-recursion-detection": "3.325.0",
        "@aws-sdk/middleware-retry": "3.325.0",
        "@aws-sdk/middleware-sdk-s3": "3.325.0",
        "@aws-sdk/middleware-serde": "3.325.0",
        "@aws-sdk/middleware-signing": "3.325.0",
        "@aws-sdk/middleware-ssec": "3.325.0",
        "@aws-sdk/middleware-stack": "3.325.0",
        "@aws-sdk/middleware-user-agent": "3.325.0",
        "@aws-sdk/node-config-provider": "3.310.0",
        "@aws-sdk/node-http-handler": "3.321.1",
        "@aws-sdk/protocol-http": "3.310.0",
        "@aws-sdk/signature-v4-multi-region": "3.310.0",
        "@aws-sdk/smithy-client": "3.325.0",
        "@aws-sdk/types": "3.310.0",
        "@aws-sdk/url-parser": "3.310.0",
        "@aws-sdk/util-base64": "3.310.0",
        "@aws-sdk/util-body-length-browser": "3.310.0",
        "@aws-sdk/util-body-length-node": "3.310.0",
        "@aws-sdk/util-defaults-mode-browser": "3.325.0",
        "@aws-sdk/util-defaults-mode-node": "3.325.0",
        "@aws-sdk/util-endpoints": "3.319.0",
        "@aws-sdk/util-retry": "3.310.0",
        "@aws-sdk/util-stream-browser": "3.310.0",
        "@aws-sdk/util-stream-node": "3.321.1",
        "@aws-sdk/util-user-agent-browser": "3.310.0",
        "@aws-sdk/util-user-agent-node": "3.310.0",
        "@aws-sdk/util-utf8": "3.310.0",
        "@aws-sdk/util-waiter": "3.310.0",
        "@aws-sdk/xml-builder": "3.310.0",
        "fast-xml-parser": "4.1.2",
        "tslib": "^2.5.0"
      },
      "engines": {
        "node": ">=14.0.0"
      }
    },

There's a lot of dependencies and these packages also have their dependencies as well. I guess for SNS, SQS & DynamoDB it will be less impact on the lock file.