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

feat: AWS S3 instrumentation #2112

Merged
merged 27 commits into from
Jun 24, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
ccc7fd7
feat: AWS S3 instrumentation
trentm Jun 16, 2021
a48477c
drop setting span.context.db.* for S3 spans for now pending discussion
trentm Jun 16, 2021
9410ae5
aws-sdk@2 S3 instrumentation should be good with this commit
trentm Jun 16, 2021
2b48d1a
first partial impl of @awk-sdk/client-s3 instrumentation; doubtful th…
trentm Jun 18, 2021
cec0575
avoid 'disableInstrumentations' test of @aws-sdk/client-s3 on node <v…
trentm Jun 18, 2021
0cf170a
v3 SDK: destination context, error capture, outcome
trentm Jun 18, 2021
26310ba
clean up TODOs in the s3 instrumentations
trentm Jun 21, 2021
0c9d3be
first crack at testing S3 aws-sdk instrumentation using localstack
trentm Jun 22, 2021
642b58d
github actions need to setup localstack as well; drop node 15 testing…
trentm Jun 22, 2021
5305b47
correct GH action yaml syntax
trentm Jun 22, 2021
bdb89df
fix the following "openssl ... No such file or directory" when exec'i…
trentm Jun 22, 2021
7006d39
debugging CI test failures
trentm Jun 22, 2021
fe0b0a1
limit tests run for debugging failures; fix AWS auth envvars for test…
trentm Jun 22, 2021
d42619b
not helping standard
trentm Jun 22, 2021
0adb071
log used endpoint for debugging; add .promise()-style case
trentm Jun 22, 2021
1eb42da
fixes; still debugging CI
trentm Jun 22, 2021
585206a
's3ForcePathStyle: true' to fix the bucketName.localstack DNS resolut…
trentm Jun 22, 2021
e8067e4
windows test fix
trentm Jun 22, 2021
b157b25
cleaning up tests
trentm Jun 22, 2021
e89a021
restore full node ver testing; modulo dropping node v15
trentm Jun 22, 2021
d1239ab
restore runing all the test files
trentm Jun 22, 2021
44c860d
drop v3 SDK instrumentation, it has moved to a separate PR
trentm Jun 22, 2021
31da8a1
Merge branch 'master' into trentm/instrument-s3
trentm Jun 22, 2021
a1892d5
method S3 is supported tech
trentm Jun 22, 2021
5455623
changelog entry
trentm Jun 22, 2021
79cfb27
Merge branch 'master' into trentm/instrument-s3
trentm Jun 23, 2021
ea9e8ed
updates from review feedback
trentm Jun 23, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .ci/.jenkins_nodejs.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
NODEJS_VERSION:
- "16"
- "16.0"
- "15"
- "14"
- "14.0"
- "12"
Expand Down
1 change: 1 addition & 0 deletions .ci/Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ def generateStepForWindows(Map params = [:]){
"ELASTIC_APM_ASYNC_HOOKS=${ELASTIC_APM_ASYNC_HOOKS}",
"CASSANDRA_HOST=${linuxIp}",
"ES_HOST=${linuxIp}",
"LOCALSTACK_HOST=${linuxIp}",
"MEMCACHED_HOST=${linuxIp}",
"MONGODB_HOST=${linuxIp}",
"MSSQL_HOST=${linuxIp}",
Expand Down
8 changes: 8 additions & 0 deletions .ci/docker/docker-compose-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ services:
extends:
file: docker-compose.yml
service: memcached
localstack:
extends:
file: docker-compose.yml
service: localstack
node_tests:
extends:
file: docker-compose-node-test.yml
Expand All @@ -54,6 +58,8 @@ services:
condition: service_healthy
memcached:
condition: service_healthy
localstack:
condition: service_healthy

volumes:
nodepgdata:
Expand All @@ -68,3 +74,5 @@ volumes:
driver: local
nodecassandradata:
driver: local
nodelocalstackdata:
driver: local
8 changes: 8 additions & 0 deletions .ci/docker/docker-compose-edge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ services:
extends:
file: docker-compose.yml
service: elasticsearch
localstack:
extends:
file: docker-compose.yml
service: localstack
memcached:
extends:
file: docker-compose.yml
Expand Down Expand Up @@ -42,6 +46,8 @@ services:
condition: service_healthy
elasticsearch:
condition: service_healthy
localstack:
condition: service_healthy
memcached:
condition: service_healthy
mongodb:
Expand All @@ -64,6 +70,8 @@ volumes:
driver: local
nodemysqldata:
driver: local
nodelocalstackdata:
driver: local
nodeesdata:
driver: local
nodecassandradata:
Expand Down
14 changes: 14 additions & 0 deletions .ci/docker/docker-compose-localstack.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
version: '2.1'

services:
localstack:
extends:
file: docker-compose.yml
service: localstack
node_tests:
extends:
file: docker-compose-node-test.yml
service: node_tests
depends_on:
localstack:
condition: service_healthy
1 change: 1 addition & 0 deletions .ci/docker/docker-compose-node-edge-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ services:
PGHOST: 'postgres'
PGUSER: 'postgres'
MEMCACHED_HOST: 'memcached'
LOCALSTACK_HOST: 'localstack'
NODE_VERSION: ${NODE_VERSION}
NVM_NODEJS_ORG_MIRROR: ${NVM_NODEJS_ORG_MIRROR}
ELASTIC_APM_ASYNC_HOOKS: ${ELASTIC_APM_ASYNC_HOOKS}
Expand Down
1 change: 1 addition & 0 deletions .ci/docker/docker-compose-node-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ services:
PGHOST: 'postgres'
PGUSER: 'postgres'
MEMCACHED_HOST: 'memcached'
LOCALSTACK_HOST: 'localstack'
NODE_VERSION: ${NODE_VERSION}
TAV: ${TAV_MODULE}
ELASTIC_APM_ASYNC_HOOKS: ${ELASTIC_APM_ASYNC_HOOKS}
Expand Down
18 changes: 18 additions & 0 deletions .ci/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,22 @@ services:
timeout: 10s
retries: 5

localstack:
# https://hub.docker.com/r/localstack/localstack/tags
image: localstack/localstack:0.12.12
environment:
- LOCALSTACK_SERVICES=s3
- DATA_DIR=/var/lib/localstack
ports:
- "4566:4566"
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:4566/health"]
interval: 30s
timeout: 10s
retries: 5
volumes:
- nodelocalstackdata:/var/lib/localstack

volumes:
nodepgdata:
driver: local
Expand All @@ -127,3 +143,5 @@ volumes:
driver: local
nodecassandradata:
driver: local
nodelocalstackdata:
driver: local
2 changes: 1 addition & 1 deletion .ci/scripts/docker-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fi
# Skip for node v8 because it results in this warning:
# openssl config failed: error:25066067:DSO support routines:DLFCN_LOAD:could not load the shared library
if [[ $major_node_version -gt 8 ]]; then
export NODE_OPTIONS="${NODE_OPTIONS:+${NODE_OPTIONS}} --openssl-config=./test/openssl-config-for-testing.cnf"
export NODE_OPTIONS="${NODE_OPTIONS:+${NODE_OPTIONS}} --openssl-config=$(pwd)/test/openssl-config-for-testing.cnf"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'))

fi

# Workaround to git <2.7
Expand Down
5 changes: 5 additions & 0 deletions .ci/scripts/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ elif [[ -n "${TAV_MODULE}" ]]; then
memcached)
DOCKER_COMPOSE_FILE=docker-compose-memcached.yml
;;
aws-sdk)
DOCKER_COMPOSE_FILE=docker-compose-localstack.yml
;;
*)
# Just the "node_tests" container. No additional services needed for testing.
DOCKER_COMPOSE_FILE=docker-compose-node-test.yml
Expand All @@ -210,6 +213,8 @@ else
DOCKER_COMPOSE_FILE=docker-compose-all.yml
fi

ELASTIC_APM_ASYNC_HOOKS=${ELASTIC_APM_ASYNC_HOOKS:-true}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

set +e
NVM_NODEJS_ORG_MIRROR=${NVM_NODEJS_ORG_MIRROR} \
ELASTIC_APM_ASYNC_HOOKS=${ELASTIC_APM_ASYNC_HOOKS} \
Expand Down
11 changes: 10 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,21 @@ jobs:
volumes:
- nodeesdata:/usr/share/elasticsearch/data

localstack:
image: localstack/localstack:0.12.12
env:
LOCALSTACK_SERVICES: 's3'
DATA_DIR: '/var/lib/localstack'
ports:
- "4566:4566"
volumes:
- nodelocalstackdata:/var/lib/localstack

strategy:
matrix:
node:
- '16'
- '16.0'
- '15'
- '14'
- '14.0'
- '12'
Expand Down
1 change: 1 addition & 0 deletions lib/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ const EMPTY_OPTS = {}
//
// Usage:
// captureError(err, opts, cb)
// captureError(err, opts)
// captureError(err, cb)
//
// where:
Expand Down
1 change: 1 addition & 0 deletions lib/instrumentation/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var shimmer = require('./shimmer')
var Transaction = require('./transaction')

var MODULES = [
'@aws-sdk/client-s3',
'@elastic/elasticsearch',
'apollo-server-core',
'aws-sdk',
Expand Down
Loading