diff --git a/src/span_context.js b/src/span_context.js index 13206e606..df9f09c65 100644 --- a/src/span_context.js +++ b/src/span_context.js @@ -53,19 +53,7 @@ export default class SpanContext { get traceId(): any { if (this._traceId == null && this._traceIdStr != null) { - // 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); - } + this._traceId = Utils.newBufferFromHex(this._traceIdStr); } return this._traceId; } @@ -86,21 +74,21 @@ export default class SpanContext { get traceIdStr(): ?string { if (this._traceIdStr == null && this._traceId != null) { - this._traceIdStr = Utils.removeLeadingZeros(this._traceId.toString('hex')); + this._traceIdStr = this._traceId.toString('hex'); } return this._traceIdStr; } get spanIdStr(): ?string { if (this._spanIdStr == null && this._spanId != null) { - this._spanIdStr = Utils.removeLeadingZeros(this._spanId.toString('hex')); + this._spanIdStr = this._spanId.toString('hex'); } return this._spanIdStr; } get parentIdStr(): ?string { if (this._parentIdStr == null && this._parentId != null) { - this._parentIdStr = Utils.removeLeadingZeros(this._parentId.toString('hex')); + this._parentIdStr = this._parentId.toString('hex'); } return this._parentIdStr; } @@ -310,9 +298,9 @@ export default class SpanContext { null, // traceId, null, // spanId, null, // parentId, - traceIdStr, - spanIdStr, - parentIdStr, + Utils.padTraceIdStrWithZeros(traceIdStr), + Utils.padTraceIdStrWithZeros(spanIdStr), + Utils.padTraceIdStrWithZeros(parentIdStr), baggage, debugId ); diff --git a/src/util.js b/src/util.js index e04faed7d..72f26f737 100644 --- a/src/util.js +++ b/src/util.js @@ -82,21 +82,26 @@ export default class Utils { } /** - * @param {string} input - the input for which leading zeros should be removed. - * @return {string} - returns the input string without leading zeros. + * @param {string} input - trace id (e.g. traceIdStr) for which leading zeros should be added. + * @return {string} - returns the id string 32 or 16 characters long. **/ - static removeLeadingZeros(input: string): string { - let counter = 0; - let length = input.length - 1; - for (let i = 0; i < length; i++) { - if (input.charAt(i) === '0') { - counter++; - } else { - break; - } + static padTraceIdStrWithZeros(input: string) { + if (!input) { + return input; } - return input.substring(counter); + // 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 = input.length > 16 ? 32 : 16; + if (input.length === traceIdExactLength) { + return input; + } else { + const padding = traceIdExactLength === 16 ? '0000000000000000' : '00000000000000000000000000000000'; + return (padding + input).slice(-traceIdExactLength); + } } static myIp(): string { diff --git a/test/propagators.js b/test/propagators.js index d45f6deeb..4f0dde8e1 100644 --- a/test/propagators.js +++ b/test/propagators.js @@ -36,7 +36,7 @@ describe('TextMapCodec', () => { let ctx = SpanContext.fromString('1:1:1:1'); let out = {}; codec.inject(ctx, out); - assert.strictEqual(out['trace-context'], '1:1:1:1'); + assert.strictEqual(out['trace-context'], '0000000000000001:0000000000000001:0000000000000001:1'); }); it('should decode baggage', () => { @@ -69,9 +69,9 @@ describe('ZipkinB3TextMapCodec', () => { }; let ctx = codec.extract(carrier); - assert.equal(ctx.parentIdStr, '123abc'); - assert.equal(ctx.spanIdStr, 'aaafff'); - assert.equal(ctx.traceIdStr, '789fed'); + assert.equal(ctx.parentIdStr, '0000000000123abc'); + assert.equal(ctx.spanIdStr, '0000000000aaafff'); + assert.equal(ctx.traceIdStr, '0000000000789fed'); assert.equal(ctx.isSampled(), true); assert.equal(ctx.isDebug(), true); }); @@ -120,9 +120,9 @@ describe('ZipkinB3TextMapCodec', () => { ctx.flags = constants.DEBUG_MASK | constants.SAMPLED_MASK; codec.inject(ctx, carrier); - assert.equal(carrier['x-b3-traceid'], '789fed'); - assert.equal(carrier['x-b3-spanid'], 'aaafff'); - assert.equal(carrier['x-b3-parentspanid'], '123abc'); + assert.equal(carrier['x-b3-traceid'], '0000000000789fed'); + assert.equal(carrier['x-b3-spanid'], '0000000000aaafff'); + assert.equal(carrier['x-b3-parentspanid'], '0000000000123abc'); assert.equal(carrier['x-b3-flags'], '1'); // > Since Debug implies Sampled, so don't also send "X-B3-Sampled: 1" diff --git a/test/samplers/sampling_state.js b/test/samplers/sampling_state.js index 07e79414b..86f376f97 100644 --- a/test/samplers/sampling_state.js +++ b/test/samplers/sampling_state.js @@ -23,8 +23,8 @@ describe('SamplingState', () => { assert.equal('dark force', s.extendedState()['sithlord']['something, something']); }); it('should recognize local root span', () => { - let s = new SamplingState('id123'); - assert.equal('id123', s.localRootSpanId()); + let s = new SamplingState('00000000000id123'); + assert.equal('00000000000id123', s.localRootSpanId()); let ctx1 = SpanContext.withStringIds('', 'id123', null, 0); let ctx2 = SpanContext.withStringIds('', 'id12345', null, 0); assert.equal(true, s.isLocalRootSpan(ctx1)); diff --git a/test/span_context.js b/test/span_context.js index 98a57dda2..d50f45aab 100644 --- a/test/span_context.js +++ b/test/span_context.js @@ -50,7 +50,7 @@ describe('SpanContext', () => { let context = SpanContext.withBinaryIds(traceId, spanId, parentId, flags); assert.deepEqual(traceId, context.traceId); - assert.deepEqual('20000000000000001', context.traceIdStr); + assert.deepEqual('00000000000000020000000000000001', context.traceIdStr); assert.deepEqual(spanId, context.spanId); assert.deepEqual(parentId, context.parentId); assert.equal(flags, context.flags); @@ -73,7 +73,7 @@ describe('SpanContext', () => { it('should format strings properly with toString', () => { let ctx1 = SpanContext.withBinaryIds(Utils.encodeInt64(0x100), Utils.encodeInt64(0x7f), null, 1); - assert.equal(ctx1.toString(), '100:7f:0:1'); + assert.equal(ctx1.toString(), '0000000000000100:000000000000007f:0:1'); let ctx2 = SpanContext.withBinaryIds( Utils.encodeInt64(255 << 4), @@ -81,7 +81,7 @@ describe('SpanContext', () => { Utils.encodeInt64(256), 0 ); - assert.equal(ctx2.toString(), 'ff0:7f:100:0'); + assert.equal(ctx2.toString(), '0000000000000ff0:000000000000007f:0000000000000100:0'); // test large numbers let ctx3 = SpanContext.withBinaryIds(LARGEST_64_BUFFER, LARGEST_64_BUFFER, LARGEST_64_BUFFER, 0); @@ -94,7 +94,7 @@ describe('SpanContext', () => { it('should turn properly formatted strings into correct span contexts', () => { let context = SpanContext.fromString('100:7f:0:1'); - assert.deepEqual('100', context.traceIdStr); + assert.deepEqual('0000000000000100', context.traceIdStr); assert.deepEqual(Utils.encodeInt64(0x100), context.traceId); assert.deepEqual(Utils.encodeInt64(0x7f), context.spanId); assert.equal(null, context.parentId); @@ -113,7 +113,7 @@ describe('SpanContext', () => { 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('00000000000000020000000000000100', 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); @@ -129,6 +129,12 @@ describe('SpanContext', () => { assert.equal(context.flags, 0x1); }); + it('should parse string ids with stripped leading zero', () => { + const ctx = SpanContext.withStringIds('ff0', '7f', '100', 0); + + assert.equal(ctx.toString(), '0000000000000ff0:000000000000007f:0000000000000100:0'); + }); + 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/tracer.js b/test/tracer.js index ed1a3ed3b..27c705e53 100644 --- a/test/tracer.js +++ b/test/tracer.js @@ -51,10 +51,10 @@ describe('tracer should', () => { }; let mycontext = mytracer.extract(opentracing.FORMAT_HTTP_HEADERS, headers); - assert.equal(mycontext.toString(), headers[ck]); + assert.equal(mycontext.toString(), '000000000000000a:000000000000000b:000000000000000c:d'); let myspan = mytracer.startSpan('myspan', { childOf: mycontext }); - assert.equal(myspan.context().traceIdStr, 'a'); + assert.equal(myspan.context().traceIdStr, '000000000000000a'); let exheaders = {}; @@ -256,10 +256,10 @@ describe('tracer should', () => { headers[ck] = 'a:b:c:d'; let mycontext = mytracer.extract(opentracing.FORMAT_HTTP_HEADERS, headers); - assert.equal(mycontext.toString(), headers[ck]); + assert.equal(mycontext.toString(), '000000000000000a:000000000000000b:000000000000000c:d'); let myspan = mytracer.startSpan('myspan', { childOf: mycontext }); - assert.equal(myspan.context().traceIdStr, 'a'); + assert.equal(myspan.context().traceIdStr, '000000000000000a'); let exheaders = Object.create(null); @@ -488,7 +488,7 @@ describe('tracer should', () => { 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'); + assert.equal('0000000000000100:', carrier['uber-trace-id'].substring(0, 17), 'preserve 64bit length'); }); it('should NOT mutate tags', () => { diff --git a/test/util.js b/test/util.js index 63907832c..e3cc6ef98 100644 --- a/test/util.js +++ b/test/util.js @@ -16,21 +16,28 @@ import Utils from '../src/util.js'; import combinations from './lib/combinations.js'; describe('utils', () => { - describe('removeLeadingZeros', () => { - it('should leave single 0 digit intact', () => { - assert.equal('0', Utils.removeLeadingZeros('0')); + describe('padTraceIdStrWithZeros', () => { + it('should return empty string for empty input', () => { + assert.equal('', Utils.padTraceIdStrWithZeros('')); }); - it('should leave single non-0 digit intact', () => { - assert.equal('1', Utils.removeLeadingZeros('1')); + it('should not pad string 16 characters long', () => { + assert.equal('0123456789abcdef', Utils.padTraceIdStrWithZeros('0123456789abcdef')); }); - it('should strip leading zeros', () => { - assert.equal('1', Utils.removeLeadingZeros('0001')); + it('should not pad string 32 characters long', () => { + assert.equal( + '0123456789abcdef0123456789abcdef', + Utils.padTraceIdStrWithZeros('0123456789abcdef0123456789abcdef') + ); }); - it('should convert all zeros to a single 0', () => { - assert.equal('0', Utils.removeLeadingZeros('0000')); + it('should pad string <16 characters long', () => { + assert.equal('0000000000000123', Utils.padTraceIdStrWithZeros('123')); + }); + + it('should pad string <32 characters long', () => { + assert.equal('0000000000000000123456789abcdef0', Utils.padTraceIdStrWithZeros('0123456789abcdef0')); }); }); diff --git a/test/zipkin_b3_text_map_codec.js b/test/zipkin_b3_text_map_codec.js index bac88380b..1c868f147 100644 --- a/test/zipkin_b3_text_map_codec.js +++ b/test/zipkin_b3_text_map_codec.js @@ -108,9 +108,9 @@ describe('Zipkin B3 Text Map Codec should', () => { const context = tracer.extract(opentracing.FORMAT_HTTP_HEADERS, headers); - assert.equal('123abc', context.traceIdStr); - assert.equal('456def', context.spanIdStr); - assert.equal('789ghi', context.parentIdStr); + assert.equal('0000000000123abc', context.traceIdStr); + assert.equal('0000000000456def', context.spanIdStr); + assert.equal('0000000000789ghi', context.parentIdStr); assert.isTrue(context.isSampled()); assert.isTrue(context.isDebug()); assert.equal('678pqr', context.debugId);