diff --git a/src/reporters/udp_sender.js b/src/reporters/udp_sender.js index 999b01677..30bd7edc0 100644 --- a/src/reporters/udp_sender.js +++ b/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, diff --git a/src/span_context.js b/src/span_context.js index 14b35c799..13206e606 100644 --- a/src/span_context.js +++ b/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; } diff --git a/src/thrift.js b/src/thrift.js index c0d83a2d6..0769808a7 100644 --- a/src/thrift.js +++ b/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): Array { let thriftTags = []; @@ -104,8 +104,8 @@ 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, }); } @@ -113,14 +113,30 @@ export default class ThriftUtils { 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, diff --git a/src/tracer.js b/src/tracer.js index ef1e9d8c4..13b89f453 100644 --- a/src/tracer.js +++ b/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)) { diff --git a/src/util.js b/src/util.js index e8d61a3ec..e04faed7d 100644 --- a/src/util.js +++ b/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 callback * after limit invocations. Useful in types like CompositeReporter that diff --git a/test/http_sender.js b/test/http_sender.js index 91e29595d..f55160e7e 100644 --- a/test/http_sender.js +++ b/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. diff --git a/test/span_context.js b/test/span_context.js index c92a3923c..98a57dda2 100644 --- a/test/span_context.js +++ b/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'); diff --git a/test/thrift.js b/test/thrift.js index ec1b0f3cf..68b80499a 100644 --- a/test/thrift.js +++ b/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); + }); }); diff --git a/test/tracer.js b/test/tracer.js index 7a81b0eee..ed1a3ed3b 100644 --- a/test/tracer.js +++ b/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', diff --git a/test/udp_sender.js b/test/udp_sender.js index 659c7bb49..c7cac2459 100644 --- a/test/udp_sender.js +++ b/test/udp_sender.js @@ -272,5 +272,5 @@ describe('udp sender', () => { tracer.close(done); } }); - }); + }).timeout(5000); }); diff --git a/test/util.js b/test/util.js index d6d077de1..63907832c 100644 --- a/test/util.js +++ b/test/util.js @@ -44,4 +44,18 @@ describe('utils', () => { ]; assert.deepEqual(expectedTags, results); }); + + it('should create new empty buffer', () => { + let results = Utils.newBuffer(8); + assert.isNotNull(results); + assert.equal(results.length, 8); + assert.deepEqual(new Buffer([0, 0, 0, 0, 0, 0, 0, 0]), results); + }); + + it('should create new buffer from hex', () => { + let expectedValue = 'deadbeef'; + let results = Utils.newBufferFromHex(expectedValue); + assert.isNotNull(results); + assert.equal(expectedValue, results.toString('hex')); + }); });