From 911e7896d5c7f0d2ee5f30e3cb6cb2e57a789091 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Wed, 9 Jun 2021 08:46:16 -0700 Subject: [PATCH 01/14] feat: initial instrumentation --- lib/instrumentation/modules/aws-sdk.js | 5 +- .../modules/aws-sdk/dynamodb.js | 189 ++++++++++++++++++ 2 files changed, 193 insertions(+), 1 deletion(-) create mode 100644 lib/instrumentation/modules/aws-sdk/dynamodb.js diff --git a/lib/instrumentation/modules/aws-sdk.js b/lib/instrumentation/modules/aws-sdk.js index 67c133c1fe..39f22ff454 100644 --- a/lib/instrumentation/modules/aws-sdk.js +++ b/lib/instrumentation/modules/aws-sdk.js @@ -2,7 +2,7 @@ const semver = require('semver') const shimmer = require('../shimmer') const { instrumentationSqs } = require('./aws-sdk/sqs') - +const {instrumentationDynamoDb} = require('./aws-sdk/dynamodb.js') // Called in place of AWS.Request.send and AWS.Request.promise // // Determines which amazon service an API request is for @@ -12,6 +12,9 @@ function instrumentOperation (orig, origArguments, request, AWS, agent, { versio if (request.service.serviceIdentifier === 'sqs') { return instrumentationSqs(orig, origArguments, request, AWS, agent, { version, enabled }) } + if (request.service.serviceIdentifier === 'dynamodb') { + return instrumentationDynamoDb(orig, origArguments, request, AWS, agent, { version, enabled }) + } // if we're still here, then we still need to call the original method return orig.apply(request, origArguments) diff --git a/lib/instrumentation/modules/aws-sdk/dynamodb.js b/lib/instrumentation/modules/aws-sdk/dynamodb.js new file mode 100644 index 0000000000..6608aa3983 --- /dev/null +++ b/lib/instrumentation/modules/aws-sdk/dynamodb.js @@ -0,0 +1,189 @@ +'use strict' +// const { URL } = require('url') +// const constants = require('../../../constants') +// const OPERATIONS_TO_ACTIONS = { +// deleteMessage: 'delete', +// deleteMessageBatch: 'delete_batch', +// receiveMessage: 'poll', +// sendMessageBatch: 'send_batch', +// sendMessage: 'send', +// unknown: 'unknown' +// } +// const OPERATIONS = Object.keys(OPERATIONS_TO_ACTIONS) +const TYPE = 'db' +const SUBTYPE = 'dynamodb' +const ACTION = 'query' +// const queueMetrics = new Map() +// +// // Returns Message Queue action from AWS SDK method name +// function getActionFromRequest (request) { +// request = request || {} +// const operation = request.operation ? request.operation : 'unknown' +// const action = OPERATIONS_TO_ACTIONS[operation] +// +// return action +// } +// +// // Returns preposition to use in span name +// // +// // POLL from ... +// // SEND to ... +// function getToFromFromOperation (operation) { +// let result = 'from' +// if (operation === 'sendMessage' || operation === 'sendMessageBatch') { +// result = 'to' +// } +// return result +// } +// +// // Parses queue/topic name from AWS queue URL +// function getQueueNameFromRequest (request) { +// const unknown = 'unknown' +// if (!request || !request.params || !request.params.QueueUrl) { +// return unknown +// } +// try { +// const url = new URL(request.params.QueueUrl) +// return url.pathname.split('/').pop() +// } catch (e) { +// return unknown +// } +// } +// +// // Parses region name from AWS service configuration +// function getRegionFromRequest (request) { +// const region = request && request.service && +// request.service.config && request.service.config.region +// return region || '' +// } +// +// // Creates message destination context suitable for setDestinationContext +// function getMessageDestinationContextFromRequest (request) { +// const destination = { +// service: { +// name: SUBTYPE, +// resource: `${SUBTYPE}/${getQueueNameFromRequest(request)}`, +// type: TYPE +// }, +// cloud: { +// region: getRegionFromRequest(request) +// } +// } +// return destination +// } +// +// // create message context suitable for setMessageContext +// function getMessageContextFromRequest (request) { +// const message = { +// queue: { +// name: getQueueNameFromRequest(request) +// } +// } +// return message +// } +// +// // Record queue related metrics +// // +// // Creates metric collector objects on first run, and +// // updates their data with data from received messages +// function recordMetrics (queueName, data, agent) { +// const messages = data && data.Messages +// if (!messages || messages.length < 1) { +// return +// } +// if (!queueMetrics.get(queueName)) { +// const collector = agent._metrics.createQueueMetricsCollector(queueName) +// queueMetrics.set(queueName, collector) +// } +// const metrics = queueMetrics.get(queueName) +// +// for (const message of messages) { +// const sentTimestamp = message.Attributes && message.Attributes.SentTimestamp +// const delay = (new Date()).getTime() - sentTimestamp +// metrics.updateStats(delay) +// } +// } +// + +function getMethodFromRequest(request) { + return request && request.operation +} + +function getTableFromRequest(request) { + const table = request && request.params && request.params.TableName + if(!table) { + return '' + } + return ` ${table}` +} + +// Creates the span name from request information +function getSpanNameFromRequest (request) { + const method = getMethodFromRequest(request) + const table = getTableFromRequest(request) + const name = `DynamoDB ${method}${table}` + return name +} + +// +function shouldIgnoreRequest (request, agent) { + // const operation = request && request.operation + // console.log(operation) + // are we interested in this operation/method call? + // if (OPERATIONS.indexOf(operation) === -1) { + // return true + // } + + if (!agent.currentTransaction) { + agent.logger.trace('no active transaction found, skipping dynamodb instrumentation') + return true + } + + return false +} + +// Main entrypoint for SQS instrumentation +// +// Must call (or one of its function calls must call) the +// `orig` function/method +function instrumentationDynamoDb (orig, origArguments, request, AWS, agent, { version, enabled }) { + if (shouldIgnoreRequest(request, agent)) { + return orig.apply(request, origArguments) + } + + const type = TYPE + const subtype = SUBTYPE + const action = ACTION + // const action = getActionFromRequest(request) + const name = getSpanNameFromRequest(request) + console.log(name) + // const span = agent.startSpan(name, type, subtype, action) + // span.setDestinationContext(getMessageDestinationContextFromRequest(request)) + // span.setMessageContext(getMessageContextFromRequest(request)) + + // request.on('complete', function (response) { + // if (response && response.error) { + // const errOpts = { + // skipOutcome: true + // } + // agent.captureError(response.error, errOpts) + // span._setOutcomeFromErrorCapture(constants.OUTCOME_FAILURE) + // } + + // // we'll need to manually mark this span as async. The actual async hop + // // is captured by the agent's async hooks instrumentation + // span.sync = false + // span.end() + + // if (request.operation === 'receiveMessage' && response && response.data) { + // recordMetrics(getQueueNameFromRequest(request), response.data, agent) + // } + // }) + console.log("SCIENCE") + const origResult = orig.apply(request, origArguments) + return origResult +} + +module.exports = { + instrumentationDynamoDb, +} From ac9b43e0373f96acf59ee9a2ecc10921ff1973f6 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Wed, 23 Jun 2021 12:08:47 -0700 Subject: [PATCH 02/14] test: in progress --- lib/agent.js | 1 + .../modules/aws-sdk/dynamodb.js | 203 ++++++------------ .../modules/aws-sdk/dynamodb.js | 170 +++++++++++++++ .../modules/aws-sdk/fixtures-dynamodb.js | 7 + 4 files changed, 245 insertions(+), 136 deletions(-) create mode 100644 test/instrumentation/modules/aws-sdk/dynamodb.js create mode 100644 test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js diff --git a/lib/agent.js b/lib/agent.js index be403523c8..139ca1dc6d 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -450,6 +450,7 @@ Agent.prototype.captureError = function (err, opts, cb) { if (agent._transport) { agent.logger.info('Sending error to Elastic APM: %o', { id }) + console.log(error) agent._transport.sendError(error, function () { agent.flush(function (err) { if (cb) cb(err, id) diff --git a/lib/instrumentation/modules/aws-sdk/dynamodb.js b/lib/instrumentation/modules/aws-sdk/dynamodb.js index 6608aa3983..21a0a5b1ad 100644 --- a/lib/instrumentation/modules/aws-sdk/dynamodb.js +++ b/lib/instrumentation/modules/aws-sdk/dynamodb.js @@ -1,114 +1,36 @@ 'use strict' -// const { URL } = require('url') -// const constants = require('../../../constants') -// const OPERATIONS_TO_ACTIONS = { -// deleteMessage: 'delete', -// deleteMessageBatch: 'delete_batch', -// receiveMessage: 'poll', -// sendMessageBatch: 'send_batch', -// sendMessage: 'send', -// unknown: 'unknown' -// } -// const OPERATIONS = Object.keys(OPERATIONS_TO_ACTIONS) +const constants = require('../../../constants') const TYPE = 'db' const SUBTYPE = 'dynamodb' const ACTION = 'query' -// const queueMetrics = new Map() -// -// // Returns Message Queue action from AWS SDK method name -// function getActionFromRequest (request) { -// request = request || {} -// const operation = request.operation ? request.operation : 'unknown' -// const action = OPERATIONS_TO_ACTIONS[operation] -// -// return action -// } -// -// // Returns preposition to use in span name -// // -// // POLL from ... -// // SEND to ... -// function getToFromFromOperation (operation) { -// let result = 'from' -// if (operation === 'sendMessage' || operation === 'sendMessageBatch') { -// result = 'to' -// } -// return result -// } -// -// // Parses queue/topic name from AWS queue URL -// function getQueueNameFromRequest (request) { -// const unknown = 'unknown' -// if (!request || !request.params || !request.params.QueueUrl) { -// return unknown -// } -// try { -// const url = new URL(request.params.QueueUrl) -// return url.pathname.split('/').pop() -// } catch (e) { -// return unknown -// } -// } -// -// // Parses region name from AWS service configuration -// function getRegionFromRequest (request) { -// const region = request && request.service && -// request.service.config && request.service.config.region -// return region || '' -// } -// -// // Creates message destination context suitable for setDestinationContext -// function getMessageDestinationContextFromRequest (request) { -// const destination = { -// service: { -// name: SUBTYPE, -// resource: `${SUBTYPE}/${getQueueNameFromRequest(request)}`, -// type: TYPE -// }, -// cloud: { -// region: getRegionFromRequest(request) -// } -// } -// return destination -// } -// -// // create message context suitable for setMessageContext -// function getMessageContextFromRequest (request) { -// const message = { -// queue: { -// name: getQueueNameFromRequest(request) -// } -// } -// return message -// } -// -// // Record queue related metrics -// // -// // Creates metric collector objects on first run, and -// // updates their data with data from received messages -// function recordMetrics (queueName, data, agent) { -// const messages = data && data.Messages -// if (!messages || messages.length < 1) { -// return -// } -// if (!queueMetrics.get(queueName)) { -// const collector = agent._metrics.createQueueMetricsCollector(queueName) -// queueMetrics.set(queueName, collector) -// } -// const metrics = queueMetrics.get(queueName) -// -// for (const message of messages) { -// const sentTimestamp = message.Attributes && message.Attributes.SentTimestamp -// const delay = (new Date()).getTime() - sentTimestamp -// metrics.updateStats(delay) -// } -// } -// + +function getRegionFromRequest(request) { + return request && request.service && + request.service.config && request.service.config.region +} + +function getPortFromRequest(request) { + return request && request.service && + request.service.endpoint && request.service.endpoint.port +} function getMethodFromRequest(request) { return request && request.operation } +function getStatementFromRequest(request) { + const method = getMethodFromRequest(request) + if('query' === method && request && request.params && request.params.KeyConditionExpression) { + return request.params.KeyConditionExpression + } + return undefined +} + +function getAddressFromRequest(request) { + return request && request.service && request.service.endpoint && + request.service.endpoint.host +} + function getTableFromRequest(request) { const table = request && request.params && request.params.TableName if(!table) { @@ -125,20 +47,7 @@ function getSpanNameFromRequest (request) { return name } -// function shouldIgnoreRequest (request, agent) { - // const operation = request && request.operation - // console.log(operation) - // are we interested in this operation/method call? - // if (OPERATIONS.indexOf(operation) === -1) { - // return true - // } - - if (!agent.currentTransaction) { - agent.logger.trace('no active transaction found, skipping dynamodb instrumentation') - return true - } - return false } @@ -156,34 +65,56 @@ function instrumentationDynamoDb (orig, origArguments, request, AWS, agent, { ve const action = ACTION // const action = getActionFromRequest(request) const name = getSpanNameFromRequest(request) - console.log(name) - // const span = agent.startSpan(name, type, subtype, action) + console.log('creating span: ' + name) + const span = agent.startSpan(name, type, subtype, action) + if(!span) { + return orig.apply(request, origArguments) + } // span.setDestinationContext(getMessageDestinationContextFromRequest(request)) // span.setMessageContext(getMessageContextFromRequest(request)) + span.setDbContext({ + instance:getRegionFromRequest(request), + statement:getStatementFromRequest(request), + type: SUBTYPE, + }) + span.setDestinationContext({ + address: getAddressFromRequest(request), + port: getPortFromRequest(request), + service: { + name: SUBTYPE, + type: 'db', + resource: SUBTYPE + }, + cloud: { + region: getRegionFromRequest(request) + } + }) - // request.on('complete', function (response) { - // if (response && response.error) { - // const errOpts = { - // skipOutcome: true - // } - // agent.captureError(response.error, errOpts) - // span._setOutcomeFromErrorCapture(constants.OUTCOME_FAILURE) - // } + request.on('complete', function (response) { + console.log(response.httpResponse.body.toString()) + if (response && response.error) { + const errOpts = { + skipOutcome: true + } + agent.captureError(response.error, errOpts) + span._setOutcomeFromErrorCapture(constants.OUTCOME_FAILURE) + } - // // we'll need to manually mark this span as async. The actual async hop - // // is captured by the agent's async hooks instrumentation - // span.sync = false - // span.end() + // we'll need to manually mark this span as async. The actual async hop + // is captured by the agent's async hooks instrumentation + span.sync = false + span.end() + }) - // if (request.operation === 'receiveMessage' && response && response.data) { - // recordMetrics(getQueueNameFromRequest(request), response.data, agent) - // } - // }) - console.log("SCIENCE") - const origResult = orig.apply(request, origArguments) - return origResult + return orig.apply(request, origArguments) } module.exports = { instrumentationDynamoDb, + + // exported for testing + getRegionFromRequest, + getPortFromRequest, + getStatementFromRequest, + getAddressFromRequest } diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js new file mode 100644 index 0000000000..b67d26fc32 --- /dev/null +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -0,0 +1,170 @@ + const agent = require('../../../..').start({ + serviceName: 'test', + secretToken: 'test', + captureExceptions: false, + metricsInterval: 0, + centralConfig: false +}) +const tape = require('tape') +const AWS = require('aws-sdk') +const express = require('express') +const bodyParser = require('body-parser') +const fixtures = require('./fixtures-dynamodb') + +const mockClient = require('../../../_mock_http_client') + +const { + getRegionFromRequest, + getPortFromRequest, + getStatementFromRequest, + getAddressFromRequest +} = + require('../../../../lib/instrumentation/modules/aws-sdk/dynamodb') + +initializeAwsSdk() + +function initializeAwsSdk () { + // SDk requires a region to be set + AWS.config.update({ region: 'us-west' }) + + // without fake credentials the aws-sdk will attempt to fetch + // credentials as though it was on an EC2 instance + process.env.AWS_ACCESS_KEY_ID = 'fake-1' + process.env.AWS_SECRET_ACCESS_KEY = 'fake-2' +} + +function createMockServer (xmlResponse) { + const app = express() + app.use(bodyParser.urlencoded({ extended: false })) + app.post('/', (req, res) => { + res.setHeader('Content-Type', 'text/xml') + res.send(xmlResponse) + }) + return app +} + +function getXmlResponse (method) { + return fixtures[method].response +} + +function resetAgent (cb) { + agent._instrumentation.currentTransaction = null + agent._transport = mockClient(cb) +} + +function getParams (method, port) { + const params = fixtures[method].request + params.QueueUrl = `http://localhost:${port}/1/our-queue` + return params +} + +tape.test('AWS DynamoDB: Unit Test Functions', function (test) { + test.test('function getRegionFromRequest', function (t) { + const request = { + service: { + config: { + region: 'us-west-2' + } + } + } + t.equals(getRegionFromRequest(request), 'us-west-2') + t.equals(getRegionFromRequest({}), undefined) + t.equals(getRegionFromRequest({service:null}), null) + t.equals(getRegionFromRequest({service:{config:null}}), null) + t.equals(getRegionFromRequest({service:{config:{region:null}}}), null) + t.equals(getRegionFromRequest(), undefined) + t.equals(getRegionFromRequest(null), null) + t.end() + }) + + test.test('function getPortFromRequest', function (t) { + const request = { + service: { + endpoint: { + port: 443 + } + } + } + t.equals(getPortFromRequest(request), 443) + t.equals(getPortFromRequest({}), undefined) + t.equals(getPortFromRequest({service:null}), null) + t.equals(getPortFromRequest({service:{endpoint:null}}), null) + t.equals(getPortFromRequest({service:{endpoint:{port:null}}}), null) + t.equals(getPortFromRequest(), undefined) + t.equals(getPortFromRequest(null), null) + t.end() + }) + + test.test('function getStatementFromRequest', function (t) { + const request = { + operation: 'query', + params: { + KeyConditionExpression: 'foo = :bar' + } + } + t.equals(getStatementFromRequest(request), 'foo = :bar') + t.equals(getStatementFromRequest({}), undefined) + t.equals(getStatementFromRequest({operation:null}), undefined) + t.equals(getStatementFromRequest({operation:'query',params:{}}), undefined) + t.equals(getStatementFromRequest({operation:'query',params:{KeyConditionExpression:null}}), undefined) + t.equals(getStatementFromRequest(), undefined) + t.equals(getStatementFromRequest(null), undefined) + t.end() + }) + + test.test('function getAddressFromRequest', function (t) { + const request = { + service: { + endpoint: { + host:'dynamodb.us-west-2.amazonaws.com' + } + } + } + t.equals(getAddressFromRequest(request), 'dynamodb.us-west-2.amazonaws.com') + t.equals(getAddressFromRequest({}), undefined) + t.equals(getAddressFromRequest({service:null}), null) + t.equals(getAddressFromRequest({service:{endpoint:null}}), null) + t.equals(getAddressFromRequest({service:{endpoint:{host:null}}}), null) + t.equals(getAddressFromRequest(), undefined) + t.equals(getAddressFromRequest(null), null) + t.end() + }) +}) + +tape.test('AWS DynamoDB: End to End Test', function (test) { + test.test('API: query', function (t) { + const app = createMockServer( + getXmlResponse('query') + ) + const listener = app.listen(0, function () { + resetAgent(function (data) { + // const [spanSqs, spanHttp] = getSqsAndOtherSpanFromData(data, t) + + // t.equals(spanSqs.name, 'SQS SEND to our-queue', 'SQS span named correctly') + // t.equals(spanSqs.type, 'messaging', 'span type set to messaging') + // t.equals(spanSqs.subtype, 'sqs', 'span subtype set to sqs') + // t.equals(spanSqs.action, 'send', 'span action matches API method called') + // t.equals(spanSqs.context.destination.service.type, 'messaging', 'messaging context set') + // t.equals(spanSqs.context.message.queue.name, 'our-queue', 'queue name context set') + + // t.equals(spanHttp.type, 'external', 'other span is for HTTP request') + t.end() + }) + agent.startTransaction('myTransaction') + // const sqs = new AWS.SQS({ apiVersion: '2012-11-05' }) + // const params = getParams('query', listener.address().port) + // sqs.sendMessage(params, function (err, data) { + // t.error(err) + // agent.endTransaction() + // listener.close() + // }) + agent.endTransaction() + listener.close() + }) + }) + + // test.test('API: no transaction', function (t) { + // t.fail() + // // t.end() + // }) +}) diff --git a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js new file mode 100644 index 0000000000..46d6c2164c --- /dev/null +++ b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js @@ -0,0 +1,7 @@ +'use strict' +module.exports = { + query: { + request:'', + response:'' + } +} From ba9f4947ff0d9de1c0256ca7ff8eea7fa36d7e3c Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Sun, 27 Jun 2021 15:49:39 -0700 Subject: [PATCH 03/14] fix: remove console.log --- lib/agent.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/agent.js b/lib/agent.js index 139ca1dc6d..be403523c8 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -450,7 +450,6 @@ Agent.prototype.captureError = function (err, opts, cb) { if (agent._transport) { agent.logger.info('Sending error to Elastic APM: %o', { id }) - console.log(error) agent._transport.sendError(error, function () { agent.flush(function (err) { if (cb) cb(err, id) From c930448f535c2f232e8c5cf877ea4723ed076457 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Sun, 27 Jun 2021 19:05:52 -0700 Subject: [PATCH 04/14] test: integartion tests --- CHANGELOG.asciidoc | 3 + lib/instrumentation/modules/aws-sdk.js | 2 +- .../modules/aws-sdk/dynamodb.js | 34 +++-- .../modules/aws-sdk/dynamodb.js | 138 +++++++++++------- .../modules/aws-sdk/fixtures-dynamodb.js | 10 +- 5 files changed, 117 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 4ffbbc6aa9..bfbe6a5c61 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -39,6 +39,9 @@ Notes: * Add instrumentation of all AWS S3 methods when using the https://www.npmjs.com/package/aws-sdk[JavaScript AWS SDK v2] (`aws-sdk`). +* Add instrumentation of all DynamoDB methods when using the + https://www.npmjs.com/package/aws-sdk[JavaScript AWS SDK v2] (`aws-sdk`). + [float] ===== Bug fixes diff --git a/lib/instrumentation/modules/aws-sdk.js b/lib/instrumentation/modules/aws-sdk.js index 9b0b04ab64..ae7dd7f344 100644 --- a/lib/instrumentation/modules/aws-sdk.js +++ b/lib/instrumentation/modules/aws-sdk.js @@ -3,7 +3,7 @@ const semver = require('semver') const shimmer = require('../shimmer') const { instrumentationS3 } = require('./aws-sdk/s3') const { instrumentationSqs } = require('./aws-sdk/sqs') -const {instrumentationDynamoDb} = require('./aws-sdk/dynamodb.js') +const { instrumentationDynamoDb } = require('./aws-sdk/dynamodb.js') const instrumentorFromSvcId = { s3: instrumentationS3, diff --git a/lib/instrumentation/modules/aws-sdk/dynamodb.js b/lib/instrumentation/modules/aws-sdk/dynamodb.js index 21a0a5b1ad..f52b08cc64 100644 --- a/lib/instrumentation/modules/aws-sdk/dynamodb.js +++ b/lib/instrumentation/modules/aws-sdk/dynamodb.js @@ -4,36 +4,39 @@ const TYPE = 'db' const SUBTYPE = 'dynamodb' const ACTION = 'query' -function getRegionFromRequest(request) { +function getRegionFromRequest (request) { return request && request.service && request.service.config && request.service.config.region } -function getPortFromRequest(request) { +function getPortFromRequest (request) { return request && request.service && request.service.endpoint && request.service.endpoint.port } -function getMethodFromRequest(request) { - return request && request.operation +function getMethodFromRequest (request) { + const method = request && request.operation + if (method) { + return method[0].toUpperCase() + method.slice(1) + } } -function getStatementFromRequest(request) { +function getStatementFromRequest (request) { const method = getMethodFromRequest(request) - if('query' === method && request && request.params && request.params.KeyConditionExpression) { + if (method === 'Query' && request && request.params && request.params.KeyConditionExpression) { return request.params.KeyConditionExpression } return undefined } -function getAddressFromRequest(request) { +function getAddressFromRequest (request) { return request && request.service && request.service.endpoint && request.service.endpoint.host } -function getTableFromRequest(request) { +function getTableFromRequest (request) { const table = request && request.params && request.params.TableName - if(!table) { + if (!table) { return '' } return ` ${table}` @@ -65,17 +68,16 @@ function instrumentationDynamoDb (orig, origArguments, request, AWS, agent, { ve const action = ACTION // const action = getActionFromRequest(request) const name = getSpanNameFromRequest(request) - console.log('creating span: ' + name) const span = agent.startSpan(name, type, subtype, action) - if(!span) { + if (!span) { return orig.apply(request, origArguments) } // span.setDestinationContext(getMessageDestinationContextFromRequest(request)) // span.setMessageContext(getMessageContextFromRequest(request)) span.setDbContext({ - instance:getRegionFromRequest(request), - statement:getStatementFromRequest(request), - type: SUBTYPE, + instance: getRegionFromRequest(request), + statement: getStatementFromRequest(request), + type: SUBTYPE }) span.setDestinationContext({ address: getAddressFromRequest(request), @@ -91,7 +93,6 @@ function instrumentationDynamoDb (orig, origArguments, request, AWS, agent, { ve }) request.on('complete', function (response) { - console.log(response.httpResponse.body.toString()) if (response && response.error) { const errOpts = { skipOutcome: true @@ -116,5 +117,6 @@ module.exports = { getRegionFromRequest, getPortFromRequest, getStatementFromRequest, - getAddressFromRequest + getAddressFromRequest, + getMethodFromRequest } diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js index b67d26fc32..acdff3394d 100644 --- a/test/instrumentation/modules/aws-sdk/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -1,4 +1,4 @@ - const agent = require('../../../..').start({ +const agent = require('../../../..').start({ serviceName: 'test', secretToken: 'test', captureExceptions: false, @@ -17,7 +17,8 @@ const { getRegionFromRequest, getPortFromRequest, getStatementFromRequest, - getAddressFromRequest + getAddressFromRequest, + getMethodFromRequest } = require('../../../../lib/instrumentation/modules/aws-sdk/dynamodb') @@ -25,7 +26,7 @@ initializeAwsSdk() function initializeAwsSdk () { // SDk requires a region to be set - AWS.config.update({ region: 'us-west' }) + AWS.config.update({ region: 'us-west-2' }) // without fake credentials the aws-sdk will attempt to fetch // credentials as though it was on an EC2 instance @@ -33,17 +34,17 @@ function initializeAwsSdk () { process.env.AWS_SECRET_ACCESS_KEY = 'fake-2' } -function createMockServer (xmlResponse) { +function createMockServer (jsonResponse) { const app = express() app.use(bodyParser.urlencoded({ extended: false })) app.post('/', (req, res) => { - res.setHeader('Content-Type', 'text/xml') - res.send(xmlResponse) + res.setHeader('Content-Type', 'application/javascript') + res.send(jsonResponse) }) return app } -function getXmlResponse (method) { +function getJsonResponse (method) { return fixtures[method].response } @@ -52,12 +53,6 @@ function resetAgent (cb) { agent._transport = mockClient(cb) } -function getParams (method, port) { - const params = fixtures[method].request - params.QueueUrl = `http://localhost:${port}/1/our-queue` - return params -} - tape.test('AWS DynamoDB: Unit Test Functions', function (test) { test.test('function getRegionFromRequest', function (t) { const request = { @@ -69,9 +64,9 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { } t.equals(getRegionFromRequest(request), 'us-west-2') t.equals(getRegionFromRequest({}), undefined) - t.equals(getRegionFromRequest({service:null}), null) - t.equals(getRegionFromRequest({service:{config:null}}), null) - t.equals(getRegionFromRequest({service:{config:{region:null}}}), null) + t.equals(getRegionFromRequest({ service: null }), null) + t.equals(getRegionFromRequest({ service: { config: null } }), null) + t.equals(getRegionFromRequest({ service: { config: { region: null } } }), null) t.equals(getRegionFromRequest(), undefined) t.equals(getRegionFromRequest(null), null) t.end() @@ -87,9 +82,9 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { } t.equals(getPortFromRequest(request), 443) t.equals(getPortFromRequest({}), undefined) - t.equals(getPortFromRequest({service:null}), null) - t.equals(getPortFromRequest({service:{endpoint:null}}), null) - t.equals(getPortFromRequest({service:{endpoint:{port:null}}}), null) + t.equals(getPortFromRequest({ service: null }), null) + t.equals(getPortFromRequest({ service: { endpoint: null } }), null) + t.equals(getPortFromRequest({ service: { endpoint: { port: null } } }), null) t.equals(getPortFromRequest(), undefined) t.equals(getPortFromRequest(null), null) t.end() @@ -104,9 +99,9 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { } t.equals(getStatementFromRequest(request), 'foo = :bar') t.equals(getStatementFromRequest({}), undefined) - t.equals(getStatementFromRequest({operation:null}), undefined) - t.equals(getStatementFromRequest({operation:'query',params:{}}), undefined) - t.equals(getStatementFromRequest({operation:'query',params:{KeyConditionExpression:null}}), undefined) + t.equals(getStatementFromRequest({ operation: null }), undefined) + t.equals(getStatementFromRequest({ operation: 'query', params: {} }), undefined) + t.equals(getStatementFromRequest({ operation: 'query', params: { KeyConditionExpression: null } }), undefined) t.equals(getStatementFromRequest(), undefined) t.equals(getStatementFromRequest(null), undefined) t.end() @@ -116,17 +111,30 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { const request = { service: { endpoint: { - host:'dynamodb.us-west-2.amazonaws.com' + host: 'dynamodb.us-west-2.amazonaws.com' } } } t.equals(getAddressFromRequest(request), 'dynamodb.us-west-2.amazonaws.com') t.equals(getAddressFromRequest({}), undefined) - t.equals(getAddressFromRequest({service:null}), null) - t.equals(getAddressFromRequest({service:{endpoint:null}}), null) - t.equals(getAddressFromRequest({service:{endpoint:{host:null}}}), null) + t.equals(getAddressFromRequest({ service: null }), null) + t.equals(getAddressFromRequest({ service: { endpoint: null } }), null) + t.equals(getAddressFromRequest({ service: { endpoint: { host: null } } }), null) + t.equals(getAddressFromRequest(), undefined) + t.equals(getAddressFromRequest(null), null) + t.end() + }) + + test.test('function getMethodFromRequest', function (t) { + const request = { + operation: 'query' + } + t.equals(getMethodFromRequest(request), 'Query') + t.equals(getMethodFromRequest({}), undefined) + t.equals(getMethodFromRequest({ operation: null }), undefined) t.equals(getAddressFromRequest(), undefined) t.equals(getAddressFromRequest(null), null) + t.end() }) }) @@ -134,37 +142,67 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { tape.test('AWS DynamoDB: End to End Test', function (test) { test.test('API: query', function (t) { const app = createMockServer( - getXmlResponse('query') + getJsonResponse('query') ) const listener = app.listen(0, function () { resetAgent(function (data) { - // const [spanSqs, spanHttp] = getSqsAndOtherSpanFromData(data, t) - - // t.equals(spanSqs.name, 'SQS SEND to our-queue', 'SQS span named correctly') - // t.equals(spanSqs.type, 'messaging', 'span type set to messaging') - // t.equals(spanSqs.subtype, 'sqs', 'span subtype set to sqs') - // t.equals(spanSqs.action, 'send', 'span action matches API method called') - // t.equals(spanSqs.context.destination.service.type, 'messaging', 'messaging context set') - // t.equals(spanSqs.context.message.queue.name, 'our-queue', 'queue name context set') - - // t.equals(spanHttp.type, 'external', 'other span is for HTTP request') + const span = data.spans.filter((span) => span.type === 'db').pop() + t.equals(span.name, 'DynamoDB Query fixture-table', 'span named correctly') + t.equals(span.type, 'db', 'span type correctly set') + t.equals(span.subtype, 'dynamodb', 'span subtype set correctly') + t.equals(span.action, 'query', 'query set correctly') + t.equals(span.context.db.statement, 'id = :foo', 'statment set in context correctly') + t.equals(span.context.destination.service.name, 'dynamodb', 'service name in destination context') t.end() }) + const port = listener.address().port + AWS.config.update({ + endpoint: `http://localhost:${port}` + }) agent.startTransaction('myTransaction') - // const sqs = new AWS.SQS({ apiVersion: '2012-11-05' }) - // const params = getParams('query', listener.address().port) - // sqs.sendMessage(params, function (err, data) { - // t.error(err) - // agent.endTransaction() - // listener.close() - // }) - agent.endTransaction() - listener.close() + var ddb = new AWS.DynamoDB({ apiVersion: '2012-08-10' }) + var params = { + TableName: 'fixture-table', + KeyConditionExpression: 'id = :foo', + ExpressionAttributeValues: { + ':foo': { S: '001' } + } + } + ddb.query(params, function (err, data) { + t.error(err) + agent.endTransaction() + listener.close() + }) }) }) - // test.test('API: no transaction', function (t) { - // t.fail() - // // t.end() - // }) + tape.test('AWS DynamoDB: No Transaction', function (test) { + test.test('API: query', function (t) { + const app = createMockServer( + getJsonResponse('query') + ) + const listener = app.listen(0, function () { + resetAgent(function (data) { + t.equals(data.spans.length, 0, 'no spans without transaction') + t.end() + }) + const port = listener.address().port + AWS.config.update({ + endpoint: `http://localhost:${port}` + }) + var ddb = new AWS.DynamoDB({ apiVersion: '2012-08-10' }) + var params = { + TableName: 'fixture-table', + KeyConditionExpression: 'id = :foo', + ExpressionAttributeValues: { + ':foo': { S: '001' } + } + } + ddb.query(params, function (err, data) { + t.error(err) + listener.close() + }) + }) + }) + }) }) diff --git a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js index 46d6c2164c..beaacf27bd 100644 --- a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js @@ -1,7 +1,11 @@ 'use strict' module.exports = { - query: { - request:'', - response:'' + query: { + response: { + Count: 1, + Items: + [{ id: { S: '001' }, name: { S: 'Richard Roe' }, number: { N: '1' } }], + ScannedCount: 1 } + } } From 8e999bdc0c14a3293a2112ff7c105b6e7bc7bc86 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 14:15:42 -0700 Subject: [PATCH 05/14] fix: error state test --- .../modules/aws-sdk/dynamodb.js | 46 +++++++++++++++---- .../modules/aws-sdk/fixtures-dynamodb.js | 7 ++- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js index acdff3394d..a67828f818 100644 --- a/test/instrumentation/modules/aws-sdk/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -34,20 +34,17 @@ function initializeAwsSdk () { process.env.AWS_SECRET_ACCESS_KEY = 'fake-2' } -function createMockServer (jsonResponse) { +function createMockServer (fixture) { const app = express() app.use(bodyParser.urlencoded({ extended: false })) app.post('/', (req, res) => { + res.status(fixture.httpStatusCode) res.setHeader('Content-Type', 'application/javascript') - res.send(jsonResponse) + res.send(fixture.response) }) return app } -function getJsonResponse (method) { - return fixtures[method].response -} - function resetAgent (cb) { agent._instrumentation.currentTransaction = null agent._transport = mockClient(cb) @@ -142,7 +139,7 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { tape.test('AWS DynamoDB: End to End Test', function (test) { test.test('API: query', function (t) { const app = createMockServer( - getJsonResponse('query') + fixtures.query ) const listener = app.listen(0, function () { resetAgent(function (data) { @@ -176,10 +173,43 @@ tape.test('AWS DynamoDB: End to End Test', function (test) { }) }) + test.test('API: error', function (t) { + const app = createMockServer( + fixtures.error + ) + const listener = app.listen(0, function () { + resetAgent(function (data) { + t.equals(data.errors.length, 1, 'expect captured error') + const span = data.spans.filter((span) => span.type === 'db').pop() + t.ok(span, 'expect a db span') + t.equals(span.outcome, 'failure', 'expect db span to have failure outcome') + t.end() + }) + const port = listener.address().port + AWS.config.update({ + endpoint: `http://localhost:${port}` + }) + agent.startTransaction('myTransaction') + var ddb = new AWS.DynamoDB({ apiVersion: '2012-08-10' }) + var params = { + TableName: 'fixture-table', + KeyConditionExpression: 'id = :foo', + ExpressionAttributeValues: { + ':foo': { S: '001' } + } + } + ddb.query(params, function (err, data) { + t.ok(err, 'expect error') + agent.endTransaction() + listener.close() + }) + }) + }) + tape.test('AWS DynamoDB: No Transaction', function (test) { test.test('API: query', function (t) { const app = createMockServer( - getJsonResponse('query') + fixtures.query ) const listener = app.listen(0, function () { resetAgent(function (data) { diff --git a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js index beaacf27bd..f6202e2aa1 100644 --- a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js @@ -6,6 +6,11 @@ module.exports = { Items: [{ id: { S: '001' }, name: { S: 'Richard Roe' }, number: { N: '1' } }], ScannedCount: 1 - } + }, + httpStatusCode: 200 + }, + error: { + response: { __type: 'com.amazonaws.dynamodb.v20120810#ResourceNotFoundException', message: 'Requested resource not found' }, + httpStatusCode: 400 } } From ac1aeea0ed81f9d963c950c9fc7e8aeb38dd358d Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 14:27:13 -0700 Subject: [PATCH 06/14] test: test second API method --- .../modules/aws-sdk/dynamodb.js | 29 +++++++++++++++++++ .../modules/aws-sdk/fixtures-dynamodb.js | 4 +++ 2 files changed, 33 insertions(+) diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js index a67828f818..7a9ebe6201 100644 --- a/test/instrumentation/modules/aws-sdk/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -173,6 +173,35 @@ tape.test('AWS DynamoDB: End to End Test', function (test) { }) }) + test.test('API: listTable', function (t) { + const app = createMockServer( + fixtures.listTable + ) + const listener = app.listen(0, function () { + resetAgent(function (data) { + const span = data.spans.filter((span) => span.type === 'db').pop() + t.equals(span.name, 'DynamoDB ListTables', 'span named correctly') + t.equals(span.type, 'db', 'span type correctly set') + t.equals(span.subtype, 'dynamodb', 'span subtype set correctly') + t.equals(span.action, 'query', 'query set correctly') + t.equals(span.context.destination.service.name, 'dynamodb', 'service name in destination context') + + t.end() + }) + const port = listener.address().port + AWS.config.update({ + endpoint: `http://localhost:${port}` + }) + agent.startTransaction('myTransaction') + var ddb = new AWS.DynamoDB({ apiVersion: '2012-08-10' }) + ddb.listTables(function (err, data) { + t.error(err) + agent.endTransaction() + listener.close() + }) + }) + }) + test.test('API: error', function (t) { const app = createMockServer( fixtures.error diff --git a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js index f6202e2aa1..a9b7ba06e3 100644 --- a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js @@ -9,6 +9,10 @@ module.exports = { }, httpStatusCode: 200 }, + listTable: { + response: { Success: { TableNames: ['fixture-table'] } }, + httpStatusCode: 200 + }, error: { response: { __type: 'com.amazonaws.dynamodb.v20120810#ResourceNotFoundException', message: 'Requested resource not found' }, httpStatusCode: 400 From a0d4f52656b070cdf67f2242430c748ebf494007 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 14:34:43 -0700 Subject: [PATCH 07/14] docs: add dynamodb to list --- docs/supported-technologies.asciidoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/supported-technologies.asciidoc b/docs/supported-technologies.asciidoc index 4f2d649241..dc8616074e 100644 --- a/docs/supported-technologies.asciidoc +++ b/docs/supported-technologies.asciidoc @@ -72,7 +72,7 @@ The Node.js agent will automatically instrument the following modules to give yo [options="header"] |======================================================================= |Module |Version |Note -|https://www.npmjs.com/package/aws-sdk[aws-sdk] |>1 <3 |Will instrument SQS send/receive/delete messages, all S3 methods +|https://www.npmjs.com/package/aws-sdk[aws-sdk] |>1 <3 |Will instrument SQS send/receive/delete messages, all S3 methods, and all DynamoDB methods |https://www.npmjs.com/package/cassandra-driver[cassandra-driver] |>=3.0.0 |Will instrument all queries |https://www.npmjs.com/package/elasticsearch[elasticsearch] |>=8.0.0 |Will instrument all queries |https://www.npmjs.com/package/@elastic/elasticsearch[@elastic/elasticsearch] |>=7.0.0 <8.0.0 |Will instrument all queries From 19c9f2241f660ad98cbe1e691a4d0407e2f4f57a Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 15:36:06 -0700 Subject: [PATCH 08/14] test: update tav --- .tav.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.tav.yml b/.tav.yml index 21690fac83..24a26c0550 100644 --- a/.tav.yml +++ b/.tav.yml @@ -413,13 +413,14 @@ body-parser: - node test/sanitize-field-names/express.js aws-sdk: - # We want this version range: + # We want a version range something like this: # versions: '>=2.858 <3' # However, awk-sdk releases *very* frequently (almost every day) and there # is no need to test *all* those releases. Instead we statically list every # N=5 releases to test. # # Maintenance note: This should be updated periodically. - versions: '2.858.0 || 2.863.0 || 2.868.0 || 2.873.0 || 2.878.0 || 2.883.0 || 2.888.0 || 2.893.0 || 2.898.0 || 2.903.0 || 2.908.0 || 2.913.0 || 2.918.0 || >2.918 <3' + versions: '2.876.0 || 2.881.0 || 2.886.0 || 2.891.0 || 2.896.0 || 2.901.0 || 2.906.0 || 2.911.0 || 2.916.0 || 2.921.0 || 2.926.0 || 2.931.0 || 2.936.0 || >2.918 <3' commands: - node test/instrumentation/modules/aws-sdk/sqs.js + - node test/instrumentation/modules/aws-sdk/dynamodb.js From 883970c85f107e75d51d1fe9e434f09060b36e75 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 22:28:33 -0700 Subject: [PATCH 09/14] test: different default configuration --- test/instrumentation/modules/aws-sdk/dynamodb.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js index 7a9ebe6201..95958a01b0 100644 --- a/test/instrumentation/modules/aws-sdk/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -3,7 +3,8 @@ const agent = require('../../../..').start({ secretToken: 'test', captureExceptions: false, metricsInterval: 0, - centralConfig: false + centralConfig: 'none', + logLevel: off }) const tape = require('tape') const AWS = require('aws-sdk') From 93f36ebab318a040828c555e752b9bc84df597e8 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 22:31:59 -0700 Subject: [PATCH 10/14] fix: move fixtures --- test/instrumentation/modules/aws-sdk/dynamodb.js | 4 ++-- .../aws-sdk/{fixtures-dynamodb.js => fixtures/dynamodb.js} | 0 .../modules/aws-sdk/{fixtures-sqs.js => fixtures/sqs.js} | 0 test/instrumentation/modules/aws-sdk/sqs.js | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename test/instrumentation/modules/aws-sdk/{fixtures-dynamodb.js => fixtures/dynamodb.js} (100%) rename test/instrumentation/modules/aws-sdk/{fixtures-sqs.js => fixtures/sqs.js} (100%) diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js index 95958a01b0..bbd2fbc622 100644 --- a/test/instrumentation/modules/aws-sdk/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -4,13 +4,13 @@ const agent = require('../../../..').start({ captureExceptions: false, metricsInterval: 0, centralConfig: 'none', - logLevel: off + logLevel: 'off' }) const tape = require('tape') const AWS = require('aws-sdk') const express = require('express') const bodyParser = require('body-parser') -const fixtures = require('./fixtures-dynamodb') +const fixtures = require('./fixtures/dynamodb') const mockClient = require('../../../_mock_http_client') diff --git a/test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js b/test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js similarity index 100% rename from test/instrumentation/modules/aws-sdk/fixtures-dynamodb.js rename to test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js diff --git a/test/instrumentation/modules/aws-sdk/fixtures-sqs.js b/test/instrumentation/modules/aws-sdk/fixtures/sqs.js similarity index 100% rename from test/instrumentation/modules/aws-sdk/fixtures-sqs.js rename to test/instrumentation/modules/aws-sdk/fixtures/sqs.js diff --git a/test/instrumentation/modules/aws-sdk/sqs.js b/test/instrumentation/modules/aws-sdk/sqs.js index e9b7575d98..01c560b166 100644 --- a/test/instrumentation/modules/aws-sdk/sqs.js +++ b/test/instrumentation/modules/aws-sdk/sqs.js @@ -12,7 +12,7 @@ const tape = require('tape') const AWS = require('aws-sdk') const express = require('express') const bodyParser = require('body-parser') -const fixtures = require('./fixtures-sqs') +const fixtures = require('./fixtures/sqs') const logging = require('../../../../lib/logging') const mockClient = require('../../../_mock_http_client') From de5afd8b211ba07cd1fa708c5ba664d0f2ab2043 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 22:34:29 -0700 Subject: [PATCH 11/14] fix: whitespace --- test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js b/test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js index a9b7ba06e3..0d47fc283e 100644 --- a/test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/fixtures/dynamodb.js @@ -3,8 +3,7 @@ module.exports = { query: { response: { Count: 1, - Items: - [{ id: { S: '001' }, name: { S: 'Richard Roe' }, number: { N: '1' } }], + Items: [{ id: { S: '001' }, name: { S: 'Richard Roe' }, number: { N: '1' } }], ScannedCount: 1 }, httpStatusCode: 200 From 7ca40d1d1d239f917ececd6f357fd7b147aa06a3 Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Tue, 29 Jun 2021 22:38:24 -0700 Subject: [PATCH 12/14] fix: pin 2.858 as lowest version to test --- .tav.yml | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/.tav.yml b/.tav.yml index 24a26c0550..9ff11462ab 100644 --- a/.tav.yml +++ b/.tav.yml @@ -413,14 +413,15 @@ body-parser: - node test/sanitize-field-names/express.js aws-sdk: - # We want a version range something like this: + # We want this version range: # versions: '>=2.858 <3' # However, awk-sdk releases *very* frequently (almost every day) and there - # is no need to test *all* those releases. Instead we statically list every - # N=5 releases to test. + # is no need to test *all* those releases. Instead we keep statically list + # every N=5 releases to test. # - # Maintenance note: This should be updated periodically. - versions: '2.876.0 || 2.881.0 || 2.886.0 || 2.891.0 || 2.896.0 || 2.901.0 || 2.906.0 || 2.911.0 || 2.916.0 || 2.921.0 || 2.926.0 || 2.931.0 || 2.936.0 || >2.918 <3' + # Maintenance note: This should be updated periodically, keeping 2.858 + # as the earliest version but updating the others. + versions: '2.858.0 || 2.881.0 || 2.886.0 || 2.891.0 || 2.896.0 || 2.901.0 || 2.906.0 || 2.911.0 || 2.916.0 || 2.921.0 || 2.926.0 || 2.931.0 || 2.936.0 || >2.936 <3' commands: - node test/instrumentation/modules/aws-sdk/sqs.js - node test/instrumentation/modules/aws-sdk/dynamodb.js From 7c66640a8b77dd1a0376a5dc169b0a87f45dab1d Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Wed, 30 Jun 2021 15:51:05 -0700 Subject: [PATCH 13/14] fix: PR feedback --- .tav.yml | 4 ++-- lib/instrumentation/modules/aws-sdk/dynamodb.js | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/.tav.yml b/.tav.yml index 9ff11462ab..8cc85e563f 100644 --- a/.tav.yml +++ b/.tav.yml @@ -416,8 +416,8 @@ aws-sdk: # We want this version range: # versions: '>=2.858 <3' # However, awk-sdk releases *very* frequently (almost every day) and there - # is no need to test *all* those releases. Instead we keep statically list - # every N=5 releases to test. + # is no need to test *all* those releases. Instead we statically list every + # N=5 releases to test. # # Maintenance note: This should be updated periodically, keeping 2.858 # as the earliest version but updating the others. diff --git a/lib/instrumentation/modules/aws-sdk/dynamodb.js b/lib/instrumentation/modules/aws-sdk/dynamodb.js index f52b08cc64..e4b7180cc3 100644 --- a/lib/instrumentation/modules/aws-sdk/dynamodb.js +++ b/lib/instrumentation/modules/aws-sdk/dynamodb.js @@ -31,7 +31,7 @@ function getStatementFromRequest (request) { function getAddressFromRequest (request) { return request && request.service && request.service.endpoint && - request.service.endpoint.host + request.service.endpoint.hostname } function getTableFromRequest (request) { @@ -66,14 +66,13 @@ function instrumentationDynamoDb (orig, origArguments, request, AWS, agent, { ve const type = TYPE const subtype = SUBTYPE const action = ACTION - // const action = getActionFromRequest(request) + const name = getSpanNameFromRequest(request) const span = agent.startSpan(name, type, subtype, action) if (!span) { return orig.apply(request, origArguments) } - // span.setDestinationContext(getMessageDestinationContextFromRequest(request)) - // span.setMessageContext(getMessageContextFromRequest(request)) + span.setDbContext({ instance: getRegionFromRequest(request), statement: getStatementFromRequest(request), @@ -101,8 +100,12 @@ function instrumentationDynamoDb (orig, origArguments, request, AWS, agent, { ve span._setOutcomeFromErrorCapture(constants.OUTCOME_FAILURE) } - // we'll need to manually mark this span as async. The actual async hop - // is captured by the agent's async hooks instrumentation + // Workaround a bug in the agent's handling of `span.sync`. + // + // The bug: Currently this span.sync is not set `false` because there is + // an HTTP span created (for this S3 request) in the same async op. That + // HTTP span becomes the "active span" for this async op, and *it* gets + // marked as sync=false in `before()` in async-hooks.js. span.sync = false span.end() }) From d7afe99137d142edac0b635c465e32392c48fadb Mon Sep 17 00:00:00 2001 From: Alan Storm Date: Wed, 30 Jun 2021 16:04:15 -0700 Subject: [PATCH 14/14] fix: fixtures --- test/instrumentation/modules/aws-sdk/dynamodb.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/instrumentation/modules/aws-sdk/dynamodb.js b/test/instrumentation/modules/aws-sdk/dynamodb.js index bbd2fbc622..44ed96862e 100644 --- a/test/instrumentation/modules/aws-sdk/dynamodb.js +++ b/test/instrumentation/modules/aws-sdk/dynamodb.js @@ -109,7 +109,7 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { const request = { service: { endpoint: { - host: 'dynamodb.us-west-2.amazonaws.com' + hostname: 'dynamodb.us-west-2.amazonaws.com' } } } @@ -117,7 +117,7 @@ tape.test('AWS DynamoDB: Unit Test Functions', function (test) { t.equals(getAddressFromRequest({}), undefined) t.equals(getAddressFromRequest({ service: null }), null) t.equals(getAddressFromRequest({ service: { endpoint: null } }), null) - t.equals(getAddressFromRequest({ service: { endpoint: { host: null } } }), null) + t.equals(getAddressFromRequest({ service: { endpoint: { hostname: null } } }), null) t.equals(getAddressFromRequest(), undefined) t.equals(getAddressFromRequest(null), null) t.end()