diff --git a/gradle/libs.gradle b/gradle/libs.gradle index 5713d69f5d..ad347a86c4 100644 --- a/gradle/libs.gradle +++ b/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', diff --git a/http-clients/src/main/java/com/palantir/remoting/http/TraceRequestInterceptor.java b/http-clients/src/main/java/com/palantir/remoting/http/TraceRequestInterceptor.java index 4f1b96a9e9..02c106c17e 100644 --- a/http-clients/src/main/java/com/palantir/remoting/http/TraceRequestInterceptor.java +++ b/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()); diff --git a/http-clients/src/main/java/feign/TraceResponseDecoder.java b/http-clients/src/main/java/feign/TraceResponseDecoder.java index 445c72b33d..f530d7c70e 100644 --- a/http-clients/src/main/java/feign/TraceResponseDecoder.java +++ b/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); diff --git a/http-servers/src/main/java/com/palantir/remoting/http/server/TraceEnrichingFilter.java b/http-servers/src/main/java/com/palantir/remoting/http/server/TraceEnrichingFilter.java index 6fad92a527..3a205bc29c 100644 --- a/http-servers/src/main/java/com/palantir/remoting/http/server/TraceEnrichingFilter.java +++ b/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 headers = responseContext.getHeaders(); - Optional maybeSpan = Traces.complete(); + Optional maybeSpan = Traces.completeSpan(); if (maybeSpan.isPresent()) { Span span = maybeSpan.get(); headers.putSingle(Traces.Headers.TRACE_ID, span.getTraceId()); diff --git a/retrofit-clients/src/main/java/com/palantir/remoting/retrofit/RetrofitClientFactory.java b/retrofit-clients/src/main/java/com/palantir/remoting/retrofit/RetrofitClientFactory.java index 44a2853ac3..fc60e199fc 100644 --- a/retrofit-clients/src/main/java/com/palantir/remoting/retrofit/RetrofitClientFactory.java +++ b/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, OkHttpClientOptions options) { @@ -48,7 +45,7 @@ private static Client newHttpClient(Optional 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() diff --git a/retrofit-clients/src/main/java/com/palantir/remoting/retrofit/TraceInterceptor.java b/retrofit-clients/src/main/java/com/palantir/remoting/retrofit/TraceInterceptor.java index 5e2643d2a3..7ed1280d45 100644 --- a/retrofit-clients/src/main/java/com/palantir/remoting/retrofit/TraceInterceptor.java +++ b/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; diff --git a/retrofit-clients/src/test/java/com/palantir/remoting/retrofit/TraceRequestInterceptorTest.java b/retrofit-clients/src/test/java/com/palantir/remoting/retrofit/TraceRequestInterceptorTest.java index 1b73b00aff..5afa07ff78 100644 --- a/retrofit-clients/src/test/java/com/palantir/remoting/retrofit/TraceRequestInterceptorTest.java +++ b/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(); diff --git a/tracing/src/main/java/com/palantir/tracing/TraceState.java b/tracing/src/main/java/com/palantir/tracing/TraceState.java index f4d2c87932..19dac20e22 100644 --- a/tracing/src/main/java/com/palantir/tracing/TraceState.java +++ b/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() { *

* 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 parentSpanId); + + Builder parentSpanId(String parentSpanId); + + Builder spanId(String spanId); + + TraceState build(); + } } diff --git a/tracing/src/main/java/com/palantir/tracing/Traces.java b/tracing/src/main/java/com/palantir/tracing/Traces.java index 6d97d2ac25..1d57242212 100644 --- a/tracing/src/main/java/com/palantir/tracing/Traces.java +++ b/tracing/src/main/java/com/palantir/tracing/Traces.java @@ -43,7 +43,7 @@ protected Deque initialValue() { public static Optional getTrace() { Deque stack = STATE.get(); - return (!stack.isEmpty()) ? Optional.of(stack.peek()) : Optional.absent(); + return stack.isEmpty() ? Optional.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 prevState = getTrace(); TraceState.Builder newStateBuilder = TraceState.builder() @@ -71,7 +71,7 @@ public static TraceState deriveTrace(String operation) { return newState; } - public static Optional complete() { + public static Optional completeSpan() { Deque stack = STATE.get(); if (stack.isEmpty()) { return Optional.absent();