Do not strip leading zeros

Fixes jaegertracing#391

Signed-off-by: Aleksei Androsov <doochik@ya.ru>

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
     );

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 {

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"

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));

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,15 +73,15 @@ 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),
       Utils.encodeInt64(127),
       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');

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', () => {

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'));
     });
   });
 

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);