Merge pull request #26 from klette/use-hex-encoded-trace-ids-in-http-…

…headers Encode TraceIds in hex when adding them to HTTP headers
openzipkin · Jul 18, 2014 · 60f54f9 · 60f54f9
2 parents 87d7c4a + 2e58822
commit 60f54f9
Show file tree

Hide file tree

Showing 7 changed files with 27 additions and 24 deletions.
diff --git a/...ptors/src/main/java/com/github/kristofa/brave/httpclient/BraveHttpRequestInterceptor.java b/...ptors/src/main/java/com/github/kristofa/brave/httpclient/BraveHttpRequestInterceptor.java
@@ -73,10 +73,10 @@ public void process(final HttpRequest request, final HttpContext context) throws
         if (newSpanId != null) {
             LOGGER.debug("Will trace request. Span Id returned from ClientTracer: {}", newSpanId);
             request.addHeader(BraveHttpHeaders.Sampled.getName(), TRUE);
-            request.addHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(newSpanId.getTraceId()));
-            request.addHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(newSpanId.getSpanId()));
+            request.addHeader(BraveHttpHeaders.TraceId.getName(), Long.toHexString((newSpanId.getTraceId())));
+            request.addHeader(BraveHttpHeaders.SpanId.getName(), Long.toHexString(newSpanId.getSpanId()));
             if (newSpanId.getParentSpanId() != null) {
-                request.addHeader(BraveHttpHeaders.ParentSpanId.getName(), String.valueOf(newSpanId.getParentSpanId()));
+                request.addHeader(BraveHttpHeaders.ParentSpanId.getName(), Long.toHexString(newSpanId.getParentSpanId()));
             }
             if (serviceAndSpanNames[0] != null) {
                 clientTracer.setCurrentClientServiceName(serviceAndSpanNames[0]);

diff --git a/...s/src/test/java/com/github/kristofa/brave/httpclient/BraveHttpRequestInterceptorTest.java b/...s/src/test/java/com/github/kristofa/brave/httpclient/BraveHttpRequestInterceptorTest.java
@@ -83,9 +83,9 @@ public void testProcessTracing() throws HttpException, IOException {
 
         inOrder.verify(mockClientTracer).startNewSpan(PATH);
         inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.Sampled.getName(), "true");
-        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(TRACE_ID));
-        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID));
-        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.ParentSpanId.getName(), String.valueOf(PARENT_SPAN_ID));
+        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.TraceId.getName(), Long.toHexString(TRACE_ID));
+        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.SpanId.getName(), Long.toHexString(SPAN_ID));
+        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.ParentSpanId.getName(), Long.toHexString(PARENT_SPAN_ID));
         inOrder.verify(mockClientTracer).setCurrentClientServiceName(CONTEXT);
         inOrder.verify(mockClientTracer).submitBinaryAnnotation("request", METHOD + " " + FULL_PATH);
         inOrder.verify(mockClientTracer).setClientSent();
@@ -108,8 +108,8 @@ public void testProcessTracingNoParentId() throws HttpException, IOException {
 
         inOrder.verify(mockClientTracer).startNewSpan(PATH);
         inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.Sampled.getName(), "true");
-        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(TRACE_ID));
-        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID));
+        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.TraceId.getName(), Long.toHexString(TRACE_ID));
+        inOrder.verify(httpRequest).addHeader(BraveHttpHeaders.SpanId.getName(), Long.toHexString(SPAN_ID));
         inOrder.verify(mockClientTracer).setCurrentClientServiceName(CONTEXT);
         inOrder.verify(mockClientTracer).submitBinaryAnnotation("request", METHOD + " " + FULL_PATH);
         inOrder.verify(mockClientTracer).setClientSent();

diff --git a/...t/java/com/github/kristofa/brave/httpclient/ITBraveHttpRequestAndResponseInterceptor.java b/...t/java/com/github/kristofa/brave/httpclient/ITBraveHttpRequestAndResponseInterceptor.java
@@ -69,8 +69,8 @@ public void testTracingTrue() throws ClientProtocolException, IOException, Unsat
 
         final HttpRequestImpl request = new HttpRequestImpl();
         request.method(Method.GET).path(FULL_PATH)
-            .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(TRACE_ID))
-            .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID))
+            .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), Long.toHexString(TRACE_ID))
+            .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), Long.toHexString(SPAN_ID))
             .httpMessageHeader(BraveHttpHeaders.Sampled.getName(), "true");
         final HttpResponseImpl response = new HttpResponseImpl(200, null, null);
         responseProvider.set(request, response);
@@ -139,8 +139,8 @@ public void testQueryParams() throws ClientProtocolException, IOException, Unsat
 
         final HttpRequestImpl request = new HttpRequestImpl();
         request.method(Method.GET).path(FULL_PATH).queryParameter("x", "1").queryParameter("y", "2")
-            .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(TRACE_ID))
-            .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID))
+            .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), Long.toHexString(TRACE_ID))
+            .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), Long.toHexString(SPAN_ID))
             .httpMessageHeader(BraveHttpHeaders.Sampled.getName(), "true");
         final HttpResponseImpl response = new HttpResponseImpl(200, null, null);
         responseProvider.set(request, response);

diff --git a/brave-jersey/src/main/java/com/github/kristofa/brave/jersey/JerseyClientTraceFilter.java b/brave-jersey/src/main/java/com/github/kristofa/brave/jersey/JerseyClientTraceFilter.java
@@ -45,10 +45,10 @@ public ClientResponse handle(ClientRequest clientRequest) throws ClientHandlerEx
     void addTracingHeaders(ClientRequest clientRequest, SpanId newSpan) {
         MultivaluedMap<String, Object> headers = clientRequest.getHeaders();
         if (newSpan != null) {
-            headers.add(BraveHttpHeaders.TraceId.getName(), String.valueOf(newSpan.getTraceId()));
-            headers.add(BraveHttpHeaders.SpanId.getName(), String.valueOf(newSpan.getSpanId()));
+            headers.add(BraveHttpHeaders.TraceId.getName(), Long.toHexString(newSpan.getTraceId()));
+            headers.add(BraveHttpHeaders.SpanId.getName(), Long.toHexString(newSpan.getSpanId()));
             if(newSpan.getParentSpanId() != null) {
-                headers.add(BraveHttpHeaders.ParentSpanId.getName(), String.valueOf(newSpan.getParentSpanId()));
+                headers.add(BraveHttpHeaders.ParentSpanId.getName(), Long.toHexString(newSpan.getParentSpanId()));
             }
             headers.add(BraveHttpHeaders.Sampled.getName(), "true");
         } else {

diff --git a/brave-jersey/src/main/java/com/github/kristofa/brave/jersey/ServletTraceFilter.java b/brave-jersey/src/main/java/com/github/kristofa/brave/jersey/ServletTraceFilter.java
@@ -94,7 +94,10 @@ private Boolean nullOrBoolean(String value) {
     }
 
     private Long longOrNull(String value) {
-        return (value == null) ? null : Long.valueOf(value);
+        if (value == null) {
+            return null;
+        }
+        return Long.parseLong(value, 16);
     }
 
     @Override

diff --git a/brave-jersey/src/test/java/com/github/kristofa/brave/jersey/JerseyClientTraceFilterTest.java b/brave-jersey/src/test/java/com/github/kristofa/brave/jersey/JerseyClientTraceFilterTest.java
@@ -32,9 +32,9 @@ public void shouldAddAllFields() {
         when(clientRequest.getHeaders()).thenReturn(new StringKeyObjectValueIgnoreCaseMultivaluedMap());
         jerseyClientTraceFilter.addTracingHeaders(clientRequest, mockSpan(123L, 456L, 789L));
         MultivaluedMap<String, Object> headers = clientRequest.getHeaders();
-        Assert.assertEquals("123", headers.getFirst(BraveHttpHeaders.TraceId.getName()));
-        Assert.assertEquals("456", headers.getFirst(BraveHttpHeaders.SpanId.getName()));
-        Assert.assertEquals("789", headers.getFirst(BraveHttpHeaders.ParentSpanId.getName()));
+        Assert.assertEquals("7b", headers.getFirst(BraveHttpHeaders.TraceId.getName()));
+        Assert.assertEquals("1c8", headers.getFirst(BraveHttpHeaders.SpanId.getName()));
+        Assert.assertEquals("315", headers.getFirst(BraveHttpHeaders.ParentSpanId.getName()));
         Assert.assertEquals("true", headers.getFirst(BraveHttpHeaders.Sampled.getName()));
     }
 
@@ -43,8 +43,8 @@ public void shouldNotIncludeMissingParentId() {
         when(clientRequest.getHeaders()).thenReturn(new StringKeyObjectValueIgnoreCaseMultivaluedMap());
         jerseyClientTraceFilter.addTracingHeaders(clientRequest, mockSpan(123L, 456L, null));
         MultivaluedMap<String, Object> headers = clientRequest.getHeaders();
-        Assert.assertEquals("123", headers.getFirst(BraveHttpHeaders.TraceId.getName()));
-        Assert.assertEquals("456", headers.getFirst(BraveHttpHeaders.SpanId.getName()));
+        Assert.assertEquals("7b", headers.getFirst(BraveHttpHeaders.TraceId.getName()));
+        Assert.assertEquals("1c8", headers.getFirst(BraveHttpHeaders.SpanId.getName()));
         Assert.assertEquals(null, headers.getFirst(BraveHttpHeaders.ParentSpanId.getName()));
         Assert.assertEquals("true", headers.getFirst(BraveHttpHeaders.Sampled.getName()));
     }

diff --git a/brave-jersey/src/test/java/com/github/kristofa/brave/jersey/ServletTraceFilterTest.java b/brave-jersey/src/test/java/com/github/kristofa/brave/jersey/ServletTraceFilterTest.java
@@ -72,9 +72,9 @@ public void shouldSubmitEndpointWithGivenPaths() throws Exception {
     public void shouldGetTraceDataFromHeaders() throws Exception {
         when(endPointSubmitter.endPointSubmitted()).thenReturn(true);
 
-        when(servletRequest.getHeader(BraveHttpHeaders.TraceId.getName())).thenReturn(String.valueOf(TRACE_ID));
-        when(servletRequest.getHeader(BraveHttpHeaders.SpanId.getName())).thenReturn(String.valueOf(SPAN_ID));
-        when(servletRequest.getHeader(BraveHttpHeaders.ParentSpanId.getName())).thenReturn(String.valueOf(PARENT_SPAN_ID));
+        when(servletRequest.getHeader(BraveHttpHeaders.TraceId.getName())).thenReturn(Long.toHexString(TRACE_ID));
+        when(servletRequest.getHeader(BraveHttpHeaders.SpanId.getName())).thenReturn(Long.toHexString(SPAN_ID));
+        when(servletRequest.getHeader(BraveHttpHeaders.ParentSpanId.getName())).thenReturn(Long.toHexString(PARENT_SPAN_ID));
         when(servletRequest.getHeader(BraveHttpHeaders.Sampled.getName())).thenReturn(String.valueOf(SAMPLED_TRUE));
         when(servletRequest.getHeader(BraveHttpHeaders.SpanName.getName())).thenReturn(SPAN_NAME);