Repository: honeycombio/beeline-nodejs
Pull request #63 (2018-07-17 21:19:37)

remove the explicit context first-parameter from event.startEvent
there's really no need for this parameter.  Everywhere that calls startEvent already makes sure we have a context (even if indirectly through `event.traceActive()` from #62 )

Remove it in favor of looking it up inside the body of startEvent.  removes the need for a local context variable (and even require-ing `async_tracker`) in a few files.


Patch:
diff --git a/lib/event.js b/lib/event.js
index dca8d80..a18da03 100644
--- a/lib/event.js
+++ b/lib/event.js
@@ -59,13 +59,14 @@ class MockEventAPI {
     let id = this.traceId++;
     let requestContext = { id: traceId || id };
     tracker.setTracked(requestContext);
-    return this.startEvent(requestContext, source, name);
+    return this.startEvent(source, name);
   }
   finishRequest(ev) {
     this.finishEvent(ev);
     tracker.deleteTracked();
   }
-  startEvent(context, source, name) {
+  startEvent(source, name) {
+    let context = tracker.getTracked();
     const eventPayload = {
       [schema.TRACE_ID]: context.id,
       [schema.TRACE_SPAN_NAME]: name,
@@ -139,7 +140,7 @@ class LibhoneyEventAPI {
       stack: [],
     };
     tracker.setTracked(requestContext);
-    return this.startEvent(requestContext, source, name, id);
+    return this.startEvent(source, name, id);
   }
 
   finishRequest(ev) {
@@ -147,7 +148,8 @@ class LibhoneyEventAPI {
     tracker.deleteTracked();
   }
 
-  startEvent(context, source, name, spanId = uuidv4()) {
+  startEvent(source, name, spanId = uuidv4()) {
+    let context = tracker.getTracked();
     let parentId;
     if (context.stack.length > 0) {
       parentId = context.stack[context.stack.length - 1][schema.TRACE_SPAN_ID];
@@ -293,8 +295,8 @@ exports.finishRequest = function finishRequest(ev) {
 exports.addContext = function addContext(map) {
   return eventAPI.addContext(map);
 };
-exports.startEvent = function startEvent(context, source, name) {
-  return eventAPI.startEvent(context, source, name);
+exports.startEvent = function startEvent(source, name) {
+  return eventAPI.startEvent(source, name);
 };
 exports.finishEvent = function finishEvent(ev, rollup) {
   return eventAPI.finishEvent(ev, rollup);
diff --git a/lib/instrumentation/child_process.js b/lib/instrumentation/child_process.js
index 8ac3eff..7134965 100644
--- a/lib/instrumentation/child_process.js
+++ b/lib/instrumentation/child_process.js
@@ -1,13 +1,11 @@
 /* global require, module */
 const shimmer = require("shimmer"),
-  tracker = require("../async_tracker"),
   event = require("../event"),
   schema = require("../schema");
 
 function wrapExecLike(name, packageVersion) {
   return function(original) {
     return function(file, args /*, options, callback */) {
-      let context = tracker.getTracked();
       if (!event.traceActive) {
         return original.apply(this, arguments);
       }
@@ -30,7 +28,7 @@ function wrapExecLike(name, packageVersion) {
         };
       });
 
-      ev = event.startEvent(context, "child_process", name);
+      ev = event.startEvent("child_process", name);
       event.addContext({
         [schema.PACKAGE_VERSION]: packageVersion,
       });
diff --git a/lib/instrumentation/http.js b/lib/instrumentation/http.js
index 8ef9ec6..6cd53da 100644
--- a/lib/instrumentation/http.js
+++ b/lib/instrumentation/http.js
@@ -1,7 +1,6 @@
 /* global require, module */
 const url = require("url"),
   shimmer = require("shimmer"),
-  tracker = require("../async_tracker"),
   event = require("../event"),
   schema = require("../schema");
 
@@ -18,7 +17,6 @@ let instrumentHTTP = (http, opts = {}) => {
 
   shimmer.wrap(http, "request", function(original) {
     return function(options, cb) {
-      let context = tracker.getTracked();
       if (!event.traceActive()) {
         return original.apply(this, [options, cb]);
       }
@@ -34,7 +32,7 @@ let instrumentHTTP = (http, opts = {}) => {
         }
       };
 
-      ev = event.startEvent(context, "http", "request");
+      ev = event.startEvent("http", "request");
       event.addContext({
         [schema.PACKAGE_VERSION]: opts.packageVersion,
       });
diff --git a/lib/instrumentation/https.js b/lib/instrumentation/https.js
index eeb5352..086e922 100644
--- a/lib/instrumentation/https.js
+++ b/lib/instrumentation/https.js
@@ -1,7 +1,6 @@
 /* global require, module */
 const url = require("url"),
   shimmer = require("shimmer"),
-  tracker = require("../async_tracker"),
   event = require("../event"),
   schema = require("../schema");
 
@@ -18,7 +17,6 @@ let instrumentHTTPS = (https, opts = {}) => {
 
   shimmer.wrap(https, "request", function(original) {
     return function(options, cb) {
-      let context = tracker.getTracked();
       if (!event.traceActive()) {
         return original.apply(this, [options, cb]);
       }
@@ -34,7 +32,7 @@ let instrumentHTTPS = (https, opts = {}) => {
         }
       };
 
-      ev = event.startEvent(context, "https", "request");
+      ev = event.startEvent("https", "request");
       event.addContext({
         [schema.PACKAGE_VERSION]: opts.packageVersion,
       });
diff --git a/lib/instrumentation/mongodb.js b/lib/instrumentation/mongodb.js
index 6d4499d..9d80c98 100644
--- a/lib/instrumentation/mongodb.js
+++ b/lib/instrumentation/mongodb.js
@@ -4,8 +4,8 @@ const shimmer = require("shimmer"),
   event = require("../event"),
   schema = require("../schema");
 
-function startCollectionEvent(context, packageVersion, name) {
-  let ev = event.startEvent(context, "mongodb", `collection.${name}`);
+function startCollectionEvent(packageVersion, name) {
+  let ev = event.startEvent("mongodb", `collection.${name}`);
   event.addContext({
     [schema.PACKAGE_VERSION]: packageVersion,
   });
@@ -43,7 +43,6 @@ function instrumentMongodb(mongodb, opts = {}) {
   function wrap(name, populateArgs, additionalContext) {
     shimmer.wrap(mongodb.Collection.prototype, name, function(original) {
       return function(...args) {
-        let context = tracker.getTracked();
         if (!event.traceActive()) {
           return original.apply(this, args);
         }
@@ -63,7 +62,7 @@ function instrumentMongodb(mongodb, opts = {}) {
           }
         });
 
-        ev = startCollectionEvent(context, opts.packageVersion, name);
+        ev = startCollectionEvent(opts.packageVersion, name);
         event.addContext(prefixKeys("db.options", options));
         if (additionalContext) {
           event.addContext(prefixKeys("db", additionalContext(...populatedArgs)));
diff --git a/lib/instrumentation/mysql2.js b/lib/instrumentation/mysql2.js
index 7cc0eb0..b7499ef 100644
--- a/lib/instrumentation/mysql2.js
+++ b/lib/instrumentation/mysql2.js
@@ -10,7 +10,6 @@ let instrumentConnection = function(conn, packageVersion) {
       if (args.length < 1) {
         return original.apply(this, args);
       }
-      let context = tracker.getTracked();
       if (!event.traceActive()) {
         return original.apply(this, args);
       }
@@ -28,7 +27,7 @@ let instrumentConnection = function(conn, packageVersion) {
       };
       args = args.slice(0, -1).concat(wrapped_cb);
 
-      ev = event.startEvent(context, "mysql2", "query");
+      ev = event.startEvent("mysql2", "query");
       event.addContext({
         [schema.PACKAGE_VERSION]: packageVersion,
       });
@@ -42,7 +41,6 @@ let instrumentConnection = function(conn, packageVersion) {
       if (args.length < 1) {
         return original.apply(this, args);
       }
-      let context = tracker.getTracked();
       if (!event.traceActive()) {
         return original.apply(this, args);
       }
@@ -62,7 +60,7 @@ let instrumentConnection = function(conn, packageVersion) {
       });
       args = args.slice(0, -1).concat(wrapped_cb);
 
-      ev = event.startEvent(context, "mysql2", "query");
+      ev = event.startEvent("mysql2", "query");
       event.addContext({
         [schema.PACKAGE_VERSION]: packageVersion,
       });
diff --git a/lib/instrumentation/react-dom-server.js b/lib/instrumentation/react-dom-server.js
index 838af61..a00cb63 100644
--- a/lib/instrumentation/react-dom-server.js
+++ b/lib/instrumentation/react-dom-server.js
@@ -1,18 +1,16 @@
 /* global require, module */
 const shimmer = require("shimmer"),
-  tracker = require("../async_tracker"),
   event = require("../event"),
   schema = require("../schema");
 
 function shimRenderMethod(reactDOMServer, name, packageVersion) {
   shimmer.wrap(reactDOMServer, name, function(original) {
     return function(...args) {
-      let context = tracker.getTracked();
       if (!event.traceActive()) {
         return original.apply(this, args);
       }
 
-      let ev = event.startEvent(context, "react", name);
+      let ev = event.startEvent("react", name);
       event.addContext({
         [schema.PACKAGE_VERSION]: packageVersion,
       });


Test patch:
diff --git a/lib/event.test.js b/lib/event.test.js
index 3acb3d7..32118ea 100644
--- a/lib/event.test.js
+++ b/lib/event.test.js
@@ -63,8 +63,8 @@ test("ending events out of order isn't allowed", () => {
 
   expect(context.stack).toEqual([eventPayload]);
 
-  let event2Payload = event.startEvent(context, "source2", "name2");
-  let event3Payload = event.startEvent(context, "source3", "name3");
+  let event2Payload = event.startEvent("source2", "name2");
+  let event3Payload = event.startEvent("source3", "name3");
   expect(context.stack).toEqual([eventPayload, event2Payload, event3Payload]);
 
   // if we end event2Payload from the stack, we should also remove event3Payload.
@@ -78,9 +78,8 @@ test("ending events out of order isn't allowed", () => {
 
 test("sub-events can opt to rollup count/duration into request events", () => {
   let requestPayload = event.startRequest("source", "name");
-  let context = tracker.getTracked();
 
-  let eventPayload = event.startEvent(context, "source2", "name2");
+  let eventPayload = event.startEvent("source2", "name2");
   event.finishEvent(eventPayload, "rollup");
 
   let durationMS = eventPayload[schema.DURATION_MS];
@@ -88,7 +87,7 @@ test("sub-events can opt to rollup count/duration into request events", () => {
   expect(requestPayload["totals.source2.rollup.duration_ms"]).toBe(durationMS);
 
   // another event from the same source
-  let event2Payload = event.startEvent(context, "source2", "name2");
+  let event2Payload = event.startEvent("source2", "name2");
   event.finishEvent(event2Payload, "rollup");
 
   let duration2MS = event2Payload[schema.DURATION_MS];
@@ -100,7 +99,7 @@ test("sub-events can opt to rollup count/duration into request events", () => {
   expect(requestPayload["totals.source2.duration_ms"]).toBe(durationMS + duration2MS);
 
   // one more event from the same source but a different name
-  let event3Payload = event.startEvent(context, "source2");
+  let event3Payload = event.startEvent("source2");
   event.finishEvent(event3Payload, "rollup2");
 
   let duration3MS = event3Payload[schema.DURATION_MS];
@@ -120,9 +119,8 @@ test("sub-events will get manual tracing fields", () => {
   const honey = event.apiForTesting().honey;
 
   let requestPayload = event.startRequest("source", "name");
-  let context = tracker.getTracked();
 
-  let eventPayload = event.startEvent(context, "source2", "name2");
+  let eventPayload = event.startEvent("source2", "name2");
   event.finishEvent(eventPayload);
 
   // finish the request
@@ -174,14 +172,13 @@ describe("custom context", () => {
     const honey = event.apiForTesting().honey;
 
     let requestPayload = event.startRequest("source", "name");
-    let context = tracker.getTracked();
 
-    let eventPayload = event.startEvent(context, "source2", "name2");
+    let eventPayload = event.startEvent("source2", "name2");
     event.finishEvent(eventPayload);
 
     event.customContext.add("testKey", "testVal");
 
-    let eventPayload2 = event.startEvent(context, "source3", "name3");
+    let eventPayload2 = event.startEvent("source3", "name3");
     event.finishEvent(eventPayload2);
 
     // finish the request