Repository: honeycombio/beeline-nodejs
Pull request #216 (2020-07-01 00:00:44)

switch UUID span and trace ids to W3C
The node version of this: https://github.com/honeycombio/beeline-go/pull/110

We're switching over to compatible-with-W3C trace and span ids to smooth out support of OTel interop. Using pretty much the exact mechanism as open-telemetry.

For now I'm sticking this in propagation where it seemed to make the most sense, but propagation.js will be refactored in a subsequent PR.

katiebayes: > I also wouldn't put them in propagation.js, since we aren't planning to tailor trace id _generation_ depending on interop, are we? likewise, I also wouldn't expect them to be used anywhere but the place where startTrace/startSpan live.
> 
> all that said, I don't care too strongly about it.

@toshok this move is actually to bring the formats in line to be friendly with each other so we don't have to tailor them unless there's a specific user input to do so. It's going to continue to be essentially, if there's a trace id, use that, if not, make a new one (in an OTel-consistent type).
toshok: I mean, I’d like to see further proposed changes before commenting further, but imo It doesn’t make sense to include fallback trace/span Id generation in the code that extracts/injects. The fallback behavior won’t be tuned by either of those things (since it’s what happens when we don’t have anything injected, and we can’t rely on extraction to provide the fallback impl.)

Patch:
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/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", () => {