diff --git a/propagation/pom.xml b/propagation/pom.xml index 13cf3bb23a..e12bdd0bd6 100644 --- a/propagation/pom.xml +++ b/propagation/pom.xml @@ -20,7 +20,7 @@ io.zipkin.brave brave-parent - 5.10.3-SNAPSHOT + 5.11.3-SNAPSHOT brave-propagation-parent diff --git a/propagation/w3c/pom.xml b/propagation/w3c/pom.xml index 700d504c19..3f8b5947cb 100644 --- a/propagation/w3c/pom.xml +++ b/propagation/w3c/pom.xml @@ -18,7 +18,7 @@ io.zipkin.brave brave-propagation-parent - 5.10.3-SNAPSHOT + 5.11.3-SNAPSHOT 4.0.0 diff --git a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextExtractor.java b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextExtractor.java index ba914671dc..6b2910d4d2 100644 --- a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextExtractor.java +++ b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextExtractor.java @@ -14,38 +14,37 @@ package brave.propagation.w3c; import brave.propagation.Propagation.Getter; -import brave.propagation.SamplingFlags; import brave.propagation.TraceContext; import brave.propagation.TraceContext.Extractor; import brave.propagation.TraceContextOrSamplingFlags; -import brave.propagation.w3c.TraceContextPropagation.Extra; -import java.util.Collections; -import java.util.List; import static brave.propagation.B3SingleFormat.parseB3SingleFormat; // TODO: this class has no useful tests wrt traceparent yet -final class TraceContextExtractor implements Extractor { - final Getter getter; +final class TraceContextExtractor implements Extractor { + static final TraceContextOrSamplingFlags EXTRACTED_EMPTY = + TraceContextOrSamplingFlags.EMPTY.toBuilder().addExtra(Tracestate.EMPTY).build(); + + final Getter getter; final K traceparentKey, tracestateKey; final TracestateFormat tracestateFormat; final B3SingleFormatHandler handler = new B3SingleFormatHandler(); - TraceContextExtractor(TraceContextPropagation propagation, Getter getter) { + TraceContextExtractor(TraceContextPropagation propagation, Getter getter) { this.getter = getter; this.traceparentKey = propagation.traceparent; this.tracestateKey = propagation.tracestate; this.tracestateFormat = new TracestateFormat(propagation.tracestateKey); } - @Override public TraceContextOrSamplingFlags extract(C carrier) { - if (carrier == null) throw new NullPointerException("carrier == null"); - String traceparent = getter.get(carrier, traceparentKey); - if (traceparent == null) return EMPTY; + @Override public TraceContextOrSamplingFlags extract(R request) { + if (request == null) throw new NullPointerException("request == null"); + String traceparentString = getter.get(request, traceparentKey); + if (traceparentString == null) return EXTRACTED_EMPTY; // TODO: add link that says tracestate itself is optional - String tracestate = getter.get(carrier, tracestateKey); - if (tracestate == null) { + String tracestateString = getter.get(request, tracestateKey); + if (tracestateString == null) { // NOTE: we may not want to pay attention to the sampled flag. Since it conflates // not-yet-sampled with sampled=false, implementations that always set flags to -00 would // never be traced! @@ -54,33 +53,20 @@ final class TraceContextExtractor implements Extractor { // span ID. Ex we don't know if upstream are sending to the same system or not, when we can't // read the tracestate header. Trusting the span ID (traceparent calls the span ID parent-id) // could result in a headless trace. - TraceContext maybeUpstream = TraceparentFormat.parseTraceparentFormat(traceparent); - return TraceContextOrSamplingFlags.newBuilder() - .context(maybeUpstream) - .extra(DEFAULT_EXTRA) // marker for outbound propagation - .build(); + TraceContext maybeUpstream = TraceparentFormat.parseTraceparentFormat(traceparentString); + return TraceContextOrSamplingFlags.newBuilder(maybeUpstream) + .addExtra(Tracestate.EMPTY) // marker for outbound propagation + .build(); } - CharSequence otherEntries = tracestateFormat.parseAndReturnOtherEntries(tracestate, handler); - - List extra; - if (otherEntries == null) { - extra = DEFAULT_EXTRA; - } else { - Extra e = new Extra(); - e.otherEntries = otherEntries; - extra = Collections.singletonList(e); - } + Tracestate tracestate = tracestateFormat.parseAndReturnOtherEntries(tracestateString, handler); TraceContext context = handler.context; if (context == null) { - if (extra == DEFAULT_EXTRA) return EMPTY; - return TraceContextOrSamplingFlags.newBuilder() - .extra(extra) - .samplingFlags(SamplingFlags.EMPTY) - .build(); + if (tracestate == Tracestate.EMPTY) return EXTRACTED_EMPTY; + return EXTRACTED_EMPTY.toBuilder().addExtra(tracestate).build(); } - return TraceContextOrSamplingFlags.newBuilder().context(context).extra(extra).build(); + return TraceContextOrSamplingFlags.newBuilder(context).addExtra(tracestate).build(); } static final class B3SingleFormatHandler implements TracestateFormat.Handler { @@ -93,11 +79,4 @@ public boolean onThisEntry(CharSequence tracestate, int beginIndex, int endIndex return context != null; } } - - /** When present, this context was created with TracestatePropagation */ - static final Extra MARKER = new Extra(); - - static final List DEFAULT_EXTRA = Collections.singletonList(MARKER); - static final TraceContextOrSamplingFlags EMPTY = - TraceContextOrSamplingFlags.EMPTY.toBuilder().extra(DEFAULT_EXTRA).build(); } diff --git a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextInjector.java b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextInjector.java index 993e209f59..2d9d0fe21e 100644 --- a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextInjector.java +++ b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextInjector.java @@ -16,37 +16,36 @@ import brave.propagation.Propagation.Setter; import brave.propagation.TraceContext; import brave.propagation.TraceContext.Injector; -import brave.propagation.w3c.TraceContextPropagation.Extra; import static brave.propagation.B3SingleFormat.writeB3SingleFormat; import static brave.propagation.w3c.TraceparentFormat.writeTraceparentFormat; -final class TraceContextInjector implements Injector { +final class TraceContextInjector implements Injector { final TracestateFormat tracestateFormat; - final Setter setter; + final Setter setter; final K traceparentKey, tracestateKey; - TraceContextInjector(TraceContextPropagation propagation, Setter setter) { + TraceContextInjector(TraceContextPropagation propagation, Setter setter) { this.tracestateFormat = new TracestateFormat(propagation.tracestateKey); this.traceparentKey = propagation.traceparent; this.tracestateKey = propagation.tracestate; this.setter = setter; } - @Override public void inject(TraceContext traceContext, C carrier) { + @Override public void inject(TraceContext traceContext, R request) { - setter.put(carrier, traceparentKey, writeTraceparentFormat(traceContext)); + setter.put(request, traceparentKey, writeTraceparentFormat(traceContext)); CharSequence otherState = null; for (int i = 0, length = traceContext.extra().size(); i < length; i++) { Object next = traceContext.extra().get(i); - if (next instanceof Extra) { - otherState = ((Extra) next).otherEntries; + if (next instanceof Tracestate) { + otherState = ((Tracestate) next).otherEntries; break; } } String tracestate = tracestateFormat.write(writeB3SingleFormat(traceContext), otherState); - setter.put(carrier, tracestateKey, tracestate); + setter.put(request, tracestateKey, tracestate); } } diff --git a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextPropagation.java b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextPropagation.java index 472cfe76cf..84c4c32f0e 100644 --- a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextPropagation.java +++ b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextPropagation.java @@ -21,7 +21,6 @@ import java.util.List; public final class TraceContextPropagation implements Propagation { - public static Factory newFactory() { return new FactoryBuilder().build(); } @@ -87,27 +86,13 @@ static final class Factory extends Propagation.Factory { return keys; } - @Override public Injector injector(Setter setter) { + @Override public Injector injector(Setter setter) { if (setter == null) throw new NullPointerException("setter == null"); return new TraceContextInjector<>(this, setter); } - @Override public Extractor extractor(Getter getter) { + @Override public Extractor extractor(Getter getter) { if (getter == null) throw new NullPointerException("getter == null"); return new TraceContextExtractor<>(this, getter); } - - /** - * This only contains other entries. The entry for the current trace is only written during - * injection. - */ - static final class Extra { // hidden intentionally - CharSequence otherEntries; - - @Override public String toString() { - return "TracestatePropagation{" - + (otherEntries != null ? ("entries=" + otherEntries.toString()) : "") - + "}"; - } - } } diff --git a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java index 7060c88bea..2aabf11077 100644 --- a/propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java +++ b/propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java @@ -16,7 +16,6 @@ import brave.internal.Nullable; import brave.internal.Platform; import brave.propagation.TraceContext; -import java.lang.ref.WeakReference; import java.nio.ByteBuffer; import static brave.internal.HexCodec.writeHexLong; @@ -29,10 +28,10 @@ final class TraceparentFormat { static final int FORMAT_LENGTH = 3 + 32 + 1 + 16 + 3; // 00-traceid128-spanid-01 static final int // instead of enum for smaller bytecode - FIELD_VERSION = 1, - FIELD_TRACE_ID = 2, - FIELD_PARENT_ID = 3, - FIELD_TRACE_FLAGS = 4; + FIELD_VERSION = 1, + FIELD_TRACE_ID = 2, + FIELD_PARENT_ID = 3, + FIELD_TRACE_FLAGS = 4; /** Writes all "traceparent" defined fields in the trace context to a hyphen delimited string. */ public static String writeTraceparentFormat(TraceContext context) { @@ -42,7 +41,7 @@ public static String writeTraceparentFormat(TraceContext context) { } /** - * Like {@link #writeTraceparentFormat(TraceContext)}, but for carriers with byte array or byte + * Like {@link #writeTraceparentFormat(TraceContext)}, but for requests with byte array or byte * buffer values. For example, {@link ByteBuffer#wrap(byte[])} can wrap the result. */ public static byte[] writeTraceparentFormatAsBytes(TraceContext context) { @@ -89,7 +88,7 @@ public static TraceContext parseTraceparentFormat(CharSequence parent) { */ @Nullable public static TraceContext parseTraceparentFormat(CharSequence value, int beginIndex, - int endIndex) { + int endIndex) { int length = endIndex - beginIndex; if (length == 0) { @@ -214,9 +213,9 @@ public static TraceContext parseTraceparentFormat(CharSequence value, int beginI } static boolean validateFieldLength(int field, int length) { - int expectedLength = - (field == FIELD_TRACE_FLAGS || field == FIELD_VERSION) ? 2 - : field == FIELD_TRACE_ID ? 32 : 16; + int expectedLength = (field == FIELD_VERSION || field == FIELD_TRACE_FLAGS) + ? 2 // There are two fields that are 2 characters long: version and flags + : field == FIELD_TRACE_ID ? 32 : 16; // trace ID or span ID if (length == 0) { log(field, "Invalid input: empty {0}"); return false; diff --git a/propagation/w3c/src/main/java/brave/propagation/w3c/Tracestate.java b/propagation/w3c/src/main/java/brave/propagation/w3c/Tracestate.java new file mode 100644 index 0000000000..63125fe247 --- /dev/null +++ b/propagation/w3c/src/main/java/brave/propagation/w3c/Tracestate.java @@ -0,0 +1,50 @@ +/* + * Copyright 2013-2020 The OpenZipkin Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package brave.propagation.w3c; + +import brave.internal.Nullable; + +/** + * This only contains other entries. The entry for the current trace is only written during + * injection. + */ +final class Tracestate { // hidden intentionally + static final Tracestate EMPTY = new Tracestate(""); + + // TODO: this will change + final String otherEntries; + + static Tracestate create(@Nullable CharSequence otherEntries) { + if (otherEntries == null || otherEntries.length() == 0) return EMPTY; + return new Tracestate(otherEntries.toString()); + } + + private Tracestate(String otherEntries) { + this.otherEntries = otherEntries; + } + + @Override public String toString() { + return "tracestate: " + otherEntries; + } + + @Override public final boolean equals(Object o) { + if (o == this) return true; + if (!(o instanceof Tracestate)) return false; + return otherEntries.equals(((Tracestate) o).otherEntries); + } + + @Override public final int hashCode() { + return otherEntries.hashCode(); + } +} \ No newline at end of file diff --git a/propagation/w3c/src/main/java/brave/propagation/w3c/TracestateFormat.java b/propagation/w3c/src/main/java/brave/propagation/w3c/TracestateFormat.java index 87ce2cbedf..58d1a72ba4 100644 --- a/propagation/w3c/src/main/java/brave/propagation/w3c/TracestateFormat.java +++ b/propagation/w3c/src/main/java/brave/propagation/w3c/TracestateFormat.java @@ -13,7 +13,6 @@ */ package brave.propagation.w3c; -import brave.internal.Nullable; import brave.internal.Platform; import static brave.propagation.w3c.TraceparentFormat.FORMAT_LENGTH; @@ -79,7 +78,7 @@ String write(String thisValue, CharSequence otherEntries) { // TODO: characters were added to the valid list, so it is possible this impl no longer works // TODO: 32 max entries https://w3c.github.io/trace-context/#tracestate-header-field-values // TODO: empty and whitespace-only allowed Ex. 'foo=' or 'foo= ' - @Nullable CharSequence parseAndReturnOtherEntries(String tracestate, Handler handler) { + Tracestate parseAndReturnOtherEntries(String tracestate, Handler handler) { StringBuilder currentString = new StringBuilder(), otherEntries = null; Op op; OUTER: @@ -120,7 +119,10 @@ String write(String thisValue, CharSequence otherEntries) { break; } } - return otherEntries; + if (otherEntries != null && otherEntries.charAt(0) == ',') { + otherEntries.deleteCharAt(0); // TODO: fix the parser so this is eaten before now + } + return Tracestate.create(otherEntries); } // Simplify other rules by allowing value-based lookup on an ASCII value. @@ -142,7 +144,7 @@ String write(String thisValue, CharSequence otherEntries) { static boolean isValidTracestateKeyChar(char c) { return (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9') - || c == '@' || c == '_' || c == '-' || c == '*' || c == '/'; + || c == '@' || c == '_' || c == '-' || c == '*' || c == '/'; } /** diff --git a/propagation/w3c/src/test/java/brave/propagation/w3c/TraceContextPropagationTest.java b/propagation/w3c/src/test/java/brave/propagation/w3c/TraceContextPropagationTest.java index 8a99e7d902..47eff3eeb1 100644 --- a/propagation/w3c/src/test/java/brave/propagation/w3c/TraceContextPropagationTest.java +++ b/propagation/w3c/src/test/java/brave/propagation/w3c/TraceContextPropagationTest.java @@ -18,92 +18,106 @@ import brave.propagation.TraceContext.Extractor; import brave.propagation.TraceContext.Injector; import brave.propagation.TraceContextOrSamplingFlags; -import brave.propagation.w3c.TraceContextPropagation.Extra; import java.util.LinkedHashMap; import java.util.Map; import org.junit.Test; import static brave.internal.HexCodec.lowerHexToUnsignedLong; import static brave.propagation.Propagation.KeyFactory.STRING; -import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; public class TraceContextPropagationTest { - Map carrier = new LinkedHashMap<>(); + Map request = new LinkedHashMap<>(); Propagation propagation = TraceContextPropagation.newFactory().create(STRING); Injector> injector = propagation.injector(Map::put); Extractor> extractor = propagation.extractor(Map::get); TraceContext sampledContext = TraceContext.newBuilder() - .traceIdHigh(lowerHexToUnsignedLong("67891233abcdef01")) - .traceId(lowerHexToUnsignedLong("2345678912345678")) - .spanId(lowerHexToUnsignedLong("463ac35c9f6413ad")) - .sampled(true) - .build(); + .traceIdHigh(lowerHexToUnsignedLong("67891233abcdef01")) + .traceId(lowerHexToUnsignedLong("2345678912345678")) + .spanId(lowerHexToUnsignedLong("463ac35c9f6413ad")) + .sampled(true) + .build(); String validTraceparent = "00-67891233abcdef012345678912345678-463ac35c9f6413ad-01"; String validB3Single = "67891233abcdef012345678912345678-463ac35c9f6413ad-1"; String otherState = "congo=lZWRzIHRoNhcm5hbCBwbGVhc3VyZS4="; @Test public void injects_b3_when_no_other_tracestate() { - Extra extra = new Extra(); + sampledContext = sampledContext.toBuilder().addExtra(Tracestate.EMPTY).build(); - sampledContext = sampledContext.toBuilder().extra(asList(extra)).build(); + injector.inject(sampledContext, request); - injector.inject(sampledContext, carrier); - - assertThat(carrier).containsEntry("tracestate", "b3=" + validB3Single); + assertThat(request).containsEntry("tracestate", "b3=" + validB3Single); } @Test public void injects_b3_before_other_tracestate() { - Extra extra = new Extra(); - extra.otherEntries = otherState; + Tracestate tracestate = Tracestate.create(otherState); - sampledContext = sampledContext.toBuilder().extra(asList(extra)).build(); + sampledContext = sampledContext.toBuilder().addExtra(tracestate).build(); - injector.inject(sampledContext, carrier); + injector.inject(sampledContext, request); - assertThat(carrier).containsEntry("tracestate", "b3=" + validB3Single + "," + otherState); + assertThat(request).containsEntry("tracestate", "b3=" + validB3Single + "," + otherState); } @Test public void extracts_b3_when_no_other_tracestate() { - carrier.put("traceparent", validTraceparent); - carrier.put("tracestate", "b3=" + validB3Single); - - assertThat(extractor.extract(carrier)).isEqualTo( - TraceContextOrSamplingFlags.newBuilder() - .addExtra(new Extra()) - .context(sampledContext) - .build() - ); + request.put("traceparent", validTraceparent); + request.put("tracestate", "b3=" + validB3Single); + + assertThat(extractor.extract(request)).isEqualTo( + TraceContextOrSamplingFlags.newBuilder(sampledContext).addExtra(Tracestate.EMPTY).build()); } @Test public void extracts_b3_before_other_tracestate() { - carrier.put("traceparent", validTraceparent); - carrier.put("tracestate", "b3=" + validB3Single + "," + otherState); - - Extra extra = new Extra(); - extra.otherEntries = otherState; - - assertThat(extractor.extract(carrier)).isEqualTo( - TraceContextOrSamplingFlags.newBuilder() - .addExtra(extra) - .context(sampledContext) - .build() - ); + request.put("traceparent", validTraceparent); + request.put("tracestate", "b3=" + validB3Single + "," + otherState); + + Tracestate tracestate = Tracestate.create(otherState); + + assertThat(extractor.extract(request)).isEqualTo( + TraceContextOrSamplingFlags.newBuilder(sampledContext).addExtra(tracestate).build()); + } + + @Test public void extracted_toString() { + request.put("traceparent", validTraceparent); + request.put("tracestate", "b3=" + validB3Single + "," + otherState); + + assertThat(extractor.extract(request)).hasToString( + "Extracted{" + + "traceContext=" + sampledContext + ", " + + "samplingFlags=SAMPLED_REMOTE, " + + "extra=[tracestate: " + otherState + "]" + + "}"); } @Test public void extracts_b3_after_other_tracestate() { - carrier.put("traceparent", validTraceparent); - carrier.put("tracestate", otherState + ",b3=" + validB3Single); - - Extra extra = new Extra(); - extra.otherEntries = otherState; - - assertThat(extractor.extract(carrier)).isEqualTo( - TraceContextOrSamplingFlags.newBuilder() - .addExtra(extra) - .context(sampledContext) - .build() - ); + request.put("traceparent", validTraceparent); + request.put("tracestate", otherState + ",b3=" + validB3Single); + + Tracestate tracestate = Tracestate.create(otherState); + + assertThat(extractor.extract(request)).isEqualTo( + TraceContextOrSamplingFlags.newBuilder(sampledContext).addExtra(tracestate).build()); + } + + @Test public void tracestate() { + Tracestate b3_withContext = Tracestate.create("b3=" + validB3Single); + Tracestate sameState = Tracestate.create("b3=" + validB3Single); + assertThat(b3_withContext).isEqualTo(sameState); + assertThat(sameState).isEqualTo(b3_withContext); + assertThat(b3_withContext).hasSameHashCodeAs(sameState); + assertThat(b3_withContext) + .hasToString("tracestate: b3=67891233abcdef012345678912345678-463ac35c9f6413ad-1"); + + assertThat(Tracestate.create("")) + .isNotEqualTo(b3_withContext) + .isSameAs(Tracestate.create(null)) + .isSameAs(Tracestate.EMPTY) + .hasToString("tracestate: "); + + Tracestate b3_debugOnly = Tracestate.create("b3=d"); + assertThat(b3_withContext).isNotEqualTo(b3_debugOnly); + assertThat(b3_debugOnly).isNotEqualTo(b3_withContext); + assertThat(b3_withContext.hashCode()).isNotEqualTo(b3_debugOnly.hashCode()); } } diff --git a/propagation/w3c/src/test/java/brave/propagation/w3c/TraceparentFormatTest.java b/propagation/w3c/src/test/java/brave/propagation/w3c/TraceparentFormatTest.java index 8870911c77..5d73a8e29f 100644 --- a/propagation/w3c/src/test/java/brave/propagation/w3c/TraceparentFormatTest.java +++ b/propagation/w3c/src/test/java/brave/propagation/w3c/TraceparentFormatTest.java @@ -59,47 +59,47 @@ public class TraceparentFormatTest { /** unsampled isn't the same as not-yet-sampled, but we have no better choice */ @Test public void writeTraceparentFormat_notYetSampled_128() { TraceContext context = TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)).build(); + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)).build(); assertThat(writeTraceparentFormat(context)) - .isEqualTo("00-" + traceIdHigh + traceId + "-" + spanId + "-00") - .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); + .isEqualTo("00-" + traceIdHigh + traceId + "-" + spanId + "-00") + .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); } @Test public void writeTraceparentFormat_unsampled() { TraceContext context = TraceContext.newBuilder() - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(false).build(); + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(false).build(); assertThat(writeTraceparentFormat(context)) - .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-00") - .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); + .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-00") + .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); } @Test public void writeTraceparentFormat_sampled() { TraceContext context = TraceContext.newBuilder() - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build(); + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build(); assertThat(writeTraceparentFormat(context)) - .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-01") - .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); + .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-01") + .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); } /** debug isn't the same as sampled, but we have no better choice */ @Test public void writeTraceparentFormat_debug() { TraceContext context = TraceContext.newBuilder() - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .debug(true).build(); + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .debug(true).build(); assertThat(writeTraceparentFormat(context)) - .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-01") - .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); + .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-01") + .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); } /** @@ -108,116 +108,116 @@ public class TraceparentFormatTest { */ @Test public void writeTraceparentFormat_parent() { TraceContext context = TraceContext.newBuilder() - .traceId(Long.parseUnsignedLong(traceId, 16)) - .parentId(Long.parseUnsignedLong(parentId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build(); + .traceId(Long.parseUnsignedLong(traceId, 16)) + .parentId(Long.parseUnsignedLong(parentId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build(); assertThat(writeTraceparentFormat(context)) - .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-01") - .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); + .isEqualTo("00-0000000000000000" + traceId + "-" + spanId + "-01") + .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); } @Test public void writeTraceparentFormat_largest() { TraceContext context = TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .parentId(Long.parseUnsignedLong(parentId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .debug(true).build(); + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .parentId(Long.parseUnsignedLong(parentId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .debug(true).build(); assertThat(writeTraceparentFormat(context)) - .isEqualTo("00-" + traceIdHigh + traceId + "-" + spanId + "-01") - .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); + .isEqualTo("00-" + traceIdHigh + traceId + "-" + spanId + "-01") + .isEqualTo(new String(writeTraceparentFormatAsBytes(context), UTF_8)); } @Test public void parseTraceparentFormat_sampled() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-01")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build() + ); } @Test public void parseTraceparentFormat_unsampled() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-00")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(false).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(false).build() + ); } @Test public void parseTraceparentFormat_padded() { assertThat(parseTraceparentFormat("00-0000000000000000" + traceId + "-" + spanId + "-01")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build() + ); } @Test public void parseTraceparentFormat_padded_right() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + "0000000000000000-" + spanId + "-01")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build() + ); } @Test public void parseTraceparentFormat_newer_version() { assertThat(parseTraceparentFormat("10-" + traceIdHigh + traceId + "-" + spanId + "-00")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(false).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(false).build() + ); } @Test public void parseTraceparentFormat_newer_version_ignores_extra_fields() { assertThat(parseTraceparentFormat("10-" + traceIdHigh + traceId + "-" + spanId + "-00-fobaly")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(false).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(false).build() + ); } @Test public void parseTraceparentFormat_newer_version_ignores_extra_flags() { assertThat(parseTraceparentFormat("10-" + traceIdHigh + traceId + "-" + spanId + "-ff")) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build() + ); } /** for example, parsing inside tracestate */ @Test public void parseTraceparentFormat_middleOfString() { String input = "tc=00-" + traceIdHigh + traceId + "-" + spanId + "-01,"; assertThat(parseTraceparentFormat(input, 3, input.length() - 1)) - .isEqualToComparingFieldByField(TraceContext.newBuilder() - .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) - .traceId(Long.parseUnsignedLong(traceId, 16)) - .spanId(Long.parseUnsignedLong(spanId, 16)) - .sampled(true).build() - ); + .isEqualToComparingFieldByField(TraceContext.newBuilder() + .traceIdHigh(Long.parseUnsignedLong(traceIdHigh, 16)) + .traceId(Long.parseUnsignedLong(traceId, 16)) + .spanId(Long.parseUnsignedLong(spanId, 16)) + .sampled(true).build() + ); } @Test public void parseTraceparentFormat_middleOfString_incorrectIndex() { String input = "tc=00-" + traceIdHigh + traceId + "-" + spanId + "-00,"; assertThat(parseTraceparentFormat(input, 0, 12)) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform) - .log("Invalid input: only valid characters are lower-hex for {0}", "version", null); + .log("Invalid input: only valid characters are lower-hex for {0}", "version", null); } /** This tests that the being index is inclusive and the end index is exclusive */ @@ -225,74 +225,74 @@ public class TraceparentFormatTest { String encoded = "00-" + traceIdHigh + traceId + "-" + spanId + "-01"; String sequence = "??" + encoded + "??"; assertThat(parseTraceparentFormat(sequence, 2, 2 + encoded.length())) - .isEqualToComparingFieldByField(parseTraceparentFormat(encoded)); + .isEqualToComparingFieldByField(parseTraceparentFormat(encoded)); } @Test public void parseTraceparentFormat_malformed() { assertThat(parseTraceparentFormat("not-a-tumor")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform) - .log("Invalid input: only valid characters are lower-hex for {0}", "version", null); + .log("Invalid input: only valid characters are lower-hex for {0}", "version", null); } @Test public void parseTraceparentFormat_malformed_notAscii() { assertThat(parseTraceparentFormat( - "00-" + traceIdHigh + traceId + "-" + spanId.substring(0, 15) + "💩-1")) - .isNull(); // instead of crashing + "00-" + traceIdHigh + traceId + "-" + spanId.substring(0, 15) + "💩-1")) + .isNull(); // instead of crashing verify(platform) - .log("Invalid input: only valid characters are lower-hex for {0}", "parent ID", null); + .log("Invalid input: only valid characters are lower-hex for {0}", "parent ID", null); } @Test public void parseTraceparentFormat_malformed_uuid() { assertThat(parseTraceparentFormat("b970dafd-0d95-40aa-95d8-1d8725aebe40")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too long", "version", null); } @Test public void parseTraceparentFormat_short_traceId() { assertThat( - parseTraceparentFormat("00-" + traceId + "-" + spanId + "-01")) - .isNull(); // instead of raising exception + parseTraceparentFormat("00-" + traceId + "-" + spanId + "-01")) + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too short", "trace ID", null); } @Test public void parseTraceparentFormat_zero_traceId() { assertThat( - parseTraceparentFormat("00-00000000000000000000000000000000-" + spanId + "-01")) - .isNull(); // instead of raising exception + parseTraceparentFormat("00-00000000000000000000000000000000-" + spanId + "-01")) + .isNull(); // instead of raising exception verify(platform).log("Invalid input: read all zeros {0}", "trace ID", null); } @Test public void parseTraceparentFormat_fails_on_extra_flags() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-ff")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: only choices are 00 or 01 {0}", "trace flags", null); } @Test public void parseTraceparentFormat_fails_on_extra_fields() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-0-")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too short", "trace flags", null); } @Test public void parseTraceparentFormat_fails_on_version_ff() { assertThat(parseTraceparentFormat("ff-" + traceIdHigh + traceId + "-" + spanId + "-01")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: ff {0}", "version", null); } @Test public void parseTraceparentFormat_zero_spanId() { assertThat( - parseTraceparentFormat("00-" + traceIdHigh + traceId + "-0000000000000000-01")) - .isNull(); // instead of raising exception + parseTraceparentFormat("00-" + traceIdHigh + traceId + "-0000000000000000-01")) + .isNull(); // instead of raising exception verify(platform).log("Invalid input: read all zeros {0}", "parent ID", null); } @@ -305,78 +305,79 @@ public class TraceparentFormatTest { @Test public void parseTraceparentFormat_empty_version() { assertThat(parseTraceparentFormat("-" + traceIdHigh + traceId + "-" + spanId + "-00")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: empty {0}", "version", null); } @Test public void parseTraceparentFormat_empty_traceId() { assertThat(parseTraceparentFormat("00--" + spanId + "-00")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: empty {0}", "trace ID", null); } @Test public void parseTraceparentFormat_empty_spanId() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "--01")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: empty {0}", "parent ID", null); } @Test public void parseTraceparentFormat_empty_flags() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: empty {0}", "trace flags", null); } @Test public void parseTraceparentFormat_truncated_traceId() { assertThat(parseTraceparentFormat("00-1-" + spanId + "-01")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too short", "trace ID", null); } @Test public void parseTraceparentFormat_truncated_traceId128() { assertThat(parseTraceparentFormat("00-1" + traceId + "-" + spanId + "-01")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too short", "trace ID", null); } @Test public void parseTraceparentFormat_truncated_spanId() { assertThat( - parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId.substring(0, 15) + "-00")) - .isNull(); // instead of raising exception + parseTraceparentFormat( + "00-" + traceIdHigh + traceId + "-" + spanId.substring(0, 15) + "-00")) + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too short", "parent ID", null); } @Test public void parseTraceparentFormat_truncated_flags() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-0")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too short", "trace flags", null); } @Test public void parseTraceparentFormat_traceIdTooLong() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "a" + "-" + spanId + "-0")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too long", "trace ID", null); } @Test public void parseTraceparentFormat_spanIdTooLong() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "a-0")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: {0} is too long", "parent ID", null); } @Test public void parseTraceparentFormat_flagsTooLong() { assertThat(parseTraceparentFormat("00-" + traceIdHigh + traceId + "-" + spanId + "-001")) - .isNull(); // instead of raising exception + .isNull(); // instead of raising exception verify(platform).log("Invalid input: too long", null); } diff --git a/propagation/w3c/src/test/java/brave/propagation/w3c/TracestateFormatTest.java b/propagation/w3c/src/test/java/brave/propagation/w3c/TracestateFormatTest.java index 90f3a88339..b31cccc599 100644 --- a/propagation/w3c/src/test/java/brave/propagation/w3c/TracestateFormatTest.java +++ b/propagation/w3c/src/test/java/brave/propagation/w3c/TracestateFormatTest.java @@ -25,26 +25,26 @@ public class TracestateFormatTest { static final String FORTY_KEY_CHARS = "abcdefghijklmnopqrstuvwxyz0123456789_-*/"; static final String TWO_HUNDRED_FORTY_KEY_CHARS = - FORTY_KEY_CHARS + FORTY_KEY_CHARS + FORTY_KEY_CHARS - + FORTY_KEY_CHARS + FORTY_KEY_CHARS + FORTY_KEY_CHARS; + FORTY_KEY_CHARS + FORTY_KEY_CHARS + FORTY_KEY_CHARS + + FORTY_KEY_CHARS + FORTY_KEY_CHARS + FORTY_KEY_CHARS; static final String LONGEST_BASIC_KEY = - TWO_HUNDRED_FORTY_KEY_CHARS + FORTY_KEY_CHARS.substring(0, 16); + TWO_HUNDRED_FORTY_KEY_CHARS + FORTY_KEY_CHARS.substring(0, 16); static final String LONGEST_TENANT_KEY = - "1" + TWO_HUNDRED_FORTY_KEY_CHARS + "@" + FORTY_KEY_CHARS.substring(0, 13); + "1" + TWO_HUNDRED_FORTY_KEY_CHARS + "@" + FORTY_KEY_CHARS.substring(0, 13); // all these need log assertions @Test public void validateKey_empty() { assertThatThrownByValidateKey("") - .hasMessage("Invalid input: empty"); + .hasMessage("Invalid input: empty"); } @Test public void validateKey_tooLong() { char[] tooMany = new char[257]; Arrays.fill(tooMany, 'a'); assertThatThrownByValidateKey(new String(tooMany)) - .hasMessage("Invalid input: too large"); + .hasMessage("Invalid input: too large"); } @Test public void validateKey_shortest_basic() { @@ -67,34 +67,34 @@ public class TracestateFormatTest { @Test public void validateKey_invalid_basic() { // zero is allowed only as when there is an '@' assertThatThrownByValidateKey("0") - .hasMessage("Invalid input: vendor must start with a-z"); + .hasMessage("Invalid input: vendor must start with a-z"); } @Test public void validateKey_invalid_basic_unicode() { Stream.of("a💩", "💩a").forEach(key -> assertThatThrownByValidateKey(key) - .hasMessage("Invalid input: valid characters are: a-z 0-9 _ - * / @")); + .hasMessage("Invalid input: valid characters are: a-z 0-9 _ - * / @")); } @Test public void validateKey_invalid_tenant() { assertThatThrownByValidateKey("_@z") - .hasMessage("Invalid input: tenant ID must start with a-z"); + .hasMessage("Invalid input: tenant ID must start with a-z"); } @Test public void validateKey_invalid_tenant_unicode() { Stream.of( - "a@a💩", - "a@💩a", - "a💩@a", - "💩a@a" + "a@a💩", + "a@💩a", + "a💩@a", + "💩a@a" ).forEach(key -> assertThatThrownByValidateKey(key) - .hasMessage("Invalid input: valid characters are: a-z 0-9 _ - * / @")); + .hasMessage("Invalid input: valid characters are: a-z 0-9 _ - * / @")); } @Test public void validateKey_invalid_tenant_empty() { assertThatThrownByValidateKey("@a") - .hasMessage("Invalid input: empty tenant ID"); + .hasMessage("Invalid input: empty tenant ID"); assertThatThrownByValidateKey("a@") - .hasMessage("Invalid input: empty vendor"); + .hasMessage("Invalid input: empty vendor"); } @Test public void validateKey_invalid_tenant_vendor_longest() { @@ -103,7 +103,7 @@ public class TracestateFormatTest { @Test public void validateKey_invalid_tenant_vendor_tooLong() { assertThatThrownByValidateKey("a@abcdef1234567890") - .hasMessage("Invalid input: vendor too long"); + .hasMessage("Invalid input: vendor too long"); } static AbstractBooleanAssert assertThatValidateKey(String key) { @@ -112,6 +112,6 @@ static AbstractBooleanAssert assertThatValidateKey(String key) { static AbstractThrowableAssert assertThatThrownByValidateKey(String key) { return assertThatThrownBy(() -> TracestateFormat.validateKey(key, true)) - .isInstanceOf(IllegalArgumentException.class); + .isInstanceOf(IllegalArgumentException.class); } }