Skip to content

Commit

Permalink
Changes 64-bit traces to left-pad 0s
Browse files Browse the repository at this point in the history
Doing so helps remove confusion with 64-bit trace ids, which are encoded
by Zipkin with 16 characters (implicitly padded left with zeros).
  • Loading branch information
Adrian Cole committed Dec 20, 2016
1 parent 7beea93 commit c4dd5cd
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,8 @@ private void finishTrace(Trace.Builder traceBuilder, Collection<Trace> converted
static String convertTraceId(Span zipkinSpan) {
// Stackdriver trace ID's are 128 bits = 16 bytes * 8
ByteBuffer idBuffer = ByteBuffer.allocate(16); // or 32 characters
// Some users call the Stackdriver API and order results by traceId to get a "random sample" of
// traces. Duplicating the 64-bit ID instead of padding the ID with zeros will remove some
// sampling bias for any projects that contain both Zipkin and Stackdriver traces. Regardless of
// if we pad-left 0s or duplicate traceId, this resulting ID won't match Zipkin anyway as it
// conditionally encodes 16 characters if high bits are unset (as opposed to padding to 32).
idBuffer.putLong(zipkinSpan.traceIdHigh == 0L ? zipkinSpan.traceId : zipkinSpan.traceIdHigh);
// Note that when 64-bit trace IDs are used, the left-most 16 characters will be zero

This comment has been minimized.

Copy link
@denyska

denyska Dec 21, 2016

Contributor

@adriancole @kevinmdavis Will the change work in mixed environments: i.e. some legacy component on Zipkin64 and newish components on Zipkin128?

This comment has been minimized.

Copy link
@codefromthecrypt

codefromthecrypt via email Dec 22, 2016

Member

This comment has been minimized.

Copy link
@denyska

denyska Dec 22, 2016

Contributor

Make sense, thnx!

idBuffer.putLong(zipkinSpan.traceIdHigh);
idBuffer.putLong(zipkinSpan.traceId);
StringBuilder idBuilder = new StringBuilder();
for (byte b : idBuffer.array()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,14 @@ public void testTranslateTrace_128bitInputTraceId() {
.containsExactly("00000000000000020000000000000001");
}

/**
* Duplicating the lower 64 bits to construct a 128-bit trace ID removes bias when users sort on
* trace ID to choose a random traces. If instead we left-padded zeros, 64-bit traces would always
* appear first.
*/
@Test
public void testTranslateTrace_64BitTraceIdCopiedTwiceToMake128bits() {
public void testTranslateTrace_64BitTraceIdLeftPadsZeros() {
Span span = Span.builder().id(1).traceId(1).name("/a").build();
TraceTranslator translator = new TraceTranslator("test-project");

assertThat(translator.translateSpans(Arrays.asList(span)))
.extracting("traceId")
.containsExactly("00000000000000010000000000000001");
.containsExactly("00000000000000000000000000000001");
}

@Test
Expand Down Expand Up @@ -101,8 +96,8 @@ public void testTranslateTrace() {
trace1.getTraceId(), trace1,
trace2.getTraceId(), trace2
);
String key1 = "00000000000000010000000000000001";
String key2 = "00000000000000020000000000000002";
String key1 = "00000000000000000000000000000001";
String key2 = "00000000000000000000000000000002";
assertTrue(traceMap.containsKey(key1));
assertTrue(traceMap.containsKey(key2));
assertEquals(2, traceMap.get(key1).getSpansCount());
Expand Down

0 comments on commit c4dd5cd

Please sign in to comment.