diff --git a/brave-apache-http-interceptors/src/test/java/com/github/kristofa/brave/httpclient/ITBraveHttpRequestAndResponseInterceptor.java b/brave-apache-http-interceptors/src/test/java/com/github/kristofa/brave/httpclient/ITBraveHttpRequestAndResponseInterceptor.java index 9e712aa075..94520c0138 100644 --- a/brave-apache-http-interceptors/src/test/java/com/github/kristofa/brave/httpclient/ITBraveHttpRequestAndResponseInterceptor.java +++ b/brave-apache-http-interceptors/src/test/java/com/github/kristofa/brave/httpclient/ITBraveHttpRequestAndResponseInterceptor.java @@ -3,6 +3,7 @@ import com.github.kristofa.brave.ClientRequestInterceptor; import com.github.kristofa.brave.ClientResponseInterceptor; import com.github.kristofa.brave.ClientTracer; +import com.github.kristofa.brave.IdConversion; import com.github.kristofa.brave.SpanId; import com.github.kristofa.brave.http.BraveHttpHeaders; import com.github.kristofa.brave.http.DefaultSpanNameProvider; @@ -43,6 +44,8 @@ public class ITBraveHttpRequestAndResponseInterceptor { private static final Long SPAN_ID = 151864L; private static final Long TRACE_ID = 8494864L; + private static final String TRACE_ID_STRING = + SpanId.builder().spanId(TRACE_ID).build().traceIdString(); private MockHttpServer mockServer; private DefaultHttpResponseProvider responseProvider; private ClientTracer clientTracer; @@ -69,7 +72,7 @@ public void testTracingTrue() throws ClientProtocolException, IOException, Unsat final HttpRequestImpl request = new HttpRequestImpl(); request.method(Method.GET).path(FULL_PATH) - .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), Long.toString(TRACE_ID, 16)) + .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), TRACE_ID_STRING) .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), Long.toString(SPAN_ID, 16)) .httpMessageHeader(BraveHttpHeaders.Sampled.getName(), "1"); final HttpResponseImpl response = new HttpResponseImpl(200, null, null); @@ -105,7 +108,7 @@ public void testTracingTrueHttpNoOk() throws ClientProtocolException, IOExceptio final HttpRequestImpl request = new HttpRequestImpl(); request.method(Method.GET).path(FULL_PATH) - .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), Long.toString(TRACE_ID, 16)) + .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), TRACE_ID_STRING) .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), Long.toString(SPAN_ID, 16)) .httpMessageHeader(BraveHttpHeaders.Sampled.getName(), "1"); final HttpResponseImpl response = new HttpResponseImpl(400, null, null); @@ -173,7 +176,7 @@ 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(), Long.toString(TRACE_ID, 16)) + .httpMessageHeader(BraveHttpHeaders.TraceId.getName(), TRACE_ID_STRING) .httpMessageHeader(BraveHttpHeaders.SpanId.getName(), Long.toString(SPAN_ID, 16)) .httpMessageHeader(BraveHttpHeaders.Sampled.getName(), "1"); final HttpResponseImpl response = new HttpResponseImpl(200, null, null); diff --git a/brave-core/README.md b/brave-core/README.md index 7dee808029..1d95ec9777 100644 --- a/brave-core/README.md +++ b/brave-core/README.md @@ -100,3 +100,23 @@ will get proper trace/span state. Instead of using `BraveExecutorService` or the `ServerSpanThreadBinder` directly you can also use the `BraveCallable` and `BraveRunnable`. These are used internally by the BraveExecutorService. + +## 128-bit trace IDs + +Traditionally, Zipkin trace IDs were 64-bit. Starting with Zipkin 1.14, +128-bit trace identifiers are supported. This can be useful in sites that +have very large traffic volume, persist traces forever, or are re-using +externally generated 128-bit IDs. + +If you want Brave to generate 128-bit trace identifiers when starting new +root spans, set `Brave.Builder.traceId128Bit(true)` + +When 128-bit trace ids are propagated, they will be twice as long as +before. For example, the `X-B3-TraceId` header will hold a 32-character +value like `163ac35c9f6413ad48485a3953bb6124`. + +Before doing this, ensure your Zipkin setup is up-to-date, and downstream +instrumented services can read the longer 128-bit trace IDs. + +Note: this only affects the trace ID, not span IDs. For example, span ids +within a trace are always 64-bit. diff --git a/brave-core/pom.xml b/brave-core/pom.xml index aa8c73704d..148dd02736 100755 --- a/brave-core/pom.xml +++ b/brave-core/pom.xml @@ -33,7 +33,7 @@ io.zipkin.reporter zipkin-reporter - 0.6.1 + 0.6.4 diff --git a/brave-core/src/main/java/com/github/kristofa/brave/Brave.java b/brave-core/src/main/java/com/github/kristofa/brave/Brave.java index 044312034c..9072957786 100644 --- a/brave-core/src/main/java/com/github/kristofa/brave/Brave.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/Brave.java @@ -45,6 +45,7 @@ public static class Builder { private Sampler sampler = Sampler.create(1.0f); private boolean allowNestedLocalSpans = false; private AnnotationSubmitter.Clock clock = AnnotationSubmitter.DefaultClock.INSTANCE; + private boolean traceId128Bit = false; /** * Builder which initializes with serviceName = "unknown". @@ -156,6 +157,12 @@ public Builder clock(AnnotationSubmitter.Clock clock) { return this; } + /** When true, new root spans will have 128-bit trace IDs. Defaults to false (64-bit) */ + public Builder traceId128Bit(boolean traceId128Bit) { + this.traceId128Bit = traceId128Bit; + return this; + } + public Brave build() { return new Brave(this); } @@ -260,6 +267,7 @@ private Brave(Builder builder) { .state(builder.state) .traceSampler(builder.sampler) .clock(builder.clock) + .traceId128Bit(builder.traceId128Bit) .build(); clientTracer = ClientTracer.builder() @@ -268,6 +276,7 @@ private Brave(Builder builder) { .state(builder.state) .traceSampler(builder.sampler) .clock(builder.clock) + .traceId128Bit(builder.traceId128Bit) .build(); localTracer = LocalTracer.builder() @@ -277,6 +286,7 @@ private Brave(Builder builder) { .spanAndEndpoint(SpanAndEndpoint.LocalSpanAndEndpoint.create(builder.state)) .traceSampler(builder.sampler) .clock(builder.clock) + .traceId128Bit(builder.traceId128Bit) .build(); serverRequestInterceptor = new ServerRequestInterceptor(serverTracer); diff --git a/brave-core/src/main/java/com/github/kristofa/brave/ClientTracer.java b/brave-core/src/main/java/com/github/kristofa/brave/ClientTracer.java index 936f72506e..5cae1b7dec 100755 --- a/brave-core/src/main/java/com/github/kristofa/brave/ClientTracer.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/ClientTracer.java @@ -1,14 +1,12 @@ package com.github.kristofa.brave; -import com.google.auto.value.AutoValue; - import com.github.kristofa.brave.SpanAndEndpoint.ClientSpanAndEndpoint; import com.github.kristofa.brave.internal.Nullable; +import com.google.auto.value.AutoValue; import com.twitter.zipkin.gen.Endpoint; import com.twitter.zipkin.gen.Span; -import zipkin.Constants; - import java.util.Random; +import zipkin.Constants; import zipkin.reporter.Reporter; /** @@ -40,6 +38,7 @@ public static Builder builder() { abstract Sampler traceSampler(); @Override abstract AnnotationSubmitter.Clock clock(); + abstract boolean traceId128Bit(); @AutoValue.Builder public abstract static class Builder { @@ -67,6 +66,7 @@ public final Builder spanCollector(SpanCollector spanCollector) { public abstract Builder traceSampler(Sampler sampler); public abstract Builder clock(AnnotationSubmitter.Clock clock); + abstract Builder traceId128Bit(boolean traceId128Bit); public abstract ClientTracer build(); } @@ -156,8 +156,13 @@ private SpanId getNewSpanId() { long newSpanId = randomGenerator().nextLong(); SpanId.Builder builder = SpanId.builder().spanId(newSpanId); - if (parentSpan == null) return builder.build(); // new trace - return builder.traceId(parentSpan.getTrace_id()).parentId(parentSpan.getId()).build(); + if (parentSpan == null) { // new trace + if (traceId128Bit()) builder.traceIdHigh(randomGenerator().nextLong()); + return builder.build(); + } + return builder.traceIdHigh(parentSpan.getTrace_id_high()) + .traceId(parentSpan.getTrace_id()) + .parentId(parentSpan.getId()).build(); } ClientTracer() { diff --git a/brave-core/src/main/java/com/github/kristofa/brave/IdConversion.java b/brave-core/src/main/java/com/github/kristofa/brave/IdConversion.java index 48aecc5aec..8a2a0c8d50 100644 --- a/brave-core/src/main/java/com/github/kristofa/brave/IdConversion.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/IdConversion.java @@ -34,16 +34,24 @@ public static String convertToString(final long id) { * Parses a 1 to 32 character lower-hex string with no prefix into an unsigned long, tossing any * bits higher than 64. */ - public static long convertToLong(final String lowerHex) { + public static long convertToLong(String lowerHex) { int length = lowerHex.length(); if (length < 1 || length > 32) throw isntLowerHexLong(lowerHex); // trim off any high bits - int i = length > 16 ? length - 16 : 0; + int beginIndex = length > 16 ? length - 16 : 0; + return convertToLong(lowerHex, beginIndex); + } + + /** + * Parses a 16 character lower-hex string with no prefix into an unsigned long, starting at the + * specified index. + */ + public static long convertToLong(String lowerHex, int index) { long result = 0; - for (; i < length; i++) { - char c = lowerHex.charAt(i); + for (int endIndex = Math.min(index + 16, lowerHex.length()); index < endIndex; index++) { + char c = lowerHex.charAt(index); result <<= 4; if (c >= '0' && c <= '9') { result |= c - '0'; diff --git a/brave-core/src/main/java/com/github/kristofa/brave/LocalTracer.java b/brave-core/src/main/java/com/github/kristofa/brave/LocalTracer.java index 33569dc6cd..d15c70fcfd 100644 --- a/brave-core/src/main/java/com/github/kristofa/brave/LocalTracer.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/LocalTracer.java @@ -60,6 +60,8 @@ static Builder builder(LocalTracer source) { @Override abstract AnnotationSubmitter.Clock clock(); + abstract boolean traceId128Bit(); + @AutoValue.Builder abstract static class Builder { @@ -75,6 +77,8 @@ abstract static class Builder { abstract Builder clock(AnnotationSubmitter.Clock clock); + abstract Builder traceId128Bit(boolean traceId128Bit); + abstract LocalTracer build(); } @@ -101,8 +105,13 @@ private SpanId getNewSpanId() { Span parentSpan = getNewSpanParent(); long newSpanId = randomGenerator().nextLong(); SpanId.Builder builder = SpanId.builder().spanId(newSpanId); - if (parentSpan == null) return builder.build(); // new trace - return builder.traceId(parentSpan.getTrace_id()).parentId(parentSpan.getId()).build(); + if (parentSpan == null) { // new trace + if (traceId128Bit()) builder.traceIdHigh(randomGenerator().nextLong()); + return builder.traceId(newSpanId).build(); + } + return builder.traceIdHigh(parentSpan.getTrace_id_high()) + .traceId(parentSpan.getTrace_id()) + .parentId(parentSpan.getId()).build(); } /** diff --git a/brave-core/src/main/java/com/github/kristofa/brave/ServerRequestInterceptor.java b/brave-core/src/main/java/com/github/kristofa/brave/ServerRequestInterceptor.java index ab620ac636..39672d8704 100644 --- a/brave-core/src/main/java/com/github/kristofa/brave/ServerRequestInterceptor.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/ServerRequestInterceptor.java @@ -41,9 +41,7 @@ public void handle(ServerRequestAdapter adapter) { boolean clientOriginatedTrace = traceData.getSpanId() != null; if (clientOriginatedTrace) { LOGGER.fine("Received span information as part of request."); - SpanId spanId = traceData.getSpanId(); - serverTracer.setStateCurrentTrace(spanId.traceId, spanId.spanId, - spanId.nullableParentId(), adapter.getSpanName()); + serverTracer.setStateCurrentTrace(traceData.getSpanId(), adapter.getSpanName()); } else { LOGGER.fine("Received no span state."); serverTracer.setStateUnknown(adapter.getSpanName()); diff --git a/brave-core/src/main/java/com/github/kristofa/brave/ServerSpan.java b/brave-core/src/main/java/com/github/kristofa/brave/ServerSpan.java index 73bd10c2f5..5bcd20d38c 100644 --- a/brave-core/src/main/java/com/github/kristofa/brave/ServerSpan.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/ServerSpan.java @@ -5,6 +5,8 @@ import com.twitter.zipkin.gen.Span; +import static com.github.kristofa.brave.internal.Util.checkNotNull; + /** * The ServerSpan is initialized by {@link ServerTracer} and keeps track of Trace/Span state of our service request. * @@ -13,8 +15,8 @@ @AutoValue public abstract class ServerSpan { - public final static ServerSpan EMPTY = ServerSpan.create(null); - static final ServerSpan NOT_SAMPLED = ServerSpan.create(false); + public static final ServerSpan EMPTY = new AutoValue_ServerSpan(null, null); + static final ServerSpan NOT_SAMPLED = new AutoValue_ServerSpan(null, false); /** * Gets the Trace/Span context. @@ -34,36 +36,8 @@ public abstract class ServerSpan { @Nullable public abstract Boolean getSample(); - static ServerSpan create(Span span, Boolean sample) { - return new AutoValue_ServerSpan(span, sample); - } - - /** - * Creates a new initializes instance. Using this constructor also indicates we need to sample this request. - * - * @param traceId Trace id. - * @param spanId Span id. - * @param parentSpanId Parent span id, can be null. - * @param name Span name. Should be lowercase and not null or empty. - */ - static ServerSpan create(long traceId, long spanId, @Nullable Long parentSpanId, String name) { - Span span = new Span(); - span.setTrace_id(traceId); - span.setId(spanId); - if (parentSpanId != null) { - span.setParent_id(parentSpanId); - } - span.setName(name); - return create(span, true); - } - - /** - * Creates a new empty instance with no Span but with sample indication. - * - * @param sample Indicates if we should sample this span. - */ - static ServerSpan create(final Boolean sample) { - return create(null, sample); + static ServerSpan create(Span span) { + return new AutoValue_ServerSpan(checkNotNull(span, "span"), true); } ServerSpan(){ diff --git a/brave-core/src/main/java/com/github/kristofa/brave/ServerTracer.java b/brave-core/src/main/java/com/github/kristofa/brave/ServerTracer.java index 196ef152a3..6f34e8037c 100755 --- a/brave-core/src/main/java/com/github/kristofa/brave/ServerTracer.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/ServerTracer.java @@ -1,14 +1,11 @@ package com.github.kristofa.brave; -import com.google.auto.value.AutoValue; - import com.github.kristofa.brave.SpanAndEndpoint.ServerSpanAndEndpoint; import com.github.kristofa.brave.internal.Nullable; +import com.google.auto.value.AutoValue; import com.twitter.zipkin.gen.Endpoint; -import com.twitter.zipkin.gen.Span; -import zipkin.Constants; - import java.util.Random; +import zipkin.Constants; import zipkin.reporter.Reporter; import static com.github.kristofa.brave.internal.Util.checkNotBlank; @@ -42,6 +39,7 @@ public static Builder builder() { abstract Sampler traceSampler(); @Override abstract AnnotationSubmitter.Clock clock(); + abstract boolean traceId128Bit(); @AutoValue.Builder public abstract static class Builder { @@ -71,6 +69,8 @@ public final Builder spanCollector(SpanCollector spanCollector) { public abstract Builder clock(AnnotationSubmitter.Clock clock); + abstract Builder traceId128Bit(boolean traceId128Bit); + public abstract ServerTracer build(); } @@ -81,27 +81,34 @@ public void clearCurrentSpan() { spanAndEndpoint().state().setCurrentServerSpan(null); } + /** + * @deprecated since 3.15 use {@link #setStateCurrentTrace(SpanId, String)} + */ + @Deprecated + public void setStateCurrentTrace(long traceId, long spanId, @Nullable Long parentSpanId, String name) { + SpanId id = SpanId.builder().traceId(traceId).spanId(spanId).parentId(parentSpanId).build(); + setStateCurrentTrace(id, name); + } + /** * Sets the current Trace/Span state. Using this method indicates we are part of an existing trace/span. * - * @param traceId Trace id. - * @param spanId Span id. - * @param parentSpanId Parent span id. Can be null. - * @param name Name should not be empty or null. + * @param spanId includes the trace identifiers extracted from the wire + * @param spanName should not be empty or null. * @see ServerTracer#setStateNoTracing() * @see ServerTracer#setStateUnknown(String) */ - public void setStateCurrentTrace(long traceId, long spanId, @Nullable Long parentSpanId, @Nullable String name) { - checkNotBlank(name, "Null or blank span name"); - spanAndEndpoint().state().setCurrentServerSpan( - ServerSpan.create(traceId, spanId, parentSpanId, name)); + public void setStateCurrentTrace(SpanId spanId, String spanName) { + checkNotBlank(spanName, "Null or blank span name"); + ServerSpan span = ServerSpan.create(spanId.toSpan().setName(spanName)); + spanAndEndpoint().state().setCurrentServerSpan(span); } /** * Sets the current Trace/Span state. Using this method indicates that a parent request has decided that we should not * trace the current request. * - * @see ServerTracer#setStateCurrentTrace(long, long, Long, String) + * @see ServerTracer#setStateCurrentTrace(SpanId, String) * @see ServerTracer#setStateUnknown(String) */ public void setStateNoTracing() { @@ -122,13 +129,17 @@ public void setStateUnknown(String spanName) { spanAndEndpoint().state().setCurrentServerSpan(ServerSpan.NOT_SAMPLED); return; } - spanAndEndpoint().state().setCurrentServerSpan( - ServerSpan.create(newTraceId, newTraceId, null, spanName)); + SpanId spanId = SpanId.builder() + .traceIdHigh(traceId128Bit() ? randomGenerator().nextLong() : 0L) + .traceId(newTraceId) + .spanId(newTraceId) + .build(); + setStateCurrentTrace(spanId, spanName); } /** * Sets server received event for current request. This should be done after setting state using one of 3 methods - * {@link ServerTracer#setStateCurrentTrace(long, long, Long, String)} , {@link ServerTracer#setStateNoTracing()} or + * {@link ServerTracer#setStateCurrentTrace(SpanId, String)} , {@link ServerTracer#setStateNoTracing()} or * {@link ServerTracer#setStateUnknown(String)}. */ public void setServerReceived() { diff --git a/brave-core/src/main/java/com/github/kristofa/brave/SpanCollectorReporterAdapter.java b/brave-core/src/main/java/com/github/kristofa/brave/SpanCollectorReporterAdapter.java index 5a90772df7..5d56803e3b 100755 --- a/brave-core/src/main/java/com/github/kristofa/brave/SpanCollectorReporterAdapter.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/SpanCollectorReporterAdapter.java @@ -37,6 +37,7 @@ public void addDefaultAnnotation(String key, String value) { /** Changes this to a brave-native span object. */ static Span toBrave(zipkin.Span input) { Span result = new Span(); + result.setTrace_id_high(input.traceIdHigh); result.setTrace_id(input.traceId); result.setId(input.id); result.setParent_id(input.parentId); diff --git a/brave-core/src/main/java/com/github/kristofa/brave/SpanId.java b/brave-core/src/main/java/com/github/kristofa/brave/SpanId.java index aae0452e74..702e4ab205 100755 --- a/brave-core/src/main/java/com/github/kristofa/brave/SpanId.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/SpanId.java @@ -49,8 +49,15 @@ public static SpanId create(long traceId, long spanId, @Nullable Long parentSpan */ @Deprecated public SpanId(long traceId, long parentId, long spanId, long flags) { - this.traceId = (parentId == traceId) ? parentId : traceId; - this.parentId = (parentId == spanId) ? traceId : parentId; + this(0, parentId == traceId ? parentId : traceId, + (parentId == spanId) ? traceId : parentId, + spanId, flags); + } + + SpanId(long traceIdHigh, long traceId, long parentId, long spanId, long flags) { + this.traceIdHigh = traceIdHigh; + this.traceId = traceId; + this.parentId = parentId; this.spanId = spanId; this.flags = flags; } @@ -58,17 +65,26 @@ public SpanId(long traceId, long parentId, long spanId, long flags) { /** Deserializes this from a big-endian byte array */ public static SpanId fromBytes(byte[] bytes) { checkNotNull(bytes, "bytes"); - if (bytes.length != 32) { - throw new IllegalArgumentException("bytes.length " + bytes.length + " != 32"); + if (bytes.length != 32 && bytes.length != 40) { + throw new IllegalArgumentException("bytes.length " + bytes.length + " != 32 or 40"); } ByteBuffer buffer = ByteBuffer.wrap(bytes); long spanId = buffer.getLong(0); long parentId = buffer.getLong(8); - long traceId = buffer.getLong(16); - long flags = buffer.getLong(24); - - return new SpanId(traceId, parentId, spanId, flags); + long traceIdHigh; + long traceId; + long flags; + if (bytes.length == 32) { + traceIdHigh = 0; + traceId = buffer.getLong(16); + flags = buffer.getLong(24); + } else { + traceIdHigh = buffer.getLong(16); + traceId = buffer.getLong(24); + flags = buffer.getLong(32); + } + return new SpanId(traceIdHigh, traceId, parentId, spanId, flags); } public static Builder builder() { @@ -109,6 +125,13 @@ public Long getParentSpanId() { return nullableParentId(); } + /** + * When non-zero, the trace containing this span uses 128-bit trace identifiers. + * + * @since 3.15 + */ + public final long traceIdHigh; + /** * Unique 8-byte identifier for a trace, set on all spans within it. */ @@ -162,12 +185,19 @@ public final Boolean sampled() { /** Serializes this into a big-endian byte array */ public byte[] bytes() { - byte[] result = new byte[32]; + boolean traceHi = traceIdHigh != 0; + byte[] result = new byte[traceHi ? 40 : 32]; ByteBuffer buffer = ByteBuffer.wrap(result); buffer.putLong(0, spanId); buffer.putLong(8, parentId); - buffer.putLong(16, traceId); - buffer.putLong(24, flags); + if (traceHi) { + buffer.putLong(16, traceIdHigh); + buffer.putLong(24, traceId); + buffer.putLong(32, flags); + } else { + buffer.putLong(16, traceId); + buffer.putLong(24, flags); + } return result; } @@ -178,13 +208,21 @@ public Builder toBuilder() { /** Returns {@code $traceId.$spanId<:$parentId} */ @Override public String toString() { - char[] result = new char[(3 * 16) + 3]; // 3 ids and the constant delimiters - writeHexLong(result, 0, traceId); - result[16] = '.'; - writeHexLong(result, 17, spanId); - result[33] = '<'; - result[34] = ':'; - writeHexLong(result, 35, parentId); + boolean traceHi = traceIdHigh != 0; + char[] result = new char[((traceHi ? 4 : 3) * 16) + 3]; // 3 ids and the constant delimiters + int pos = 0; + if (traceHi) { + writeHexLong(result, pos, traceIdHigh); + pos += 16; + } + writeHexLong(result, pos, traceId); + pos += 16; + result[pos++] = '.'; + writeHexLong(result, pos, spanId); + pos += 16; + result[pos++] = '<'; + result[pos++] = ':'; + writeHexLong(result, pos, parentId); return new String(result); } @@ -195,7 +233,8 @@ public boolean equals(Object o) { } if (o instanceof SpanId) { SpanId that = (SpanId) o; - return (this.traceId == that.traceId) + return (this.traceIdHigh == that.traceIdHigh) + && (this.traceId == that.traceId) && (this.parentId == that.parentId) && (this.spanId == that.spanId); } @@ -206,6 +245,8 @@ public boolean equals(Object o) { public int hashCode() { int h = 1; h *= 1000003; + h ^= (traceIdHigh >>> 32) ^ traceIdHigh; + h *= 1000003; h ^= (traceId >>> 32) ^ traceId; h *= 1000003; h ^= (parentId >>> 32) ^ parentId; @@ -214,10 +255,28 @@ public int hashCode() { return h; } + /** + * Returns the hex representation of the span's trace ID + * + * @since 3.15 + */ + public String traceIdString() { + if (traceIdHigh != 0) { + char[] result = new char[32]; + writeHexLong(result, 0, traceIdHigh); + writeHexLong(result, 16, traceId); + return new String(result); + } + char[] result = new char[16]; + writeHexLong(result, 0, traceId); + return new String(result); + } + /** Preferred way to create spans, as it properly deals with the parent id */ public Span toSpan() { Span result = new Span(); result.setId(spanId); + result.setTrace_id_high(traceIdHigh); result.setTrace_id(traceId); result.setParent_id(nullableParentId()); result.setName(""); // avoid NPE on equals @@ -226,8 +285,9 @@ public Span toSpan() { } public static final class Builder { + long traceIdHigh = 0; Long traceId; - Long parentId; + Long nullableParentId; Long spanId; long flags; @@ -235,12 +295,19 @@ public static final class Builder { } Builder(SpanId source) { + this.traceIdHigh = source.traceIdHigh; this.traceId = source.traceId; - this.parentId = source.nullableParentId(); + this.nullableParentId = source.nullableParentId(); this.spanId = source.spanId; this.flags = source.flags; } + /** @see SpanId#traceIdHigh */ + public Builder traceIdHigh(long traceIdHigh) { + this.traceIdHigh = traceIdHigh; + return this; + } + /** @see SpanId#traceId */ public Builder traceId(long traceId) { this.traceId = traceId; @@ -258,7 +325,7 @@ public Builder parentId(@Nullable Long parentId) { } else { this.flags &= ~FLAG_IS_ROOT; } - this.parentId = parentId; + this.nullableParentId = parentId; return this; } @@ -300,9 +367,11 @@ public Builder sampled(@Nullable Boolean sampled) { } public SpanId build() { - long traceId = this.traceId != null ? this.traceId : checkNotNull(spanId, "spanId"); - long parentId = this.parentId != null ? this.parentId : traceId; - return new SpanId(traceId, parentId, checkNotNull(spanId, "spanId"), flags); + checkNotNull(spanId, "spanId"); + long traceId = this.traceId != null ? this.traceId : spanId; + long parentId = nullableParentId != null ? nullableParentId : traceId; + if (parentId == spanId) parentId = traceId; + return new SpanId(traceIdHigh, traceId, parentId, spanId, flags); } } diff --git a/brave-core/src/main/java/com/github/kristofa/brave/internal/DefaultSpanCodec.java b/brave-core/src/main/java/com/github/kristofa/brave/internal/DefaultSpanCodec.java index 6c8b8dbc8e..b304681bde 100644 --- a/brave-core/src/main/java/com/github/kristofa/brave/internal/DefaultSpanCodec.java +++ b/brave-core/src/main/java/com/github/kristofa/brave/internal/DefaultSpanCodec.java @@ -38,6 +38,7 @@ public byte[] writeSpans(List spans) { public Span readSpan(byte[] bytes) { zipkin.Span in = codec.readSpan(bytes); Span result = new Span(); + result.setTrace_id_high(in.traceIdHigh); result.setTrace_id(in.traceId); result.setId(in.id); result.setParent_id(in.parentId); diff --git a/brave-core/src/main/java/com/twitter/zipkin/gen/Span.java b/brave-core/src/main/java/com/twitter/zipkin/gen/Span.java index b3255f5313..47167564dc 100644 --- a/brave-core/src/main/java/com/twitter/zipkin/gen/Span.java +++ b/brave-core/src/main/java/com/twitter/zipkin/gen/Span.java @@ -25,6 +25,7 @@ public class Span implements Serializable { public volatile Long startTick; private long trace_id; // required + private long trace_id_high; // optional (default to zero) private String name; // required private long id; // required private Long parent_id; // optional @@ -43,6 +44,21 @@ public Span setTrace_id(long trace_id) { return this; } + /** + * When non-zero, the trace containing this span uses 128-bit trace identifiers. + * + * @since 3.15 + * @see zipkin.Span#traceIdHigh + */ + public long getTrace_id_high() { + return this.trace_id_high; + } + + public Span setTrace_id_high(long trace_id_high) { + this.trace_id_high = trace_id_high; + return this; + } + /** * Span name in lowercase, rpc method for example * @@ -213,7 +229,8 @@ public boolean equals(Object o) { } if (o instanceof Span) { Span that = (Span) o; - return (this.trace_id == that.trace_id) + return (this.trace_id_high == that.trace_id_high) + && (this.trace_id == that.trace_id) && (this.name.equals(that.name)) && (this.id == that.id) && equal(this.parent_id, that.parent_id) @@ -230,6 +247,8 @@ && equal(this.binary_annotations, that.binary_annotations) public int hashCode() { int h = 1; h *= 1000003; + h ^= (trace_id_high >>> 32) ^ trace_id_high; + h *= 1000003; h ^= (trace_id >>> 32) ^ trace_id; h *= 1000003; h ^= name.hashCode(); @@ -259,6 +278,7 @@ public String toString() { public zipkin.Span toZipkin() { zipkin.Span.Builder result = zipkin.Span.builder(); result.traceId(getTrace_id()); + result.traceIdHigh(getTrace_id_high()); result.id(getId()); result.parentId(getParent_id()); result.name(getName()); diff --git a/brave-core/src/test/java/com/github/kristofa/brave/ClientTracerTest.java b/brave-core/src/test/java/com/github/kristofa/brave/ClientTracerTest.java index fcdd094ebf..c3e491c63c 100755 --- a/brave-core/src/test/java/com/github/kristofa/brave/ClientTracerTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/ClientTracerTest.java @@ -26,11 +26,10 @@ @PrepareForTest(AnnotationSubmitter.class) public class ClientTracerTest { - private final static long CURRENT_TIME_MICROSECONDS = System.currentTimeMillis() * 1000; - private final static String REQUEST_NAME = "requestname"; - private static final long PARENT_SPAN_ID = 103; - private static final long PARENT_TRACE_ID = 105; - private static final long TRACE_ID = 32534; + private static final long CURRENT_TIME_MICROSECONDS = System.currentTimeMillis() * 1000; + private static final String REQUEST_NAME = "requestname"; + private static final long TRACE_ID = 105; + private static final SpanId PARENT_SPAN_ID = SpanId.builder().traceId(TRACE_ID).spanId(103).build(); private ServerClientAndLocalSpanState state = new TestServerClientAndLocalSpanStateCompilation(); private Random mockRandom; @@ -54,6 +53,7 @@ public void setup() { .spanCollector(mockCollector) .traceSampler(mockSampler) .clock(AnnotationSubmitter.DefaultClock.INSTANCE) + .traceId128Bit(false) .build(); } @@ -159,7 +159,7 @@ public void testStartNewSpanSampleFalse() { @Test public void testStartNewSpanSampleNullNotPartOfExistingSpan() { - state.setCurrentServerSpan(ServerSpan.create(null)); + state.setCurrentServerSpan(ServerSpan.EMPTY); when(mockRandom.nextLong()).thenReturn(TRACE_ID); when(mockSampler.isSampled(TRACE_ID)).thenReturn(true); @@ -171,7 +171,7 @@ public void testStartNewSpanSampleNullNotPartOfExistingSpan() { assertNull(newSpanId.nullableParentId()); assertEquals( - new Span().setTrace_id(TRACE_ID).setId(TRACE_ID).setName(REQUEST_NAME), + SpanId.builder().spanId(TRACE_ID).build().toSpan().setName(REQUEST_NAME), state.getCurrentClientSpan() ); @@ -180,40 +180,20 @@ public void testStartNewSpanSampleNullNotPartOfExistingSpan() { verifyNoMoreInteractions(mockCollector, mockSampler); } - @Test - public void testStartNewSpanSampleTrueNotPartOfExistingSpan() { - state.setCurrentServerSpan(ServerSpan.create(true)); - - when(mockRandom.nextLong()).thenReturn(TRACE_ID); - - final SpanId newSpanId = clientTracer.startNewSpan(REQUEST_NAME); - assertNotNull(newSpanId); - assertEquals(TRACE_ID, newSpanId.traceId); - assertEquals(TRACE_ID, newSpanId.spanId); - assertNull(newSpanId.nullableParentId()); - - assertEquals( - new Span().setTrace_id(TRACE_ID).setId(TRACE_ID).setName(REQUEST_NAME), - state.getCurrentClientSpan() - ); - - verifyNoMoreInteractions(mockCollector, mockSampler); - } - @Test public void testStartNewSpanSampleTruePartOfExistingSpan() { - final ServerSpan parentSpan = ServerSpan.create(PARENT_TRACE_ID, PARENT_SPAN_ID, null, "name"); + final ServerSpan parentSpan = ServerSpan.create(PARENT_SPAN_ID.toSpan().setName("name")); state.setCurrentServerSpan(parentSpan); when(mockRandom.nextLong()).thenReturn(1L); final SpanId newSpanId = clientTracer.startNewSpan(REQUEST_NAME); assertNotNull(newSpanId); - assertEquals(PARENT_TRACE_ID, newSpanId.traceId); + assertEquals(TRACE_ID, newSpanId.traceId); assertEquals(1L, newSpanId.spanId); - assertEquals(PARENT_SPAN_ID, newSpanId.parentId); + assertEquals(PARENT_SPAN_ID.spanId, newSpanId.parentId); assertEquals( - new Span().setTrace_id(PARENT_TRACE_ID).setId(1).setParent_id(PARENT_SPAN_ID).setName(REQUEST_NAME), + newSpanId.toSpan().setName(REQUEST_NAME), state.getCurrentClientSpan() ); @@ -222,7 +202,7 @@ public void testStartNewSpanSampleTruePartOfExistingSpan() { @Test public void testSamplerFalse() { - state.setCurrentServerSpan(ServerSpan.create(null, null)); + state.setCurrentServerSpan(ServerSpan.EMPTY); when(mockSampler.isSampled(TRACE_ID)).thenReturn(false); when(mockRandom.nextLong()).thenReturn(TRACE_ID); @@ -268,4 +248,38 @@ public void setClientReceived_lessThanMicrosRoundUp() { assertEquals(1L, finished.getDuration().longValue()); } + + @Test + public void startNewSpan_whenParentHas128bitTraceId() { + ServerSpan parentSpan = ServerSpan.create( + PARENT_SPAN_ID.toBuilder().traceIdHigh(3).build().toSpan().setName("name")); + state.setCurrentServerSpan(parentSpan); + when(mockRandom.nextLong()).thenReturn(1L); + + SpanId newSpanId = clientTracer.startNewSpan(REQUEST_NAME); + assertEquals(3, newSpanId.traceIdHigh); + assertEquals(TRACE_ID, newSpanId.traceId); + } + + @Test + public void startNewSpan_rootSpanWith64bitTraceId() { + when(mockRandom.nextLong()).thenReturn(TRACE_ID); + when(mockSampler.isSampled(TRACE_ID)).thenReturn(true); + + SpanId newSpanId = clientTracer.startNewSpan(REQUEST_NAME); + assertEquals(0, newSpanId.traceIdHigh); + assertEquals(TRACE_ID, newSpanId.traceId); + } + + @Test + public void startNewSpan_rootSpanWith128bitTraceId() { + clientTracer = new AutoValue_ClientTracer.Builder(clientTracer) + .traceId128Bit(true).build(); + when(mockRandom.nextLong()).thenReturn(TRACE_ID, TRACE_ID + 1); + when(mockSampler.isSampled(TRACE_ID)).thenReturn(true); + + SpanId newSpanId = clientTracer.startNewSpan(REQUEST_NAME); + assertEquals(TRACE_ID + 1, newSpanId.traceIdHigh); + assertEquals(TRACE_ID, newSpanId.traceId); + } } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/ITBrave.java b/brave-core/src/test/java/com/github/kristofa/brave/ITBrave.java index 3149ef5cce..eb29958871 100755 --- a/brave-core/src/test/java/com/github/kristofa/brave/ITBrave.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/ITBrave.java @@ -65,7 +65,8 @@ public Integer call() throws Exception { final Random random = new Random(); final String serverSpanName = "server span name " + random.nextLong(); - serverTracer.setStateCurrentTrace(random.nextLong(), random.nextLong(), null, serverSpanName); + SpanId spanId = SpanId.builder().spanId(random.nextLong()).build(); + serverTracer.setStateCurrentTrace(spanId, serverSpanName); serverTracer.setServerReceived(); diff --git a/brave-core/src/test/java/com/github/kristofa/brave/IdConversionTest.java b/brave-core/src/test/java/com/github/kristofa/brave/IdConversionTest.java index 6c02ef3a22..4abc7f4434 100644 --- a/brave-core/src/test/java/com/github/kristofa/brave/IdConversionTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/IdConversionTest.java @@ -73,8 +73,14 @@ public void testIdShouldntBeUppercase() { } @Test - public void lowerHexToUnsignedLong_downgrades128bitIdsByDroppingHighBits() { + public void convertToLong_downgrades128bitIdsByDroppingHighBits() { assertThat(IdConversion.convertToLong("463ac35c9f6413ad48485a3953bb6124")) .isEqualTo(IdConversion.convertToLong("48485a3953bb6124")); } + + @Test + public void convertToLong_worksOnOffset() { + assertThat(IdConversion.convertToLong("463ac35c9f6413ad48485a3953bb6124", 0)) + .isEqualTo(IdConversion.convertToLong("463ac35c9f6413ad")); + } } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/InheritableServerClientAndLocalSpanStateTest.java b/brave-core/src/test/java/com/github/kristofa/brave/InheritableServerClientAndLocalSpanStateTest.java index da764fd667..3836413f56 100644 --- a/brave-core/src/test/java/com/github/kristofa/brave/InheritableServerClientAndLocalSpanStateTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/InheritableServerClientAndLocalSpanStateTest.java @@ -52,7 +52,7 @@ public void tearDown() { @Test public void testGetAndSetCurrentServerSpan() { - assertEquals(ServerSpan.create(null), state.getCurrentServerSpan()); + assertEquals(ServerSpan.EMPTY, state.getCurrentServerSpan()); state.setCurrentServerSpan(mockServerSpan); assertSame(mockServerSpan, state.getCurrentServerSpan()); assertNull("Should not have been modified.", state.getCurrentClientSpan()); @@ -63,7 +63,7 @@ public void testGetAndSetCurrentClientSpan() { assertNull(state.getCurrentClientSpan()); state.setCurrentClientSpan(mockSpan); assertSame(mockSpan, state.getCurrentClientSpan()); - assertEquals("Should not have been modified.", ServerSpan.create(null), + assertEquals("Should not have been modified.", ServerSpan.EMPTY, state.getCurrentServerSpan()); } @@ -73,8 +73,7 @@ public void testGetAndSetCurrentLocalSpan() { assertNull(state.getCurrentLocalSpan()); state.setCurrentLocalSpan(mockSpan); assertSame(mockSpan, state.getCurrentLocalSpan()); - assertEquals("Should not have been modified.", ServerSpan.create(null), - state.getCurrentServerSpan()); + assertEquals("Should not have been modified.", ServerSpan.EMPTY, state.getCurrentServerSpan()); assertNull(state.getCurrentClientSpan()); } @@ -84,7 +83,7 @@ public void testGetAndSetCurrentLocalSpanInheritence() { assertNull(state.getCurrentLocalSpan()); state.setCurrentLocalSpan(mockSpan); assertSame(mockSpan, state.getCurrentLocalSpan()); - assertEquals("Should not have been modified.", ServerSpan.create(null), + assertEquals("Should not have been modified.", ServerSpan.EMPTY, state.getCurrentServerSpan()); assertNull(state.getCurrentClientSpan()); } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/LocalTracerTest.java b/brave-core/src/test/java/com/github/kristofa/brave/LocalTracerTest.java index e9f1b47529..962a49be73 100755 --- a/brave-core/src/test/java/com/github/kristofa/brave/LocalTracerTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/LocalTracerTest.java @@ -25,9 +25,8 @@ @RunWith(PowerMockRunner.class) @PrepareForTest({AnnotationSubmitter.class, LocalTracer.class}) public class LocalTracerTest { - - private static final long PARENT_TRACE_ID = 105; - private static final long PARENT_SPAN_ID = 103; + private static final long TRACE_ID = 105; + private static final SpanId PARENT_SPAN_ID = SpanId.builder().traceId(TRACE_ID).spanId(103).build(); private static final String COMPONENT_NAME = "componentname"; private static final String OPERATION_NAME = "operationname"; @@ -52,6 +51,7 @@ public void setup() { .allowNestedLocalSpans(false) .traceSampler(Sampler.create(1.0f)) .clock(AnnotationSubmitter.DefaultClock.INSTANCE) + .traceId128Bit(false) .build(); } @@ -66,13 +66,12 @@ public void setup() { */ @Test public void startNewSpan() { - state.setCurrentServerSpan(ServerSpan.create(PARENT_TRACE_ID, PARENT_SPAN_ID, null, "name")); + state.setCurrentServerSpan(ServerSpan.create(PARENT_SPAN_ID.toSpan().setName("name"))); PowerMockito.when(System.currentTimeMillis()).thenReturn(1L); PowerMockito.when(System.nanoTime()).thenReturn(500L); - SpanId expectedSpanId = - SpanId.builder().traceId(PARENT_TRACE_ID).spanId(555L).parentId(PARENT_SPAN_ID).build(); + SpanId expectedSpanId = PARENT_SPAN_ID.toBuilder().spanId(555L).parentId(PARENT_SPAN_ID.spanId).build(); assertEquals(expectedSpanId, localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME)); @@ -98,7 +97,7 @@ public void startNewSpan() { */ @Test public void startSpan_userSuppliedTimestamp() { - state.setCurrentServerSpan(ServerSpan.create(PARENT_TRACE_ID, PARENT_SPAN_ID, null, "name")); + state.setCurrentServerSpan(ServerSpan.create(PARENT_SPAN_ID.toSpan().setName("name"))); localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME, 1000L); @@ -259,14 +258,14 @@ public void startSpan_with_inheritable_nested_local_spans() { .build(); assertNull(localTracer.getNewSpanParent()); - state.setCurrentServerSpan(ServerSpan.create(PARENT_TRACE_ID, PARENT_SPAN_ID, null, "name")); + state.setCurrentServerSpan(ServerSpan.create(PARENT_SPAN_ID.toSpan().setName("name"))); SpanId span1 = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); - assertEquals(SpanId.builder().traceId(PARENT_TRACE_ID).spanId(555L).parentId(PARENT_SPAN_ID).build(), span1); + assertEquals(PARENT_SPAN_ID.toBuilder().spanId(555L).parentId(PARENT_SPAN_ID.spanId).build(), span1); assertEquals(span1.spanId, localTracer.getNewSpanParent().getId()); SpanId span2 = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); - assertEquals(SpanId.builder().traceId(PARENT_TRACE_ID).spanId(555L).parentId(span1.spanId).build(), span2); + assertEquals(PARENT_SPAN_ID.toBuilder().spanId(555L).parentId(span1.spanId).build(), span2); assertEquals(span2.spanId, localTracer.getNewSpanParent().getId()); assertEquals(span2.spanId, state.getCurrentLocalSpan().getId()); @@ -286,16 +285,15 @@ public void startSpan_nested_local_spans_disabled() { .build(); assertNull(localTracer.getNewSpanParent()); - final ServerSpan serverSpan = ServerSpan.create(PARENT_TRACE_ID, PARENT_SPAN_ID, null, "name"); - state.setCurrentServerSpan(serverSpan); + state.setCurrentServerSpan(ServerSpan.create(PARENT_SPAN_ID.toSpan().setName("name"))); SpanId span1 = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); - assertEquals(SpanId.builder().traceId(PARENT_TRACE_ID).spanId(555L).parentId(PARENT_SPAN_ID).build(), span1); - assertEquals(serverSpan.getSpan(), localTracer.getNewSpanParent()); + assertEquals(PARENT_SPAN_ID.toBuilder().spanId(555L).parentId(PARENT_SPAN_ID.spanId).build(), span1); + assertEquals(state.getCurrentServerSpan().getSpan(), localTracer.getNewSpanParent()); SpanId span2 = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); - assertEquals(SpanId.builder().traceId(PARENT_TRACE_ID).spanId(555L).parentId(PARENT_SPAN_ID).build(), span2); - assertEquals(serverSpan.getSpan(), localTracer.getNewSpanParent()); + assertEquals(PARENT_SPAN_ID.toBuilder().spanId(555L).parentId(PARENT_SPAN_ID.spanId).build(), span2); + assertEquals(state.getCurrentServerSpan().getSpan(), localTracer.getNewSpanParent()); assertEquals(span2.spanId, state.getCurrentLocalSpan().getId()); localTracer.finishSpan(); @@ -304,4 +302,35 @@ public void startSpan_nested_local_spans_disabled() { assertNull(state.getCurrentLocalSpan()); } + @Test + public void startNewSpan_whenParentHas128bitTraceId() { + ServerSpan parentSpan = ServerSpan.create( + PARENT_SPAN_ID.toBuilder().traceIdHigh(3).build().toSpan().setName("name")); + state.setCurrentServerSpan(parentSpan); + when(mockRandom.nextLong()).thenReturn(1L); + + SpanId newSpanId = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); + assertEquals(3, newSpanId.traceIdHigh); + assertEquals(TRACE_ID, newSpanId.traceId); + } + + @Test + public void startNewSpan_rootSpanWith64bitTraceId() { + when(mockRandom.nextLong()).thenReturn(1L); + + SpanId newSpanId = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); + assertEquals(0, newSpanId.traceIdHigh); + assertEquals(1, newSpanId.traceId); + } + + @Test + public void startNewSpan_rootSpanWith128bitTraceId() { + localTracer = new AutoValue_LocalTracer.Builder(localTracer) + .traceId128Bit(true).build(); + when(mockRandom.nextLong()).thenReturn(1L, 3L); + + SpanId newSpanId = localTracer.startNewSpan(COMPONENT_NAME, OPERATION_NAME); + assertEquals(3, newSpanId.traceIdHigh); + assertEquals(1, newSpanId.traceId); + } } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/ServerRequestInterceptorTest.java b/brave-core/src/test/java/com/github/kristofa/brave/ServerRequestInterceptorTest.java index a56248d6d7..739adb43f0 100644 --- a/brave-core/src/test/java/com/github/kristofa/brave/ServerRequestInterceptorTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/ServerRequestInterceptorTest.java @@ -1,7 +1,6 @@ package com.github.kristofa.brave; -import com.twitter.zipkin.gen.Annotation; import com.twitter.zipkin.gen.Endpoint; import com.twitter.zipkin.gen.Span; import java.util.Random; @@ -12,7 +11,6 @@ import java.util.Arrays; import java.util.Collections; -import static com.github.kristofa.brave.Sampler.ALWAYS_SAMPLE; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; @@ -72,6 +70,7 @@ public void handleSampleRequestWithParentSpanId() { .spanCollector(mock(SpanCollector.class)) .traceSampler(Sampler.ALWAYS_SAMPLE) .clock(AnnotationSubmitter.DefaultClock.INSTANCE) + .traceId128Bit(false) .build(); interceptor = new ServerRequestInterceptor(serverTracer); @@ -105,12 +104,13 @@ public void handleSampleRequestWithParentSpanId() { public void handleSampleRequestWithoutParentSpanId() { InheritableServerClientAndLocalSpanState state = new InheritableServerClientAndLocalSpanState(mock(Endpoint.class)); - serverTracer = ServerTracer.builder() + serverTracer = new AutoValue_ServerTracer.Builder(serverTracer) .state(state) .randomGenerator(mock(Random.class)) .spanCollector(mock(SpanCollector.class)) .traceSampler(Sampler.ALWAYS_SAMPLE) .clock(AnnotationSubmitter.DefaultClock.INSTANCE) + .traceId128Bit(false) .build(); interceptor = new ServerRequestInterceptor(serverTracer); diff --git a/brave-core/src/test/java/com/github/kristofa/brave/ServerSpanTest.java b/brave-core/src/test/java/com/github/kristofa/brave/ServerSpanTest.java index c4d6495ca2..6c4164791a 100644 --- a/brave-core/src/test/java/com/github/kristofa/brave/ServerSpanTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/ServerSpanTest.java @@ -1,9 +1,7 @@ package com.github.kristofa.brave; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import org.junit.Assert; @@ -13,30 +11,15 @@ import com.twitter.zipkin.gen.Span; public class ServerSpanTest { - private static final long TRACE_ID = 1; - private static final long SPAN_ID = 2; - private static final Long PARENT_SPAN_ID = Long.valueOf(3); + private static final SpanId SPAN_ID = SpanId.builder().traceId(TRACE_ID).spanId(2).parentId(3L).build(); private static final String NAME = "name"; + private ServerSpan serverSpan; @Before public void setup() { - serverSpan = ServerSpan.create(TRACE_ID, SPAN_ID, PARENT_SPAN_ID, NAME); - } - - @Test - public void testServerSpanSampleNull() { - final ServerSpan serverSpan = ServerSpan.create(null); - assertNull(serverSpan.getSample()); - assertNull(serverSpan.getSpan()); - } - - @Test - public void testServerSpanSampleFalse() { - final ServerSpan serverSpan = ServerSpan.create(false); - assertFalse(serverSpan.getSample()); - assertNull(serverSpan.getSpan()); + serverSpan = ServerSpan.create(SPAN_ID.toSpan().setName(NAME)); } @Test @@ -44,13 +27,22 @@ public void testGetSpan() { final Span span = serverSpan.getSpan(); assertNotNull(span); assertEquals(TRACE_ID, span.getTrace_id()); - assertEquals(SPAN_ID, span.getId()); - assertEquals(PARENT_SPAN_ID.longValue(), span.getParent_id().longValue()); + assertEquals(SPAN_ID.spanId, span.getId()); + assertEquals(SPAN_ID.parentId, span.getParent_id().longValue()); assertEquals(NAME, span.getName()); assertTrue(span.getAnnotations().isEmpty()); assertTrue(span.getBinary_annotations().isEmpty()); } + @Test + public void testGetSpan_128() { + serverSpan = ServerSpan.create(SPAN_ID.toSpan().setTrace_id_high(5).setName(NAME)); + + Span span = serverSpan.getSpan(); + assertEquals(5, span.getTrace_id_high()); + assertEquals(TRACE_ID, span.getTrace_id()); + } + @Test public void testGetSample() { assertTrue(serverSpan.getSample()); @@ -59,13 +51,13 @@ public void testGetSample() { @Test public void testEqualsObject() { - final ServerSpan equalServerSpan = ServerSpan.create(TRACE_ID, SPAN_ID, PARENT_SPAN_ID, NAME); + ServerSpan equalServerSpan = ServerSpan.create(SPAN_ID.toSpan().setName(NAME)); assertTrue(serverSpan.equals(equalServerSpan)); } @Test public void testHashCode() { - final ServerSpan equalServerSpan = ServerSpan.create(TRACE_ID, SPAN_ID, PARENT_SPAN_ID, NAME); + ServerSpan equalServerSpan = ServerSpan.create(SPAN_ID.toSpan().setName(NAME)); Assert.assertEquals(serverSpan.hashCode(), equalServerSpan.hashCode()); } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/ServerTracerTest.java b/brave-core/src/test/java/com/github/kristofa/brave/ServerTracerTest.java index d8da130bef..ce6fc38aeb 100755 --- a/brave-core/src/test/java/com/github/kristofa/brave/ServerTracerTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/ServerTracerTest.java @@ -27,11 +27,10 @@ @PrepareForTest(AnnotationSubmitter.class) public class ServerTracerTest { - private final static long CURRENT_TIME_MICROSECONDS = System.currentTimeMillis() * 1000; - private final static long TRACE_ID = 1L; - private final static long SPAN_ID = 2L; - private final static Long PARENT_SPANID = 3L; - private final static String SPAN_NAME = "span name"; + private static final long CURRENT_TIME_MICROSECONDS = System.currentTimeMillis() * 1000; + private static final long TRACE_ID = 1; + private static final SpanId SPAN_ID = SpanId.builder().traceId(TRACE_ID).spanId(2).parentId(3L).build(); + private static final String SPAN_NAME = "span name"; private ServerTracer serverTracer; private ServerSpanState mockServerSpanState; @@ -60,6 +59,7 @@ public void setup() { .spanCollector(mockSpanCollector) .traceSampler(mockSampler) .clock(AnnotationSubmitter.DefaultClock.INSTANCE) + .traceId128Bit(false) .build(); } @@ -72,8 +72,8 @@ public void testClearCurrentSpan() { @Test public void testSetStateCurrentTrace() { - serverTracer.setStateCurrentTrace(TRACE_ID, SPAN_ID, PARENT_SPANID, SPAN_NAME); - final ServerSpan expectedServerSpan = ServerSpan.create(TRACE_ID, SPAN_ID, PARENT_SPANID, SPAN_NAME); + serverTracer.setStateCurrentTrace(SPAN_ID, SPAN_NAME); + ServerSpan expectedServerSpan = ServerSpan.create(SPAN_ID.toSpan().setName(SPAN_NAME)); verify(mockServerSpanState).setCurrentServerSpan(expectedServerSpan); verifyNoMoreInteractions(mockServerSpanState, mockSpanCollector); } @@ -81,8 +81,7 @@ public void testSetStateCurrentTrace() { @Test public void testSetStateNoTracing() { serverTracer.setStateNoTracing(); - final ServerSpan expectedServerSpan = ServerSpan.create(false); - verify(mockServerSpanState).setCurrentServerSpan(expectedServerSpan); + verify(mockServerSpanState).setCurrentServerSpan(ServerSpan.NOT_SAMPLED); verifyNoMoreInteractions(mockServerSpanState, mockSpanCollector); } @@ -93,7 +92,8 @@ public void testSetStateUnknownSamplerTrue() { when(mockSampler.isSampled(TRACE_ID)).thenReturn(true); serverTracer.setStateUnknown(SPAN_NAME); - final ServerSpan expectedServerSpan = ServerSpan.create(TRACE_ID, TRACE_ID, null, SPAN_NAME); + ServerSpan expectedServerSpan = ServerSpan.create( + SpanId.builder().spanId(TRACE_ID).build().toSpan().setName(SPAN_NAME)); final InOrder inOrder = inOrder(mockSampler, mockRandom, mockServerSpanState); @@ -105,20 +105,41 @@ public void testSetStateUnknownSamplerTrue() { } @Test - public void testSetStateUnknownSamplerFalse() { + public void testSetStateUnknownSamplerTrue_128Bit() { + serverTracer = new AutoValue_ServerTracer.Builder(serverTracer) + .traceId128Bit(true).build(); + + when(mockRandom.nextLong()).thenReturn(TRACE_ID, TRACE_ID + 1); + when(mockSampler.isSampled(TRACE_ID)).thenReturn(true); + + serverTracer.setStateUnknown(SPAN_NAME); + ServerSpan expectedServerSpan = ServerSpan.create(SpanId.builder() + .traceIdHigh(TRACE_ID + 1) + .traceId(TRACE_ID) + .spanId(TRACE_ID).build().toSpan().setName(SPAN_NAME)); + + final InOrder inOrder = inOrder(mockSampler, mockRandom, mockServerSpanState); + + inOrder.verify(mockRandom).nextLong(); + inOrder.verify(mockSampler).isSampled(TRACE_ID); + inOrder.verify(mockRandom).nextLong(); + inOrder.verify(mockServerSpanState).setCurrentServerSpan(expectedServerSpan); + + verifyNoMoreInteractions(mockServerSpanState, mockSpanCollector, mockRandom); + } + @Test + public void testSetStateUnknownSamplerFalse() { when(mockRandom.nextLong()).thenReturn(TRACE_ID); when(mockSampler.isSampled(TRACE_ID)).thenReturn(false); - final ServerSpan expectedServerSpan = ServerSpan.create(false); - serverTracer.setStateUnknown(SPAN_NAME); final InOrder inOrder = inOrder(mockSampler, mockRandom, mockServerSpanState); inOrder.verify(mockRandom).nextLong(); inOrder.verify(mockSampler).isSampled(TRACE_ID); - inOrder.verify(mockServerSpanState).setCurrentServerSpan(expectedServerSpan); + inOrder.verify(mockServerSpanState).setCurrentServerSpan(ServerSpan.NOT_SAMPLED); verifyNoMoreInteractions(mockServerSpanState, mockSpanCollector, mockRandom); } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/SpanIdTest.java b/brave-core/src/test/java/com/github/kristofa/brave/SpanIdTest.java index 2b8fd41c13..2cbe978bae 100644 --- a/brave-core/src/test/java/com/github/kristofa/brave/SpanIdTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/SpanIdTest.java @@ -2,6 +2,7 @@ import com.twitter.finagle.tracing.TraceId; import com.twitter.finagle.tracing.TraceId$; +import com.twitter.zipkin.gen.Span; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -12,7 +13,6 @@ public class SpanIdTest { @Rule public ExpectedException thrown = ExpectedException.none(); - @Test public void rootSpan_whenTraceIdsAreSpanIds() { SpanId id = SpanId.builder().spanId(333L).build(); @@ -156,6 +156,24 @@ public void testToStringNullParent() { .isEqualTo(TraceId$.MODULE$.deserialize(id.bytes()).get().toString()); } + @Test + public void testToString_hi() { + SpanId id = SpanId.builder().traceIdHigh(1).traceId(2).spanId(3).parentId(2L).build(); + + assertThat(id.toString()) + .isEqualTo("00000000000000010000000000000002.0000000000000003<:0000000000000002"); + } + + @Test + public void testToStringNullParent_hi() { + SpanId id = SpanId.builder().traceIdHigh(1).traceId(2).spanId(1).build(); + + assertThat(id.toString()) + .isEqualTo("00000000000000010000000000000002.0000000000000001<:0000000000000002"); + } + + // TODO: update finagle to 128-bit and check accordingly + // https://github.com/twitter/finagle/issues/564 static void checkAgainstFinagle(SpanId brave) { TraceId finagle = TraceId$.MODULE$.deserialize(brave.bytes()).get(); @@ -171,4 +189,40 @@ static void checkAgainstFinagle(SpanId brave) { assertThat(SpanId.fromBytes(TraceId.serialize(finagle))) .isEqualTo(brave); } + + @Test + public void traceIdString() { + SpanId id = SpanId.builder().traceId(1).spanId(1).build(); + + assertThat(id.traceIdString()) + .isEqualTo("0000000000000001"); + } + + @Test + public void traceIdString_128() { + SpanId id = SpanId.builder().traceIdHigh(1).traceId(2).spanId(3).parentId(2L).build(); + + assertThat(id.traceIdString()) + .isEqualTo("00000000000000010000000000000002"); + } + + + @Test + public void serializeRoundTrip_128() { + SpanId id = SpanId.builder().traceIdHigh(1).traceId(2).spanId(3).parentId(2L).build(); + + assertThat(SpanId.fromBytes(id.bytes())) + .isEqualTo(id); + } + + @Test + public void toSpan_128() { + SpanId id = SpanId.builder().traceIdHigh(1).traceId(2).spanId(3).parentId(2L).build(); + + Span span = id.toSpan(); + assertThat(span.getTrace_id_high()).isEqualTo(id.traceIdHigh); + assertThat(span.getTrace_id()).isEqualTo(id.traceId); + assertThat(span.getId()).isEqualTo(id.spanId); + assertThat(span.getParent_id()).isEqualTo(id.parentId); + } } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/ThreadLocalServerClientAndLocalSpanStateTest.java b/brave-core/src/test/java/com/github/kristofa/brave/ThreadLocalServerClientAndLocalSpanStateTest.java index a7a3a1ac25..e41d37b44c 100755 --- a/brave-core/src/test/java/com/github/kristofa/brave/ThreadLocalServerClientAndLocalSpanStateTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/ThreadLocalServerClientAndLocalSpanStateTest.java @@ -9,19 +9,15 @@ import org.junit.Before; import org.junit.Test; -import com.twitter.zipkin.gen.Endpoint; import com.twitter.zipkin.gen.Span; public class ThreadLocalServerClientAndLocalSpanStateTest { private static final short PORT = 80; private static final String SERVICE_NAME = "service"; - private static final long DURATION1 = 10; - private static final long DURATION2 = 30; private ThreadLocalServerClientAndLocalSpanState serverAndClientSpanState; private ServerSpan mockServerSpan; private Span mockSpan; - private Endpoint mockEndpoint; @Before public void setup() { @@ -29,7 +25,6 @@ public void setup() { serverAndClientSpanState = new ThreadLocalServerClientAndLocalSpanState(-1062731775, PORT, SERVICE_NAME); mockServerSpan = mock(ServerSpan.class); mockSpan = mock(Span.class); - mockEndpoint = mock(Endpoint.class); } @After @@ -40,7 +35,7 @@ public void tearDown() { @Test public void testGetAndSetCurrentServerSpan() { - assertEquals(ServerSpan.create(null), serverAndClientSpanState.getCurrentServerSpan()); + assertEquals(ServerSpan.EMPTY, serverAndClientSpanState.getCurrentServerSpan()); serverAndClientSpanState.setCurrentServerSpan(mockServerSpan); assertSame(mockServerSpan, serverAndClientSpanState.getCurrentServerSpan()); assertNull("Should not have been modified.", serverAndClientSpanState.getCurrentClientSpan()); @@ -51,7 +46,7 @@ public void testGetAndSetCurrentClientSpan() { assertNull(serverAndClientSpanState.getCurrentClientSpan()); serverAndClientSpanState.setCurrentClientSpan(mockSpan); assertSame(mockSpan, serverAndClientSpanState.getCurrentClientSpan()); - assertEquals("Should not have been modified.", ServerSpan.create(null), + assertEquals("Should not have been modified.", ServerSpan.EMPTY, serverAndClientSpanState.getCurrentServerSpan()); } diff --git a/brave-core/src/test/java/com/github/kristofa/brave/internal/DefaultSpanCodecTest.java b/brave-core/src/test/java/com/github/kristofa/brave/internal/DefaultSpanCodecTest.java index 70a363e5d6..bee9a6eddf 100644 --- a/brave-core/src/test/java/com/github/kristofa/brave/internal/DefaultSpanCodecTest.java +++ b/brave-core/src/test/java/com/github/kristofa/brave/internal/DefaultSpanCodecTest.java @@ -35,9 +35,25 @@ public void roundTripSpan_thrift() { assertEquals(span, DefaultSpanCodec.THRIFT.readSpan(encoded)); } + @Test + public void roundTripSpan_thrift_128() { + span.setTrace_id_high(3L); + + byte[] encoded = DefaultSpanCodec.THRIFT.writeSpan(span); + assertEquals(span, DefaultSpanCodec.THRIFT.readSpan(encoded)); + } + @Test public void roundTripSpan_json() { byte[] encoded = DefaultSpanCodec.JSON.writeSpan(span); assertEquals(span, DefaultSpanCodec.JSON.readSpan(encoded)); } + + @Test + public void roundTripSpan_json_128() { + span.setTrace_id_high(3L); + + byte[] encoded = DefaultSpanCodec.JSON.writeSpan(span); + assertEquals(span, DefaultSpanCodec.JSON.readSpan(encoded)); + } } diff --git a/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcClientInterceptor.java b/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcClientInterceptor.java index 80689734f5..e2cce88514 100644 --- a/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcClientInterceptor.java +++ b/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcClientInterceptor.java @@ -88,7 +88,7 @@ public void addSpanIdToRequest(@Nullable SpanId spanId) { headers.put(BravePropagationKeys.Sampled, "0"); } else { headers.put(BravePropagationKeys.Sampled, "1"); - headers.put(BravePropagationKeys.TraceId, IdConversion.convertToString(spanId.traceId)); + headers.put(BravePropagationKeys.TraceId, spanId.traceIdString()); headers.put(BravePropagationKeys.SpanId, IdConversion.convertToString(spanId.spanId)); if (spanId.nullableParentId() != null) { headers.put(BravePropagationKeys.ParentSpanId, IdConversion.convertToString(spanId.parentId)); diff --git a/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcServerInterceptor.java b/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcServerInterceptor.java index c4714a1591..d03f31c841 100644 --- a/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcServerInterceptor.java +++ b/brave-grpc/src/main/java/com/github/kristofa/brave/grpc/BraveGrpcServerInterceptor.java @@ -125,6 +125,7 @@ public Collection responseAnnotations() { static SpanId getSpanId(String traceId, String spanId, String parentSpanId) { return SpanId.builder() + .traceIdHigh(traceId.length() == 32 ? convertToLong(traceId, 0) : 0) .traceId(convertToLong(traceId)) .spanId(convertToLong(spanId)) .parentId(parentSpanId == null ? null : convertToLong(parentSpanId)).build(); diff --git a/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/BraveGrpcInterceptorsTest.java b/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/BraveGrpcInterceptorsTest.java index 7e4997bf76..f11be2e215 100644 --- a/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/BraveGrpcInterceptorsTest.java +++ b/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/BraveGrpcInterceptorsTest.java @@ -141,6 +141,28 @@ public void noSamplesWhenSamplingDisabled() throws Exception { assertThat(spans.size()).isEqualTo(0); } + @Test + public void propagatesAndReads128BitTraceId() throws Exception { + SpanId spanId = SpanId.builder().traceIdHigh(1).traceId(2).spanId(3).parentId(2L).build(); + brave.localSpanThreadBinder().setCurrentSpan(spanId.toSpan().setName("myop")); + GreeterBlockingStub stub = GreeterGrpc.newBlockingStub(channel); + //This call will be made using hte context of the localTracer as it's parent + HelloReply reply = stub.sayHello(HELLO_REQUEST); + assertThat(reply.getMessage()).isEqualTo("Hello brave"); + validateSpans(); + List spans = SpanCollectorForTesting.INSTANCE.getCollectedSpans(); + Optional maybeSpan = spans.stream() + .filter(s -> s.getAnnotations().stream().anyMatch(a -> "ss".equals(a.value))) + .findFirst(); + assertTrue("Could not find expected server span", maybeSpan.isPresent()); + Span span = maybeSpan.get(); + + //Verify that the 128-bit trace id and span id were propagated to the server + assertThat(span.getTrace_id_high()).isEqualTo(spanId.traceIdHigh); + assertThat(span.getTrace_id()).isEqualTo(spanId.traceId); + assertThat(span.getParent_id()).isEqualTo(spanId.spanId); + } + /** * Validating that two spans were generated indicates that a span was generated by both the * server and the client. diff --git a/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/GrpcClientRequestAdapterTest.java b/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/GrpcClientRequestAdapterTest.java index 554f69ac3f..d7e2fee646 100644 --- a/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/GrpcClientRequestAdapterTest.java +++ b/brave-grpc/src/test/java/com/github/kristofa/brave/grpc/GrpcClientRequestAdapterTest.java @@ -35,7 +35,7 @@ public void sampled_rootSpan() throws Exception { assertThat(metadata.get(BravePropagationKeys.Sampled)) .isEqualTo("1"); assertThat(metadata.get(BravePropagationKeys.TraceId)) - .isEqualTo("4d2"); + .isEqualTo("00000000000004d2"); assertThat(metadata.get(BravePropagationKeys.SpanId)) .isEqualTo("4d2"); } @@ -50,10 +50,25 @@ public void sampled_childSpan() throws Exception { assertThat(metadata.get(BravePropagationKeys.Sampled)) .isEqualTo("1"); assertThat(metadata.get(BravePropagationKeys.TraceId)) - .isEqualTo("4d2"); + .isEqualTo("00000000000004d2"); assertThat(metadata.get(BravePropagationKeys.ParentSpanId)) .isEqualTo("4d2"); assertThat(metadata.get(BravePropagationKeys.SpanId)) .isEqualTo("162e"); } + + @Test + public void traceId_when128bit() throws Exception { + adapter.addSpanIdToRequest(SpanId.builder().traceIdHigh(1L).traceId(2L).spanId(2L).build()); + + assertThat(metadata.keys()) + .containsExactly("x-b3-sampled", "x-b3-traceid", "x-b3-spanid"); + + assertThat(metadata.get(BravePropagationKeys.Sampled)) + .isEqualTo("1"); + assertThat(metadata.get(BravePropagationKeys.TraceId)) + .isEqualTo("00000000000000010000000000000002"); + assertThat(metadata.get(BravePropagationKeys.SpanId)) + .isEqualTo("2"); + } } diff --git a/brave-http/src/main/java/com/github/kristofa/brave/http/HttpClientRequestAdapter.java b/brave-http/src/main/java/com/github/kristofa/brave/http/HttpClientRequestAdapter.java index 0358b6a078..c283877920 100644 --- a/brave-http/src/main/java/com/github/kristofa/brave/http/HttpClientRequestAdapter.java +++ b/brave-http/src/main/java/com/github/kristofa/brave/http/HttpClientRequestAdapter.java @@ -32,7 +32,7 @@ public void addSpanIdToRequest(@Nullable SpanId spanId) { request.addHeader(BraveHttpHeaders.Sampled.getName(), "0"); } else { request.addHeader(BraveHttpHeaders.Sampled.getName(), "1"); - request.addHeader(BraveHttpHeaders.TraceId.getName(), IdConversion.convertToString(spanId.traceId)); + request.addHeader(BraveHttpHeaders.TraceId.getName(), spanId.traceIdString()); request.addHeader(BraveHttpHeaders.SpanId.getName(), IdConversion.convertToString(spanId.spanId)); if (spanId.nullableParentId() != null) { request.addHeader(BraveHttpHeaders.ParentSpanId.getName(), IdConversion.convertToString(spanId.parentId)); diff --git a/brave-http/src/main/java/com/github/kristofa/brave/http/HttpServerRequestAdapter.java b/brave-http/src/main/java/com/github/kristofa/brave/http/HttpServerRequestAdapter.java index 063e8a1e2f..6bd403f7f2 100644 --- a/brave-http/src/main/java/com/github/kristofa/brave/http/HttpServerRequestAdapter.java +++ b/brave-http/src/main/java/com/github/kristofa/brave/http/HttpServerRequestAdapter.java @@ -57,6 +57,7 @@ public Collection requestAnnotations() { private SpanId getSpanId(String traceId, String spanId, String parentSpanId) { return SpanId.builder() + .traceIdHigh(traceId.length() == 32 ? convertToLong(traceId, 0) : 0) .traceId(convertToLong(traceId)) .spanId(convertToLong(spanId)) .parentId(parentSpanId == null ? null : convertToLong(parentSpanId)).build(); diff --git a/brave-http/src/test/java/com/github/kristofa/brave/http/HttpClientRequestAdapterTest.java b/brave-http/src/test/java/com/github/kristofa/brave/http/HttpClientRequestAdapterTest.java index ee1bc04847..a48fe3d75b 100644 --- a/brave-http/src/test/java/com/github/kristofa/brave/http/HttpClientRequestAdapterTest.java +++ b/brave-http/src/test/java/com/github/kristofa/brave/http/HttpClientRequestAdapterTest.java @@ -54,7 +54,7 @@ public void addSpanIdToRequest_WithParentSpanId() { SpanId id = SpanId.builder().traceId(TRACE_ID).spanId(SPAN_ID).parentId(PARENT_SPAN_ID).build(); clientRequestAdapter.addSpanIdToRequest(id); verify(request).addHeader(BraveHttpHeaders.Sampled.getName(), "1"); - verify(request).addHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(TRACE_ID)); + verify(request).addHeader(BraveHttpHeaders.TraceId.getName(), "0000000000000001"); verify(request).addHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID)); verify(request).addHeader(BraveHttpHeaders.ParentSpanId.getName(), String.valueOf(PARENT_SPAN_ID)); verifyNoMoreInteractions(request, spanNameProvider); @@ -65,7 +65,7 @@ public void addSpanIdToRequest_WithoutParentSpanId() { SpanId id = SpanId.builder().traceId(TRACE_ID).spanId(SPAN_ID).parentId(null).build(); clientRequestAdapter.addSpanIdToRequest(id); verify(request).addHeader(BraveHttpHeaders.Sampled.getName(), "1"); - verify(request).addHeader(BraveHttpHeaders.TraceId.getName(), String.valueOf(TRACE_ID)); + verify(request).addHeader(BraveHttpHeaders.TraceId.getName(), "0000000000000001"); verify(request).addHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID)); verifyNoMoreInteractions(request, spanNameProvider); } @@ -82,4 +82,13 @@ public void requestAnnotations() { verifyNoMoreInteractions(request, spanNameProvider); } + @Test + public void traceId_when128bit() throws Exception { + SpanId id = SpanId.builder().traceIdHigh(TRACE_ID).traceId(TRACE_ID).spanId(SPAN_ID).parentId(null).build(); + clientRequestAdapter.addSpanIdToRequest(id); + verify(request).addHeader(BraveHttpHeaders.Sampled.getName(), "1"); + verify(request).addHeader(BraveHttpHeaders.TraceId.getName(), "00000000000000010000000000000001"); + verify(request).addHeader(BraveHttpHeaders.SpanId.getName(), String.valueOf(SPAN_ID)); + verifyNoMoreInteractions(request, spanNameProvider); + } } diff --git a/brave-http/src/test/java/com/github/kristofa/brave/http/HttpServerRequestAdapterTest.java b/brave-http/src/test/java/com/github/kristofa/brave/http/HttpServerRequestAdapterTest.java index 48552a1361..e56fc11947 100644 --- a/brave-http/src/test/java/com/github/kristofa/brave/http/HttpServerRequestAdapterTest.java +++ b/brave-http/src/test/java/com/github/kristofa/brave/http/HttpServerRequestAdapterTest.java @@ -118,9 +118,10 @@ public void getTraceDataSampledOneNoParentId() { } @Test - public void tolerates128BitTraceIdByDroppingHighBits() { - String hex128Bits = "463ac35c9f6413ad48485a3953bb6124"; + public void supports128BitTraceIdHeader() { + String upper64Bits = "48485a3953bb6124"; String lower64Bits = "48485a3953bb6124"; + String hex128Bits = upper64Bits + lower64Bits; when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.Sampled.getName())).thenReturn("1"); when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.TraceId.getName())).thenReturn(hex128Bits); when(serverRequest.getHttpHeaderValue(BraveHttpHeaders.SpanId.getName())).thenReturn(lower64Bits); @@ -129,6 +130,7 @@ public void tolerates128BitTraceIdByDroppingHighBits() { assertTrue(traceData.getSample()); SpanId spanId = traceData.getSpanId(); assertNotNull(spanId); + assertEquals(IdConversion.convertToLong(upper64Bits), spanId.traceIdHigh); assertEquals(IdConversion.convertToLong(lower64Bits), spanId.traceId); assertEquals(IdConversion.convertToLong(lower64Bits), spanId.spanId); assertNull(spanId.nullableParentId()); diff --git a/brave-okhttp/src/main/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptor.java b/brave-okhttp/src/main/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptor.java index e09abd887b..98f9252f26 100644 --- a/brave-okhttp/src/main/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptor.java +++ b/brave-okhttp/src/main/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptor.java @@ -145,7 +145,7 @@ public Response intercept(Chain chain) throws IOException { static Request.Builder addTraceHeaders(Request request, SpanId spanId) { Request.Builder tracedRequest = request.newBuilder(); - tracedRequest.header(BraveHttpHeaders.TraceId.getName(), convertToString(spanId.traceId)); + tracedRequest.header(BraveHttpHeaders.TraceId.getName(), spanId.traceIdString()); tracedRequest.header(BraveHttpHeaders.SpanId.getName(), convertToString(spanId.spanId)); if (spanId.nullableParentId() != null) { tracedRequest.header(BraveHttpHeaders.ParentSpanId.getName(), diff --git a/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveOkHttpRequestResponseInterceptorTest.java b/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveOkHttpRequestResponseInterceptorTest.java index 65478c8056..aa10a91198 100644 --- a/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveOkHttpRequestResponseInterceptorTest.java +++ b/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveOkHttpRequestResponseInterceptorTest.java @@ -31,6 +31,8 @@ public class BraveOkHttpRequestResponseInterceptorTest { private static final Long SPAN_ID = 151864L; private static final Long TRACE_ID = 8494864L; + private static final String TRACE_ID_STRING = + SpanId.builder().spanId(TRACE_ID).build().traceIdString(); private static final String HTTP_METHOD_GET = "GET"; @Rule @@ -81,7 +83,7 @@ public void testTracingTrue() throws IOException, InterruptedException { RecordedRequest serverRequest = server.takeRequest(); assertEquals(HTTP_METHOD_GET, serverRequest.getMethod()); assertEquals("1", serverRequest.getHeader(BraveHttpHeaders.Sampled.getName())); - assertEquals(Long.toString(TRACE_ID, 16), serverRequest.getHeader(BraveHttpHeaders.TraceId.getName())); + assertEquals(TRACE_ID_STRING, serverRequest.getHeader(BraveHttpHeaders.TraceId.getName())); assertEquals(Long.toString(SPAN_ID, 16), serverRequest.getHeader(BraveHttpHeaders.SpanId.getName())); } @@ -114,7 +116,7 @@ public void testTracingTrueHttpNoOk() throws IOException, InterruptedException { RecordedRequest serverRequest = server.takeRequest(); assertEquals(HTTP_METHOD_GET, serverRequest.getMethod()); assertEquals("1", serverRequest.getHeader(BraveHttpHeaders.Sampled.getName())); - assertEquals(Long.toString(TRACE_ID, 16), serverRequest.getHeader(BraveHttpHeaders.TraceId.getName())); + assertEquals(TRACE_ID_STRING, serverRequest.getHeader(BraveHttpHeaders.TraceId.getName())); assertEquals(Long.toString(SPAN_ID, 16), serverRequest.getHeader(BraveHttpHeaders.SpanId.getName())); } @@ -172,7 +174,7 @@ public void testQueryParams() throws IOException, InterruptedException { RecordedRequest serverRequest = server.takeRequest(); assertEquals(HTTP_METHOD_GET, serverRequest.getMethod()); assertEquals("1", serverRequest.getHeader(BraveHttpHeaders.Sampled.getName())); - assertEquals(Long.toString(TRACE_ID, 16), serverRequest.getHeader(BraveHttpHeaders.TraceId.getName())); + assertEquals(TRACE_ID_STRING, serverRequest.getHeader(BraveHttpHeaders.TraceId.getName())); assertEquals(Long.toString(SPAN_ID, 16), serverRequest.getHeader(BraveHttpHeaders.SpanId.getName())); } diff --git a/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptorTest.java b/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptorTest.java index 4c66c9eeb6..78f8761952 100644 --- a/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptorTest.java +++ b/brave-okhttp/src/test/java/com/github/kristofa/brave/okhttp/BraveTracingInterceptorTest.java @@ -39,6 +39,7 @@ import static com.github.kristofa.brave.http.BraveHttpHeaders.Sampled; import static com.github.kristofa.brave.http.BraveHttpHeaders.SpanId; import static com.github.kristofa.brave.http.BraveHttpHeaders.TraceId; +import static com.github.kristofa.brave.okhttp.BraveTracingInterceptor.addTraceHeaders; import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; @@ -233,6 +234,17 @@ public void reportsToZipkin_followupsAsNewSpans() throws Exception { ); } + @Test + public void addTraceHeaders_128() { + com.github.kristofa.brave.SpanId id = com.github.kristofa.brave.SpanId.builder() + .traceIdHigh(1).traceId(2).spanId(3).parentId(2L).build(); + + Request original = new Request.Builder().url("http://localhost").build(); + + assertThat(addTraceHeaders(original, id).build().header(TraceId.getName())) + .isEqualTo("00000000000000010000000000000002"); + } + BraveTracingInterceptor.Builder interceptorBuilder(Sampler sampler) { com.twitter.zipkin.gen.Endpoint localEndpoint = com.twitter.zipkin.gen.Endpoint.builder() .ipv4(local.ipv4) diff --git a/pom.xml b/pom.xml index 351e65e579..6a1b889edb 100755 --- a/pom.xml +++ b/pom.xml @@ -47,7 +47,7 @@ 4.3.2.RELEASE 8.1.20.v20160902 - 1.13.0 + 1.14.1 2.3 4.4.1 1.6.5