Update for PR comments

gradle/libs.gradle

@@ -12,7 +12,7 @@ project.ext.libs = [
     ],
     'guava': 'com.google.guava:guava:18.0',
     'hamcrest': 'org.hamcrest:hamcrest-all:1.3',
-    'immutables': 'org.immutables:value:2.0.21',
+    'immutables': 'org.immutables:value:2.2',
     'jackson': [
         'databind': 'com.fasterxml.jackson.core:jackson-databind:2.6.1',
         'guava': 'com.fasterxml.jackson.datatype:jackson-datatype-guava:2.6.1',

http-clients/src/main/java/com/palantir/remoting/http/TraceRequestInterceptor.java

@@ -25,7 +25,7 @@ public final class TraceRequestInterceptor implements RequestInterceptor {
 
     @Override
     public void apply(RequestTemplate template) {
-        TraceState callState = Traces.deriveTrace(template.method() + " " + template.url());
+        TraceState callState = Traces.startSpan(template.method() + " " + template.url());
         template.header(Traces.Headers.TRACE_ID, callState.getTraceId());
         if (callState.getParentSpanId().isPresent()) {
             template.header(Traces.Headers.PARENT_SPAN_ID, callState.getParentSpanId().get());

http-clients/src/main/java/feign/TraceResponseDecoder.java

@@ -32,7 +32,7 @@ public Object decode(Response response, Type type) throws IOException, DecodeExc
             if (trace.get().getTraceId().equals(traceId)
                     && trace.get().getSpanId().equals(spanId)) {
                 // this trace is for the traceId and spanId on top of the tracing stack, complete it
-                Traces.complete();
+                Traces.completeSpan();
             }
         }
         return delegate.decode(response, type);

http-servers/src/main/java/com/palantir/remoting/http/server/TraceEnrichingFilter.java

@@ -45,7 +45,7 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
 
         if (Strings.isNullOrEmpty(traceId)) {
             // no trace for this request, just derive a new one
-            Traces.deriveTrace(operation);
+            Traces.startSpan(operation);
         } else {
             // defend against badly formed requests
             if (spanId == null) {
@@ -65,7 +65,7 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
     public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext)
             throws IOException {
         MultivaluedMap<String, Object> headers = responseContext.getHeaders();
-        Optional<Span> maybeSpan = Traces.complete();
+        Optional<Span> maybeSpan = Traces.completeSpan();
         if (maybeSpan.isPresent()) {
             Span span = maybeSpan.get();
             headers.putSingle(Traces.Headers.TRACE_ID, span.getTraceId());

retrofit-clients/src/main/java/com/palantir/remoting/retrofit/RetrofitClientFactory.java

@@ -18,7 +18,6 @@
 
 import com.google.common.base.Optional;
 import com.palantir.remoting.retrofit.errors.RetrofitSerializableErrorErrorHandler;
-import com.squareup.okhttp.Interceptor;
 import com.squareup.okhttp.OkHttpClient;
 import java.util.concurrent.TimeUnit;
 import javax.net.ssl.SSLSocketFactory;
@@ -33,8 +32,6 @@
  */
 public final class RetrofitClientFactory {
 
-    private static final Interceptor TRACE_INTERCEPTOR = new TraceInterceptor();
-
     private RetrofitClientFactory() {}
 
     private static Client newHttpClient(Optional<SSLSocketFactory> sslSocketFactory, OkHttpClientOptions options) {
@@ -48,7 +45,7 @@ private static Client newHttpClient(Optional<SSLSocketFactory> sslSocketFactory,
                 options.getWriteTimeoutMs().or((long) okClient.getWriteTimeout()), TimeUnit.MILLISECONDS);
 
         // tracing
-        okClient.interceptors().add(TRACE_INTERCEPTOR);
+        okClient.interceptors().add(TraceInterceptor.INSTANCE);
 
         // retries
         RetryInterceptor retryInterceptor = options.getMaxNumberRetries().isPresent()

retrofit-clients/src/main/java/com/palantir/remoting/retrofit/TraceInterceptor.java

@@ -11,14 +11,16 @@
 import com.squareup.okhttp.Response;
 import java.io.IOException;
 
-public final class TraceInterceptor implements Interceptor {
+public enum TraceInterceptor implements Interceptor {
+
+    INSTANCE;
 
     @Override
     public Response intercept(Chain chain) throws IOException {
         Request request = chain.request();
 
         // instrument request
-        TraceState callState = Traces.deriveTrace(request.method() + " " + request.urlString());
+        TraceState callState = Traces.startSpan(request.method() + " " + request.urlString());
         Request.Builder instrumentedRequest = new Request.Builder()
                 .headers(request.headers())
                 .url(request.url())
@@ -33,8 +35,7 @@ public Response intercept(Chain chain) throws IOException {
         try {
             response = chain.proceed(instrumentedRequest.build());
         } finally {
-            // complete response
-            Traces.complete();
+            Traces.completeSpan();
         }
 
         return response;

retrofit-clients/src/test/java/com/palantir/remoting/retrofit/TraceRequestInterceptorTest.java

@@ -54,7 +54,7 @@ public void before() {
 
     @Test
     public void testTraceRequestInterceptor_sendsAValidTraceId() throws InterruptedException {
-        TraceState parentTrace = Traces.deriveTrace("");
+        TraceState parentTrace = Traces.startSpan("");
         service.get();
 
         RecordedRequest request = server.takeRequest();

tracing/src/main/java/com/palantir/tracing/TraceState.java

@@ -19,8 +19,6 @@
 import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
 import com.fasterxml.jackson.databind.annotation.JsonSerialize;
 import com.google.common.base.Optional;
-import java.nio.ByteBuffer;
-import java.util.UUID;
 import java.util.concurrent.ThreadLocalRandom;
 import org.immutables.value.Value;
 
@@ -82,28 +80,31 @@ public String getSpanId() {
      * <p>
      * Users should not set the {@code startTimeMs} value manually.
      */
-    public static final Builder builder() {
-        return new Builder()
+    public static Builder builder() {
+        return ImmutableTraceState.builder()
                 .startTimeMs(System.currentTimeMillis())
                 .startClockNs(System.nanoTime());
     }
 
     public static String randomId() {
-        // non-secure random generated UUID because speed is important here and security is not
-        byte[] randomBytes = new byte[16];
-        ThreadLocalRandom.current().nextBytes(randomBytes);
-
-        randomBytes[6]  &= 0x0f;  /* clear version        */
-        randomBytes[6]  |= 0x40;  /* set to version 4     */
-        randomBytes[8]  &= 0x3f;  /* clear variant        */
-        randomBytes[8]  |= 0x80;  /* set to IETF variant  */
-
-        ByteBuffer buffer = ByteBuffer.wrap(randomBytes);
-        long msb = buffer.getLong(0);
-        long lsb = buffer.getLong(8);
-        return new UUID(msb, lsb).toString();
+        return Long.toHexString(ThreadLocalRandom.current().nextLong());
     }
 
-    public static class Builder extends ImmutableTraceState.Builder {}
+    /**
+     * A safe builder interface that does not expose setting generated values.
+     */
+    public interface Builder {
+        Builder operation(String operation);
+
+        Builder traceId(String traceId);
+
+        Builder parentSpanId(Optional<String> parentSpanId);
+
+        Builder parentSpanId(String parentSpanId);
+
+        Builder spanId(String spanId);
+
+        TraceState build();
+    }
 
 }

tracing/src/main/java/com/palantir/tracing/Traces.java

@@ -43,7 +43,7 @@ protected Deque<TraceState> initialValue() {
 
     public static Optional<TraceState> getTrace() {
         Deque<TraceState> stack = STATE.get();
-        return (!stack.isEmpty()) ? Optional.of(stack.peek()) : Optional.<TraceState>absent();
+        return stack.isEmpty() ? Optional.<TraceState>absent() : Optional.of(stack.peek());
     }
 
     public static void setTrace(TraceState state) {
@@ -55,7 +55,7 @@ public static void setTrace(TraceState state) {
      * Derives a new call trace from the currently known call trace labeled with the
      * provided operation.
      */
-    public static TraceState deriveTrace(String operation) {
+    public static TraceState startSpan(String operation) {
         Optional<TraceState> prevState = getTrace();
 
         TraceState.Builder newStateBuilder = TraceState.builder()
@@ -71,7 +71,7 @@ public static TraceState deriveTrace(String operation) {
         return newState;
     }
 
-    public static Optional<Span> complete() {
+    public static Optional<Span> completeSpan() {
         Deque<TraceState> stack = STATE.get();
         if (stack.isEmpty()) {
             return Optional.absent();