-
Notifications
You must be signed in to change notification settings - Fork 131
Adding 128 bit traceId support #361
Changes from 4 commits
82f5a97
678561f
cd7cf05
0c23d6f
945237a
16e1d91
d5aa139
eb918d3
4ec48e6
a00710e
5059f19
69ac16c
50b49fe
cfd324c
4d4a573
1f02e9c
df66094
ec0374a
eee5ab6
fb29d69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ npm-debug.log | |
dist/ | ||
|
||
.idea/ | ||
.vscode/ | ||
*.iml | ||
|
||
crossdock/jaeger-docker-compose.yml |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,4 +20,5 @@ declare type startSpanOptions = { | |
references?: Array<Reference>, | ||
tags?: any, | ||
startTime?: number, | ||
traceId128bit?: boolean, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. didn't know that, I'll move it |
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,8 @@ import * as constants from './constants.js'; | |
import Utils from './util.js'; | ||
|
||
export default class SpanContext { | ||
_traceId: any; | ||
_traceIdLow: any; | ||
_traceIdHigh: any; | ||
_spanId: any; | ||
_parentId: any; | ||
_traceIdStr: ?string; | ||
|
@@ -43,7 +44,8 @@ export default class SpanContext { | |
_samplingFinalized: boolean; | ||
|
||
constructor( | ||
traceId: any, | ||
traceIdLow: any, | ||
traceIdHigh: any, | ||
spanId: any, | ||
parentId: any, | ||
traceIdStr: ?string, | ||
|
@@ -54,7 +56,8 @@ export default class SpanContext { | |
debugId: ?string = '', | ||
samplingFinalized: boolean = false | ||
) { | ||
this._traceId = traceId; | ||
this._traceIdLow = traceIdLow; | ||
this._traceIdHigh = traceIdHigh; | ||
this._spanId = spanId; | ||
this._parentId = parentId; | ||
this._traceIdStr = traceIdStr; | ||
|
@@ -66,11 +69,33 @@ export default class SpanContext { | |
this._samplingFinalized = samplingFinalized; | ||
} | ||
|
||
get traceId(): any { | ||
if (this._traceId == null && this._traceIdStr != null) { | ||
this._traceId = Utils.encodeInt64(this._traceIdStr); | ||
get traceIdLow(): any { | ||
this._tryParseTraceIdStr(); | ||
return this._traceIdLow; | ||
} | ||
|
||
get traceIdHigh(): any { | ||
this._tryParseTraceIdStr(); | ||
return this._traceIdHigh; | ||
} | ||
|
||
_tryParseTraceIdStr() { | ||
if (this._traceIdLow == null && this._traceIdHigh == null && this._traceIdStr != null) { | ||
if (this._traceIdStr.length > 16) { | ||
const safeTraceIdStr = this._traceIdStr.length % 2 == 0 ? this._traceIdStr : '0' + this._traceIdStr; | ||
const traceId = new Buffer(safeTraceIdStr, 'hex'); | ||
this._traceIdHigh = traceId.slice(-16, -8); | ||
if (this._traceIdHigh.length != 8) { | ||
const tmpBuffer = new Buffer(8); | ||
tmpBuffer.fill(0); | ||
this._traceIdHigh.copy(tmpBuffer, 8 - this._traceIdHigh.length); | ||
this._traceIdHigh = tmpBuffer; | ||
} | ||
this._traceIdLow = traceId.slice(-8); | ||
} else { | ||
this._traceIdLow = Utils.encodeInt64(this._traceIdStr); | ||
} | ||
} | ||
return this._traceId; | ||
} | ||
|
||
get spanId(): any { | ||
|
@@ -88,8 +113,14 @@ export default class SpanContext { | |
} | ||
|
||
get traceIdStr(): ?string { | ||
if (this._traceIdStr == null && this._traceId != null) { | ||
this._traceIdStr = Utils.removeLeadingZeros(this._traceId.toString('hex')); | ||
if (this._traceIdStr == null && this._traceIdLow != null) { | ||
if (this._traceIdHigh != null) { | ||
this._traceIdStr = Utils.removeLeadingZeros( | ||
this._traceIdHigh.toString('hex') + this._traceIdLow.toString('hex') | ||
); | ||
} else { | ||
this._traceIdStr = Utils.removeLeadingZeros(this._traceIdLow.toString('hex')); | ||
} | ||
} | ||
return this._traceIdStr; | ||
} | ||
|
@@ -124,8 +155,13 @@ export default class SpanContext { | |
return this._samplingFinalized; | ||
} | ||
|
||
set traceId(traceId: Buffer): void { | ||
this._traceId = traceId; | ||
set traceIdLow(traceIdLow: Buffer): void { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need to expose low/high fields explicitly? It seems like the Buffer type can represent the full ID as a single buffer, and do it transparently to the user. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I'll work on that |
||
this._traceIdLow = traceIdLow; | ||
this._traceIdStr = null; | ||
} | ||
|
||
set traceIdHigh(traceIdHigh: Buffer): void { | ||
this._traceIdHigh = traceIdHigh; | ||
this._traceIdStr = null; | ||
} | ||
|
||
|
@@ -152,7 +188,7 @@ export default class SpanContext { | |
} | ||
|
||
get isValid(): boolean { | ||
return !!((this._traceId || this._traceIdStr) && (this._spanId || this._spanIdStr)); | ||
return !!((this._traceIdLow || this._traceIdStr) && (this._spanId || this._spanIdStr)); | ||
} | ||
|
||
finalizeSampling(): void { | ||
|
@@ -181,7 +217,8 @@ export default class SpanContext { | |
let newBaggage = Utils.clone(this._baggage); | ||
newBaggage[key] = value; | ||
return new SpanContext( | ||
this._traceId, | ||
this._traceIdLow, | ||
this._traceIdHigh, | ||
this._spanId, | ||
this._parentId, | ||
this._traceIdStr, | ||
|
@@ -238,15 +275,17 @@ export default class SpanContext { | |
} | ||
|
||
static withBinaryIds( | ||
traceId: any, | ||
traceIdLow: any, | ||
traceIdHigh: any, | ||
spanId: any, | ||
parentId: any, | ||
flags: number, | ||
baggage: any = {}, | ||
debugId: ?string = '' | ||
): SpanContext { | ||
return new SpanContext( | ||
traceId, | ||
traceIdLow, | ||
traceIdHigh, | ||
spanId, | ||
parentId, | ||
null, // traceIdStr: string, | ||
|
@@ -267,7 +306,8 @@ export default class SpanContext { | |
debugId: ?string = '' | ||
): SpanContext { | ||
return new SpanContext( | ||
null, // traceId, | ||
null, // traceIdLow, | ||
null, // traceIdHigh, | ||
null, // spanId, | ||
null, // parentId, | ||
traceIdStr, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -188,6 +188,8 @@ export default class Tracer { | |
* the created Span object. The time should be specified in | ||
* milliseconds as Unix timestamp. Decimal value are supported | ||
* to represent time values with sub-millisecond accuracy. | ||
* @param {number} [options.traceId128bit] - generate root span with a | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this is a boolean based on the config and a test, so it should be |
||
* 128bit traceId. | ||
* @return {Span} - a new Span object. | ||
**/ | ||
startSpan(operationName: string, options: ?startSpanOptions): Span { | ||
|
@@ -239,12 +241,18 @@ export default class Tracer { | |
ctx.baggage = parent.baggage; | ||
} | ||
|
||
ctx.traceId = randomId; | ||
if (options.traceId128bit) { | ||
ctx.traceIdLow = randomId; | ||
ctx.traceIdHigh = Utils.getRandom64(); | ||
} else { | ||
ctx.traceIdLow = randomId; | ||
} | ||
ctx.spanId = randomId; | ||
ctx.parentId = null; | ||
ctx.flags = flags; | ||
} else { | ||
ctx.traceId = parent.traceId; | ||
ctx.traceIdLow = parent.traceIdLow; | ||
ctx.traceIdHigh = parent.traceIdHigh; | ||
ctx.spanId = Utils.getRandom64(); | ||
ctx.parentId = parent.spanId; | ||
ctx.flags = parent.flags; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,14 +24,31 @@ describe('SpanContext should', () => { | |
}); | ||
|
||
it('return given values as they were set', () => { | ||
let traceId = Utils.encodeInt64(1); | ||
let traceIdLow = Utils.encodeInt64(1); | ||
let spanId = Utils.encodeInt64(2); | ||
let parentId = Utils.encodeInt64(3); | ||
let flags = 1; | ||
|
||
let context = SpanContext.withBinaryIds(traceId, spanId, parentId, flags); | ||
let context = SpanContext.withBinaryIds(traceIdLow, null, spanId, parentId, flags); | ||
|
||
assert.deepEqual(traceId, context.traceId); | ||
assert.deepEqual(traceIdLow, context.traceIdLow); | ||
assert.deepEqual(spanId, context.spanId); | ||
assert.deepEqual(parentId, context.parentId); | ||
assert.equal(flags, context.flags); | ||
}); | ||
|
||
it('return given values as they were set 128 bit', () => { | ||
let traceIdLow = Utils.encodeInt64(1); | ||
let traceIdHigh = Utils.encodeInt64(2); | ||
let spanId = Utils.encodeInt64(3); | ||
let parentId = Utils.encodeInt64(4); | ||
let flags = 1; | ||
|
||
let context = SpanContext.withBinaryIds(traceIdLow, traceIdHigh, spanId, parentId, flags); | ||
|
||
assert.deepEqual(traceIdLow, context.traceIdLow); | ||
assert.deepEqual(traceIdHigh, context.traceIdHigh); | ||
assert.deepEqual('20000000000000001', context.traceIdStr); | ||
assert.deepEqual(spanId, context.spanId); | ||
assert.deepEqual(parentId, context.parentId); | ||
assert.equal(flags, context.flags); | ||
|
@@ -40,6 +57,7 @@ describe('SpanContext should', () => { | |
it('return IsSampled properly', () => { | ||
let context = SpanContext.withBinaryIds( | ||
Utils.encodeInt64(1), | ||
null, | ||
Utils.encodeInt64(2), | ||
Utils.encodeInt64(3), | ||
3 | ||
|
@@ -53,19 +71,20 @@ describe('SpanContext should', () => { | |
}); | ||
|
||
it('format strings properly with toString', () => { | ||
let ctx1 = SpanContext.withBinaryIds(Utils.encodeInt64(0x100), Utils.encodeInt64(0x7f), null, 1); | ||
let ctx1 = SpanContext.withBinaryIds(Utils.encodeInt64(0x100), null, Utils.encodeInt64(0x7f), null, 1); | ||
assert.equal(ctx1.toString(), '100:7f:0:1'); | ||
|
||
let ctx2 = SpanContext.withBinaryIds( | ||
Utils.encodeInt64(255 << 4), | ||
null, | ||
Utils.encodeInt64(127), | ||
Utils.encodeInt64(256), | ||
0 | ||
); | ||
assert.equal(ctx2.toString(), 'ff0:7f:100:0'); | ||
|
||
// test large numbers | ||
let ctx3 = SpanContext.withBinaryIds(LARGEST_64_BUFFER, LARGEST_64_BUFFER, LARGEST_64_BUFFER, 0); | ||
let ctx3 = SpanContext.withBinaryIds(LARGEST_64_BUFFER, null, LARGEST_64_BUFFER, LARGEST_64_BUFFER, 0); | ||
assert.equal(ctx3.toString(), 'ffffffffffffffff:ffffffffffffffff:ffffffffffffffff:0'); | ||
assert.equal('ffffffffffffffff', ctx3.traceIdStr); | ||
assert.equal('ffffffffffffffff', ctx3.spanIdStr); | ||
|
@@ -90,6 +109,28 @@ describe('SpanContext should', () => { | |
assert.equal(context.flags, 0x1); | ||
}); | ||
|
||
it('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(Utils.encodeInt64(0x100), context.traceIdLow); | ||
assert.deepEqual(Utils.encodeInt64(0x2), context.traceIdHigh); | ||
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(LARGEST_64_BUFFER, context.traceIdLow); | ||
assert.deepEqual(LARGEST_64_BUFFER, context.traceIdHigh); | ||
assert.deepEqual(LARGEST_64_BUFFER, context.spanId); | ||
assert.deepEqual(LARGEST_64_BUFFER, context.spanId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is the same as the previous line. |
||
assert.deepEqual(Utils.encodeInt64(0x5), context.parentId); | ||
assert.equal(context.flags, 0x1); | ||
}); | ||
|
||
it('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'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this for, and it is indented correctly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latest version of node seems to be unstable at this time, so I added the lts version to the build and move the latest to the 'allow_failures' sections which will still build it but won't fail the entire build if it fails
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes total sense, thanks