From c1179b1cb041a4905a5907b255c476261d91fb7c Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 15:00:21 -0600 Subject: [PATCH 01/13] Update the zipkinCore thrift to latest from https://github.com/openzipkin/zipkin/blob/master/zipkin-thrift/src/main/thrift/com/twitter/zipkin/zipkinCore.thrift New fields (Span.debug, Annotation.duration) are optional, all tests still passing --- lib/_thrift/zipkinCore.thrift | 36 ++++++----- lib/_thrift/zipkinCore/zipkinCore_types.js | 75 ++++++++++++++++------ 2 files changed, 74 insertions(+), 37 deletions(-) diff --git a/lib/_thrift/zipkinCore.thrift b/lib/_thrift/zipkinCore.thrift index cac827d..50ea914 100644 --- a/lib/_thrift/zipkinCore.thrift +++ b/lib/_thrift/zipkinCore.thrift @@ -11,29 +11,31 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -namespace java com.twitter.zipkin.gen +namespace java com.twitter.zipkin.thriftjava +#@namespace scala com.twitter.zipkin.thriftscala namespace rb Zipkin -//************** Collection related structs ************** +#************** Collection related structs ************** -// these are the annotations we always expect to find in a span +# these are the annotations we always expect to find in a span const string CLIENT_SEND = "cs" const string CLIENT_RECV = "cr" const string SERVER_SEND = "ss" const string SERVER_RECV = "sr" -// this represents a host and port in a network +# this represents a host and port in a network struct Endpoint { 1: i32 ipv4, - 2: i16 port // beware that this will give us negative ports. some conversion needed - 3: string service_name // which service did this operation happen on? + 2: i16 port # beware that this will give us negative ports. some conversion needed + 3: string service_name # which service did this operation happen on? } -// some event took place, either one by the framework or by the user +# some event took place, either one by the framework or by the user struct Annotation { - 1: i64 timestamp // microseconds from epoch - 2: string value // what happened at the timestamp? - 3: optional Endpoint host // host this happened on + 1: i64 timestamp # microseconds from epoch + 2: string value # what happened at the timestamp? + 3: optional Endpoint host # host this happened on + 4: optional i32 duration # how long did the operation take? microseconds } enum AnnotationType { BOOL, BYTES, I16, I32, I64, DOUBLE, STRING } @@ -46,10 +48,12 @@ struct BinaryAnnotation { } struct Span { - 1: i64 trace_id // unique trace id, use for all spans in trace - 3: string name, // span name, rpc method for example - 4: i64 id, // unique span id, only used for this span - 5: optional i64 parent_id, // parent span id - 6: list annotations, // list of all annotations/events that occured - 8: list binary_annotations // any binary annotations + 1: i64 trace_id # unique trace id, use for all spans in trace + 3: string name, # span name, rpc method for example + 4: i64 id, # unique span id, only used for this span + 5: optional i64 parent_id, # parent span id + 6: list annotations, # list of all annotations/events that occured + 8: list binary_annotations # any binary annotations + 9: optional bool debug = 0 # if true, we DEMAND that this span passes all samplers } + diff --git a/lib/_thrift/zipkinCore/zipkinCore_types.js b/lib/_thrift/zipkinCore/zipkinCore_types.js index a5369b1..6b9a0e0 100644 --- a/lib/_thrift/zipkinCore/zipkinCore_types.js +++ b/lib/_thrift/zipkinCore/zipkinCore_types.js @@ -1,9 +1,10 @@ // -// Autogenerated by Thrift Compiler (0.8.0) +// Autogenerated by Thrift Compiler (0.9.1) // // DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING // var Thrift = require('thrift').Thrift; + var ttypes = module.exports = {}; ttypes.AnnotationType = { 'BOOL' : 0, @@ -14,7 +15,7 @@ ttypes.AnnotationType = { 'DOUBLE' : 5, 'STRING' : 6 }; -var Endpoint = module.exports.Endpoint = function(args) { +Endpoint = module.exports.Endpoint = function(args) { this.ipv4 = null; this.port = null; this.service_name = null; @@ -76,17 +77,17 @@ Endpoint.prototype.read = function(input) { Endpoint.prototype.write = function(output) { output.writeStructBegin('Endpoint'); - if (this.ipv4) { + if (this.ipv4 !== null && this.ipv4 !== undefined) { output.writeFieldBegin('ipv4', Thrift.Type.I32, 1); output.writeI32(this.ipv4); output.writeFieldEnd(); } - if (this.port) { + if (this.port !== null && this.port !== undefined) { output.writeFieldBegin('port', Thrift.Type.I16, 2); output.writeI16(this.port); output.writeFieldEnd(); } - if (this.service_name) { + if (this.service_name !== null && this.service_name !== undefined) { output.writeFieldBegin('service_name', Thrift.Type.STRING, 3); output.writeString(this.service_name); output.writeFieldEnd(); @@ -96,10 +97,11 @@ Endpoint.prototype.write = function(output) { return; }; -var Annotation = module.exports.Annotation = function(args) { +Annotation = module.exports.Annotation = function(args) { this.timestamp = null; this.value = null; this.host = null; + this.duration = null; if (args) { if (args.timestamp !== undefined) { this.timestamp = args.timestamp; @@ -110,6 +112,9 @@ var Annotation = module.exports.Annotation = function(args) { if (args.host !== undefined) { this.host = args.host; } + if (args.duration !== undefined) { + this.duration = args.duration; + } } }; Annotation.prototype = {}; @@ -148,6 +153,13 @@ Annotation.prototype.read = function(input) { input.skip(ftype); } break; + case 4: + if (ftype == Thrift.Type.I32) { + this.duration = input.readI32(); + } else { + input.skip(ftype); + } + break; default: input.skip(ftype); } @@ -159,27 +171,32 @@ Annotation.prototype.read = function(input) { Annotation.prototype.write = function(output) { output.writeStructBegin('Annotation'); - if (this.timestamp) { + if (this.timestamp !== null && this.timestamp !== undefined) { output.writeFieldBegin('timestamp', Thrift.Type.I64, 1); output.writeI64(this.timestamp); output.writeFieldEnd(); } - if (this.value) { + if (this.value !== null && this.value !== undefined) { output.writeFieldBegin('value', Thrift.Type.STRING, 2); output.writeString(this.value); output.writeFieldEnd(); } - if (this.host) { + if (this.host !== null && this.host !== undefined) { output.writeFieldBegin('host', Thrift.Type.STRUCT, 3); this.host.write(output); output.writeFieldEnd(); } + if (this.duration !== null && this.duration !== undefined) { + output.writeFieldBegin('duration', Thrift.Type.I32, 4); + output.writeI32(this.duration); + output.writeFieldEnd(); + } output.writeFieldStop(); output.writeStructEnd(); return; }; -var BinaryAnnotation = module.exports.BinaryAnnotation = function(args) { +BinaryAnnotation = module.exports.BinaryAnnotation = function(args) { this.key = null; this.value = null; this.annotation_type = null; @@ -253,22 +270,22 @@ BinaryAnnotation.prototype.read = function(input) { BinaryAnnotation.prototype.write = function(output) { output.writeStructBegin('BinaryAnnotation'); - if (this.key) { + if (this.key !== null && this.key !== undefined) { output.writeFieldBegin('key', Thrift.Type.STRING, 1); output.writeString(this.key); output.writeFieldEnd(); } - if (this.value) { + if (this.value !== null && this.value !== undefined) { output.writeFieldBegin('value', Thrift.Type.STRING, 2); output.writeString(this.value); output.writeFieldEnd(); } - if (this.annotation_type) { + if (this.annotation_type !== null && this.annotation_type !== undefined) { output.writeFieldBegin('annotation_type', Thrift.Type.I32, 3); output.writeI32(this.annotation_type); output.writeFieldEnd(); } - if (this.host) { + if (this.host !== null && this.host !== undefined) { output.writeFieldBegin('host', Thrift.Type.STRUCT, 4); this.host.write(output); output.writeFieldEnd(); @@ -278,13 +295,14 @@ BinaryAnnotation.prototype.write = function(output) { return; }; -var Span = module.exports.Span = function(args) { +Span = module.exports.Span = function(args) { this.trace_id = null; this.name = null; this.id = null; this.parent_id = null; this.annotations = null; this.binary_annotations = null; + this.debug = false; if (args) { if (args.trace_id !== undefined) { this.trace_id = args.trace_id; @@ -304,6 +322,9 @@ var Span = module.exports.Span = function(args) { if (args.binary_annotations !== undefined) { this.binary_annotations = args.binary_annotations; } + if (args.debug !== undefined) { + this.debug = args.debug; + } } }; Span.prototype = {}; @@ -390,6 +411,13 @@ Span.prototype.read = function(input) { input.skip(ftype); } break; + case 9: + if (ftype == Thrift.Type.BOOL) { + this.debug = input.readBool(); + } else { + input.skip(ftype); + } + break; default: input.skip(ftype); } @@ -401,27 +429,27 @@ Span.prototype.read = function(input) { Span.prototype.write = function(output) { output.writeStructBegin('Span'); - if (this.trace_id) { + if (this.trace_id !== null && this.trace_id !== undefined) { output.writeFieldBegin('trace_id', Thrift.Type.I64, 1); output.writeI64(this.trace_id); output.writeFieldEnd(); } - if (this.name) { + if (this.name !== null && this.name !== undefined) { output.writeFieldBegin('name', Thrift.Type.STRING, 3); output.writeString(this.name); output.writeFieldEnd(); } - if (this.id) { + if (this.id !== null && this.id !== undefined) { output.writeFieldBegin('id', Thrift.Type.I64, 4); output.writeI64(this.id); output.writeFieldEnd(); } - if (this.parent_id) { + if (this.parent_id !== null && this.parent_id !== undefined) { output.writeFieldBegin('parent_id', Thrift.Type.I64, 5); output.writeI64(this.parent_id); output.writeFieldEnd(); } - if (this.annotations) { + if (this.annotations !== null && this.annotations !== undefined) { output.writeFieldBegin('annotations', Thrift.Type.LIST, 6); output.writeListBegin(Thrift.Type.STRUCT, this.annotations.length); for (var iter14 in this.annotations) @@ -435,7 +463,7 @@ Span.prototype.write = function(output) { output.writeListEnd(); output.writeFieldEnd(); } - if (this.binary_annotations) { + if (this.binary_annotations !== null && this.binary_annotations !== undefined) { output.writeFieldBegin('binary_annotations', Thrift.Type.LIST, 8); output.writeListBegin(Thrift.Type.STRUCT, this.binary_annotations.length); for (var iter15 in this.binary_annotations) @@ -449,6 +477,11 @@ Span.prototype.write = function(output) { output.writeListEnd(); output.writeFieldEnd(); } + if (this.debug !== null && this.debug !== undefined) { + output.writeFieldBegin('debug', Thrift.Type.BOOL, 9); + output.writeBool(this.debug); + output.writeFieldEnd(); + } output.writeFieldStop(); output.writeStructEnd(); return; From a7d3deabb7da96baf107fc95cc6c6d57996efc9e Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 15:44:17 -0600 Subject: [PATCH 02/13] added failing tests to make sure optional debug parameter is being set on the Trace and being passed down into the child spans as well. Updated existing Trace tests to make sure that the debug property stays undefined when not explicitly set to true --- tests/test_trace.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/test_trace.js b/tests/test_trace.js index fe6a3e3..3082c9e 100644 --- a/tests/test_trace.js +++ b/tests/test_trace.js @@ -76,6 +76,15 @@ module.exports = { var t = new trace.Trace('test_trace'); test.equal(t.name, 'test_trace'); test.equal(t.parentSpanId, undefined); + test.equal(t.debug, undefined); + assert_is_valid_trace(test, t); + test.done(); + }, + test_new_trace_debug: function(test){ + var t = new trace.Trace('test_trace', {debug: true}); + test.equal(t.name, 'test_trace'); + test.equal(t.parentSpanId, undefined); + test.equal(t.debug, true); assert_is_valid_trace(test, t); test.done(); }, @@ -84,6 +93,15 @@ module.exports = { var c = t.child('child_test_trace'); test.equal(c.traceId, 1); test.equal(c.parentSpanId, 1); + test.equal(c.debug, undefined); + test.done(); + }, + test_trace_child_debug: function(test){ + var t = new trace.Trace('test_trace', {traceId: 1, spanId: 1, debug: true}); + var c = t.child('child_test_trace'); + test.equal(c.traceId, 1); + test.equal(c.parentSpanId, 1); + test.equal(c.debug, true); test.done(); }, test_trace_child_passes_tracers: function(test){ From 4062b44d108a6b976293eefbe2540800509c73b0 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 15:45:18 -0600 Subject: [PATCH 03/13] optional debug option is being set on the Trace and passed down to child spans. All tests passing again --- lib/trace.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/trace.js b/lib/trace.js index 381b8d6..dfc5900 100644 --- a/lib/trace.js +++ b/lib/trace.js @@ -76,6 +76,10 @@ options = options || {}; + if (options.debug === true) { + this.debug = true + } + ['traceId', 'spanId', 'parentSpanId'].forEach(function(idName) { if (has(options, idName) && options[idName] !== null && options[idName] !== undefined) { @@ -107,6 +111,7 @@ var optionalArgs = { traceId: this.traceId, parentSpanId: this.spanId, + debug: this.debug, tracers: this._tracers }; var trace = new Trace(name, optionalArgs); From 765697431fd888f482a9e3897318a197470f8d49 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 15:49:36 -0600 Subject: [PATCH 04/13] restkin formatter now includes debug property on the span if its defined --- lib/formatters.js | 4 ++++ tests/test_formatters.js | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/formatters.js b/lib/formatters.js index 5329b87..f21205d 100644 --- a/lib/formatters.js +++ b/lib/formatters.js @@ -57,6 +57,10 @@ if (trace.parentSpanId !== undefined) { restkinTrace.parent_span_id = trace.parentSpanId; } + + if (trace.debug !== undefined) { + restkinTrace.debug = trace.debug; + } Array.prototype.forEach.call(annotations, function(ann, idx) { annJson = { diff --git a/tests/test_formatters.js b/tests/test_formatters.js index 996a160..04108cf 100644 --- a/tests/test_formatters.js +++ b/tests/test_formatters.js @@ -32,6 +32,11 @@ var testcases = { annotations: [new trace.Annotation.timestamp('name1', 1), new trace.Annotation.string('name2', '2')] }, + trace_with_optional_params: { + trace: new trace.Trace('test', {spanId: 10, traceId:1, debug: true}), + annotations: [new trace.Annotation.timestamp('name1', 1), + new trace.Annotation.string('name2', '2')] + }, trace_with_parentSpanId: { trace: new trace.Trace('test', {parentSpanId:5, spanId: 10, traceId:1}), annotations: [] @@ -98,6 +103,26 @@ module.exports = { ] }]); }, + test_trace_with_optional_params: function (test) { + testRestkinFormatter(test, testcases.trace_with_optional_params, [{ + trace_id: '0000000000000001', + span_id: '000000000000000a', + name: 'test', + debug: true, + annotations: [ + { + key: 'name1', + value: 1, + type: 'timestamp' + }, + { + key: 'name2', + value: '2', + type: 'string' + } + ] + }]); + }, test_trace_with_parentSpanId: function(test){ testRestkinFormatter(test, testcases.trace_with_parentSpanId, [{ trace_id: '0000000000000001', From ddbe571b5ae908ea5a45d501456a3aa9c4a07ca9 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 15:58:28 -0600 Subject: [PATCH 05/13] zipkin formatter will write the Span.debug property if defined --- lib/formatters.js | 1 + tests/test_formatters.js | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/formatters.js b/lib/formatters.js index f21205d..6ee53ca 100644 --- a/lib/formatters.js +++ b/lib/formatters.js @@ -181,6 +181,7 @@ name: trace.name, id: trace.spanId, parent_id: trace.parentSpanId, + debug: trace.debug, annotations: thriftAnn, binary_annotations: binaryAnn }); diff --git a/tests/test_formatters.js b/tests/test_formatters.js index 04108cf..7f66774 100644 --- a/tests/test_formatters.js +++ b/tests/test_formatters.js @@ -172,6 +172,25 @@ module.exports = { })] })); }, + test_trace_with_optional_params: function(test){ + testZipkinFormatter( + test, testcases.trace_with_optional_params, + new zipkinCore_types.Span({ + trace_id: new Int64(1), + id: new Int64(10), + name: 'test', + debug: true, + annotations: [ new zipkinCore_types.Annotation({ + timestamp: new Int64(1), + value: 'name1' + })], + binary_annotations: [ new zipkinCore_types.BinaryAnnotation({ + value: '2', + key: 'name2', + annotation_type: zipkinCore_types.AnnotationType.STRING + })] + })); + }, test_trace_with_parentSpanId: function(test){ testZipkinFormatter( test, testcases.trace_with_parentSpanId, From 83f6652b5ba099b06e9ff450346180386f9753d9 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 21:23:01 -0600 Subject: [PATCH 06/13] compile with thrift version 9.2 Fix up the leaked global variables --- lib/_thrift/zipkinCore/zipkinCore_types.js | 33 ++++++++++++---------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/_thrift/zipkinCore/zipkinCore_types.js b/lib/_thrift/zipkinCore/zipkinCore_types.js index 6b9a0e0..b69a8fd 100644 --- a/lib/_thrift/zipkinCore/zipkinCore_types.js +++ b/lib/_thrift/zipkinCore/zipkinCore_types.js @@ -1,21 +1,24 @@ // -// Autogenerated by Thrift Compiler (0.9.1) +// Autogenerated by Thrift Compiler (0.9.2) // // DO NOT EDIT UNLESS YOU ARE SURE THAT YOU KNOW WHAT YOU ARE DOING // -var Thrift = require('thrift').Thrift; +var thrift = require('thrift'); +var Thrift = thrift.Thrift; +var Q = thrift.Q; + var ttypes = module.exports = {}; ttypes.AnnotationType = { -'BOOL' : 0, -'BYTES' : 1, -'I16' : 2, -'I32' : 3, -'I64' : 4, -'DOUBLE' : 5, -'STRING' : 6 + 'BOOL' : 0, + 'BYTES' : 1, + 'I16' : 2, + 'I32' : 3, + 'I64' : 4, + 'DOUBLE' : 5, + 'STRING' : 6 }; -Endpoint = module.exports.Endpoint = function(args) { +var Endpoint = module.exports.Endpoint = function(args) { this.ipv4 = null; this.port = null; this.service_name = null; @@ -97,7 +100,7 @@ Endpoint.prototype.write = function(output) { return; }; -Annotation = module.exports.Annotation = function(args) { +var Annotation = module.exports.Annotation = function(args) { this.timestamp = null; this.value = null; this.host = null; @@ -196,7 +199,7 @@ Annotation.prototype.write = function(output) { return; }; -BinaryAnnotation = module.exports.BinaryAnnotation = function(args) { +var BinaryAnnotation = module.exports.BinaryAnnotation = function(args) { this.key = null; this.value = null; this.annotation_type = null; @@ -239,7 +242,7 @@ BinaryAnnotation.prototype.read = function(input) { break; case 2: if (ftype == Thrift.Type.STRING) { - this.value = input.readString(); + this.value = input.readBinary(); } else { input.skip(ftype); } @@ -277,7 +280,7 @@ BinaryAnnotation.prototype.write = function(output) { } if (this.value !== null && this.value !== undefined) { output.writeFieldBegin('value', Thrift.Type.STRING, 2); - output.writeString(this.value); + output.writeBinary(this.value); output.writeFieldEnd(); } if (this.annotation_type !== null && this.annotation_type !== undefined) { @@ -295,7 +298,7 @@ BinaryAnnotation.prototype.write = function(output) { return; }; -Span = module.exports.Span = function(args) { +var Span = module.exports.Span = function(args) { this.trace_id = null; this.name = null; this.id = null; From 4ade9196e505a6ef558bb536a31db6723724d076 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 22:00:23 -0600 Subject: [PATCH 07/13] fix missing semi-colon --- lib/trace.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/trace.js b/lib/trace.js index dfc5900..c098f14 100644 --- a/lib/trace.js +++ b/lib/trace.js @@ -77,7 +77,7 @@ options = options || {}; if (options.debug === true) { - this.debug = true + this.debug = true; } ['traceId', 'spanId', 'parentSpanId'].forEach(function(idName) { From d29531580a840383e70c28e03ad42873ba97b951 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 22:01:33 -0600 Subject: [PATCH 08/13] as of node-thrift version 9.2, all binary types are read in as node Buffers. Convert the binary annotation values into strings for test comparisons --- tests/test_formatters.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_formatters.js b/tests/test_formatters.js index 7f66774..666e431 100644 --- a/tests/test_formatters.js +++ b/tests/test_formatters.js @@ -73,6 +73,11 @@ var testZipkinFormatter = function (test, testcase, expected) { var prot = new tprotocol.TBinaryProtocol(trans); var span = new zipkinCore_types.Span(); span.read(prot); + _.each(span.binary_annotations, function (annotation) { + if (annotation.value) { + annotation.value = annotation.value.toString(); + } + }); test.deepEqual(span, expected); test.done(); }); From be9bcbd067945982934b89409be0a59c0889b827 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 22:30:20 -0600 Subject: [PATCH 09/13] allow optional duration argument to be set on timestamp Annotations --- lib/trace.js | 11 ++++++++--- tests/test_trace.js | 7 ++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/trace.js b/lib/trace.js index c098f14..e6874dc 100644 --- a/lib/trace.js +++ b/lib/trace.js @@ -298,11 +298,12 @@ * zipkinCore_types.CLIENT_RECV, * or String} annotationType * @param {Endpoint} endpoint + * @param {number} duration of the event in microseconds (timestamp only) - optional * * NOTE: If the annotationType is NOT a timestamp, the value must be a string * (for now) */ - Annotation = function(name, value, annotationType, endpoint) { + Annotation = function(name, value, annotationType, endpoint, duration) { var self = this; self.name = name; self.value = value; @@ -310,6 +311,9 @@ if (endpoint !== undefined) { self.endpoint = endpoint; } + if (duration !== undefined) { + self.duration = duration; + } }; /** @@ -318,12 +322,13 @@ * @param {String} name The name of this timestamp * @param {number} timestamp The time of the event in microseconds since the * epoch (UTC) - optional + * @param {number} duration of the event in microseconds - optional */ - Annotation.timestamp = function (name, timestamp) { + Annotation.timestamp = function (name, timestamp, duration) { if (timestamp === undefined) { timestamp = getNowMicros(); } - return new Annotation(name, timestamp, 'timestamp'); + return new Annotation(name, timestamp, 'timestamp', undefined, duration); }; /** diff --git a/tests/test_trace.js b/tests/test_trace.js index 3082c9e..3591678 100644 --- a/tests/test_trace.js +++ b/tests/test_trace.js @@ -55,10 +55,11 @@ var assert_is_valid_trace = function(test, t) { }; // generate an Annotation test -var runAnnotationTest = function(test, ann, name, value, ann_type) { +var runAnnotationTest = function(test, ann, name, value, ann_type, duration) { test.equal(ann.name, name); test.equal(ann.value, value); test.equal(ann.annotationType, ann_type); + test.equal(ann.duration, duration); test.done(); }; @@ -254,6 +255,10 @@ module.exports = { runAnnotationTest(test, trace.Annotation.timestamp('test'), 'test', 1000000, 'timestamp'); }, + test_timestamp_with_duration: function(test) { + runAnnotationTest(test, trace.Annotation.timestamp('test', undefined, 123), 'test', + 1000000, 'timestamp', 123); + }, test_client_send: function(test) { runAnnotationTest(test, trace.Annotation.clientSend(), 'cs', 1000000, 'timestamp'); From cfb0ec443afa9b04ad8a9897a08759b0ae50bce9 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 22:31:13 -0600 Subject: [PATCH 10/13] when timestamp Annotation has duration defined, include it in restkin JSON --- lib/formatters.js | 4 ++++ tests/test_formatters.js | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/formatters.js b/lib/formatters.js index 6ee53ca..d8b7deb 100644 --- a/lib/formatters.js +++ b/lib/formatters.js @@ -77,6 +77,10 @@ }; } + if (ann.duration !== undefined) { + annJson.duration = ann.duration; + } + restkinTrace.annotations.push(annJson); }); diff --git a/tests/test_formatters.js b/tests/test_formatters.js index 666e431..75106a4 100644 --- a/tests/test_formatters.js +++ b/tests/test_formatters.js @@ -34,7 +34,7 @@ var testcases = { }, trace_with_optional_params: { trace: new trace.Trace('test', {spanId: 10, traceId:1, debug: true}), - annotations: [new trace.Annotation.timestamp('name1', 1), + annotations: [new trace.Annotation.timestamp('name1', 1, 123), new trace.Annotation.string('name2', '2')] }, trace_with_parentSpanId: { @@ -118,7 +118,8 @@ module.exports = { { key: 'name1', value: 1, - type: 'timestamp' + type: 'timestamp', + duration: 123 }, { key: 'name2', From 1cf55b803b125b97035eca5102e94e35e0ce7a85 Mon Sep 17 00:00:00 2001 From: Niels Nielsen Date: Wed, 22 Jul 2015 22:34:20 -0600 Subject: [PATCH 11/13] include timestamp Annotation duration in zipkin thrift message --- lib/formatters.js | 1 + tests/test_formatters.js | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/formatters.js b/lib/formatters.js index d8b7deb..81465a0 100644 --- a/lib/formatters.js +++ b/lib/formatters.js @@ -171,6 +171,7 @@ if (ann.annotationType === 'timestamp') { ttype_args.timestamp = ann.value; ttype_args.value = ann.name; + ttype_args.duration = ann.duration; thriftAnn.push(new zipkinCore_types.Annotation(ttype_args)); } else { ttype_args.key = ann.name; diff --git a/tests/test_formatters.js b/tests/test_formatters.js index 75106a4..882d09a 100644 --- a/tests/test_formatters.js +++ b/tests/test_formatters.js @@ -188,7 +188,8 @@ module.exports = { debug: true, annotations: [ new zipkinCore_types.Annotation({ timestamp: new Int64(1), - value: 'name1' + value: 'name1', + duration: 123 })], binary_annotations: [ new zipkinCore_types.BinaryAnnotation({ value: '2', From 90108cec82c1bddd916c84e19e0d049b0ca2a1bb Mon Sep 17 00:00:00 2001 From: Chris Thompson Date: Mon, 14 Dec 2015 12:07:19 -0700 Subject: [PATCH 12/13] Add detection for IPv6 addresses in Trace#fromRequest --- lib/trace.js | 39 +++++++++++++++++++++++++++++++++++--- tests/test_trace.js | 46 ++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/lib/trace.js b/lib/trace.js index 381b8d6..088b692 100644 --- a/lib/trace.js +++ b/lib/trace.js @@ -15,7 +15,7 @@ (function () { var tracers, formatters, zipkinCore_types; var Trace, Annotation, Endpoint, Int64; - var forEach, has, getUniqueId, getNowMicros, getHex64; + var forEach, has, getUniqueId, getNowMicros, getHex64, isIPv4; forEach = function(obj, f){ Array.prototype.forEach.call(obj, f); @@ -54,6 +54,28 @@ return Date.now() * 1000; }; + /** + * Returns true if an IP address is v4 dotted decimal + * @param {String} address + * @returns {Boolean} + */ + isIPv4 = function(address) { + var net = require('net'); + // Use native check first if possible + if (typeof net.isIPv4 === 'function') { + return net.isIPv4(address); + } else { + var octets = address.split('.'); + if (octets.length === 4) { + return octets.reduce(function (result, value) { + var n = parseInt(value, 10); + return result && (!isNaN(n) && n >= 0 && n <= 255); + }); + } + } + return false; + }; + /** * A Trace encapsulates information about the current span of this trace * and provides a mechanism for creating new child spans and recording @@ -85,7 +107,7 @@ } }); - if (has(options, 'tracers') && options.tracers){ + if (has(options, 'tracers') && options.tracers) { self._tracers = options.tracers; } else { self._tracers = tracers.getTracers(); @@ -236,7 +258,18 @@ request.method || 'GET', request.headers || {}); var host = (request.socket && request.socket.address ? - request.socket.address() : {address: '127.0.0.1', port: 80}); + request.socket.address() : { address: '127.0.0.1', port: 80 }); + + // Check IPv4 mapped IPv6 addresses + if (/^::ffff:(\d{1,3}\.){3,3}\d{1,3}$/.test(host.address)) { + host.address = host.address.replace(/^::ffff:/, ''); + } + + if (host.address === '::1') { + host.address = '127.0.0.1'; + } else if (!isIPv4(host.address)) { + host.address = '0.0.0.0'; + } if (serviceName === undefined || serviceName === null) { serviceName = 'http'; diff --git a/tests/test_trace.js b/tests/test_trace.js index fe6a3e3..1735d41 100644 --- a/tests/test_trace.js +++ b/tests/test_trace.js @@ -15,7 +15,6 @@ var util = require('util'); var ass = require('nodeunit').assert; -var _ = require("underscore"); var trace = require('..').trace; var tracers = require('..').tracers; @@ -23,12 +22,11 @@ var formatters = require('..').formatters; var MAX_ID = 'ffffffffffffffff'; -ass.isNum = function(num, message){ +ass.isNum = function(num){ ass.equal(isNaN(num), false, util.format("%s is not number like", num)); }; - var mockTracer = function(name, id, endpoint){ var self = this; self.name = name; @@ -223,6 +221,48 @@ module.exports = { {method: 'GET', headers: {}}, 'this_is_a_service'); test.equal(t.endpoint.serviceName, "this_is_a_service"); test.done(); + }, + test_fromRequest_endpoint_with_ipv6_localhost: function(test) { + var t = new trace.Trace.fromRequest({ + method: 'GET', + headers: {}, + socket: { + address: function() { + return {family: 2, port: 8888, address: '::1'}; + } + } + }); + test.equal(t.endpoint.ipv4, '127.0.0.1'); + test.equal(t.endpoint.port, 8888); + test.done(); + }, + test_fromRequest_endpoint_with_ipv6_mapped_localhost: function(test) { + var t = new trace.Trace.fromRequest({ + method: 'GET', + headers: {}, + socket: { + address: function() { + return {family: 2, port: 8888, address: '::ffff:127.0.0.1'}; + } + } + }); + test.equal(t.endpoint.ipv4, '127.0.0.1'); + test.equal(t.endpoint.port, 8888); + test.done(); + }, + test_fromRequest_endpoint_with_other_ipv6: function(test) { + var t = new trace.Trace.fromRequest({ + method: 'GET', + headers: {}, + socket: { + address: function() { + return {family: 2, port: 8888, address: '2001:db8::ff00:42:8329'}; + } + } + }); + test.equal(t.endpoint.ipv4, '0.0.0.0'); + test.equal(t.endpoint.port, 8888); + test.done(); } }, annotationTests: { From 0d938984193443faddd768f3d4cc16fa937fccbd Mon Sep 17 00:00:00 2001 From: Chris Thompson Date: Mon, 14 Dec 2015 12:40:49 -0700 Subject: [PATCH 13/13] Add node 4.2 and 5.2 to travis.yml --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 6063ca4..ba22674 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,9 @@ language: node_js node_js: + - 5.2 + - 4.2 + - 0.12 - 0.10 - 0.8 - 0.6