feat: AWS S3 instrumentation

[trentm]

Fixes: #1954

Note that this PR will only include aws-sdk@2 instrumentation. A separate PR will address instrumentation of @awk-sdk/client-s3 (aka "v3" or the "Modular AWS SDK for JavaScript").

Checklist

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

[apmmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Build Cause: Pull request #2112 updated

• Start Time: 2021-06-23T23:15:34.843+0000

• Duration: 18 min 14 sec

• Commit: ea9e8ed

Test stats 🧪

Test	Results
Failed	0
Passed	19040
Skipped	0
Total	19040

Trends 🧪

[trentm]

The state of instrumenting v3 of the JavaScript AWS SDK (https://github.com/aws/aws-sdk-js-v3/tree/main/clients/client-s3) is as follows. (See https://github.com/aws/aws-sdk-js-v3/tree/main/clients/client-s3#import for what I mean by the import styles.)

1. We can get it to automatically instrument if the CommonJS (a.k.a. ES5) require-style is used to import it.
2. We cannot automatically instrument if ESM (a.k.a. ES module, ES6+) import-style is used to import it. We cannot instrument it with known workarounds/hacks (e.g. this technique) because the package (@aws-sdk/client-s3) distributes separate source trees for CommonJS and ESM. (An import { ... } from "@aws-sdk/client-s3" and a const { ... } = require("@aws-sdk/client-s3") will load separate source files.)
3. As a possible documented workaround, we can provide a manual mechanism for users of import { ... } from "@aws-sdk/client-s3" to instrument their created client. It would look like this:

import apm from 'elastic-apm-node/start.js'
import { S3Client, HeadObjectCommand } from "@aws-sdk/client-s3";

const s3client = new S3Client({ region: 'us-west-1' })
apm.instrumentAwsSdkClient(s3client)   // <--- this is the only new line

// ...

I'm not sure if that is worthwhile.

[trentm]

testing access points

S3 Access Points is a
separate AWS service that allows you to create a way to access a bucket
(or parts of a bucket) with policy separate from the bucket itself. It can
be useful for having multiple access policies for a single bucket. An access
point is tied to a specific S3 bucket and region. You then use the access
point ARN -- of the form
arn:aws:s3:$region:$accountId:accesspoint/$accesspointName -- instead of the
bucket name with the usual S3 client APIs, e.g.:

s3Client.getObject({
    Bucket: 'arn:aws:s3:us-west-1:123456789012:accesspoint/trentm-play-ap1',
    Key: 'aDir/aFile.txt'
}, function (err, data) {
    // ...
})

This comment documents my testing of the S3 instrumentation with Access Points.
It is not included in the test suite because, AFAIK, localstack doesn't support
access points.

First I created one to use:

% aws s3control create-access-point --account-id $accountId --bucket trentm-play-s3-bukkit2 --name trentm-play-ap1 --region us-west-1  # bucket is in us-west-1
% aws s3control list-access-points --account-id=$accountId --region=us-west-1
{
    "AccessPointList": [
        ...
        {
            "Name": "trentm-play-ap1",
            "NetworkOrigin": "Internet",
            "Bucket": "trentm-play-s3-bukkit2"
        }
    ]

In the v2 JavaScript AWS SDK, using an access point and the useArnRegion:true
config option for the client allows accessing a bucket in one region with a
client configured for a different region

With an access point the context.destination.address changes to a different
endpoint:

trentm-play-ap1-123456789012.s3-accesspoint.us-west-1.amazonaws.com
$accesspointName-$accountId.s3-accesspoint.$region.amazonaws.com

I used the following ARN in my dev script that exercises the S3 API a little bit.

const ARN1 = `arn:aws:s3:us-west-1:${accountId}:accesspoint/trentm-play-ap1`

A resulting span for a HeadObject call is:

    {
        "span": {
            "name": "S3 HeadObject accesspoint/trentm-play-ap1",
            "type": "storage",
            "subtype": "s3",
            "action": "HeadObject",
            "context": {
                "destination": {
                    "address": "trentm-play-ap1-123456789012.s3-accesspoint.us-west-1.amazonaws.com",
                    "port": 443,
                    "service": {
                        "name": "s3",
                        "type": "storage",
                        "resource": "accesspoint/trentm-play-ap1"
                    },
                    "cloud": {
                        "region": "us-west-1"
                    }
                }
            },
            "outcome": "success",
            // ...

[trentm]

REVIEW NOTE: This was necessary to avoid this error message:

openssl config failed: error:02001002:system library:fopen:No such file or directory

when executing a subprocess to run node fixtures/use-s3.js as part of s3.test.js below... because that subprocess is run in a separate directory (in "test/instrumentation/modules/aws-sdk") rather than at the top-level. Using an absolute path fixes this -- which is the same thing that is done for this in test/test.js:

args.unshift('--require', path.join(__dirname, '_promise_rejection.js'))

[trentm]

REVIEW NOTE: This ensures that ELASTIC_APM_ASYNC_HOOKS is either "true" or "false", and not "" or "false". When it is the empty string, things are fine, but the agent logs:

{"log.level":"warn","@timestamp":"2021-06-22T23:22:55.871Z","log":{"logger":"elastic-apm-node"},"ecs":{"version":"1.6.0"},"message":"unrecognized boolean value \"\" for \"asyncHooks\""}
{"log.level":"warn","@timestamp":"2021-06-22T23:22:55.872Z","log":{"logger":"elastic-apm-node"},"ecs":{"version":"1.6.0"},"message":"unrecognized boolean value \"\" for \"asyncHooks\""}

which is just noise in test output.

[trentm]

@astorm I haven't yet added a changelog entry, but this is ready for review. Our AWS instrumentation might benefit from a separate documentation page, but I think that could be done in a separate issue.

[trentm]

That "Integration Tests failure" above (https://apm-ci.elastic.co/job/apm-agent-nodejs/job/apm-agent-nodejs-mbp/job/PR-2112/) has this in the log:

[2021-06-23T00:26:18.539Z] ERROR: for apm-server  Container "83e98b03806a" is unhealthy.
[2021-06-23T00:26:18.539Z] Encountered errors while bringing up the project.
[2021-06-23T00:26:18.539Z] Starting/Building stack services..
[2021-06-23T00:26:18.539Z] 
[2021-06-23T00:26:18.539Z] Traceback (most recent call last):
[2021-06-23T00:26:18.539Z]   File "scripts/compose.py", line 19, in <module>
[2021-06-23T00:26:18.539Z]     main()
[2021-06-23T00:26:18.539Z]   File "scripts/compose.py", line 15, in main
[2021-06-23T00:26:18.539Z]     setup()
[2021-06-23T00:26:18.539Z]   File "/var/lib/jenkins/workspace/ration-tests-selector-mbp_master/src/github.com/elastic/apm-integration-testing/scripts/modules/cli.py", line 204, in __call__
[2021-06-23T00:26:18.539Z]     self.args.func()
[2021-06-23T00:26:18.539Z]   File "/var/lib/jenkins/workspace/ration-tests-selector-mbp_master/src/github.com/elastic/apm-integration-testing/scripts/modules/cli.py", line 586, in start_handler
[2021-06-23T00:26:18.539Z]     self.build_start_handler("start")
[2021-06-23T00:26:18.540Z]   File "/var/lib/jenkins/workspace/ration-tests-selector-mbp_master/src/github.com/elastic/apm-integration-testing/scripts/modules/cli.py", line 732, in build_start_handler
[2021-06-23T00:26:18.540Z]     self.run_docker_compose_process(docker_compose_cmd + up_params)
[2021-06-23T00:26:18.540Z]   File "/var/lib/jenkins/workspace/ration-tests-selector-mbp_master/src/github.com/elastic/apm-integration-testing/scripts/modules/cli.py", line 472, in run_docker_compose_process
[2021-06-23T00:26:18.540Z]     subprocess.check_call(docker_compose_cmd)
[2021-06-23T00:26:18.540Z]   File "/usr/lib/python3.6/subprocess.py", line 311, in check_call
[2021-06-23T00:26:18.540Z]     raise CalledProcessError(retcode, cmd)
[2021-06-23T00:26:18.540Z] subprocess.CalledProcessError: Command '['docker-compose', '-f', '/var/lib/jenkins/workspace/ration-tests-selector-mbp_master/src/github.com/elastic/apm-integration-testing/docker-compose.yml', '--no-ansi', '--log-level', 'ERROR', 'up', '-d', '--quiet-pull']' returned non-zero exit status 1.

I don't know whta that is about. I'll try to run it again -> https://apm-ci.elastic.co/job/apm-integration-tests-selector-mbp/job/master/18016/

[astorm]

Overall looks good.

Just to say it out loud I do have mixed feeling about incorporating another dependency (localstack) directly into our CI ... but I think I'm slightly in favor of it if it can get us out of the business of mocking requests (especially with the specre of a new SDK with different requests to mock out)

Review is just some due diligence style questions and some nits. Trending towards approval once those questions are answered. 👍

[astorm]

👍