updates from review feedback

lib/instrumentation/modules/aws-sdk/s3.js

@@ -64,20 +64,22 @@ function instrumentationS3 (orig, origArguments, request, AWS, agent, { version,
       // This can be asynchronously determined -- e.g. if it differs from the
       // configured service endpoint region -- so this won't be set until
       // 'complete'.
-      const region = request.httpRequest.region
+      const region = request.httpRequest && request.httpRequest.region
 
       // Destination context.
       // '.httpRequest.endpoint' might differ from '.service.endpoint' if
       // the bucket is in a different region.
-      const endpoint = request.httpRequest.endpoint
+      const endpoint = request.httpRequest && request.httpRequest.endpoint
       const destContext = {
-        address: endpoint.hostname,
-        port: endpoint.port,
         service: {
           name: SUBTYPE,
           type: TYPE
         }
       }
+      if (endpoint) {
+        destContext.address = endpoint.hostname
+        destContext.port = endpoint.port
+      }
       if (resource) {
         destContext.service.resource = resource
       }

test/instrumentation/modules/aws-sdk/fixtures/use-s3.js

@@ -24,7 +24,7 @@
 //    node use-s3.js | ecslog
 //
 //    # Testing against localstack.
-//    docker run --rm -it -e SERVICES=s3 -p 4566:4566 -p 4571:4571 localstack/localstack
+//    docker run --rm -it -e SERVICES=s3 -p 4566:4566 localstack/localstack
 //    TEST_ENDPOINT=http://localhost:4566 node use-s3.js | ecslog
 //
 //    # Use TEST_BUCKET_NAME to re-use an existing bucket (and not delete it).

test/instrumentation/modules/aws-sdk/s3.test.js

@@ -51,14 +51,16 @@ tape.test('simple S3 usage scenario', function (t) {
         // Sort the events by timestamp, then work through each expected span.
         const events = server.events.slice(1)
         events.sort((a, b) => {
-          const aTimestamp = (a.transaction || a.span).timestamp
-          const bTimestamp = (b.transaction || b.span).timestamp
+          const aTimestamp = (a.transaction || a.span || {}).timestamp
+          const bTimestamp = (b.transaction || b.span || {}).timestamp
           return aTimestamp < bTimestamp ? -1 : 1
         })
 
         // First the transaction.
         t.ok(events[0].transaction, 'got the transaction')
         const tx = events.shift().transaction
+        t.equal(events.filter(e => e.span).length, events.length,
+          'all remaining events are spans')
 
         // Currently HTTP spans under each S3 span are included. Eventually
         // those will be excluded. Filter those out for now.

test/script/local-deps-start.sh

@@ -8,5 +8,7 @@ memcached -d -P /tmp/memcached.pid
 redis-server /usr/local/etc/redis.conf --daemonize yes
 mysql.server start
 
-# Note: localstack is not included here because running localstack
-# outside of Docker is deprecated/not supported.
+# Note: Running a "local" (i.e. outside of Docker) localstack is deprecated/not
+# supported. So we run it in Docker.
+docker run --name dev-localstack -d --rm -e SERVICES=s3 -p 4566:4566 localstack/localstack
+

test/script/local-deps-stop.sh

@@ -7,3 +7,4 @@ kill `cat /tmp/cassandra.pid`
 kill `cat /tmp/memcached.pid`
 redis-cli shutdown
 mysql.server stop
+docker stop dev-localstack