Skip to content

Commit

Permalink
Simplifies b3 single format by formalizing debug as a sampling status
Browse files Browse the repository at this point in the history 
As discussed, this makes debug use the letter 'd' in the same slot as
unsampled '0' or sampled '1'. The result is much simpler code and
ideally more a more intuitive header.

See openzipkin/b3-propagation#21 (comment)
  • Loading branch information
Adrian Cole committed Aug 24, 2018
1 parent 7c6575f commit aab6783
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 99 deletions.
102 changes: 36 additions & 66 deletions brave/src/main/java/brave/propagation/B3SingleFormat.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,34 @@
* following manner.
*
* <pre>{@code
* b3: {x-b3-traceid}-{x-b3-spanid}-{x-b3-sampled}-{x-b3-parentspanid}-{x-b3-flags}
* b3: {x-b3-traceid}-{x-b3-spanid}-{if x-b3-flags 'd' else x-b3-sampled}-{x-b3-parentspanid}
* }</pre>
*
* <p>For example, a sampled root span would look like:
* {@code 4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-1}
*
* <p>... a not yet sampled root span would look like:
* {@code 4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7}
*
* <p>... and a debug RPC child span would look like:
* {@code 4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-d-5b4185666d50f68b}
*
* <p>Like normal B3, it is valid to omit trace identifiers in order to only propagate a sampling
* decision. For example, the following are valid downstream hints:
* <ul>
* <li>sampled - {@code b3: 1}</li>
* <li>unsampled - {@code b3: 0}</li>
* <li>debug - {@code b3: 1-1}</li>
* <li>debug - {@code b3: d}</li>
* </ul>
* Note: {@code b3: 0-1} isn't supported as it doesn't make sense. Debug boosts ordinary sampling
* decision to also affect the collector tier. {@code b3: 0-1} would be like saying, don't sample,
* except at the collector tier, which is impossible as if you don't sample locally the data will
* never arrive at a collector.
*
* Reminder: debug (previously {@code X-B3-Flags: 1}), is a boosted sample signal which is recorded
* to ensure it reaches the collector tier. See {@link TraceContext#debug()}.
*
* <p>See <a href="https://github.com/openzipkin/b3-propagation">B3 Propagation</a>
*/
public final class B3SingleFormat {
static final Logger logger = Logger.getLogger(B3SingleFormat.class.getName());
static final int FORMAT_MAX_LENGTH = 32 + 1 + 16 + 2 + 16 + 2; // traceid128-spanid-1-parentid-1
static final int FORMAT_MAX_LENGTH = 32 + 1 + 16 + 2 + 16; // traceid128-spanid-1-parentid

/**
* Writes all B3 defined fields in the trace context, except {@link TraceContext#parentIdAsLong()
Expand Down Expand Up @@ -106,19 +111,14 @@ static int writeB3SingleFormat(TraceContext context, long parentId, char[] resul
Boolean sampled = context.sampled();
if (sampled != null) {
result[pos++] = '-';
result[pos++] = sampled ? '1' : '0';
result[pos++] = context.debug() ? 'd' : sampled ? '1' : '0';
}

if (parentId != 0) {
if (parentId != 0L) {
result[pos++] = '-';
writeHexLong(result, pos, parentId);
pos += 16;
}

if (context.debug()) {
result[pos++] = '-';
result[pos++] = '1';
}
return pos;
}

Expand All @@ -134,11 +134,16 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3) {
@Nullable
public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, int beginIndex,
int endIndex) {
if (endIndex <= beginIndex + 3) { // possibly sampling flags
return decode(b3, beginIndex, endIndex);
if (beginIndex == endIndex) {
logger.log(FINE, "Invalid input: empty");
return null;
}

int pos = beginIndex;
if (pos + 1 == endIndex) { // possibly sampling flags
return tryParseSamplingFlags(b3, pos);
}

// At this point we minimally expect a traceId-spanId pair
if (endIndex < 16 + 1 + 16 /* traceid64-spanid */) {
logger.fine("Invalid input: truncated");
Expand Down Expand Up @@ -194,31 +199,17 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, i
}

if (endIndex > pos) {
// If we are at this point, we have only two possible fields left, parent and/or debug
// If the parent field is present, we'll have at least 17 characters. If it is absent, but debug
// is present, we'll have we'll have a delimiter 2 characters from now. Ex "-1"
// Therefore, if we have less than two characters, the input is truncated.
if (endIndex == pos + 1) {
// If we are at this point, we should have a parent ID, encoded as "-[0-9a-f]{16}"
if (endIndex != pos + 17) {
logger.fine("Invalid input: truncated");
return null;
}

if (endIndex > pos + 2) {
if (!checkHyphen(b3, pos++)) return null;
parentId = tryParse16HexCharacters(b3, pos, endIndex);
if (parentId == 0L) {
logger.log(FINE, "Invalid input: expected a 16 lower hex parent ID at offset {0}", pos);
return null;
}
pos += 16;
}

// the only option at this point is that we have a debug flag
if (endIndex == pos + 2) {
if (!checkHyphen(b3, pos)) return null;
pos++; // consume the hyphen
flags = parseDebugFlag(b3, pos, flags);
if (flags == 0) return null;
if (!checkHyphen(b3, pos++)) return null;
parentId = tryParse16HexCharacters(b3, pos, endIndex);
if (parentId == 0L) {
logger.log(FINE, "Invalid input: expected a 16 lower hex parent ID at offset {0}", pos);
return null;
}
}
}
Expand All @@ -233,25 +224,9 @@ public static TraceContextOrSamplingFlags parseB3SingleFormat(CharSequence b3, i
));
}

@Nullable
static TraceContextOrSamplingFlags decode(CharSequence b3, int beginIndex, int endIndex) {
int pos = beginIndex;
if (pos == endIndex) { // empty
logger.log(FINE, "Invalid input: expected 0 or 1 for sampled at offset {0}", pos);
return null;
}

int flags = parseSampledFlag(b3, pos++);
static TraceContextOrSamplingFlags tryParseSamplingFlags(CharSequence b3, int pos) {
int flags = parseSampledFlag(b3, pos);
if (flags == 0) return null;
if (endIndex > pos) {
if (!checkHyphen(b3, pos++)) return null;
if (endIndex == pos) {
logger.fine("Invalid input: truncated");
return null;
}
flags = parseDebugFlag(b3, pos, flags);
if (flags == 0) return null;
}
return TraceContextOrSamplingFlags.create(SamplingFlags.toSamplingFlags(flags));
}

Expand All @@ -274,26 +249,21 @@ static long tryParse16HexCharacters(CharSequence lowerHex, int index, int end) {
static int parseSampledFlag(CharSequence b3, int pos) {
int flags;
char sampledChar = b3.charAt(pos);
if (sampledChar == '1') {
if (sampledChar == 'd') {
flags = FLAG_SAMPLED_SET | FLAG_SAMPLED | FLAG_DEBUG;
} else if (sampledChar == '1') {
flags = FLAG_SAMPLED_SET | FLAG_SAMPLED;
} else if (sampledChar == '0') {
flags = FLAG_SAMPLED_SET;
} else {
logger.log(FINE, "Invalid input: expected 0 or 1 for sampled at offset {0}", pos);
logInvalidSampled(pos);
flags = 0;
}
return flags;
}

static int parseDebugFlag(CharSequence b3, int pos, int flags) {
char lastChar = b3.charAt(pos);
if (lastChar == '1') {
flags = FLAG_DEBUG | FLAG_SAMPLED_SET | FLAG_SAMPLED;
} else if (lastChar != '0') { // redundant to say debug false, but whatev
logger.log(FINE, "Invalid input: expected 1 for debug at offset {0}", pos);
flags = 0;
}
return flags;
static void logInvalidSampled(int pos) {
logger.log(FINE, "Invalid input: expected 0, 1 or d for sampled at offset {0}", pos);
}

static byte[] asciiToNewByteArray(char[] buffer, int length) {
Expand Down
2 changes: 1 addition & 1 deletion brave/src/test/java/brave/TracerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ public class TracerTest {
assertThat(tracer.toSpan(context))
.isInstanceOf(RealSpan.class)
.extracting(Span::context)
.isEqualToComparingFieldByField(context);
.isEqualTo(context);
}

@Test public void toSpan_noop() {
Expand Down
63 changes: 31 additions & 32 deletions brave/src/test/java/brave/propagation/B3SingleFormatTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import org.junit.Test;

import static brave.propagation.B3SingleFormat.parseB3SingleFormat;
import static brave.propagation.B3SingleFormat.writeB3SingleFormatAsBytes;
import static brave.propagation.B3SingleFormat.writeB3SingleFormat;
import static brave.propagation.B3SingleFormat.writeB3SingleFormatAsBytes;
import static brave.propagation.B3SingleFormat.writeB3SingleFormatWithoutParentId;
import static brave.propagation.B3SingleFormat.writeB3SingleFormatWithoutParentIdAsBytes;
import static java.nio.charset.StandardCharsets.UTF_8;
Expand All @@ -23,6 +23,14 @@ public class B3SingleFormatTest {
.isEqualTo(new String(writeB3SingleFormatAsBytes(context), UTF_8));
}

@Test public void writeB3SingleFormat_notYetSampled_128() {
TraceContext context = TraceContext.newBuilder().traceIdHigh(9).traceId(1).spanId(3).build();

assertThat(writeB3SingleFormat(context))
.isEqualTo("0000000000000009" + traceId + "-" + spanId)
.isEqualTo(new String(writeB3SingleFormatAsBytes(context), UTF_8));
}

@Test public void writeB3SingleFormat_unsampled() {
TraceContext context = TraceContext.newBuilder().traceId(1).spanId(3).sampled(false).build();

Expand All @@ -43,7 +51,7 @@ public class B3SingleFormatTest {
TraceContext context = TraceContext.newBuilder().traceId(1).spanId(3).debug(true).build();

assertThat(writeB3SingleFormat(context))
.isEqualTo(traceId + "-" + spanId + "-1-1")
.isEqualTo(traceId + "-" + spanId + "-d")
.isEqualTo(new String(writeB3SingleFormatAsBytes(context), UTF_8));
}

Expand Down Expand Up @@ -87,7 +95,7 @@ public class B3SingleFormatTest {
TraceContext.newBuilder().traceId(1).parentId(2).spanId(3).debug(true).build();

assertThat(writeB3SingleFormatWithoutParentId(context))
.isEqualTo(traceId + "-" + spanId + "-1-1")
.isEqualTo(traceId + "-" + spanId + "-d")
.isEqualTo(new String(writeB3SingleFormatWithoutParentIdAsBytes(context), UTF_8));
}

Expand All @@ -101,12 +109,18 @@ public class B3SingleFormatTest {
}

/** for example, parsing a w3c context */
@Test public void parseB3SingleFormat_middleOfString_flags() {
String input = "b2=foo,b3=1-1,b4=bar";
assertThat(parseB3SingleFormat(input, 10, 13).samplingFlags())
@Test public void parseB3SingleFormat_middleOfString_debugOnly() {
String input = "b2=foo,b3=d,b4=bar";
assertThat(parseB3SingleFormat(input, 10, 11).samplingFlags())
.isSameAs(SamplingFlags.DEBUG);
}

@Test public void parseB3SingleFormat_middleOfString_incorrectOffset() {
String input = "b2=foo,b3=d,b4=bar";
assertThat(parseB3SingleFormat(input, 10, 12))
.isNull(); // instead of raising exception
}

@Test public void parseB3SingleFormat_idsNotYetSampled() {
assertThat(parseB3SingleFormat(traceId + "-" + spanId).context())
.isEqualToComparingFieldByField(
Expand Down Expand Up @@ -136,31 +150,19 @@ public class B3SingleFormatTest {
}

@Test public void parseB3SingleFormat_parent_debug() {
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-1-" + parentId + "-1").context())
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-d-" + parentId).context())
.isEqualToComparingFieldByField(
TraceContext.newBuilder().traceId(1).parentId(2).spanId(3).debug(true).build()
);
}

@Test public void parseB3SingleFormat_idsWithDebug() {
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-1-1").context())
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-d").context())
.isEqualToComparingFieldByField(
TraceContext.newBuilder().traceId(1).spanId(3).debug(true).build()
);
}

@Test public void parseB3SingleFormat_idsUnsampled_with_redundant_debug() {
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-0-0").context())
.isEqualToComparingFieldByField(
TraceContext.newBuilder().traceId(1).spanId(3).sampled(false).build()
);
}

@Test public void parseB3SingleFormat_idsUnsampled_with_malformed_debug() {
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-0-?"))
.isNull(); // instead of raising exception
}

@Test public void parseB3SingleFormat_sampledFalse() {
assertThat(parseB3SingleFormat("0"))
.isEqualTo(TraceContextOrSamplingFlags.NOT_SAMPLED);
Expand All @@ -171,21 +173,11 @@ public class B3SingleFormatTest {
.isEqualTo(TraceContextOrSamplingFlags.SAMPLED);
}

@Test public void parseB3SingleFormat_sampled_redundant() {
assertThat(parseB3SingleFormat("1-0"))
.isEqualTo(TraceContextOrSamplingFlags.SAMPLED);
}

@Test public void parseB3SingleFormat_debug() {
assertThat(parseB3SingleFormat("1-1"))
assertThat(parseB3SingleFormat("d"))
.isEqualTo(TraceContextOrSamplingFlags.DEBUG);
}

@Test public void parseB3SingleFormat_debug_malformed() {
assertThat(parseB3SingleFormat("1-?"))
.isNull(); // instead of raising exception
}

@Test public void parseB3SingleFormat_malformed_traceId() {
assertThat(parseB3SingleFormat(traceId.substring(0, 15) + "?-" + spanId))
.isNull(); // instead of raising exception
Expand All @@ -196,7 +188,14 @@ public class B3SingleFormatTest {
.isNull(); // instead of raising exception
}

@Test public void parseB3SingleFormat_malformed_parentid() {
@Test public void parseB3SingleFormat_malformed_sampled_parentid() {
assertThat(
parseB3SingleFormat(traceId + "-" + spanId + "-1-" + parentId.substring(0, 15) + "?"))
.isNull(); // instead of raising exception
}

// odd but possible to not yet sample a child
@Test public void parseB3SingleFormat_malformed_parentid_notYetSampled() {
assertThat(parseB3SingleFormat(traceId + "-" + spanId + "-" + parentId.substring(0, 15) + "?"))
.isNull(); // instead of raising exception
}
Expand Down

0 comments on commit aab6783

Please sign in to comment.