Merge pull request #150 from bizob2828/fix-dynamo-port

default dynamodb port to 443 when undefined on endpoint instance

Author: bizob2828

lib/util.js

@@ -4,6 +4,8 @@
  */
 
 'use strict'
+const UNKNOWN = 'Unknown'
+
 function grabLastUrlSegment(url = '/') {
   // cast URL as string, and an empty
   // string for null, undefined, NaN etc.
@@ -14,6 +16,22 @@ function grabLastUrlSegment(url = '/') {
   return lastItem
 }
 
+/**
+ * Retrieves the db segment params from endpoint and command parameters
+ *
+ * @param {Object} endpoint instance of ddb endpoint
+ * @param {Object} params parameters passed to a ddb command
+ * @returns {Object}
+ */
+function setDynamoParameters(endpoint, params) {
+  return {
+    host: endpoint && (endpoint.host || endpoint.hostname),
+    port_path_or_id: (endpoint && endpoint.port) || 443,
+    collection: (params && params.TableName) || UNKNOWN
+  }
+}
+
 module.exports = {
-  grabLastUrlSegment
+  grabLastUrlSegment,
+  setDynamoParameters
 }

lib/v2/dynamodb.js

@@ -29,6 +29,8 @@ const DOC_CLIENT_OPERATIONS = [
   'scan'
 ]
 
+const { setDynamoParameters } = require('../util')
+
 function instrument(shim, AWS) {
   shim.setDatastore(shim.DYNAMODB)
 
@@ -43,11 +45,7 @@ function instrument(shim, AWS) {
 
         return {
           name: operationName,
-          parameters: {
-            host: this.endpoint && this.endpoint.host,
-            port_path_or_id: this.endpoint && this.endpoint.port,
-            collection: (params && params.TableName) || 'Unknown'
-          },
+          parameters: setDynamoParameters(this.endpoint, params),
           callback: shim.LAST,
           opaque: true
         }

lib/v3/dynamodb-util.js

@@ -4,8 +4,7 @@
  */
 
 'use strict'
-
-const UNKNOWN = 'Unknown'
+const { setDynamoParameters } = require('../util')
 
 /**
  * Returns the spec for Dynamo commands
@@ -18,12 +17,9 @@ const UNKNOWN = 'Unknown'
  */
 function getDynamoSpec(shim, original, name, args) {
   const [{ input }] = args
-  const collection = (input && input.TableName) || UNKNOWN
-  const host = this.endpoint && this.endpoint.hostname
-  const portPathOrId = this.endpoint && this.endpoint.port
   return {
     name: this.commandName,
-    parameters: { host, port_path_or_id: portPathOrId, collection },
+    parameters: setDynamoParameters(this.endpoint, input),
     callback: shim.LAST,
     opaque: true,
     promise: true

tests/unit/util.tap.js

@@ -5,7 +5,7 @@
 
 'use strict'
 const tap = require('tap')
-const { grabLastUrlSegment } = require('../../lib/util')
+const { grabLastUrlSegment, setDynamoParameters } = require('../../lib/util')
 tap.test('Utility Functions', (t) => {
   t.ok(grabLastUrlSegment, 'imported function successfully')
 
@@ -30,7 +30,61 @@ tap.test('Utility Functions', (t) => {
 
   for (const [, fixture] of fixtures.entries()) {
     const result = grabLastUrlSegment(fixture.input)
-    t.equals(result, fixture.output, `expecting ${result} to equal ${fixture.output}`)
+    t.equal(result, fixture.output, `expecting ${result} to equal ${fixture.output}`)
   }
   t.end()
 })
+
+tap.test('DB parameters', (t) => {
+  t.autoend()
+
+  t.test('default values', (t) => {
+    const input = {}
+    const endpoint = {}
+    const result = setDynamoParameters(endpoint, input)
+    t.same(
+      result,
+      {
+        host: undefined,
+        port_path_or_id: 443,
+        collection: 'Unknown'
+      },
+      'should set default values for parameters'
+    )
+    t.end()
+  })
+
+  // v2 uses host key
+  t.test('host, port, collection', (t) => {
+    const input = { TableName: 'unit-test' }
+    const endpoint = { host: 'unit-test-host', port: '123' }
+    const result = setDynamoParameters(endpoint, input)
+    t.same(
+      result,
+      {
+        host: endpoint.host,
+        port_path_or_id: endpoint.port,
+        collection: input.TableName
+      },
+      'should set appropriate parameters'
+    )
+    t.end()
+  })
+
+  // v3 uses hostname key
+  t.test('hostname, port, collection', (t) => {
+    const input = { TableName: 'unit-test' }
+    const endpoint = { hostname: 'unit-test-host', port: '123' }
+    const result = setDynamoParameters(endpoint, input)
+    t.same(
+      result,
+      {
+        host: endpoint.hostname,
+        port_path_or_id: endpoint.port,
+        collection: input.TableName
+      },
+      'should set appropriate parameters'
+    )
+    t.end()
+  })
+})

tests/versioned/v2/dynamodb.tap.js

@@ -128,11 +128,12 @@ function finish(t, tests, tx) {
       'should have operation in segment name'
     )
     const attrs = segment.attributes.get(common.SEGMENT_DESTINATION)
+    attrs.port_path_or_id = parseInt(attrs.port_path_or_id, 10)
     t.matches(
       attrs,
       {
         'host': String,
-        'port_path_or_id': String,
+        'port_path_or_id': Number,
         'product': 'DynamoDB',
         'collection': String,
         'aws.operation': operation,

tests/versioned/v3/client-dynamodb.tap.js

@@ -202,11 +202,13 @@ tap.test('DynamoDB', (t) => {
         'should have operation in segment name'
       )
       const attrs = segment.attributes.get(common.SEGMENT_DESTINATION)
+      attrs.port_path_or_id = parseInt(attrs.port_path_or_id, 10)
+
       t.match(
         attrs,
         {
           'host': String,
-          'port_path_or_id': String,
+          'port_path_or_id': Number,
           'product': 'DynamoDB',
           'collection': String,
           'aws.operation': command.constructor.name,

tests/versioned/v3/lib-dynamodb.tap.js

@@ -165,11 +165,12 @@ function finish(t, tests, tx) {
       'should have operation in segment name'
     )
     const attrs = segment.attributes.get(common.SEGMENT_DESTINATION)
+    attrs.port_path_or_id = parseInt(attrs.port_path_or_id, 10)
     t.match(
       attrs,
       {
         'host': String,
-        'port_path_or_id': String,
+        'port_path_or_id': Number,
         'product': 'DynamoDB',
         'collection': String,
         'aws.operation': operation,