backfills tests for traceparent

propagation/pom.xml

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!--
 
-    Copyright 2013-2020 The OpenZipkin Authors
+    Copyright 2013-2018 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

propagation/w3c/pom.xml

@@ -31,6 +31,22 @@
     <main.signature.artifact>java16</main.signature.artifact>
   </properties>
 
+  <dependencies>
+    <!-- to mock Platform calls -->
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-module-junit4</artifactId>
+      <version>${powermock.version}</version>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.powermock</groupId>
+      <artifactId>powermock-api-mockito2</artifactId>
+      <version>${powermock.version}</version>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
+
   <build>
     <plugins>
       <plugin>

propagation/w3c/src/main/java/brave/propagation/w3c/TraceContextExtractor.java

@@ -22,9 +22,7 @@
 import java.util.Collections;
 import java.util.List;
 
-import static brave.propagation.w3c.TraceparentFormat.FORMAT_LENGTH;
-import static brave.propagation.w3c.TraceparentFormat.maybeExtractParent;
-import static brave.propagation.w3c.TraceparentFormat.validateFormat;
+import static brave.propagation.w3c.TraceparentFormat.parseTraceparentFormat;
 
 final class TraceContextExtractor<C, K> implements Extractor<C> {
   final Getter<C, K> getter;
@@ -57,22 +55,20 @@ final class TraceContextExtractor<C, K> implements Extractor<C> {
     if (handler.context == null) {
       if (extra == DEFAULT_EXTRA) return EMPTY;
       return TraceContextOrSamplingFlags.newBuilder()
-          .extra(extra)
-          .samplingFlags(SamplingFlags.EMPTY)
-          .build();
+        .extra(extra)
+        .samplingFlags(SamplingFlags.EMPTY)
+        .build();
     }
     return TraceContextOrSamplingFlags.newBuilder().context(handler.context).extra(extra).build();
   }
 
   static final class TraceparentFormatHandler implements TracestateFormat.Handler {
     TraceContext context;
 
-    @Override public boolean onThisState(CharSequence tracestateString, int pos) {
-      if (validateFormat(tracestateString, pos) < FORMAT_LENGTH) {
-        return false;
-      }
-      context = maybeExtractParent(tracestateString, pos);
-      return true;
+    @Override
+    public boolean onThisState(CharSequence tracestateString, int beginIndex, int endIndex) {
+      context = parseTraceparentFormat(tracestateString, beginIndex, endIndex);
+      return context != null;
     }
   }
 
@@ -81,5 +77,5 @@ static final class TraceparentFormatHandler implements TracestateFormat.Handler
 
   static final List<Object> DEFAULT_EXTRA = Collections.singletonList(MARKER);
   static final TraceContextOrSamplingFlags EMPTY =
-      TraceContextOrSamplingFlags.EMPTY.toBuilder().extra(DEFAULT_EXTRA).build();
+    TraceContextOrSamplingFlags.EMPTY.toBuilder().extra(DEFAULT_EXTRA).build();
 }

propagation/w3c/src/main/java/brave/propagation/w3c/TraceparentFormat.java

@@ -13,100 +13,201 @@
  */
 package brave.propagation.w3c;
 
+import brave.internal.HexCodec;
 import brave.internal.Nullable;
+import brave.internal.Platform;
 import brave.propagation.TraceContext;
-import java.util.logging.Logger;
+import java.nio.ByteBuffer;
 
-import static brave.internal.HexCodec.lenientLowerHexToUnsignedLong;
 import static brave.internal.HexCodec.writeHexLong;
 
-// TODO: this format may have changed since writing. Check for drift then not the spec version
+/** Implements https://w3c.github.io/trace-context/#traceparent-header */
 final class TraceparentFormat {
-  static final Logger logger = Logger.getLogger(TraceparentFormat.class.getName());
-  static final int FORMAT_LENGTH = 55;
-
-  static String writeTraceparentFormat(TraceContext context) {
-    // 00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01
-    char[] result = new char[FORMAT_LENGTH];
-    result[0] = '0'; // version
-    result[1] = '0'; // version
-    result[2] = '-'; // delimiter
-    writeHexLong(result, 3, context.traceIdHigh());
-    writeHexLong(result, 19, context.traceId());
-    result[35] = '-'; // delimiter
-    writeHexLong(result, 36, context.spanId());
-    result[52] = '-'; // delimiter
-    result[53] = '0'; // options
-    result[54] = context.sampled() != null && context.sampled() ? '1' : '0'; // options
-    return new String(result);
+  static final int FORMAT_LENGTH = 3 + 32 + 1 + 16 + 3; // 00-traceid128-spanid-01
+
+  /**
+   * Writes all B3 defined fields in the trace context, except {@link TraceContext#parentIdAsLong()
+   * parent ID}, to a hyphen delimited string.
+   *
+   * <p>This is appropriate for receivers who understand "b3" single header format, and always do
+   * work in a child span. For example, message consumers always do work in child spans, so message
+   * producers can use this format to save bytes on the wire. On the other hand, RPC clients should
+   * use {@link #writeTraceparentFormat(TraceContext)} instead, as RPC servers often share a span ID
+   * with the client.
+   */
+  public static String writeTraceparentFormat(TraceContext context) {
+    char[] buffer = getCharBuffer();
+    int length = writeTraceparentFormat(context, buffer);
+    return new String(buffer, 0, length);
+  }
+
+  /**
+   * Like {@link #writeTraceparentFormat(TraceContext)}, but for carriers with byte array or byte
+   * buffer values. For example, {@link ByteBuffer#wrap(byte[])} can wrap the result.
+   */
+  public static byte[] writeTraceparentFormatAsBytes(TraceContext context) {
+    char[] buffer = getCharBuffer();
+    int length = writeTraceparentFormat(context, buffer);
+    return asciiToNewByteArray(buffer, length);
   }
 
-  /** returns the count of valid characters read from the input position */
-  static int validateFormat(CharSequence parent, int pos) {
-    int length = Math.max(parent.length() - pos, FORMAT_LENGTH);
+  static int writeTraceparentFormat(TraceContext context, char[] result) {
+    int pos = 0;
+    result[pos++] = '0';
+    result[pos++] = '0';
+    result[pos++] = '-';
+    long traceIdHigh = context.traceIdHigh();
+    writeHexLong(result, pos, traceIdHigh);
+    pos += 16;
+    writeHexLong(result, pos, context.traceId());
+    pos += 16;
+    result[pos++] = '-';
+    writeHexLong(result, pos, context.spanId());
+    pos += 16;
+
+    Boolean sampled = context.sampled();
+    if (sampled != null) {
+      result[pos++] = '-';
+      result[pos++] = '0';
+      result[pos++] = sampled ? '1' : '0';
+    }
+
+    return pos;
+  }
+
+  @Nullable
+  public static TraceContext parseTraceparentFormat(CharSequence parent) {
+    return parseTraceparentFormat(parent, 0, parent.length());
+  }
+
+  /**
+   * @param beginIndex the start index, inclusive
+   * @param endIndex the end index, exclusive
+   */
+  @Nullable
+  public static TraceContext parseTraceparentFormat(CharSequence parent, int beginIndex,
+    int endIndex) {
+    int length = endIndex - beginIndex;
+
+    if (length == 0) {
+      Platform.get().log("Invalid input: empty", null);
+      return null;
+    }
+
     if (length < FORMAT_LENGTH) {
-      logger.fine("Bad length.");
-      return 0;
+      Platform.get().log("Invalid input: truncated", null);
+      return null;
+    } else if (length > FORMAT_LENGTH) {
+      Platform.get().log("Invalid input: too long", null);
+      return null;
     }
 
-    for (int i = 0; i < length; i++) {
-      char c = parent.charAt(i + pos);
-      if (c == '-') {
-        // There are delimiters separating the version, trace ID, span ID and options fields.
-        if (i != 2 && i != 35 && i != 52) {
-          logger.fine("Expected hyphen at " + (i + pos));
-          return i;
-        }
-        // Everything else is hex
-      } else if ((c < '0' || c > '9') && (c < 'a' || c > 'f')) {
-        logger.fine("Expected lower hex at " + (i + pos));
-        return i;
+    // Cheaply check for only ASCII characters. This allows for more precise messages later, but
+    // kicks out early on data such as unicode.
+    for (int i = beginIndex; i < endIndex; i++) {
+      char c = parent.charAt(i);
+      if (c != '-' && ((c < '0' || c > '9') && (c < 'a' || c > 'f'))) {
+        Platform.get().log("Invalid input: neither hyphen, nor lower-hex at offset {0}", i, null);
+        return null;
       }
     }
-    return length;
-  }
 
-  static @Nullable TraceContext maybeExtractParent(CharSequence parent, int pos) {
+    int pos = beginIndex;
     int version = parseUnsigned16BitLowerHex(parent, pos);
     if (version == -1) {
-      logger.fine("Malformed version.");
+      Platform.get().log("Invalid input: expected version at offset {0}", beginIndex, null);
       return null;
     }
+
     if (version != 0) {
-      logger.fine("Unsupported version.");
+      // Parsing higher versions is a SHOULD not MUST
+      // https://w3c.github.io/trace-context/#versioning-of-traceparent
+      Platform.get().log("Invalid input: expected version 00 at offset {0}", beginIndex, null);
       return null;
     }
+    pos += 3;
+
+    long traceIdHigh = tryParse16HexCharacters(parent, pos, endIndex);
+    pos += 16;
+    long traceId = tryParse16HexCharacters(parent, pos, endIndex);
+    pos += 16;
 
-    long traceIdHigh = lenientLowerHexToUnsignedLong(parent, pos + 3, pos + 19);
-    long traceId = lenientLowerHexToUnsignedLong(parent, pos + 19, pos + 35);
     if (traceIdHigh == 0L && traceId == 0L) {
-      logger.fine("Invalid input: expected non-zero trace ID");
+      Platform.get()
+        .log("Invalid input: expected a 32 lower hex trace ID at offset {0}", pos - 32, null);
       return null;
     }
 
-    long spanId = lenientLowerHexToUnsignedLong(parent, pos + 36, pos + 52);
-    if (spanId == 0L) {
-      logger.fine("Invalid input: expected non-zero span ID");
+    if (isLowerHex(parent.charAt(pos))) {
+      Platform.get().log("Invalid input: trace ID is too long", null);
       return null;
     }
 
-    int traceOptions = parseUnsigned16BitLowerHex(parent, pos + 53);
-    if (traceOptions == -1) {
-      logger.fine("Malformed trace options.");
+    if (!checkHyphen(parent, pos++)) return null;
+
+    // Confusingly, the spec calls the span ID field parentId
+    // https://w3c.github.io/trace-context/#parent-id
+    long parentId = tryParse16HexCharacters(parent, pos, endIndex);
+    if (parentId == 0L) {
+      Platform.get()
+        .log("Invalid input: expected a 16 lower hex parent ID at offset {0}", pos, null);
       return null;
     }
+    pos += 16; // parentId
 
-    // TODO: treat it as a bitset?
-    // https://github.com/w3c/distributed-tracing/issues/8#issuecomment-382958021
-    // TODO handle deferred decision https://github.com/w3c/distributed-tracing/issues/8
-    boolean sampled = (traceOptions & 1) == 1;
+    if (isLowerHex(parent.charAt(pos))) {
+      Platform.get().log("Invalid input: parent ID is too long", null);
+      return null;
+    }
+
+    // If the sampling field is present, we'll have a delimiter 2 characters from now. Ex "-1"
+    if (endIndex == pos + 1) {
+      Platform.get().log("Invalid input: truncated", null);
+      return null;
+    }
+    if (!checkHyphen(parent, pos++)) return null;
+
+    boolean sampled;
+    // Only one flag is defined: sampled
+    // https://w3c.github.io/trace-context/#sampled-flag
+    Boolean maybeSampled = parseSampledFromFlags(parent, pos);
+    if (maybeSampled == null) return null;
+    sampled = maybeSampled;
 
     return TraceContext.newBuilder()
-        .traceIdHigh(traceIdHigh)
-        .traceId(traceId)
-        .spanId(spanId)
-        .sampled(sampled)
-        .build();
+      .traceIdHigh(traceIdHigh)
+      .traceId(traceId)
+      .spanId(parentId)
+      .sampled(sampled)
+      .build();
+  }
+
+  static boolean checkHyphen(CharSequence b3, int pos) {
+    if (b3.charAt(pos) == '-') return true;
+    Platform.get().log("Invalid input: expected a hyphen(-) delimiter at offset {0}", pos, null);
+    return false;
+  }
+
+  static long tryParse16HexCharacters(CharSequence lowerHex, int index, int end) {
+    int endIndex = index + 16;
+    if (endIndex > end) return 0L;
+    return HexCodec.lenientLowerHexToUnsignedLong(lowerHex, index, endIndex);
+  }
+
+  @Nullable static Boolean parseSampledFromFlags(CharSequence parent, int pos) {
+    char sampledChar = parent.charAt(pos) == '0' ? parent.charAt(pos + 1) : '?';
+    if (sampledChar == '1') {
+      return true;
+    } else if (sampledChar == '0') {
+      return false;
+    } else {
+      logInvalidFlags(pos);
+    }
+    return null;
+  }
+
+  static void logInvalidFlags(int pos) {
+    Platform.get().log("Invalid input: expected 00 or 01 for flags at offset {0}", pos, null);
   }
 
   /** Returns -1 if it wasn't hex */
@@ -125,4 +226,30 @@ static int parseUnsigned16BitLowerHex(CharSequence lowerHex, int pos) {
     }
     return result;
   }
+
+  static byte[] asciiToNewByteArray(char[] buffer, int length) {
+    byte[] result = new byte[length];
+    for (int i = 0; i < length; i++) {
+      result[i] = (byte) buffer[i];
+    }
+    return result;
+  }
+
+  static boolean isLowerHex(char c) {
+    return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f');
+  }
+
+  static final ThreadLocal<char[]> CHAR_BUFFER = new ThreadLocal<>();
+
+  static char[] getCharBuffer() {
+    char[] charBuffer = CHAR_BUFFER.get();
+    if (charBuffer == null) {
+      charBuffer = new char[FORMAT_LENGTH];
+      CHAR_BUFFER.set(charBuffer);
+    }
+    return charBuffer;
+  }
+
+  TraceparentFormat() {
+  }
 }