what is this for, and it is indented correctly?

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

makes total sense, thanks

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.

ok, I'll work on that

I believe this is a boolean based on the config and a test, so it should be @param {boolean} ... instead

It seems as new Buffer is deprecated. We should probably use the new API. So this line and the subsequent line can jointed into:

this._traceId = Buffer.alloc(size)

Then we can rely on babel to provide backwards compatibility.

@everett980 ~~I don't think babel will provide backwards compatibility for the Buffer.from Buffer.alloc because it's not syntax related.

Buffer.alloc was added in 5.10. Seems like guarding on the presence of Buffer.alloc and falling back to the deprecated constructor might be preferred.

I believe you can just drop the new to avoid the deprecated constructor

I think the use of new is unnecessary, regardless of the constructor being deprecated.

This line is the same as the previous line.

This should be Buffer.from(safeTraceIdStr, 'hex');

@everett980 Buffer.from is not available in node 0.10.x. It was added in 5.10.

I suggest using a factory which checks for the presence of the 5.10 factory functions and falls back to the constructor if they're not available.

@everett980 Buffer.from is not available in node 0.10.x. It was added in 5.10.

I suggest using a factory which checks for the presence of the 5.10 factory functions and falls back to the constructor if they're not available.

why is this needed? startSpan() is an OpenTracing API function, we cannot change its signature in Jaeger. This flag is only needed as tracer-level flag, not span-level.

didn't know that, I'll move it

why do we need to allocate two buffers?

I need to create a 64 or 128 bits buffer and decode the hex string at the same, I don't think that can be done in one shot

It was unfortunate oversight that Jaeger clients allowed shortened string representation of trace/span IDs by dropping leading zeroes (an optimization that only affects ~1/16th of all traces, wasn't worth it), instead of always insisting on exactly 16 or 32 characters. I booked a meta-issue for this: jaegertracing/jaeger#1657

To maintain backwards compatibility we do need to be able to accept strings <16 or <32 chars, but always output strings of the exact length.

The overall implementation would probably be simpler if internal representation always uses a buffer of exact length, as you're doing above (an alternative would be use use the buffer of whatever length the input string is). But we should still avoid double-allocating the buffer. For example, in L73 instead of padding with a single zero, you could pad with as many zeros as needed to reach 16/32 length, and then your Utils.newBufferFromHex() function will return the right-sized buffer in one allocation. We still lose an allocation due to padding, but that will only affect about 1/16th of all traces, and once jaegertracing/jaeger#1657 is fixed for all languages padding won't be happening in practice.

this can probably made more efficient by allocating 16-bytes buffer, copying randomId, and filling high bytes with more random values. Saves at least one allocation.

why do we need all of these?

There are no common code paths based on this param, and encoding is only used for string. I think this should be split into separate functions.

I'll split the code

this is not OpenTracing-compliant code.

sorry didn't know that

can combine this into a single assignment with traceIdExactLength === 16 ? and avoid allocating array

ok