Repository: honeycombio/beeline-nodejs (js)

Issue:
http.request patch is incorrect since Node.js v10.9.0
Node.js v10.9.0 added a function signature to [http.request](https://nodejs.org/docs/latest-v12.x/api/http.html#http_http_request_options_callback) which accepts three arguments: `http.request(url[, options][, callback])`

This broke the patch in [http](https://github.com/honeycombio/beeline-nodejs/blob/master/lib/instrumentation/http.js#L12) and [https](https://github.com/honeycombio/beeline-nodejs/blob/master/lib/instrumentation/https.js#L13).

It also has the unfortunate side-effect of breaking other modules which make use of the new signature, in our case, [got](https://github.com/sindresorhus/got/issues/951).

The issue should be trivially reproducible: make a request with the latest version of got in an app where beeline-nodejs tracing is enabled.

Pull request description:
[http] make .get/.request work for both 2 (all node versions) and 3 (node 10.9.0+) arg versions
Node 10.9.0 added a 3 arg form for .get/.request in `http` and `https`, so we now have 2 forms to support:

```
http.request(options[, callback])
http.request(url[, options][, callback])
```

Instead of including a semver check in the instrumentation, handle the 3-arg form everywhere (this may or may not be the right approach, but it's definitely easier to write.)

Fixes #204 

Patch:
diff --git a/lib/api/index.js b/lib/api/index.js
index f1264ce..c38cbe3 100644
--- a/lib/api/index.js
+++ b/lib/api/index.js
@@ -9,6 +9,8 @@ const path = require("path"),
   LibhoneyImpl = require("./libhoney"),
   MockImpl = require("./mock");
 
+const { honeycomb, aws } = propagation;
+
 let apiImpl;
 
 function mapFieldsToApp(map) {
@@ -49,12 +51,24 @@ module.exports = {
     return tracker.getTracked();
   },
 
-  TRACE_HTTP_HEADER: propagation.TRACE_HTTP_HEADER,
-  AMAZON_TRACE_HTTP_HEADER: propagation.AMAZON_TRACE_HTTP_HEADER,
   REQUEST_ID_HTTP_HEADER: propagation.REQUEST_ID_HTTP_HEADER,
-  marshalTraceContext: propagation.marshalTraceContext,
+  marshalTraceContext: honeycomb.marshalTraceContext,
+
+  // deprecated. Use provider-specific functions and constants
+  TRACE_HTTP_HEADER: honeycomb.TRACE_HTTP_HEADER,
+  AMAZON_TRACE_HTTP_HEADER: aws.TRACE_HTTP_HEADER,
   unmarshalTraceContext: propagation.unmarshalTraceContext,
 
+  honeycomb: {
+    TRACE_HTTP_HEADER: honeycomb.TRACE_HTTP_HEADER,
+    unmarshalTraceContext: honeycomb.unmarshalTraceContext,
+  },
+
+  aws: {
+    TRACE_HTTP_HEADER: aws.TRACE_HTTP_HEADER,
+    unmarshalTraceContext: aws.unmarshalTraceContext,
+  },
+
   startTrace(metadataContext, withTraceId, withParentSpanId, withDataset) {
     return apiImpl.startTrace(metadataContext, withTraceId, withParentSpanId, withDataset);
   },
diff --git a/lib/api/libhoney.js b/lib/api/libhoney.js
index fff9a22..4c9defe 100644
--- a/lib/api/libhoney.js
+++ b/lib/api/libhoney.js
@@ -3,8 +3,8 @@ const libhoney = require("libhoney"),
   os = require("os"),
   process = require("process"),
   path = require("path"),
-  uuidv4 = require("uuid/v4"),
   url = require("url"),
+  crypto = require("crypto"),
   tracker = require("../async_tracker"),
   instrumentation = require("../instrumentation"),
   deterministicSampler = require("../deterministic_sampler"),
@@ -19,6 +19,18 @@ const incr = (payload, key, val = 1) => {
   payload[key] = (payload[key] || 0) + val;
 };
 
+// id generation
+const SPAN_ID_BYTES = 8;
+const TRACE_ID_BYTES = 16;
+
+function generateTraceId() {
+  return crypto.randomBytes(TRACE_ID_BYTES).toString("hex");
+}
+
+function generateSpanId() {
+  return crypto.randomBytes(SPAN_ID_BYTES).toString("hex");
+}
+
 const getFilteredOptions = opts => {
   let filteredOptions = {};
   Object.keys(opts).forEach(optionKey => {
@@ -91,7 +103,7 @@ module.exports = class LibhoneyEventAPI {
   }
 
   startTrace(metadataContext, traceId, parentSpanId, dataset) {
-    const id = traceId || uuidv4();
+    const id = traceId || generateTraceId();
 
     tracker.setTracked({
       id,
@@ -107,7 +119,7 @@ module.exports = class LibhoneyEventAPI {
     tracker.deleteTracked();
   }
 
-  startSpan(metadataContext, spanId = uuidv4(), parentId = undefined) {
+  startSpan(metadataContext, spanId = generateSpanId(), parentId = undefined) {
     let context = tracker.getTracked();
     if (!context) {
       // valid, since we can end up in our instrumentation outside of requests we're tracking
@@ -139,7 +151,7 @@ module.exports = class LibhoneyEventAPI {
 
   startAsyncSpan(metadataContext, spanFn) {
     let parentId;
-    let spanId = uuidv4();
+    let spanId = generateSpanId();
     let parentContext = tracker.getTracked();
     if (!parentContext) {
       // valid, since we can end up in our instrumentation outside of requests we're tracking
diff --git a/lib/api/mock.js b/lib/api/mock.js
index 6f2fb9a..0c5c4e2 100644
--- a/lib/api/mock.js
+++ b/lib/api/mock.js
@@ -83,6 +83,9 @@ module.exports = class MockEventAPI {
 
   addContext(map) {
     let context = tracker.getTracked();
+    if (!context || context.stack.length === 0) {
+      return;
+    }
     Object.assign(context.stack[0], map);
   }
 
diff --git a/lib/instrumentation/http.js b/lib/instrumentation/http.js
index d8224fb..ed0180b 100644
--- a/lib/instrumentation/http.js
+++ b/lib/instrumentation/http.js
@@ -8,36 +8,50 @@ let instrumentHTTP = (http, opts = {}) => {
   shimmer.wrap(http, "get", function(_original) {
     // we have to replace http.get since it references request through
     // a closure (so we can't replace the value it uses..)
-    return function(options, cb) {
-      var req = http.request(options, cb);
+    return function(_url, _options, _cb) {
+      let req = http.request.apply(this, Array.from(arguments));
       req.end();
       return req;
     };
   });
 
   shimmer.wrap(http, "request", function(original) {
-    return function(options, cb) {
+    // node 10+ supports a three arg form.  Instead of writing both implementations,
+    // dynamically switch on arg types.  I _hate_ code that does this, but it's easier
+    // and cleaner than the alternative.
+    return function(_url, options, cb) {
       if (!api.traceActive()) {
-        return original.apply(this, [options, cb]);
+        return original.apply(this, Array.from(arguments));
       }
 
-      // let's make sure we have an object (or a url) for options.
-      if (typeof options === "string") {
-        options = url.parse(options); // node 10 doesn't use url.parse (except as a fallback).  figure out how to handle that?
-      } else {
-        // shallow-clone options since we'll be modifying it below (adding our header)
-        options = Object.assign({}, options);
-      }
+      let combinedOptions = {};
+      let callback;
 
-      // another copy so we can normalize things
-      let formatOptions = Object.assign({}, options);
+      if (typeof _url === "string") {
+        combinedOptions = Object.assign({}, url.parse(_url)); // node 10 doesn't use url.parse (except as a fallback).  figure out how to handle that?
+        delete combinedOptions.href;
+        if (typeof options === "object") {
+          Object.assign(combinedOptions, options);
+          // we need to fix up `host` after this (if it wasn't specified in options), since the port might have changed.
+          if (!options.host) {
+            combinedOptions.host = `${combinedOptions.hostname}:${combinedOptions.port}`;
+          }
+          callback = cb;
+        } else {
+          callback = options;
+        }
+      } else if (typeof _url === "object") {
+        // the two-arg form.
+        combinedOptions = Object.assign({}, _url);
+        callback = options;
+      }
 
       // make sure options is populated enough for url.format to give us reasonable results.  these are all defaults from nodejs docs.
-      formatOptions.protocol = formatOptions.protocol || "http:";
-      formatOptions.port = formatOptions.port !== 80 ? formatOptions.port : null;
-      formatOptions.pathname = formatOptions.path || "/";
-      if (!formatOptions.hostname && !formatOptions.host) {
-        formatOptions.hostname = "localhost";
+      combinedOptions.protocol = combinedOptions.protocol || "http:";
+      combinedOptions.port = combinedOptions.port !== 80 ? combinedOptions.port : null;
+      combinedOptions.pathname = combinedOptions.path || "/";
+      if (!combinedOptions.hostname && !combinedOptions.host) {
+        combinedOptions.hostname = "localhost";
       }
 
       return api.startAsyncSpan(
@@ -45,26 +59,26 @@ let instrumentHTTP = (http, opts = {}) => {
           [schema.EVENT_TYPE]: "http",
           [schema.PACKAGE_VERSION]: opts.packageVersion,
           [schema.TRACE_SPAN_NAME]: options.method || "GET",
-          url: url.format(formatOptions),
+          url: url.format(combinedOptions),
         },
         span => {
           let wrapped_cb = function(res) {
             api.finishSpan(span, "request");
-            if (cb) {
-              return cb.apply(this, [res]);
+            if (callback) {
+              return callback.apply(this, [res]);
             }
           };
 
-          // shallow clone options.headers, adding our tracing headers in
-          options.headers = Object.assign({}, options.headers, {
+          // shallow clone combinedOptions.headers, adding our tracing headers in
+          combinedOptions.headers = Object.assign({}, combinedOptions.headers, {
             [api.TRACE_HTTP_HEADER]: api.marshalTraceContext(api.getTraceContext()),
           });
 
-          if (cb) {
-            return original.apply(this, [options, wrapped_cb]);
+          if (callback) {
+            return original.apply(this, [combinedOptions, wrapped_cb]);
           } else {
             // the user didn't specify a callback, add it as a "response" handler ourselves
-            return original.apply(this, [options]).on("response", wrapped_cb);
+            return original.apply(this, [combinedOptions]).on("response", wrapped_cb);
           }
         }
       );
diff --git a/lib/instrumentation/https.js b/lib/instrumentation/https.js
index a871416..bdbdd48 100644
--- a/lib/instrumentation/https.js
+++ b/lib/instrumentation/https.js
@@ -9,36 +9,50 @@ let instrumentHTTPS = (https, opts = {}) => {
   shimmer.wrap(https, "get", function(_original) {
     // we have to replace http.get since it references request through
     // a closure (so we can't replace the value it uses..)
-    return function(options, cb) {
-      var req = https.request(options, cb);
+    return function(_url, _options, _cb) {
+      let req = https.request.apply(this, Array.from(arguments));
       req.end();
       return req;
     };
   });
 
   shimmer.wrap(https, "request", function(original) {
-    return function(options, cb) {
+    // node 10+ supports a three arg form.  Instead of writing both implementations,
+    // dynamically switch on arg types.  I _hate_ code that does this, but it's easier
+    // and cleaner than the alternative.
+    return function(_url, options, cb) {
       if (!api.traceActive()) {
-        return original.apply(this, [options, cb]);
+        return original.apply(this, Array.from(arguments));
       }
 
-      // let's make sure we have an object (or a url) for options.
-      if (typeof options === "string") {
-        options = url.parse(options); // node 10 doesn't use url.parse (except as a fallback).  figure out how to handle that?
-      } else {
-        // shallow-clone options since we'll be modifying it below (adding our header)
-        options = Object.assign({}, options);
-      }
+      let combinedOptions = {};
+      let callback;
 
-      // another copy so we can normalize things
-      let formatOptions = Object.assign({}, options);
+      if (typeof _url === "string") {
+        combinedOptions = Object.assign({}, url.parse(_url)); // node 10 doesn't use url.parse (except as a fallback).  figure out how to handle that?
+        delete combinedOptions.href;
+        if (typeof options === "object") {
+          Object.assign(combinedOptions, options);
+          // we need to fix up `host` after this (if it wasn't specified in options), since the port might have changed.
+          if (!options.host) {
+            combinedOptions.host = `${combinedOptions.hostname}:${combinedOptions.port}`;
+          }
+          callback = cb;
+        } else {
+          callback = options;
+        }
+      } else if (typeof _url === "object") {
+        // the two-arg form.
+        combinedOptions = Object.assign({}, _url);
+        callback = options;
+      }
 
       // make sure options is populated enough for url.format to give us reasonable results.  these are all defaults from nodejs docs.
-      formatOptions.protocol = formatOptions.protocol || "https:";
-      formatOptions.port = formatOptions.port !== 443 ? formatOptions.port : null;
-      formatOptions.pathname = formatOptions.path || "/";
-      if (!formatOptions.hostname && !formatOptions.host) {
-        formatOptions.hostname = "localhost";
+      combinedOptions.protocol = combinedOptions.protocol || "https:";
+      combinedOptions.port = combinedOptions.port !== 443 ? combinedOptions.port : null;
+      combinedOptions.pathname = combinedOptions.path || "/";
+      if (!combinedOptions.hostname && !combinedOptions.host) {
+        combinedOptions.hostname = "localhost";
       }
 
       return api.startAsyncSpan(
@@ -46,7 +60,7 @@ let instrumentHTTPS = (https, opts = {}) => {
           [schema.EVENT_TYPE]: "https",
           [schema.PACKAGE_VERSION]: opts.packageVersion,
           [schema.TRACE_SPAN_NAME]: options.method || "GET",
-          url: url.format(formatOptions),
+          url: url.format(combinedOptions),
         },
         span => {
           // in node 8.x, https calls into http.  currently we don't have a way
@@ -57,21 +71,21 @@ let instrumentHTTPS = (https, opts = {}) => {
           // callback here.
           let wrapped_cb = tracker.bindFunction(function(res) {
             api.finishSpan(span, "request");
-            if (cb) {
-              return cb.apply(this, [res]);
+            if (callback) {
+              return callback.apply(this, [res]);
             }
           });
 
           // shallow clone options.headers, adding our tracing headers in
-          options.headers = Object.assign({}, options.headers, {
+          combinedOptions.headers = Object.assign({}, combinedOptions.headers, {
             [api.TRACE_HTTP_HEADER]: api.marshalTraceContext(api.getTraceContext()),
           });
 
-          if (cb) {
-            return original.apply(this, [options, wrapped_cb]);
+          if (callback) {
+            return original.apply(this, [combinedOptions, wrapped_cb]);
           } else {
             // the user didn't specify a callback, add it as a "response" handler ourselves
-            return original.apply(this, [options]).on("response", wrapped_cb);
+            return original.apply(this, [combinedOptions]).on("response", wrapped_cb);
           }
         }
       );
diff --git a/lib/instrumentation/trace-util.js b/lib/instrumentation/trace-util.js
index 37c9368..e4f4f4b 100644
--- a/lib/instrumentation/trace-util.js
+++ b/lib/instrumentation/trace-util.js
@@ -21,6 +21,16 @@ const getValueFromHeaders = (requestHeaders, headers) => {
   return undefined;
 };
 
+// adds trace source field to the context and returns the updated context
+function _includeSource(context, key) {
+  if (!context) {
+    return {};
+  }
+  return Object.assign({}, context, {
+    source: `${key} http header`,
+  });
+}
+
 exports.getTraceContext = (traceIdSource, req) => {
   if (typeof traceIdSource === "undefined" || typeof traceIdSource === "string") {
     let headers =
@@ -33,52 +43,29 @@ exports.getTraceContext = (traceIdSource, req) => {
       return {};
     }
     let { value, header } = valueAndHeader;
+    let parsed = {};
 
     switch (header) {
-      case api.TRACE_HTTP_HEADER: {
-        let parsed = api.unmarshalTraceContext(value);
-        if (!parsed) {
-          return {};
-        }
-        return Object.assign({}, parsed, {
-          source: `${header} http header`,
-        });
+      //honeycomb trace header
+      case api.honeycomb.TRACE_HTTP_HEADER: {
+        header = api.honeycomb.TRACE_HTTP_HEADER;
+        parsed = api.honeycomb.unmarshalTraceContext(value);
+        return _includeSource(parsed, header);
       }
 
-      case api.AMAZON_TRACE_HTTP_HEADER: {
-        let traceId, parentSpanId;
-
-        const split = value.split(";");
-        for (const s of split) {
-          const [name, value] = s.split("=");
-          if (name === "Root") {
-            traceId = value;
-          } else if (name === "Parent") {
-            parentSpanId = value;
-          }
-        }
-
-        if (!traceId) {
-          // if we didn't even get a 'Root=' clause, bail.
-          return {};
-        }
-
-        if (!parentSpanId) {
-          parentSpanId = traceId;
-        }
-
-        return {
-          traceId,
-          parentSpanId,
-          source: `${header} http header`,
-        };
+      case api.aws.TRACE_HTTP_HEADER: {
+        header = api.aws.TRACE_HTTP_HEADER;
+        parsed = api.aws.unmarshalTraceContext(value);
+        return _includeSource(parsed, header);
       }
 
       default: {
-        return {
-          traceId: value,
-          source: `${header} http header`,
-        };
+        return _includeSource(
+          {
+            traceId: value,
+          },
+          header
+        );
       }
     }
   } else {
diff --git a/lib/propagation/aws.js b/lib/propagation/aws.js
new file mode 100644
index 0000000..6722b0c
--- /dev/null
+++ b/lib/propagation/aws.js
@@ -0,0 +1,34 @@
+/* global exports */
+const TRACE_HTTP_HEADER = "X-Amzn-Trace-Id";
+exports.TRACE_HTTP_HEADER = TRACE_HTTP_HEADER;
+
+// unmarshalTraceContext takes a string trace header and returns a context
+exports.unmarshalTraceContext = unmarshalTraceContext;
+
+function unmarshalTraceContext(header) {
+  let traceId, parentSpanId;
+
+  const split = header.split(";");
+  for (const s of split) {
+    const [name, value] = s.split("=");
+    if (name === "Root") {
+      traceId = value;
+    } else if (name === "Parent") {
+      parentSpanId = value;
+    }
+  }
+
+  if (!traceId) {
+    // if we didn't even get a 'Root=' clause, bail.
+    return;
+  }
+
+  if (!parentSpanId) {
+    parentSpanId = traceId;
+  }
+
+  return {
+    traceId,
+    parentSpanId,
+  };
+}
diff --git a/lib/propagation.js b/lib/propagation/honeycomb.js
similarity index 82%
rename from lib/propagation.js
rename to lib/propagation/honeycomb.js
index e07548a..fe94f5d 100644
--- a/lib/propagation.js
+++ b/lib/propagation/honeycomb.js
@@ -1,12 +1,11 @@
 /* global require, exports, Buffer, __dirname */
 const path = require("path"),
-  pkg = require(path.join(__dirname, "..", "package.json")),
+  pkg = require(path.join(__dirname, "..", "..", "package.json")),
   debug = require("debug")(`${pkg.name}:propagation`),
-  schema = require("./schema");
+  schema = require("../schema");
 
-exports.TRACE_HTTP_HEADER = "X-Honeycomb-Trace";
-exports.AMAZON_TRACE_HTTP_HEADER = "X-Amzn-Trace-Id";
-exports.REQUEST_ID_HTTP_HEADER = "X-Request-ID";
+const TRACE_HTTP_HEADER = "X-Honeycomb-Trace";
+exports.TRACE_HTTP_HEADER = TRACE_HTTP_HEADER;
 
 const VERSION = "1";
 exports.VERSION = VERSION;
@@ -27,7 +26,10 @@ exports.VERSION = VERSION;
 //
 // ex: X-Honeycomb-Trace: 1;trace_id=weofijwoeifj,parent_id=owefjoweifj,context=SGVsbG8gV29ybGQ=
 
+// marshalTraceContext takes a trace context object
+// and returns a serialized honeycomb trace header
 exports.marshalTraceContext = marshalTraceContextv1;
+
 function marshalTraceContextv1(context) {
   let traceId = context.id;
   let spanId = context.stack[context.stack.length - 1].payload[schema.TRACE_SPAN_ID];
@@ -44,8 +46,12 @@ function marshalTraceContextv1(context) {
   ).toString("base64")}`;
 }
 
+// unmarshalTraceContext takes a string trace header and returns a context
 exports.unmarshalTraceContext = unmarshalTraceContext;
+
 function unmarshalTraceContext(contextStr) {
+  // this trim and split just splits out the version from the remaining trace header
+  // payload is trace header minus version
   let [version, payload] = contextStr.trim().split(";");
   switch (version) {
     case "1":
@@ -101,5 +107,10 @@ function unmarshalTraceContextv1(payload) {
     }
   }
 
-  return { traceId, parentSpanId, customContext, dataset };
+  return {
+    traceId,
+    parentSpanId,
+    customContext,
+    dataset,
+  };
 }
diff --git a/lib/propagation/index.js b/lib/propagation/index.js
new file mode 100644
index 0000000..1316c61
--- /dev/null
+++ b/lib/propagation/index.js
@@ -0,0 +1,31 @@
+/* global require, exports */
+const honeycomb = require("./honeycomb");
+const aws = require("./aws");
+
+exports.REQUEST_ID_HTTP_HEADER = "X-Request-ID";
+
+// VERSION points to honeycomb.VERSION for backwards compatibility.
+
+// Deprecated: Use honeycomb.VERSION
+exports.VERSION = honeycomb.VERSION;
+
+// unmarshalTraceContext wraps honeycomb.unmarshalTraceContext for backwards compatibility.
+
+// Deprecated: Use honeycomb.unmarshalTraceContext
+exports.unmarshalTraceContext = unmarshalTraceContext;
+
+function unmarshalTraceContext(traceHeader) {
+  return honeycomb.unmarshalTraceContext(traceHeader);
+}
+
+// marshalTraceContext wraps honeycomb.marshalTraceContext for backwards compatibility.
+
+// Deprecated: Use honeycomb.marshalTraceContext
+exports.marshalTraceContext = marshalTraceContext;
+
+function marshalTraceContext(context) {
+  return honeycomb.marshalTraceContext(context);
+}
+
+exports.honeycomb = honeycomb;
+exports.aws = aws;
diff --git a/package-lock.json b/package-lock.json
index 9f2b318..d139c0e 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1478,6 +1478,11 @@
         "which": "^1.2.9"
       }
     },
+    "crypto": {
+      "version": "1.0.1",
+      "resolved": "https://registry.npmjs.org/crypto/-/crypto-1.0.1.tgz",
+      "integrity": "sha512-VxBKmeNcqQdiUQUW2Tzq0t377b54N2bMtXO/qiLa+6eRRmmC4qT3D4OnTGoT/U6O9aklQ/jTwbOtRMTTY8G0Ig=="
+    },
     "cssom": {
       "version": "0.3.8",
       "resolved": "https://registry.npmjs.org/cssom/-/cssom-0.3.8.tgz",
@@ -7868,7 +7873,8 @@
     "uuid": {
       "version": "3.3.2",
       "resolved": "https://registry.npmjs.org/uuid/-/uuid-3.3.2.tgz",
-      "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA=="
+      "integrity": "sha512-yXJmeNaw3DnnKAOKJE51sL/ZaYfWJRl1pK9dr19YFCu0ObS231AB1/LbqTKRAQ5kw8A90rA6fr4riOUpTZvQZA==",
+      "dev": true
     },
     "validate-npm-package-license": {
       "version": "3.0.4",
diff --git a/package.json b/package.json
index 3291df1..c02d78e 100644
--- a/package.json
+++ b/package.json
@@ -48,11 +48,11 @@
   },
   "dependencies": {
     "array-flatten": "^2.1.2",
+    "crypto": "^1.0.1",
     "debug": "^4.1.1",
     "libhoney": "^2.0.2",
     "on-headers": "^1.0.2",
-    "shimmer": "^1.2.1",
-    "uuid": "^3.3.2"
+    "shimmer": "^1.2.1"
   },
   "jest": {
     "verbose": true,


Test patch:
diff --git a/lib/api/index.test.js b/lib/api/index.test.js
index 47f09c1..8a40b4b 100644
--- a/lib/api/index.test.js
+++ b/lib/api/index.test.js
@@ -117,6 +117,46 @@ test("sample rates are propagated", () => {
   expect(postData[schema.DURATION_MS]).not.toBeUndefined();
 });
 
+describe("generate trace id", () => {
+  api._resetForTesting();
+  api.configure({
+    impl: "libhoney-event",
+    transmission: "mock",
+    writeKey: "abc123",
+  });
+
+  let rootSpan = api.startTrace({
+    [schema.EVENT_TYPE]: "source",
+    [schema.TRACE_SPAN_NAME]: "name",
+  });
+
+  test("should create a 8-byte hex id", () => {
+    const regex = /^(([\da-f]){32})$/;
+    expect(rootSpan.payload[schema.TRACE_ID]).toMatch(regex);
+  });
+  api.finishTrace(rootSpan);
+});
+
+describe("generate span id", () => {
+  api._resetForTesting();
+  api.configure({
+    impl: "libhoney-event",
+    transmission: "mock",
+    writeKey: "abc123",
+  });
+
+  let rootSpan = api.startTrace({
+    [schema.EVENT_TYPE]: "source",
+    [schema.TRACE_SPAN_NAME]: "name",
+  });
+
+  test("should create a 8-byte hex id", () => {
+    const regex = /^(([\da-f]){16})$/;
+    expect(rootSpan.payload[schema.TRACE_SPAN_ID]).toMatch(regex);
+  });
+  api.finishTrace(rootSpan);
+});
+
 describe("sampler hook", () => {
   describe("if samplerHook option provided", () => {
     test("it enqueues event during finalSpan if shouldSample is true", () => {
diff --git a/lib/instrumentation/http.test.js b/lib/instrumentation/http.test.js
index 0d9366f..b920d5b 100644
--- a/lib/instrumentation/http.test.js
+++ b/lib/instrumentation/http.test.js
@@ -1,5 +1,6 @@
 /* eslint-env node, jest */
-const http = require("http"),
+const semver = require("semver"),
+  http = require("http"),
   instrumentHttp = require("./http"),
   schema = require("../schema"),
   tracker = require("../async_tracker"),
@@ -12,13 +13,16 @@ function newMockContext() {
 instrumentHttp(http);
 
 let server;
-beforeAll(() => {
+beforeAll(done => {
   server = http.createServer((req, res) => {
-    res.setHeader(api.TRACE_HTTP_HEADER, req.headers[api.TRACE_HTTP_HEADER.toLowerCase()]);
+    res.setHeader(
+      api.TRACE_HTTP_HEADER,
+      req.headers[api.TRACE_HTTP_HEADER.toLowerCase()] || "missing"
+    );
     res.end();
   });
 
-  server.listen(9009, "localhost");
+  server.listen(9009, "localhost", done);
 });
 
 afterAll(() => {
@@ -74,3 +78,55 @@ test("url as options", done => {
     }
   );
 });
+
+test("node 10.9.0+ url + options", done => {
+  if (semver.lt(process.version, "10.9.0")) {
+    // don't run this test for these versions
+    done();
+    return;
+  }
+
+  tracker.setTracked(newMockContext());
+
+  http.get(
+    "http://localhost:80",
+    {
+      hostname: "localhost",
+      port: 9009,
+    },
+    res => {
+      expect(res.headers[api.TRACE_HTTP_HEADER.toLowerCase()]).toBe(
+        "1;trace_id=0,parent_id=51001,context=e30="
+      );
+      expect(api._apiForTesting().sentEvents).toEqual([
+        expect.objectContaining({
+          [schema.EVENT_TYPE]: "http",
+          name: "GET",
+          url: "http://localhost:9009/",
+        }),
+      ]);
+      done();
+    }
+  );
+});
+
+test("node 10.9.0+ url + options, without active trace", done => {
+  if (semver.lt(process.version, "10.9.0")) {
+    // don't run this test for these versions
+    done();
+    return;
+  }
+
+  http.get(
+    "http://localhost:80",
+    {
+      hostname: "localhost",
+      port: 9009,
+    },
+    res => {
+      expect(res.headers[api.TRACE_HTTP_HEADER.toLowerCase()]).toBe("missing");
+      expect(api._apiForTesting().sentEvents).toEqual([]);
+      done();
+    }
+  );
+});
diff --git a/lib/instrumentation/https.test.js b/lib/instrumentation/https.test.js
index 5851840..37bc517 100644
--- a/lib/instrumentation/https.test.js
+++ b/lib/instrumentation/https.test.js
@@ -88,3 +88,49 @@ test("url as options", done => {
     }
   );
 });
+
+test("node 10.9.0+ url + options", done => {
+  if (semver.lt(process.version, "10.9.0")) {
+    // don't run this test for these versions
+    done();
+    return;
+  }
+
+  tracker.setTracked(newMockContext());
+
+  https.get(
+    "https://google.com:80",
+    {
+      port: 443,
+    },
+    _res => {
+      expect(api._apiForTesting().sentEvents).toEqual([
+        expect.objectContaining({
+          [schema.EVENT_TYPE]: "https",
+          name: "GET",
+          url: "https://google.com:443/",
+        }),
+      ]);
+      done();
+    }
+  );
+});
+
+test("node 10.9.0+ url + options, without active trace", done => {
+  if (semver.lt(process.version, "10.9.0")) {
+    // don't run this test for these versions
+    done();
+    return;
+  }
+
+  https.get(
+    "https://google.com:80",
+    {
+      port: 443,
+    },
+    _res => {
+      expect(api._apiForTesting().sentEvents).toEqual([]);
+      done();
+    }
+  );
+});
diff --git a/lib/propagation.test.js b/lib/propagation/propagation.test.js
similarity index 93%
rename from lib/propagation.test.js
rename to lib/propagation/propagation.test.js
index 9357a6c..101c66e 100644
--- a/lib/propagation.test.js
+++ b/lib/propagation/propagation.test.js
@@ -1,9 +1,10 @@
 /* global require describe test expect */
-const propagation = require("./propagation"),
-  schema = require("./schema"),
-  Span = require("./api/span"),
+const propagation = require("."),
+  schema = require("../schema"),
+  Span = require("../api/span"),
   cases = require("jest-in-case");
 
+const { honeycomb } = propagation;
 // this context structure is the same as what the libhoney event api implementation generate.
 let testContext = {
   id: "abcdef123456",
@@ -19,7 +20,7 @@ let testContext = {
 describe("marshaling", () => {
   test("version string prefix", () => {
     expect(
-      propagation.marshalTraceContext(testContext).startsWith(`${propagation.VERSION};`)
+      propagation.marshalTraceContext(testContext).startsWith(`${honeycomb.VERSION};`)
     ).toBeTruthy();
   });
 });