Adding 128 bit traceId support (#361)

* Adding 128 bit traceId support
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* fix node 0.10 issue
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Replace node latest to lts in the travis config
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Fix travis file, adding back build for node latest
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Refactoring traceId
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Adding unit test
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Fixed issues
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Force travis to use ubuntu trusty
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Fix issues
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Refactoring
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Fix node 0.10 compatibility
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Increase timeout on some unit tests
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Increase timeout on some unit tests
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Small refactor
Signed-off-by: Paul Biccherai <paul.biccherai@chronwell.com>

* Add one more test; create Buffer via Utils

Signed-off-by: Yuri Shkuro <ys@uber.com>

* Avoid Buffer.fill() which is void on Node 0.10

Signed-off-by: Yuri Shkuro <ys@uber.com>

src/reporters/udp_sender.js

@@ -15,8 +15,9 @@ import dgram from 'dgram';
 import fs from 'fs';
 import path from 'path';
 import { Thrift } from 'thriftrw';
-import NullLogger from '../logger.js';
-import SenderUtils from './sender_utils.js';
+import NullLogger from '../logger';
+import SenderUtils from './sender_utils';
+import Utils from '../util';
 
 const HOST = 'localhost';
 const PORT = 6832;
@@ -128,7 +129,7 @@ export default class UDPSender {
     }
 
     const bufferLen = this._totalSpanBytes + this._emitSpanBatchOverhead;
-    const thriftBuffer = new Buffer(bufferLen);
+    const thriftBuffer = Utils.newBuffer(bufferLen);
     const writeResult = this._agentThrift.Agent.emitBatch.argumentsMessageRW.writeInto(
       this._convertBatchToThriftMessage(),
       thriftBuffer,

src/span_context.js

@@ -53,7 +53,19 @@ export default class SpanContext {
 
   get traceId(): any {
     if (this._traceId == null && this._traceIdStr != null) {
-      this._traceId = Utils.encodeInt64(this._traceIdStr);
+      // make sure that the string is 32 or 16 characters long to generate the
+      // corresponding 64 or 128 bits buffer by padding the start with zeros if necessary
+      // https://github.com/jaegertracing/jaeger/issues/1657
+      // At the same time this will enforce that the HEX has an even number of digits, node is expecting 2 HEX characters per byte
+      // https://github.com/nodejs/node/issues/21242
+      const traceIdExactLength = this._traceIdStr.length > 16 ? 32 : 16;
+      if (this._traceIdStr.length === traceIdExactLength) {
+        this._traceId = Utils.newBufferFromHex(this._traceIdStr);
+      } else {
+        const padding = traceIdExactLength === 16 ? '0000000000000000' : '00000000000000000000000000000000';
+        const safeTraceIdStr = (padding + this._traceIdStr).slice(-traceIdExactLength);
+        this._traceId = Utils.newBufferFromHex(safeTraceIdStr);
+      }
     }
     return this._traceId;
   }

src/thrift.js

@@ -22,7 +22,7 @@ export default class ThriftUtils {
     source: fs.readFileSync(path.join(__dirname, './jaeger-idl/thrift/jaeger.thrift'), 'ascii'),
     allowOptionalArguments: true,
   });
-  static emptyBuffer: Buffer = new Buffer([0, 0, 0, 0, 0, 0, 0, 0]);
+  static emptyBuffer: Buffer = Utils.newBuffer(8);
 
   static getThriftTags(initialTags: Array<Tag>): Array<any> {
     let thriftTags = [];
@@ -104,23 +104,39 @@ export default class ThriftUtils {
 
       thriftRefs.push({
         refType: refEnum,
-        traceIdLow: context.traceId,
-        traceIdHigh: ThriftUtils.emptyBuffer,
+        traceIdLow: ThriftUtils.getTraceIdLow(context.traceId),
+        traceIdHigh: ThriftUtils.getTraceIdHigh(context.traceId),
         spanId: context.spanId,
       });
     }
 
     return thriftRefs;
   }
 
+  static getTraceIdLow(traceId: Buffer) {
+    if (traceId != null) {
+      return traceId.slice(-8);
+    }
+
+    return ThriftUtils.emptyBuffer;
+  }
+
+  static getTraceIdHigh(traceId: Buffer) {
+    if (traceId != null && traceId.length > 8) {
+      return traceId.slice(-16, -8);
+    }
+
+    return ThriftUtils.emptyBuffer;
+  }
+
   static spanToThrift(span: Span): any {
     let tags = ThriftUtils.getThriftTags(span._tags);
     let logs = ThriftUtils.getThriftLogs(span._logs);
     let unsigned = true;
 
     return {
-      traceIdLow: span._spanContext.traceId,
-      traceIdHigh: ThriftUtils.emptyBuffer, // TODO(oibe) implement 128 bit ids
+      traceIdLow: ThriftUtils.getTraceIdLow(span._spanContext.traceId),
+      traceIdHigh: ThriftUtils.getTraceIdHigh(span._spanContext.traceId),
       spanId: span._spanContext.spanId,
       parentSpanId: span._spanContext.parentId || ThriftUtils.emptyBuffer,
       operationName: span._operationName,

src/tracer.js

@@ -44,6 +44,7 @@ export default class Tracer {
   _baggageSetter: BaggageSetter;
   _debugThrottler: Throttler & ProcessSetter;
   _process: Process;
+  _traceId128bit: boolean;
 
   /**
    * @param {String} [serviceName] - name of the current service or application.
@@ -57,6 +58,7 @@ export default class Tracer {
    * @param {Object} [options.baggageRestrictionManager] - a baggageRestrictionManager matching
    * @param {Object} [options.contextKey] - a name of the key to extract/inject context from headers
    * @param {Object} [options.baggagePrefix] - a name of the context baggage key prefix
+   * @param {boolean} [options.traceId128bit] - generate root span with a 128bit traceId.
    * BaggageRestrictionManager API from ./baggage.js.
    */
   constructor(
@@ -117,6 +119,8 @@ export default class Tracer {
     this._debugThrottler.setProcess(this._process);
     // TODO update reporter to implement ProcessSetter
     this._reporter.setProcess(this._process.serviceName, this._process.tags);
+
+    this._traceId128bit = options.traceId128bit;
   }
 
   _startInternalSpan(
@@ -230,8 +234,13 @@ export default class Tracer {
     let internalTags: any = {};
     let hasValidParent = false;
     if (!parent || !parent.isValid) {
-      let randomId = Utils.getRandom64();
-      ctx = new SpanContext(randomId, randomId);
+      if (this._traceId128bit) {
+        let randomId = Utils.getRandom128();
+        ctx = new SpanContext(randomId, randomId.slice(-8));
+      } else {
+        let randomId = Utils.getRandom64();
+        ctx = new SpanContext(randomId, randomId);
+      }
       if (parent) {
         // fake parent, doesn't contain a parent trace-id, but may contain debug-id/baggage
         if (parent.isDebugIDContainerOnly() && this._isDebugAllowed(operationName)) {

src/util.js

@@ -42,19 +42,36 @@ export default class Utils {
   }
 
   /**
-   * Determines whether a string contains a given prefix.
+   * Get a random buffer representing a random 64 bit.
    *
    * @return {Buffer}  - returns a buffer representing a random 64 bit
    * number.
    **/
   static getRandom64(): Buffer {
     let randint = xorshift.randomint();
-    let buf = new Buffer(8);
+    let buf = this.newBuffer(8);
     buf.writeUInt32BE(randint[0], 0);
     buf.writeUInt32BE(randint[1], 4);
     return buf;
   }
 
+  /**
+   * Get a random buffer representing a random 128 bit.
+   *
+   * @return {Buffer}  - returns a buffer representing a random 128 bit
+   * number.
+   **/
+  static getRandom128(): Buffer {
+    let randint1 = xorshift.randomint();
+    let randint2 = xorshift.randomint();
+    let buf = this.newBuffer(16);
+    buf.writeUInt32BE(randint1[0], 0);
+    buf.writeUInt32BE(randint1[1], 4);
+    buf.writeUInt32BE(randint2[0], 8);
+    buf.writeUInt32BE(randint2[1], 12);
+    return buf;
+  }
+
   /**
    * @param {string|number} numberValue - a string or number to be encoded
    * as a 64 bit byte array.
@@ -149,6 +166,33 @@ export default class Utils {
       });
   }
 
+  /**
+   * @param {string|number} input - a hex encoded string to store in the buffer.
+   * @return {Buffer} - returns a buffer representing the hex encoded string.
+   **/
+  static newBufferFromHex(input: string): Buffer {
+    const encoding = 'hex';
+    // check that 'Buffer.from' exists based on node's documentation
+    // https://nodejs.org/en/docs/guides/buffer-constructor-deprecation/#variant-3
+    if (Buffer.from && Buffer.from !== Uint8Array.from) {
+      return Buffer.from(input, encoding);
+    }
+    return new Buffer(input, encoding);
+  }
+
+  /**
+   * @param {number} input - a number of octets to allocate.
+   * @return {Buffer} - returns an empty buffer.
+   **/
+  static newBuffer(size: number): Buffer {
+    if (Buffer.alloc) {
+      return Buffer.alloc(size);
+    }
+    const buffer = new Buffer(size);
+    buffer.fill(0);
+    return buffer;
+  }
+
   /**
    * Creates a callback function that only delegates to passed <code>callback</code>
    * after <code>limit</code> invocations. Useful in types like CompositeReporter that

test/http_sender.js

@@ -254,7 +254,7 @@ describe('http sender', () => {
       expect(err).to.have.string('error sending spans over HTTP: Error: getaddrinfo ENOTFOUND');
       tracer.close(done);
     });
-  });
+  }).timeout(5000);
 
   it('should handle HTTPS collectors', done => {
     // Make it ignore the fact that our cert isn't valid.

test/span_context.js

@@ -41,6 +41,21 @@ describe('SpanContext', () => {
     assert.equal(flags, context.flags);
   });
 
+  it('should return given values as they were set 128 bit', () => {
+    let traceId = Buffer.concat([Utils.encodeInt64(2), Utils.encodeInt64(1)]);
+    let spanId = Utils.encodeInt64(3);
+    let parentId = Utils.encodeInt64(4);
+    let flags = 1;
+
+    let context = SpanContext.withBinaryIds(traceId, spanId, parentId, flags);
+
+    assert.deepEqual(traceId, context.traceId);
+    assert.deepEqual('20000000000000001', context.traceIdStr);
+    assert.deepEqual(spanId, context.spanId);
+    assert.deepEqual(parentId, context.parentId);
+    assert.equal(flags, context.flags);
+  });
+
   it('should expose IsSampled properly', () => {
     let context = SpanContext.withBinaryIds(
       Utils.encodeInt64(1),
@@ -80,6 +95,7 @@ describe('SpanContext', () => {
     let context = SpanContext.fromString('100:7f:0:1');
 
     assert.deepEqual('100', context.traceIdStr);
+    assert.deepEqual(Utils.encodeInt64(0x100), context.traceId);
     assert.deepEqual(Utils.encodeInt64(0x7f), context.spanId);
     assert.equal(null, context.parentId);
     assert.equal(1, context.flags);
@@ -94,6 +110,25 @@ describe('SpanContext', () => {
     assert.equal(context.flags, 0x1);
   });
 
+  it('should turn properly formatted strings into correct span contexts 128 bit', () => {
+    let context = SpanContext.fromString('20000000000000100:7f:0:1');
+
+    assert.deepEqual('20000000000000100', context.traceIdStr);
+    assert.deepEqual(Buffer.concat([Utils.encodeInt64(0x2), Utils.encodeInt64(0x100)]), context.traceId);
+    assert.deepEqual(Utils.encodeInt64(0x7f), context.spanId);
+    assert.equal(null, context.parentId);
+    assert.equal(1, context.flags);
+
+    // test large numbers
+    context = SpanContext.fromString('ffffffffffffffffffffffffffffffff:ffffffffffffffff:5:1');
+    assert.equal('ffffffffffffffffffffffffffffffff', context.traceIdStr);
+    assert.equal('ffffffffffffffff', context.spanIdStr);
+    assert.deepEqual(Buffer.concat([LARGEST_64_BUFFER, LARGEST_64_BUFFER]), context.traceId);
+    assert.deepEqual(LARGEST_64_BUFFER, context.spanId);
+    assert.deepEqual(Utils.encodeInt64(0x5), context.parentId);
+    assert.equal(context.flags, 0x1);
+  });
+
   it('should return null on malformed traces', () => {
     assert.equal(SpanContext.fromString('bad value'), null);
     assert.equal(SpanContext.fromString('1:1:1:1:1'), null, 'Too many colons');

test/thrift.js

@@ -11,6 +11,7 @@
 // the License.
 
 import { assert } from 'chai';
+import opentracing from 'opentracing';
 import ConstSampler from '../src/samplers/const_sampler.js';
 import InMemoryReporter from '../src/reporters/in_memory_reporter.js';
 import Tracer from '../src/tracer.js';
@@ -64,4 +65,34 @@ describe('ThriftUtils', () => {
     assert.deepEqual(tSpan.duration, Utils.encodeInt64((123.678 - 123.456) * 1000));
     assert.deepEqual(tSpan.logs[0].timestamp, Utils.encodeInt64(123567));
   });
+
+  it('should convert span with 128 bit traceId', () => {
+    let reporter = new InMemoryReporter();
+    let tracer = new Tracer('test-service-name', reporter, new ConstSampler(true), { traceId128bit: true });
+    let span = tracer.startSpan('some operation');
+    let childOfRef = new opentracing.Reference(opentracing.REFERENCE_CHILD_OF, span.context());
+    let childSpan = tracer.startSpan('some child operation', { references: [childOfRef] });
+    childSpan.finish();
+    span.finish();
+    tracer.close();
+    let tSpan = ThriftUtils.spanToThrift(childSpan);
+    assert.deepEqual(tSpan.traceIdLow, childSpan.context().traceId.slice(-8));
+    assert.deepEqual(tSpan.traceIdHigh, childSpan.context().traceId.slice(-16, -8));
+    assert.deepEqual(tSpan.spanId, childSpan.context().spanId);
+    assert.deepEqual(tSpan.references[0].traceIdLow, span.context().traceId.slice(-8));
+    assert.deepEqual(tSpan.references[0].traceIdHigh, span.context().traceId.slice(-16, -8));
+    assert.deepEqual(tSpan.references[0].spanId, span.context().spanId);
+  });
+
+  it('should convert extract traceIdLow and traceIdHigh', () => {
+    let traceIdLow = Utils.encodeInt64(1);
+    let traceIdHigh = Utils.encodeInt64(2);
+    let traceId = Buffer.concat([traceIdHigh, traceIdLow]);
+
+    assert.deepEqual(ThriftUtils.getTraceIdLow(traceId), traceIdLow);
+    assert.deepEqual(ThriftUtils.getTraceIdHigh(traceId), traceIdHigh);
+
+    assert.deepEqual(ThriftUtils.getTraceIdLow(null), ThriftUtils.emptyBuffer);
+    assert.deepEqual(ThriftUtils.getTraceIdHigh(null), ThriftUtils.emptyBuffer);
+  });
 });

test/tracer.js

@@ -299,6 +299,38 @@ describe('tracer should', () => {
     assertByFormat(opentracing.FORMAT_HTTP_HEADERS);
   });
 
+  it('inject plain text headers into carrier, and extract span context with the same value 128bits', () => {
+    let keyOne = 'keyOne';
+    let keyTwo = 'keyTwo';
+    let baggage = {
+      keyOne: 'leela',
+      keyTwo: 'bender',
+    };
+    let savedContext = SpanContext.withBinaryIds(
+      Buffer.concat([Utils.encodeInt64(1), Utils.encodeInt64(2)]),
+      Utils.encodeInt64(2),
+      Utils.encodeInt64(3),
+      constants.SAMPLED_MASK,
+      baggage
+    );
+
+    let assertByFormat = format => {
+      let carrier = {};
+      tracer.inject(savedContext, format, carrier);
+      let extractedContext = tracer.extract(format, carrier);
+
+      assert.deepEqual(savedContext.traceId, extractedContext.traceId);
+      assert.deepEqual(savedContext.spanId, extractedContext.spanId);
+      assert.deepEqual(savedContext.parentId, extractedContext.parentId);
+      assert.equal(savedContext.flags, extractedContext.flags);
+      assert.equal(savedContext.baggage[keyOne], extractedContext.baggage[keyOne]);
+      assert.equal(savedContext.baggage[keyTwo], extractedContext.baggage[keyTwo]);
+    };
+
+    assertByFormat(opentracing.FORMAT_TEXT_MAP);
+    assertByFormat(opentracing.FORMAT_HTTP_HEADERS);
+  });
+
   it('inject url encoded values into headers', () => {
     let baggage = {
       keyOne: 'Leela vs. Bender',
@@ -430,6 +462,35 @@ describe('tracer should', () => {
     });
   });
 
+  it('start a root span with 128 bit traceId', () => {
+    tracer = new Tracer('test-service-name', reporter, new ConstSampler(true), { traceId128bit: true });
+    let span = tracer.startSpan('test-name');
+
+    assert.deepEqual(span.context().traceId.slice(-8), span.context().spanId);
+    assert.equal(16, span.context().traceId.length);
+  });
+
+  it('preserve 64bit traceId even when in 128bit mode', () => {
+    // NB: because we currently trim leading zeros, this test is not as effective as it could be.
+    // But once https://github.com/jaegertracing/jaeger-client-node/issues/391 is fixed, this test
+    // will be more useful as it can catch regression.
+    tracer = new Tracer('test-service-name', reporter, new ConstSampler(true), { traceId128bit: true });
+    let span = tracer.startSpan('test-name');
+    assert.equal(16, span.context().traceId.length, 'new traces use 128bit IDs');
+
+    let parent = SpanContext.fromString('100:7f:0:1');
+    assert.equal(8, parent.traceId.length, 'respect 64bit length');
+
+    let child = tracer.startSpan('test-name', { childOf: parent });
+    assert.equal(8, child.context().traceId.length, 'preserve 64bit length');
+
+    let carrier = {};
+    tracer.inject(child.context(), opentracing.FORMAT_TEXT_MAP, carrier);
+    // Once https://github.com/jaegertracing/jaeger-client-node/issues/391 is fixed, the following
+    // asset will fail and will need to be changed to compare against '0000000000000100' string.
+    assert.equal('100:', carrier['uber-trace-id'].substring(0, 4), 'preserve 64bit length');
+  });
+
   it('should NOT mutate tags', () => {
     const tags = {
       robot: 'bender',

test/udp_sender.js

@@ -272,5 +272,5 @@ describe('udp sender', () => {
         tracer.close(done);
       }
     });
-  });
+  }).timeout(5000);
 });