diff options
3 files changed, 92 insertions, 90 deletions
diff --git a/benchmarks/src/jmh/java/com/google/instrumentation/trace/PropagationUtilBenchmark.java b/benchmarks/src/jmh/java/com/google/instrumentation/trace/PropagationUtilBenchmark.java index 3b302186..2834db03 100644 --- a/benchmarks/src/jmh/java/com/google/instrumentation/trace/PropagationUtilBenchmark.java +++ b/benchmarks/src/jmh/java/com/google/instrumentation/trace/PropagationUtilBenchmark.java @@ -14,6 +14,7 @@ package com.google.instrumentation.trace; import java.io.IOException; +import java.text.ParseException; import java.util.concurrent.TimeUnit; import org.openjdk.jmh.annotations.Benchmark; import org.openjdk.jmh.annotations.BenchmarkMode; @@ -61,7 +62,7 @@ public class PropagationUtilBenchmark { @Benchmark @BenchmarkMode(Mode.SampleTime) @OutputTimeUnit(TimeUnit.NANOSECONDS) - public SpanContext fromBinaryValueSpanContext() throws IOException { + public SpanContext fromBinaryValueSpanContext() throws ParseException { return PropagationUtil.fromBinaryValue(spanContextBinary); } @@ -73,7 +74,7 @@ public class PropagationUtilBenchmark { @Benchmark @BenchmarkMode(Mode.SampleTime) @OutputTimeUnit(TimeUnit.NANOSECONDS) - public SpanContext toFromBinarySpanContext() throws IOException { + public SpanContext toFromBinarySpanContext() throws ParseException { return PropagationUtil.fromBinaryValue(PropagationUtil.toBinaryValue(spanContext)); } } diff --git a/core/src/main/java/com/google/instrumentation/trace/PropagationUtil.java b/core/src/main/java/com/google/instrumentation/trace/PropagationUtil.java index 1ccb28cb..842961f3 100644 --- a/core/src/main/java/com/google/instrumentation/trace/PropagationUtil.java +++ b/core/src/main/java/com/google/instrumentation/trace/PropagationUtil.java @@ -13,10 +13,9 @@ package com.google.instrumentation.trace; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; -import java.io.IOException; +import java.text.ParseException; /** * This is a helper class for {@link SpanContext} propagation on the wire. @@ -28,15 +27,18 @@ import java.io.IOException; * <li>version_id: 1-byte representing the version id. * <li>For version_id = 0: * <ul> - * <li>version_format: <field><field>... + * <li>version_format: <field><field> * <li>field_format: <field_id><field_format> - * <li>TraceId: (filed_id = 0, len = 16, default = “0000000000000000”) - 16-byte array - * representing the trace_id. - * <li>SpanId: (filed_id = 1, len = 8, default = “00000000”) - 8-byte array representing the - * span_id. - * <li>TraceOptions: (filed_id = 2, len = 4, default = “0”) - 1-byte array representing the - * trace_options. It is in little-endian order, if represented as an int. - * <li>Fields MUST be encoded using the filed id order (smaller to higher). + * <li>Fields: + * <ul> + * <li>TraceId: (field_id = 0, len = 16, default = "0000000000000000") - 16-byte + * array representing the trace_id. + * <li>SpanId: (field_id = 1, len = 8, default = "00000000") - 8-byte array + * representing the span_id. + * <li>TraceOptions: (field_id = 2, len = 1, default = "0") - 1-byte array + * representing the trace_options. + * </ul> + * <li>Fields MUST be encoded using the field id order (smaller to higher). * <li>Valid value example: * <ul> * <li>{0, 0, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 1, 97, 98, 99, @@ -72,19 +74,11 @@ public final class PropagationUtil { * @param bytes a binary encoded buffer from which the {@code SpanContext} will be parsed. * @return the parsed {@code SpanContext}. * @throws NullPointerException if the {@code input} is {@code null}. - * @throws IOException if the version is not supported or the input is invalid + * @throws ParseException if the version is not supported or the input is invalid */ - public static SpanContext fromBinaryValue(byte[] bytes) throws IOException { + public static SpanContext fromBinaryValue(byte[] bytes) throws ParseException { checkNotNull(bytes, "bytes"); - try { - return binaryHandler.fromBinaryFormat(bytes); - } catch (IllegalArgumentException e) { - throw new IOException("Invalid input.", e); - } catch (ArrayIndexOutOfBoundsException e) { - throw new IOException("Invalid input.", e); - } catch (IndexOutOfBoundsException e) { - throw new IOException("Invalid input.", e); - } + return binaryHandler.fromBinaryFormat(bytes); } /** @@ -111,8 +105,9 @@ public final class PropagationUtil { * * @param bytes a binary encoded buffer from which the {@code SpanContext} will be parsed. * @return the parsed {@code SpanContext}. + * @throws ParseException is any parsing error. */ - public abstract SpanContext fromBinaryFormat(byte[] bytes); + public abstract SpanContext fromBinaryFormat(byte[] bytes) throws ParseException; } /** Version 0 implementation of the {@code VersionHandler}. */ @@ -150,24 +145,32 @@ public final class PropagationUtil { } @Override - public SpanContext fromBinaryFormat(byte[] bytes) { - checkArgument(bytes.length > 0 && bytes[0] == VERSION_ID, "Unsupported version."); + public SpanContext fromBinaryFormat(byte[] bytes) throws ParseException { + if (bytes.length == 0 || bytes[0] != VERSION_ID) { + throw new ParseException("Unsupported version.", 0); + } TraceId traceId = TraceId.INVALID; SpanId spanId = SpanId.INVALID; TraceOptions traceOptions = TraceOptions.DEFAULT; int pos = 1; - if (bytes.length > pos && bytes[pos] == TRACE_ID_FIELD_ID) { - traceId = TraceId.fromBytes(bytes, pos + ID_SIZE); - pos += ID_SIZE + TraceId.SIZE; - } - if (bytes.length > pos && bytes[pos] == SPAN_ID_FIELD_ID) { - spanId = SpanId.fromBytes(bytes, pos + ID_SIZE); - pos += ID_SIZE + SpanId.SIZE; - } - if (bytes.length > pos && bytes[pos] == TRACE_OPTION_FIELD_ID) { - traceOptions = TraceOptions.fromBytes(bytes, pos + ID_SIZE); + try { + if (bytes.length > pos && bytes[pos] == TRACE_ID_FIELD_ID) { + traceId = TraceId.fromBytes(bytes, pos + ID_SIZE); + pos += ID_SIZE + TraceId.SIZE; + } + if (bytes.length > pos && bytes[pos] == SPAN_ID_FIELD_ID) { + spanId = SpanId.fromBytes(bytes, pos + ID_SIZE); + pos += ID_SIZE + SpanId.SIZE; + } + if (bytes.length > pos && bytes[pos] == TRACE_OPTION_FIELD_ID) { + traceOptions = TraceOptions.fromBytes(bytes, pos + ID_SIZE); + } + return new SpanContext(traceId, spanId, traceOptions); + } catch (ArrayIndexOutOfBoundsException e) { + throw new ParseException("Invalid input: " + e.toString(), pos); + } catch (IndexOutOfBoundsException e) { + throw new ParseException("Invalid input: " + e.toString(), pos); } - return new SpanContext(traceId, spanId, traceOptions); } private DefaultBinaryHandler() {} diff --git a/core/src/test/java/com/google/instrumentation/trace/PropagationUtilTest.java b/core/src/test/java/com/google/instrumentation/trace/PropagationUtilTest.java index 9b479e16..e87c57e5 100644 --- a/core/src/test/java/com/google/instrumentation/trace/PropagationUtilTest.java +++ b/core/src/test/java/com/google/instrumentation/trace/PropagationUtilTest.java @@ -15,14 +15,15 @@ package com.google.instrumentation.trace; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; -import static org.junit.Assert.fail; import static org.mockito.Matchers.same; import static org.mockito.Mockito.when; import com.google.instrumentation.trace.PropagationUtil.BinaryHandler; import com.google.instrumentation.trace.PropagationUtil.DefaultBinaryHandler; -import java.io.IOException; +import java.text.ParseException; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; import org.mockito.Mock; @@ -31,13 +32,13 @@ import org.mockito.MockitoAnnotations; /** Unit tests for {@link PropagationUtil}. */ @RunWith(JUnit4.class) public class PropagationUtilTest { - private static final byte[] traceIdBytes = + private static final byte[] TRACE_ID_BYTES = new byte[] {64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79}; - private static final TraceId traceId = TraceId.fromBytes(traceIdBytes); - private static final byte[] spanIdBytes = new byte[] {97, 98, 99, 100, 101, 102, 103, 104}; - private static final SpanId spanId = SpanId.fromBytes(spanIdBytes); - private static final byte[] traceOptionsBytes = new byte[] {1}; - private static final TraceOptions traceOptions = TraceOptions.fromBytes(traceOptionsBytes); + private static final TraceId TRACE_ID = TraceId.fromBytes(TRACE_ID_BYTES); + private static final byte[] SPAN_ID_BYTES = new byte[] {97, 98, 99, 100, 101, 102, 103, 104}; + private static final SpanId SPAN_ID = SpanId.fromBytes(SPAN_ID_BYTES); + private static final byte[] TRACE_OPTIONS_BYTES = new byte[] {1}; + private static final TraceOptions TRACE_OPTIONS = TraceOptions.fromBytes(TRACE_OPTIONS_BYTES); private static final byte[] EXAMPLE_BYTES = new byte[] { 0, 0, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 1, 97, 98, 99, 100, @@ -50,44 +51,31 @@ public class PropagationUtilTest { SpanId.fromBytes(new byte[] {97, 98, 99, 100, 101, 102, 103, 104}), TraceOptions.fromBytes(new byte[] {1})); @Mock private BinaryHandler mockHandler; + @Rule public ExpectedException expectedException = ExpectedException.none(); - private static void testHttpSpanContextConversion(SpanContext spanContext) { + private static void testSpanContextConversion(SpanContext spanContext) throws ParseException { SpanContext propagatedBinarySpanContext = null; - try { - propagatedBinarySpanContext = - PropagationUtil.fromBinaryValue(PropagationUtil.toBinaryValue(spanContext)); - } catch (IOException e) { - fail("Unexpected exception " + e.toString()); - } + propagatedBinarySpanContext = + PropagationUtil.fromBinaryValue(PropagationUtil.toBinaryValue(spanContext)); + assertWithMessage("Binary propagated context is not equal with the initial context.") .that(propagatedBinarySpanContext) .isEqualTo(spanContext); } - // For valid requests to avoid using try-catch everywhere. - private static SpanContext fromBinaryValue(byte[] bytes) { - try { - return PropagationUtil.fromBinaryValue(bytes); - } catch (IOException e) { - fail("Unexpected exception " + e.toString()); - } - // Should not happen. - return SpanContext.INVALID; - } - @Test - public void propagate_SpanContextTracingEnabled() { - testHttpSpanContextConversion( - new SpanContext(traceId, spanId, TraceOptions.builder().setIsSampled().build())); + public void propagate_SpanContextTracingEnabled() throws ParseException { + testSpanContextConversion( + new SpanContext(TRACE_ID, SPAN_ID, TraceOptions.builder().setIsSampled().build())); } @Test - public void propagate_SpanContextNoTracing() { - testHttpSpanContextConversion(new SpanContext(traceId, spanId, TraceOptions.DEFAULT)); + public void propagate_SpanContextNoTracing() throws ParseException { + testSpanContextConversion(new SpanContext(TRACE_ID, SPAN_ID, TraceOptions.DEFAULT)); } @Test - public void toHttpHeaderValue_InvalidSpanContext() { + public void toBinaryValue_InvalidSpanContext() { assertThat(PropagationUtil.toBinaryValue(SpanContext.INVALID)) .isEqualTo( new byte[] { @@ -96,24 +84,28 @@ public class PropagationUtilTest { } @Test - public void parseHeaderValue_BinaryExampleValue() { - assertThat(fromBinaryValue(EXAMPLE_BYTES)) - .isEqualTo(new SpanContext(traceId, spanId, traceOptions)); - assertThat(fromBinaryValue(EXAMPLE_BYTES).getTraceOptions().getOptions()).isEqualTo(1); + public void parseBinaryValue_BinaryExampleValue() throws ParseException { + assertThat(PropagationUtil.fromBinaryValue(EXAMPLE_BYTES)) + .isEqualTo(new SpanContext(TRACE_ID, SPAN_ID, TRACE_OPTIONS)); + assertThat(PropagationUtil.fromBinaryValue(EXAMPLE_BYTES).getTraceOptions().getOptions()) + .isEqualTo(1); } @Test(expected = NullPointerException.class) - public void parseBinaryValue_NullInput() throws IOException { + public void parseBinaryValue_NullInput() throws ParseException { PropagationUtil.fromBinaryValue(null); } - @Test(expected = IOException.class) - public void parseBinaryValue_EmptyInput() throws IOException { + public void parseBinaryValue_EmptyInput() throws ParseException { + expectedException.expect(ParseException.class); + expectedException.expectMessage("Unsupported version."); PropagationUtil.fromBinaryValue(new byte[0]); } - @Test(expected = IOException.class) - public void parseBinaryValue_UnsupportedVersionId() throws IOException { + @Test + public void parseBinaryValue_UnsupportedVersionId() throws ParseException { + expectedException.expect(ParseException.class); + expectedException.expectMessage("Unsupported version."); PropagationUtil.fromBinaryValue( new byte[] { 66, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 97, 98, 99, 100, 101, @@ -122,9 +114,9 @@ public class PropagationUtilTest { } @Test - public void parseBinaryValue_UnsupportedFieldIdFirst() { + public void parseBinaryValue_UnsupportedFieldIdFirst() throws ParseException { assertThat( - fromBinaryValue( + PropagationUtil.fromBinaryValue( new byte[] { 0, 4, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 1, 97, 98, 99, 100, 101, 102, 103, 104, 2, 1 @@ -133,9 +125,9 @@ public class PropagationUtilTest { } @Test - public void parseBinaryValue_UnsupportedFieldIdSecond() { + public void parseBinaryValue_UnsupportedFieldIdSecond() throws ParseException { assertThat( - fromBinaryValue( + PropagationUtil.fromBinaryValue( new byte[] { 0, 0, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 3, 97, 98, 99, 100, 101, 102, 103, 104, 2, 1 @@ -148,19 +140,25 @@ public class PropagationUtilTest { TraceOptions.DEFAULT)); } - @Test(expected = IOException.class) - public void parseBinaryValue_ShorterTraceId() throws IOException { + @Test + public void parseBinaryValue_ShorterTraceId() throws ParseException { + expectedException.expect(ParseException.class); + expectedException.expectMessage("Invalid input: java.lang.ArrayIndexOutOfBoundsException"); PropagationUtil.fromBinaryValue( new byte[] {0, 0, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76}); } - @Test(expected = IOException.class) - public void parseBinaryValue_ShorterSpanId() throws IOException { + @Test + public void parseBinaryValue_ShorterSpanId() throws ParseException { + expectedException.expect(ParseException.class); + expectedException.expectMessage("Invalid input: java.lang.ArrayIndexOutOfBoundsException"); PropagationUtil.fromBinaryValue(new byte[] {0, 1, 97, 98, 99, 100, 101, 102, 103}); } - @Test(expected = IOException.class) - public void parseBinaryValue_ShorterTraceOptions() throws IOException { + @Test + public void parseBinaryValue_ShorterTraceOptions() throws ParseException { + expectedException.expect(ParseException.class); + expectedException.expectMessage("Invalid input: java.lang.IndexOutOfBoundsException"); PropagationUtil.fromBinaryValue( new byte[] { 0, 0, 64, 65, 66, 67, 68, 69, 70, 71, 72, 73, 74, 75, 76, 77, 78, 79, 1, 97, 98, 99, 100, @@ -169,12 +167,12 @@ public class PropagationUtilTest { } @Test - public void parseBinaryValue_WithNewAddedFormat() { + public void parseBinaryValue_WithNewAddedFormat() throws ParseException { MockitoAnnotations.initMocks(this); when(mockHandler.fromBinaryFormat(same(EXAMPLE_BYTES))).thenReturn(EXAMPLE_SPAN_CONTEXT); PropagationUtil.setBinaryHandler(mockHandler); try { - assertThat(fromBinaryValue(EXAMPLE_BYTES)).isEqualTo(EXAMPLE_SPAN_CONTEXT); + assertThat(PropagationUtil.fromBinaryValue(EXAMPLE_BYTES)).isEqualTo(EXAMPLE_SPAN_CONTEXT); } finally { PropagationUtil.setBinaryHandler(DefaultBinaryHandler.INSTANCE); } |