Don't break on 128bit X-B3-TraceId by tossing high bits

[codefromthecrypt]

We want to phase in 128bit trace ids per See openzipkin/b3-propagation#6. The first step is not breaking if a longer trace id is found in an http header.

Expected behavior

The first step of transitioning to 128bit X-B3-TraceId is tolerantly reading 32 character long ids by throwing away the high bits (any characters left of 16 characters). This allows the tracing system to more flexibly introduce 128bit trace id support in the future.

Ex. when X-B3-TraceId: 463ac35c9f6413ad48485a3953bb6124 is received, parse the lower 64 bits (right most 16 characters ex48485a3953bb6124) as the trace id.

Actual behavior

Right now, the http codec parses the X-B3-TraceId header via SpanId.fromString.

  def fromString(spanId: String): Option[SpanId] =
    try {
      Some(SpanId(new RichU64String(spanId).toU64Long))
    } catch {
      case NonFatal(_) => None
    }

We could either change this to toss left-most characters over 16, or define a new function for parsing a trace id. For example, like this:

  /**
   * 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 lowerHexToUnsignedLong(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;

    long result = 0;
    for (; i < length; i++) {
      char c = lowerHex.charAt(i);
      result <<= 4;
      if (c >= '0' && c <= '9') {
        result |= c - '0';
      } else if (c >= 'a' && c <= 'f') {
        result |= c - 'a' + 10;
      } else {
        throw isntLowerHexLong(lowerHex);
      }
    }
    return result;
  }

  static NumberFormatException isntLowerHexLong(String lowerHex) {
    throw new NumberFormatException(
        lowerHex + " should be a 1 to 32 character lower-hex string with no prefix");
  }

Steps to reproduce the behavior

Once this is in, we should be able to test by ensuring that the following parse the same value:

X-B3-TraceId: 463ac35c9f6413ad48485a3953bb6124
X-B3-TraceId: 48485a3953bb6124

[kevinoliver]

+@mosesn
Seems like a reasonable approach to me.

[mosesn]

@adriancole what's the expected migration strategy? use a zipkin which writes all 128 bits, but only uses 64, then when everyone is switched over, cut over?

[codefromthecrypt]

@adriancole https://github.com/adriancole what's the expected migration
strategy? use a zipkin which writes all 128 bits, but only uses 64, then
when everyone is switched over, cut over?

pretty much. There would be a little dancing inside zipkin api when tracers
send mixed bits in the same trace, but yeah.

Main thing is the transition design is made to have no impact until 128bit
ids are provisioned and propagated. This change request, is just a
safeguard in case some non-finagle callers propagate a 128bit id before
finagle is update, or the underlying zipkin one is.

[codefromthecrypt]

#553 I just converted java to scala.. hope it looks alright.

[mosesn]

@adriancole fixed this back in #553, thanks!