You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zipkin.apache.org by ad...@apache.org on 2019/05/12 02:42:55 UTC
[incubator-zipkin] branch more-efficient-strings created (now
a603181)
This is an automated email from the ASF dual-hosted git repository.
adriancole pushed a change to branch more-efficient-strings
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin.git.
at a603181 Consolidates buffers and generally improves string decoding
This branch includes the following new commits:
new a603181 Consolidates buffers and generally improves string decoding
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[incubator-zipkin] 01/01: Consolidates buffers and generally
improves string decoding
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adriancole pushed a commit to branch more-efficient-strings
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin.git
commit a6031815427a391613ca792a29b2af685a6fc845
Author: Adrian Cole <ac...@pivotal.io>
AuthorDate: Sun May 12 10:42:39 2019 +0800
Consolidates buffers and generally improves string decoding
---
.../src/main/java/zipkin2/SpanBenchmarks.java | 30 ++++++
.../main/java/zipkin2/codec/CodecBenchmarks.java | 105 ++++++++-------------
.../java/zipkin2/codec/ProtobufSpanDecoder.java | 2 +-
.../main/java/zipkin2/codec/WireSpanDecoder.java | 2 +-
.../storage/cassandra/v1/CassandraUtil.java | 25 ++---
.../zipkin2/storage/cassandra/CassandraUtil.java | 30 +++---
.../elasticsearch/ElasticsearchSpanConsumer.java | 4 +-
.../elasticsearch/VersionSpecificTemplates.java | 4 +-
.../elasticsearch/internal/BulkCallBuilder.java | 1 -
.../elasticsearch/internal/BulkIndexWriter.java | 6 +-
zipkin/src/main/java/zipkin2/Endpoint.java | 16 +---
zipkin/src/main/java/zipkin2/Span.java | 6 +-
.../src/main/java/zipkin2/internal/Platform.java | 24 ++---
.../main/java/zipkin2/internal/Proto3Codec.java | 10 +-
.../main/java/zipkin2/internal/Proto3Fields.java | 11 ++-
.../java/zipkin2/internal/Proto3ZipkinFields.java | 4 +-
.../main/java/zipkin2/internal/ThriftCodec.java | 49 ++++++++--
.../java/zipkin2/internal/ThriftEndpointCodec.java | 3 +
.../main/java/zipkin2/internal/UnsafeBuffer.java | 18 +++-
.../java/zipkin2/internal/V1ThriftSpanReader.java | 21 +++--
.../java/zipkin2/codec/SpanBytesDecoderTest.java | 28 ++++--
.../java/zipkin2/codec/V1SpanBytesDecoderTest.java | 27 +++++-
.../java/zipkin2/internal/Proto3FieldsTest.java | 2 +-
23 files changed, 262 insertions(+), 166 deletions(-)
diff --git a/benchmarks/src/main/java/zipkin2/SpanBenchmarks.java b/benchmarks/src/main/java/zipkin2/SpanBenchmarks.java
index 40eed40..a30a1d2 100644
--- a/benchmarks/src/main/java/zipkin2/SpanBenchmarks.java
+++ b/benchmarks/src/main/java/zipkin2/SpanBenchmarks.java
@@ -16,6 +16,10 @@
*/
package zipkin2;
+import com.esotericsoftware.kryo.Kryo;
+import com.esotericsoftware.kryo.io.Input;
+import com.esotericsoftware.kryo.io.Output;
+import com.esotericsoftware.kryo.serializers.JavaSerializer;
import java.util.concurrent.TimeUnit;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
@@ -46,6 +50,7 @@ public class SpanBenchmarks {
Endpoint.newBuilder().serviceName("frontend").ip("127.0.0.1").build();
static final Endpoint BACKEND =
Endpoint.newBuilder().serviceName("backend").ip("192.168.99.101").port(9000).build();
+ static final Span clientSpan = buildClientSpan(Span.newBuilder());
final Span.Builder sharedBuilder;
@@ -113,6 +118,31 @@ public class SpanBenchmarks {
return sharedBuilder.clone().build();
}
+ static final Kryo kryo = new Kryo();
+ static final byte[] clientSpanSerialized;
+
+ static {
+ kryo.register(Span.class, new JavaSerializer());
+ Output output = new Output(4096);
+ kryo.writeObject(output, clientSpan);
+ output.flush();
+ clientSpanSerialized = output.getBuffer();
+ }
+
+ /** manually implemented with json so not as slow as normal java */
+ @Benchmark
+ public Span serialize_kryo() {
+ return kryo.readObject(new Input(clientSpanSerialized), Span.class);
+ }
+
+ @Benchmark
+ public byte[] deserialize_kryo() {
+ Output output = new Output(clientSpanSerialized.length);
+ kryo.writeObject(output, clientSpan);
+ output.flush();
+ return output.getBuffer();
+ }
+
// Convenience main entry-point
public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder()
diff --git a/benchmarks/src/main/java/zipkin2/codec/CodecBenchmarks.java b/benchmarks/src/main/java/zipkin2/codec/CodecBenchmarks.java
index 7591b88..b808ffc 100644
--- a/benchmarks/src/main/java/zipkin2/codec/CodecBenchmarks.java
+++ b/benchmarks/src/main/java/zipkin2/codec/CodecBenchmarks.java
@@ -16,10 +16,6 @@
*/
package zipkin2.codec;
-import com.esotericsoftware.kryo.Kryo;
-import com.esotericsoftware.kryo.io.Input;
-import com.esotericsoftware.kryo.io.Output;
-import com.esotericsoftware.kryo.serializers.JavaSerializer;
import com.google.common.io.ByteStreams;
import java.io.IOException;
import java.util.Collections;
@@ -61,137 +57,112 @@ import zipkin2.Span;
public class CodecBenchmarks {
static final byte[] clientSpanJsonV2 = read("/zipkin2-client.json");
static final Span clientSpan = SpanBytesDecoder.JSON_V2.decodeOne(clientSpanJsonV2);
+ static final byte[] clientSpanJsonV1 = SpanBytesEncoder.JSON_V1.encode(clientSpan);
static final byte[] clientSpanProto3 = SpanBytesEncoder.PROTO3.encode(clientSpan);
- static final zipkin2.proto3.Span clientSpan_wire;
+ static final byte[] clientSpanThrift = SpanBytesEncoder.THRIFT.encode(clientSpan);
static final List<Span> tenClientSpans = Collections.nCopies(10, clientSpan);
static final byte[] tenClientSpansJsonV2 = SpanBytesEncoder.JSON_V2.encodeList(tenClientSpans);
- static final Kryo kryo = new Kryo();
- static final byte[] clientSpanSerialized;
-
- static {
- kryo.register(Span.class, new JavaSerializer());
- Output output = new Output(4096);
- kryo.writeObject(output, clientSpan);
- output.flush();
- clientSpanSerialized = output.getBuffer();
- try {
- clientSpan_wire = zipkin2.proto3.Span.ADAPTER.decode(clientSpanProto3);
- } catch (IOException e) {
- throw new AssertionError(e);
- }
- }
-
- /** manually implemented with json so not as slow as normal java */
- @Benchmark
- public Span readClientSpan_kryo() {
- return kryo.readObject(new Input(clientSpanSerialized), Span.class);
- }
@Benchmark
- public byte[] writeClientSpan_kryo() {
- Output output = new Output(clientSpanSerialized.length);
- kryo.writeObject(output, clientSpan);
- output.flush();
- return output.getBuffer();
+ public Span readClientSpan_JSON_V1() {
+ return SpanBytesDecoder.JSON_V1.decodeOne(clientSpanJsonV1);
}
@Benchmark
- public Span readClientSpan_json() {
+ public Span readClientSpan_JSON_V2() {
return SpanBytesDecoder.JSON_V2.decodeOne(clientSpanJsonV2);
}
@Benchmark
- public Span readClientSpan_proto3() {
+ public Span readClientSpan_PROTO3() {
return SpanBytesDecoder.PROTO3.decodeOne(clientSpanProto3);
}
@Benchmark
- public zipkin2.proto3.Span readClientSpan_proto3_wire() throws Exception {
- return zipkin2.proto3.Span.ADAPTER.decode(clientSpanProto3);
+ public Span readClientSpan_THRIFT() {
+ return SpanBytesDecoder.THRIFT.decodeOne(clientSpanThrift);
}
@Benchmark
- public List<Span> readTenClientSpans_json() {
- return SpanBytesDecoder.JSON_V2.decodeList(tenClientSpansJsonV2);
- }
-
- @Benchmark
- public byte[] writeClientSpan_json() {
+ public byte[] writeClientSpan_JSON_V2() {
return SpanBytesEncoder.JSON_V2.encode(clientSpan);
}
@Benchmark
- public byte[] writeTenClientSpans_json() {
- return SpanBytesEncoder.JSON_V2.encodeList(tenClientSpans);
+ public byte[] writeClientSpan_JSON_V1() {
+ return SpanBytesEncoder.JSON_V1.encode(clientSpan);
}
@Benchmark
- public byte[] writeClientSpan_json_v1() {
- return SpanBytesEncoder.JSON_V1.encode(clientSpan);
+ public byte[] writeClientSpan_PROTO3() {
+ return SpanBytesEncoder.PROTO3.encode(clientSpan);
}
@Benchmark
- public byte[] writeTenClientSpans_json_v1() {
- return SpanBytesEncoder.JSON_V1.encodeList(tenClientSpans);
+ public byte[] writeClientSpan_THRIFT() {
+ return SpanBytesEncoder.THRIFT.encode(clientSpan);
}
@Benchmark
- public byte[] writeClientSpan_proto3() {
- return SpanBytesEncoder.PROTO3.encode(clientSpan);
+ public List<Span> readTenClientSpans_JSON_V2() {
+ return SpanBytesDecoder.JSON_V2.decodeList(tenClientSpansJsonV2);
}
@Benchmark
- public byte[] writeClientSpan_proto3_wire() {
- return clientSpan_wire.encode();
+ public byte[] writeTenClientSpans_JSON_V2() {
+ return SpanBytesEncoder.JSON_V2.encodeList(tenClientSpans);
}
static final byte[] chineseSpanJsonV2 = read("/zipkin2-chinese.json");
static final Span chineseSpan = SpanBytesDecoder.JSON_V2.decodeOne(chineseSpanJsonV2);
- static final zipkin2.proto3.Span chineseSpan_wire;
static final byte[] chineseSpanProto3 = SpanBytesEncoder.PROTO3.encode(chineseSpan);
+ static final byte[] chineseSpanJsonV1 = SpanBytesEncoder.JSON_V1.encode(chineseSpan);
+ static final byte[] chineseSpanThrift = SpanBytesEncoder.THRIFT.encode(chineseSpan);
- static {
- try {
- chineseSpan_wire = zipkin2.proto3.Span.ADAPTER.decode(chineseSpanProto3);
- } catch (IOException e) {
- throw new AssertionError(e);
- }
+ @Benchmark
+ public Span readChineseSpan_JSON_V1() {
+ return SpanBytesDecoder.JSON_V1.decodeOne(chineseSpanJsonV1);
}
@Benchmark
- public Span readChineseSpan_json() {
+ public Span readChineseSpan_JSON_V2() {
return SpanBytesDecoder.JSON_V2.decodeOne(chineseSpanJsonV2);
}
@Benchmark
- public Span readChineseSpan_proto3() {
+ public Span readChineseSpan_PROTO3() {
return SpanBytesDecoder.PROTO3.decodeOne(chineseSpanProto3);
}
@Benchmark
- public zipkin2.proto3.Span readChineseSpan_proto3_wire() throws Exception {
- return zipkin2.proto3.Span.ADAPTER.decode(chineseSpanProto3);
+ public Span readChineseSpan_THRIFT() {
+ return SpanBytesDecoder.THRIFT.decodeOne(chineseSpanThrift);
}
@Benchmark
- public byte[] writeChineseSpan_json() {
+ public byte[] writeChineseSpan_JSON_V2() {
return SpanBytesEncoder.JSON_V2.encode(chineseSpan);
}
@Benchmark
- public byte[] writeChineseSpan_proto3() {
+ public byte[] writeChineseSpan_JSON_V1() {
+ return SpanBytesEncoder.JSON_V1.encode(chineseSpan);
+ }
+
+ @Benchmark
+ public byte[] writeChineseSpan_PROTO3() {
return SpanBytesEncoder.PROTO3.encode(chineseSpan);
}
@Benchmark
- public byte[] writeChineseSpan_proto3_wire() {
- return chineseSpan_wire.encode();
+ public byte[] writeChineseSpan_THRIFT() {
+ return SpanBytesEncoder.THRIFT.encode(chineseSpan);
}
// Convenience main entry-point
public static void main(String[] args) throws RunnerException {
Options opt = new OptionsBuilder()
- .include(".*" + CodecBenchmarks.class.getSimpleName())
+ .include(".*" + CodecBenchmarks.class.getSimpleName() +".*read.*Span_.*")
.addProfiler("gc")
.build();
diff --git a/benchmarks/src/main/java/zipkin2/codec/ProtobufSpanDecoder.java b/benchmarks/src/main/java/zipkin2/codec/ProtobufSpanDecoder.java
index 0df0d03..e53b8f8 100644
--- a/benchmarks/src/main/java/zipkin2/codec/ProtobufSpanDecoder.java
+++ b/benchmarks/src/main/java/zipkin2/codec/ProtobufSpanDecoder.java
@@ -279,7 +279,7 @@ public class ProtobufSpanDecoder {
throw new AssertionError("hex field greater than 32 chars long: " + length);
}
- char[] result = Platform.get().idBuffer();
+ char[] result = Platform.shortStringBuffer();
for (int i = 0; i < length; i += 2) {
byte b = input.readRawByte();
diff --git a/benchmarks/src/main/java/zipkin2/codec/WireSpanDecoder.java b/benchmarks/src/main/java/zipkin2/codec/WireSpanDecoder.java
index d5b1f84..52a5f8f 100644
--- a/benchmarks/src/main/java/zipkin2/codec/WireSpanDecoder.java
+++ b/benchmarks/src/main/java/zipkin2/codec/WireSpanDecoder.java
@@ -294,7 +294,7 @@ public class WireSpanDecoder {
throw new AssertionError("hex field greater than 32 chars long: " + length);
}
- char[] result = Platform.get().idBuffer();
+ char[] result = Platform.shortStringBuffer();
for (int i = 0; i < bytes.size(); i ++) {
byte b = bytes.getByte(i);
diff --git a/zipkin-storage/cassandra-v1/src/main/java/zipkin2/storage/cassandra/v1/CassandraUtil.java b/zipkin-storage/cassandra-v1/src/main/java/zipkin2/storage/cassandra/v1/CassandraUtil.java
index d051f63..4510e71 100644
--- a/zipkin-storage/cassandra-v1/src/main/java/zipkin2/storage/cassandra/v1/CassandraUtil.java
+++ b/zipkin-storage/cassandra-v1/src/main/java/zipkin2/storage/cassandra/v1/CassandraUtil.java
@@ -37,9 +37,12 @@ import zipkin2.Annotation;
import zipkin2.Call;
import zipkin2.Span;
import zipkin2.internal.Nullable;
+import zipkin2.internal.Platform;
+import zipkin2.internal.UnsafeBuffer;
import zipkin2.storage.QueryRequest;
import static com.google.common.base.Preconditions.checkArgument;
+import static zipkin2.internal.Platform.SHORT_STRING_LENGTH;
final class CassandraUtil {
static final Charset UTF_8 = Charset.forName("UTF-8");
@@ -47,13 +50,6 @@ final class CassandraUtil {
static final List<String> CORE_ANNOTATIONS =
ImmutableList.of("cs", "cr", "ss", "sr", "ms", "mr", "ws", "wr");
- /**
- * Zipkin's {@link QueryRequest#annotationQuery()} are equals match. Not all tags are lookup keys.
- * For example, sql query isn't something that is likely to be looked up by value and indexing
- * that could add a potentially kilobyte partition key on {@link Tables#ANNOTATIONS_INDEX}
- */
- static final int LONGEST_VALUE_TO_INDEX = 256;
-
private static final ThreadLocal<CharsetEncoder> UTF8_ENCODER =
new ThreadLocal<CharsetEncoder>() {
@Override
@@ -73,6 +69,11 @@ final class CassandraUtil {
/**
* Returns keys that concatenate the serviceName associated with an annotation or tag.
*
+ * <p>Values over {@link Platform#SHORT_STRING_LENGTH} are not considered. Zipkin's {@link
+ * QueryRequest#annotationQuery()} are equals match. Not all values are lookup values. For
+ * example, {@code sql.query} isn't something that is likely to be looked up by value and indexing
+ * that could add a potentially kilobyte partition key on {@link Tables#ANNOTATIONS_INDEX}
+ *
* @see QueryRequest#annotationQuery()
*/
static Set<String> annotationKeys(Span span) {
@@ -80,15 +81,17 @@ final class CassandraUtil {
String localServiceName = span.localServiceName();
if (localServiceName == null) return Collections.emptySet();
for (Annotation a : span.annotations()) {
+ if (a.value().length() > SHORT_STRING_LENGTH) continue;
+
// don't index core annotations as they aren't queryable
if (CORE_ANNOTATIONS.contains(a.value())) continue;
annotationKeys.add(localServiceName + ":" + a.value());
}
for (Map.Entry<String, String> e : span.tags().entrySet()) {
- if (e.getValue().length() <= LONGEST_VALUE_TO_INDEX) {
- annotationKeys.add(localServiceName + ":" + e.getKey());
- annotationKeys.add(localServiceName + ":" + e.getKey() + ":" + e.getValue());
- }
+ if (e.getValue().length() > SHORT_STRING_LENGTH) continue;
+
+ annotationKeys.add(localServiceName + ":" + e.getKey());
+ annotationKeys.add(localServiceName + ":" + e.getKey() + ":" + e.getValue());
}
return annotationKeys;
}
diff --git a/zipkin-storage/cassandra/src/main/java/zipkin2/storage/cassandra/CassandraUtil.java b/zipkin-storage/cassandra/src/main/java/zipkin2/storage/cassandra/CassandraUtil.java
index af8b19c..71de159 100644
--- a/zipkin-storage/cassandra/src/main/java/zipkin2/storage/cassandra/CassandraUtil.java
+++ b/zipkin-storage/cassandra/src/main/java/zipkin2/storage/cassandra/CassandraUtil.java
@@ -33,23 +33,18 @@ import zipkin2.Call;
import zipkin2.Span;
import zipkin2.internal.DateUtil;
import zipkin2.internal.Nullable;
+import zipkin2.internal.Platform;
import zipkin2.storage.QueryRequest;
-final class CassandraUtil {
- /**
- * Zipkin's {@link QueryRequest#annotationQuery()} are equals match. Not all tag serviceSpanKeys
- * are lookup serviceSpanKeys. For example, {@code sql.query} isn't something that is likely to be
- * looked up by value and indexing that could add a potentially kilobyte partition key on {@link
- * Schema#TABLE_SPAN}
- */
- static final int LONGEST_VALUE_TO_INDEX = 256;
+import static zipkin2.internal.Platform.SHORT_STRING_LENGTH;
+final class CassandraUtil {
/**
* Time window covered by a single bucket of the {@link Schema#TABLE_TRACE_BY_SERVICE_SPAN} and
* {@link Schema#TABLE_TRACE_BY_SERVICE_REMOTE_SERVICE}, in seconds. Default: 1 day
*/
private static final long DURATION_INDEX_BUCKET_WINDOW_SECONDS =
- Long.getLong("zipkin.store.cassandra.internal.durationIndexBucket", 24 * 60 * 60);
+ Long.getLong("zipkin.store.cassandra.internal.durationIndexBucket", 24 * 60 * 60);
public static int durationIndexBucket(long ts_micro) {
// if the window constant has microsecond precision, the division produces negative getValues
@@ -59,6 +54,11 @@ final class CassandraUtil {
/**
* Returns a set of annotation getValues and tags joined on equals, delimited by ░
*
+ * <p>Values over {@link Platform#SHORT_STRING_LENGTH} are not considered. Zipkin's {@link
+ * QueryRequest#annotationQuery()} are equals match. Not all values are lookup values. For
+ * example, {@code sql.query} isn't something that is likely to be looked up by value and indexing
+ * that could add a potentially kilobyte partition key on {@link Schema#TABLE_SPAN}
+ *
* @see QueryRequest#annotationQuery()
*/
static @Nullable String annotationQuery(Span span) {
@@ -67,16 +67,16 @@ final class CassandraUtil {
char delimiter = '░'; // as very unlikely to be in the query
StringBuilder result = new StringBuilder().append(delimiter);
for (Annotation a : span.annotations()) {
- if (a.value().length() > LONGEST_VALUE_TO_INDEX) continue;
+ if (a.value().length() > SHORT_STRING_LENGTH) continue;
result.append(a.value()).append(delimiter);
}
for (Map.Entry<String, String> tag : span.tags().entrySet()) {
- if (tag.getValue().length() > LONGEST_VALUE_TO_INDEX) continue;
+ if (tag.getValue().length() > SHORT_STRING_LENGTH) continue;
result.append(tag.getKey()).append(delimiter); // search is possible by key alone
- result.append(tag.getKey() + "=" + tag.getValue()).append(delimiter);
+ result.append(tag.getKey()).append('=').append(tag.getValue()).append(delimiter);
}
return result.length() == 1 ? null : result.toString();
}
@@ -107,9 +107,9 @@ final class CassandraUtil {
SortedMap<BigInteger, String> sorted = new TreeMap<>(Collections.reverseOrder());
for (Map.Entry<String, Long> entry : map.entrySet()) {
BigInteger uncollided =
- BigInteger.valueOf(entry.getValue())
- .multiply(OFFSET)
- .add(BigInteger.valueOf(RAND.nextInt() & Integer.MAX_VALUE));
+ BigInteger.valueOf(entry.getValue())
+ .multiply(OFFSET)
+ .add(BigInteger.valueOf(RAND.nextInt() & Integer.MAX_VALUE));
sorted.put(uncollided, entry.getKey());
}
return new LinkedHashSet<>(sorted.values());
diff --git a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/ElasticsearchSpanConsumer.java b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/ElasticsearchSpanConsumer.java
index cf2db5d..4667327 100644
--- a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/ElasticsearchSpanConsumer.java
+++ b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/ElasticsearchSpanConsumer.java
@@ -31,7 +31,7 @@ import zipkin2.storage.SpanConsumer;
import static zipkin2.elasticsearch.ElasticsearchAutocompleteTags.AUTOCOMPLETE;
import static zipkin2.elasticsearch.ElasticsearchSpanStore.SPAN;
-import static zipkin2.elasticsearch.internal.BulkCallBuilder.INDEX_CHARS_LIMIT;
+import static zipkin2.internal.Platform.SHORT_STRING_LENGTH;
class ElasticsearchSpanConsumer implements SpanConsumer { // not final for testing
@@ -107,7 +107,7 @@ class ElasticsearchSpanConsumer implements SpanConsumer { // not final for testi
String idx = consumer.formatTypeAndTimestampForInsert(AUTOCOMPLETE, indexTimestamp);
for (Map.Entry<String, String> tag : span.tags().entrySet()) {
int length = tag.getKey().length() + tag.getValue().length() + 1;
- if (length > INDEX_CHARS_LIMIT) continue;
+ if (length > SHORT_STRING_LENGTH) continue;
// If the autocomplete whitelist doesn't contain the key, skip storing its value
if (!consumer.autocompleteKeys.contains(tag.getKey())) continue;
diff --git a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/VersionSpecificTemplates.java b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/VersionSpecificTemplates.java
index f9e6bc9..0e401dd 100644
--- a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/VersionSpecificTemplates.java
+++ b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/VersionSpecificTemplates.java
@@ -26,6 +26,7 @@ import static zipkin2.elasticsearch.ElasticsearchAutocompleteTags.AUTOCOMPLETE;
import static zipkin2.elasticsearch.ElasticsearchSpanStore.DEPENDENCY;
import static zipkin2.elasticsearch.ElasticsearchSpanStore.SPAN;
import static zipkin2.elasticsearch.internal.JsonReaders.enterPath;
+import static zipkin2.internal.Platform.SHORT_STRING_LENGTH;
/** Returns a version-specific span and dependency index template */
final class VersionSpecificTemplates {
@@ -109,7 +110,8 @@ final class VersionSpecificTemplates {
+ " {\n"
+ " \"strings\": {\n"
+ " \"mapping\": {\n"
- + " \"type\": \"keyword\",\"norms\": false, \"ignore_above\": 256\n"
+ + " \"type\": \"keyword\",\"norms\": false,"
+ + " \"ignore_above\": " + SHORT_STRING_LENGTH + "\n"
+ " },\n"
+ " \"match_mapping_type\": \"string\",\n"
+ " \"match\": \"*\"\n"
diff --git a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkCallBuilder.java b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkCallBuilder.java
index 2361907..6c6e03e 100644
--- a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkCallBuilder.java
+++ b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkCallBuilder.java
@@ -35,7 +35,6 @@ import zipkin2.elasticsearch.internal.client.HttpCall;
// See https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-bulk.html
// exposed to re-use for testing writes of dependency links
public final class BulkCallBuilder {
- public static final int INDEX_CHARS_LIMIT = 256;
static final MediaType APPLICATION_JSON = MediaType.parse("application/json");
final String tag;
diff --git a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkIndexWriter.java b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkIndexWriter.java
index e9fd409..d037ef1 100644
--- a/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkIndexWriter.java
+++ b/zipkin-storage/elasticsearch/src/main/java/zipkin2/elasticsearch/internal/BulkIndexWriter.java
@@ -28,7 +28,7 @@ import zipkin2.Annotation;
import zipkin2.Endpoint;
import zipkin2.Span;
-import static zipkin2.elasticsearch.internal.BulkCallBuilder.INDEX_CHARS_LIMIT;
+import static zipkin2.internal.Platform.SHORT_STRING_LENGTH;
public abstract class BulkIndexWriter<T> {
@@ -163,12 +163,12 @@ public abstract class BulkIndexWriter<T> {
writer.name("_q");
writer.beginArray();
for (Annotation a : span.annotations()) {
- if (a.value().length() > INDEX_CHARS_LIMIT) continue;
+ if (a.value().length() > SHORT_STRING_LENGTH) continue;
writer.value(a.value());
}
for (Map.Entry<String, String> tag : span.tags().entrySet()) {
int length = tag.getKey().length() + tag.getValue().length() + 1;
- if (length > INDEX_CHARS_LIMIT) continue;
+ if (length > SHORT_STRING_LENGTH) continue;
writer.value(tag.getKey()); // search is possible by key alone
writer.value(tag.getKey() + "=" + tag.getValue());
}
diff --git a/zipkin/src/main/java/zipkin2/Endpoint.java b/zipkin/src/main/java/zipkin2/Endpoint.java
index be6a214..9684541 100644
--- a/zipkin/src/main/java/zipkin2/Endpoint.java
+++ b/zipkin/src/main/java/zipkin2/Endpoint.java
@@ -25,6 +25,7 @@ import java.net.InetAddress;
import java.nio.ByteBuffer;
import java.util.Locale;
import zipkin2.internal.Nullable;
+import zipkin2.internal.Platform;
import static zipkin2.internal.UnsafeBuffer.HEX_DIGITS;
@@ -198,7 +199,7 @@ public final class Endpoint implements Serializable { // for Spark and Flink job
}
static String writeIpV4(byte[] ipBytes) {
- char[] buf = ipBuffer();
+ char[] buf = Platform.shortStringBuffer();
int pos = 0;
pos = writeBackwards(ipBytes[0] & 0xff, pos, buf);
buf[pos++] = '.';
@@ -368,20 +369,9 @@ public final class Endpoint implements Serializable { // for Spark and Flink job
return (c < '0' || c > '9') && (c < 'a' || c > 'f') && (c < 'A' || c > 'F');
}
- static final ThreadLocal<char[]> IP_BUFFER = new ThreadLocal<>();
-
- static char[] ipBuffer() {
- char[] ipBuffer = IP_BUFFER.get();
- if (ipBuffer == null) {
- ipBuffer = new char[39]; // maximum length of encoded ipv6
- IP_BUFFER.set(ipBuffer);
- }
- return ipBuffer;
- }
-
static String writeIpV6(byte[] ipv6) {
int pos = 0;
- char[] buf = ipBuffer();
+ char[] buf = Platform.shortStringBuffer();
// Compress the longest string of zeros
int zeroCompressionIndex = -1;
diff --git a/zipkin/src/main/java/zipkin2/Span.java b/zipkin/src/main/java/zipkin2/Span.java
index 1f07e63..6ea685d 100644
--- a/zipkin/src/main/java/zipkin2/Span.java
+++ b/zipkin/src/main/java/zipkin2/Span.java
@@ -416,7 +416,7 @@ public final class Span implements Serializable { // for Spark and Flink jobs
*/
public Builder traceId(long high, long low) {
if (high == 0L && low == 0L) throw new IllegalArgumentException("empty trace ID");
- char[] data = Platform.get().idBuffer();
+ char[] data = Platform.shortStringBuffer();
int pos = 0;
if (high != 0L) {
writeHexLong(data, pos, high);
@@ -660,7 +660,7 @@ public final class Span implements Serializable { // for Spark and Flink jobs
}
static String padLeft(String id, int desiredLength) {
- char[] data = Platform.get().idBuffer();
+ char[] data = Platform.shortStringBuffer();
int i = 0, length = id.length(), offset = desiredLength - length;
for (; i < offset; i++) data[i] = '0';
for (int j = 0; j < length; j++) data[i++] = id.charAt(j);
@@ -668,7 +668,7 @@ public final class Span implements Serializable { // for Spark and Flink jobs
}
static String toLowerHex(long v) {
- char[] data = Platform.get().idBuffer();
+ char[] data = Platform.shortStringBuffer();
writeHexLong(data, 0, v);
return new String(data, 0, 16);
}
diff --git a/zipkin/src/main/java/zipkin2/internal/Platform.java b/zipkin/src/main/java/zipkin2/internal/Platform.java
index 6e10fb8..c64faa0 100644
--- a/zipkin/src/main/java/zipkin2/internal/Platform.java
+++ b/zipkin/src/main/java/zipkin2/internal/Platform.java
@@ -27,23 +27,25 @@ import org.jvnet.animal_sniffer.IgnoreJRERequirement;
public abstract class Platform {
private static final Platform PLATFORM = findPlatform();
- private static final ThreadLocal<char[]> ID_BUFFER = new ThreadLocal<>();
-
Platform() {
}
+ static final ThreadLocal<char[]> SHORT_STRING_BUFFER = new ThreadLocal<>();
+ /** Maximum character length constraint of most names, IP literals and IDs. */
+ public static final int SHORT_STRING_LENGTH = 256;
+
/**
- * Returns a {@link ThreadLocal} reused {@code char[]} for use when decoding bytes into a hex
- * string. The buffer should be immediately copied into a {@link String} after decoding within the
- * same method.
+ * Returns a {@link ThreadLocal} reused {@code char[]} for use when decoding bytes into hex, IP
+ * literals, or {@link #SHORT_STRING_LENGTH short strings}. The buffer must never be leaked
+ * outside the method. Most will {@link String#String(char[], int, int) copy it into a string}.
*/
- public char[] idBuffer() {
- char[] idBuffer = ID_BUFFER.get();
- if (idBuffer == null) {
- idBuffer = new char[32];
- ID_BUFFER.set(idBuffer);
+ public static char[] shortStringBuffer() {
+ char[] shortStringBuffer = SHORT_STRING_BUFFER.get();
+ if (shortStringBuffer == null) {
+ shortStringBuffer = new char[SHORT_STRING_LENGTH];
+ SHORT_STRING_BUFFER.set(shortStringBuffer);
}
- return idBuffer;
+ return shortStringBuffer;
}
public RuntimeException uncheckedIOException(IOException e) {
diff --git a/zipkin/src/main/java/zipkin2/internal/Proto3Codec.java b/zipkin/src/main/java/zipkin2/internal/Proto3Codec.java
index c5ae215..52d646e 100644
--- a/zipkin/src/main/java/zipkin2/internal/Proto3Codec.java
+++ b/zipkin/src/main/java/zipkin2/internal/Proto3Codec.java
@@ -52,13 +52,17 @@ public final class Proto3Codec {
if (span == null) return false;
out.add(span);
return true;
- } catch (Exception e) {
+ } catch (RuntimeException e) {
throw exceptionReading("Span", e);
}
}
public static @Nullable Span readOne(byte[] bytes) {
- return SPAN.read(UnsafeBuffer.wrap(bytes, 0));
+ try {
+ return SPAN.read(UnsafeBuffer.wrap(bytes, 0));
+ } catch (RuntimeException e) {
+ throw exceptionReading("Span", e);
+ }
}
public static boolean readList(byte[] bytes, Collection<Span> out) {
@@ -71,7 +75,7 @@ public final class Proto3Codec {
if (span == null) return false;
out.add(span);
}
- } catch (Exception e) {
+ } catch (RuntimeException e) {
throw exceptionReading("List<Span>", e);
}
return true;
diff --git a/zipkin/src/main/java/zipkin2/internal/Proto3Fields.java b/zipkin/src/main/java/zipkin2/internal/Proto3Fields.java
index 0761795..e580e46 100644
--- a/zipkin/src/main/java/zipkin2/internal/Proto3Fields.java
+++ b/zipkin/src/main/java/zipkin2/internal/Proto3Fields.java
@@ -124,13 +124,18 @@ final class Proto3Fields {
* is returned when the length prefix is zero.
*/
final T readLengthPrefixAndValue(UnsafeBuffer b) {
- int length = readLengthPrefix(b);
+ int length = guardLength(b);
if (length == 0) return null;
return readValue(b, length);
}
- final int readLengthPrefix(UnsafeBuffer b) {
- return b.readVarint32();
+ final int guardLength(UnsafeBuffer buffer) {
+ int length = buffer.readVarint32();
+ if (length > buffer.remaining()) {
+ throw new IllegalArgumentException(
+ "Truncated: length " + length + " > bytes remaining " + buffer.remaining());
+ }
+ return length;
}
abstract int sizeOfValue(T value);
diff --git a/zipkin/src/main/java/zipkin2/internal/Proto3ZipkinFields.java b/zipkin/src/main/java/zipkin2/internal/Proto3ZipkinFields.java
index 7bc63a9..36a0bb9 100644
--- a/zipkin/src/main/java/zipkin2/internal/Proto3ZipkinFields.java
+++ b/zipkin/src/main/java/zipkin2/internal/Proto3ZipkinFields.java
@@ -139,7 +139,7 @@ final class Proto3ZipkinFields {
}
@Override boolean readLengthPrefixAndValue(UnsafeBuffer b, Span.Builder builder) {
- int length = readLengthPrefix(b);
+ int length = guardLength(b);
if (length == 0) return false;
int endPos = b.pos() + length;
@@ -187,7 +187,7 @@ final class Proto3ZipkinFields {
}
@Override boolean readLengthPrefixAndValue(UnsafeBuffer b, Span.Builder builder) {
- int length = readLengthPrefix(b);
+ int length = guardLength(b);
if (length == 0) return false;
int endPos = b.pos() + length;
diff --git a/zipkin/src/main/java/zipkin2/internal/ThriftCodec.java b/zipkin/src/main/java/zipkin2/internal/ThriftCodec.java
index 2355b4d..258c5a5 100644
--- a/zipkin/src/main/java/zipkin2/internal/ThriftCodec.java
+++ b/zipkin/src/main/java/zipkin2/internal/ThriftCodec.java
@@ -127,8 +127,9 @@ public final class ThriftCodec {
static IllegalArgumentException exceptionReading(String type, Exception e) {
String cause = e.getMessage() == null ? "Error" : e.getMessage();
if (e instanceof EOFException) cause = "EOF";
- if (e instanceof IllegalStateException || e instanceof BufferUnderflowException)
+ if (e instanceof IllegalStateException || e instanceof BufferUnderflowException) {
cause = "Malformed";
+ }
String message = String.format("%s reading %s from TBinary", cause, type);
throw new IllegalArgumentException(message, e);
}
@@ -187,29 +188,59 @@ public final class ThriftCodec {
static void skip(ByteBuffer bytes, int count) {
// avoid java.lang.NoSuchMethodError: java.nio.ByteBuffer.position(I)Ljava/nio/ByteBuffer;
// bytes.position(bytes.position() + count);
- for (int i = 0; i< count && bytes.hasRemaining(); i++) {
+ for (int i = 0; i < count && bytes.hasRemaining(); i++) {
bytes.get();
}
}
static byte[] readByteArray(ByteBuffer bytes) {
- byte[] result = new byte[guardLength(bytes)];
- bytes.get(result);
- return result;
+ return readByteArray(bytes, guardLength(bytes));
+ }
+
+ static final String ONE = Character.toString((char) 1);
+
+ static byte[] readByteArray(ByteBuffer bytes, int length) {
+ byte[] copy = new byte[length];
+ if (!bytes.hasArray()) {
+ bytes.get(copy);
+ return copy;
+ }
+
+ byte[] original = bytes.array();
+ int offset = bytes.arrayOffset() + bytes.position();
+ System.arraycopy(original, offset, copy, 0, length);
+ bytes.position(bytes.position() + length);
+ return copy;
}
static String readUtf8(ByteBuffer bytes) {
- // TODO: optimize out the array copy here
- return new String(readByteArray(bytes), UTF_8);
+ int length = guardLength(bytes);
+ if (length == 0) return ""; // ex empty name
+ if (length == 1) {
+ byte single = bytes.get();
+ if (single == 1) return ONE; // special case for address annotations
+ return Character.toString((char) single);
+ }
+
+ if (!bytes.hasArray()) return new String(readByteArray(bytes, length), UTF_8);
+
+ int offset = bytes.arrayOffset() + bytes.position();
+ String result = UnsafeBuffer.wrap(bytes.array(), offset).readUtf8(length);
+ bytes.position(bytes.position() + length);
+ return result;
}
static int guardLength(ByteBuffer buffer) {
int length = buffer.getInt();
+ guardLength(buffer, length);
+ return length;
+ }
+
+ static void guardLength(ByteBuffer buffer, int length) {
if (length > buffer.remaining()) {
throw new IllegalArgumentException(
- "Truncated: length " + length + " > bytes remaining " + buffer.remaining());
+ "Truncated: length " + length + " > bytes remaining " + buffer.remaining());
}
- return length;
}
static void writeListBegin(UnsafeBuffer buffer, int size) {
diff --git a/zipkin/src/main/java/zipkin2/internal/ThriftEndpointCodec.java b/zipkin/src/main/java/zipkin2/internal/ThriftEndpointCodec.java
index 9c0eb93..fd646ca 100644
--- a/zipkin/src/main/java/zipkin2/internal/ThriftEndpointCodec.java
+++ b/zipkin/src/main/java/zipkin2/internal/ThriftEndpointCodec.java
@@ -19,6 +19,7 @@ package zipkin2.internal;
import java.nio.ByteBuffer;
import zipkin2.Endpoint;
+import static zipkin2.internal.ThriftCodec.guardLength;
import static zipkin2.internal.ThriftCodec.skip;
import static zipkin2.internal.ThriftField.TYPE_I16;
import static zipkin2.internal.ThriftField.TYPE_I32;
@@ -41,6 +42,7 @@ final class ThriftEndpointCodec {
if (thriftField.type == TYPE_STOP) break;
if (thriftField.isEqualTo(IPV4)) {
+ guardLength(bytes, 4);
int ipv4 = bytes.getInt();
if (ipv4 != 0) {
result.parseIp( // allocation is ok here as Endpoint.ipv4Bytes would anyway
@@ -52,6 +54,7 @@ final class ThriftEndpointCodec {
});
}
} else if (thriftField.isEqualTo(PORT)) {
+ guardLength(bytes, 2);
result.port(bytes.getShort() & 0xFFFF);
} else if (thriftField.isEqualTo(SERVICE_NAME)) {
result.serviceName(ThriftCodec.readUtf8(bytes));
diff --git a/zipkin/src/main/java/zipkin2/internal/UnsafeBuffer.java b/zipkin/src/main/java/zipkin2/internal/UnsafeBuffer.java
index 5089b35..2b1bc71 100644
--- a/zipkin/src/main/java/zipkin2/internal/UnsafeBuffer.java
+++ b/zipkin/src/main/java/zipkin2/internal/UnsafeBuffer.java
@@ -98,11 +98,25 @@ public final class UnsafeBuffer {
String readUtf8(int length) {
require(length);
- String result = new String(buf, pos, length, UTF_8);
+ String result = maybeDecodeShortAsciiString(buf, pos, length);
+ if (result == null) new String(buf, pos, length, UTF_8);
pos += length;
return result;
}
+ // Speculatively assume all 7-bit ASCII characters.. common in normal tags and names
+ static String maybeDecodeShortAsciiString(byte[] buf, int offset, int length) {
+ if (length == 0) return ""; // ex error tag with no value
+ if (length > Platform.SHORT_STRING_LENGTH) return null;
+ char[] buffer = Platform.shortStringBuffer();
+ for (int i = 0; i < length; i++) {
+ byte b = buf[offset + i];
+ if ((b & 0x80) != 0) return null; // Not 7-bit ASCII character
+ buffer[i] = (char) b;
+ }
+ return new String(buffer, 0, length);
+ }
+
String readBytesAsHex(int length) {
// All our hex fields are at most 32 characters.
if (length > 32) {
@@ -110,7 +124,7 @@ public final class UnsafeBuffer {
}
require(length);
- char[] result = Platform.get().idBuffer();
+ char[] result = Platform.get().shortStringBuffer();
int hexLength = length * 2;
for (int i = 0; i < hexLength; i += 2) {
diff --git a/zipkin/src/main/java/zipkin2/internal/V1ThriftSpanReader.java b/zipkin/src/main/java/zipkin2/internal/V1ThriftSpanReader.java
index ac5ee35..541e0cc 100644
--- a/zipkin/src/main/java/zipkin2/internal/V1ThriftSpanReader.java
+++ b/zipkin/src/main/java/zipkin2/internal/V1ThriftSpanReader.java
@@ -20,8 +20,8 @@ import java.nio.ByteBuffer;
import zipkin2.Endpoint;
import zipkin2.v1.V1Span;
-import static zipkin2.internal.ThriftCodec.UTF_8;
-import static zipkin2.internal.ThriftCodec.readByteArray;
+import static zipkin2.internal.ThriftCodec.ONE;
+import static zipkin2.internal.ThriftCodec.guardLength;
import static zipkin2.internal.ThriftCodec.readListLength;
import static zipkin2.internal.ThriftCodec.readUtf8;
import static zipkin2.internal.ThriftCodec.skip;
@@ -62,14 +62,18 @@ public final class V1ThriftSpanReader {
if (thriftField.type == TYPE_STOP) break;
if (thriftField.isEqualTo(TRACE_ID_HIGH)) {
+ guardLength(bytes, 8);
builder.traceIdHigh(bytes.getLong());
} else if (thriftField.isEqualTo(TRACE_ID)) {
+ guardLength(bytes, 8);
builder.traceId(bytes.getLong());
} else if (thriftField.isEqualTo(NAME)) {
builder.name(readUtf8(bytes));
} else if (thriftField.isEqualTo(ID)) {
+ guardLength(bytes, 8);
builder.id(bytes.getLong());
} else if (thriftField.isEqualTo(PARENT_ID)) {
+ guardLength(bytes, 8);
builder.parentId(bytes.getLong());
} else if (thriftField.isEqualTo(ANNOTATIONS)) {
int length = readListLength(bytes);
@@ -82,10 +86,13 @@ public final class V1ThriftSpanReader {
BinaryAnnotationReader.read(bytes, builder);
}
} else if (thriftField.isEqualTo(DEBUG)) {
+ guardLength(bytes, 1);
builder.debug(bytes.get() == 1);
} else if (thriftField.isEqualTo(TIMESTAMP)) {
+ guardLength(bytes, 8);
builder.timestamp(bytes.getLong());
} else if (thriftField.isEqualTo(DURATION)) {
+ guardLength(bytes, 8);
builder.duration(bytes.getLong());
} else {
skip(bytes, thriftField.type);
@@ -111,6 +118,7 @@ public final class V1ThriftSpanReader {
if (thriftField.type == TYPE_STOP) break;
if (thriftField.isEqualTo(TIMESTAMP)) {
+ guardLength(bytes, 8);
timestamp = bytes.getLong();
} else if (thriftField.isEqualTo(VALUE)) {
value = readUtf8(bytes);
@@ -134,7 +142,7 @@ public final class V1ThriftSpanReader {
static void read(ByteBuffer bytes, V1Span.Builder builder) {
String key = null;
- byte[] value = null;
+ String value = null;
Endpoint endpoint = null;
boolean isBoolean = false;
boolean isString = false;
@@ -145,8 +153,9 @@ public final class V1ThriftSpanReader {
if (thriftField.isEqualTo(KEY)) {
key = readUtf8(bytes);
} else if (thriftField.isEqualTo(VALUE)) {
- value = readByteArray(bytes);
+ value = readUtf8(bytes);
} else if (thriftField.isEqualTo(TYPE)) {
+ guardLength(bytes, 4);
switch (bytes.getInt()) {
case 0:
isBoolean = true;
@@ -163,8 +172,8 @@ public final class V1ThriftSpanReader {
}
if (key == null || value == null) return;
if (isString) {
- builder.addBinaryAnnotation(key, new String(value, UTF_8), endpoint);
- } else if (isBoolean && value.length == 1 && value[0] == 1 && endpoint != null) {
+ builder.addBinaryAnnotation(key, value, endpoint);
+ } else if (isBoolean && ONE.equals(value) && endpoint != null) {
if (key.equals("sa") || key.equals("ca") || key.equals("ma")) {
builder.addBinaryAnnotation(key, endpoint);
}
diff --git a/zipkin/src/test/java/zipkin2/codec/SpanBytesDecoderTest.java b/zipkin/src/test/java/zipkin2/codec/SpanBytesDecoderTest.java
index f2875ef..679c3e6 100644
--- a/zipkin/src/test/java/zipkin2/codec/SpanBytesDecoderTest.java
+++ b/zipkin/src/test/java/zipkin2/codec/SpanBytesDecoderTest.java
@@ -17,6 +17,7 @@
package zipkin2.codec;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.junit.Rule;
@@ -40,6 +41,22 @@ public class SpanBytesDecoderTest {
@Rule public ExpectedException thrown = ExpectedException.none();
+ @Test public void niceErrorOnTruncatedSpans_PROTO3() {
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Truncated: length 66 > bytes remaining 8 reading List<Span> from proto3");
+
+ byte[] encoded = SpanBytesEncoder.PROTO3.encodeList(TRACE);
+ SpanBytesDecoder.PROTO3.decodeList(Arrays.copyOfRange(encoded, 0, 10));
+ }
+
+ @Test public void niceErrorOnTruncatedSpan_PROTO3() {
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Truncated: length 179 > bytes remaining 7 reading Span from proto3");
+
+ byte[] encoded = SpanBytesEncoder.PROTO3.encode(SPAN);
+ SpanBytesDecoder.PROTO3.decodeOne(Arrays.copyOfRange(encoded, 0, 10));
+ }
+
@Test public void emptyListOk_JSON_V1() {
assertThat(SpanBytesDecoder.JSON_V1.decodeList(new byte[0]))
.isEmpty(); // instead of throwing an exception
@@ -59,15 +76,6 @@ public class SpanBytesDecoderTest {
.isEmpty(); // instead of throwing an exception
}
- @Test public void emptyListOk_THRIFT() {
- assertThat(SpanBytesDecoder.THRIFT.decodeList(new byte[0]))
- .isEmpty(); // instead of throwing an exception
-
- byte[] emptyListLiteral = {12 /* TYPE_STRUCT */, 0, 0, 0, 0 /* zero length */};
- assertThat(SpanBytesDecoder.THRIFT.decodeList(emptyListLiteral))
- .isEmpty(); // instead of throwing an exception
- }
-
@Test public void spanRoundTrip_JSON_V2() {
assertThat(SpanBytesDecoder.JSON_V2.decodeOne(SpanBytesEncoder.JSON_V2.encode(span)))
.isEqualTo(span);
@@ -164,7 +172,7 @@ public class SpanBytesDecoderTest {
@Test public void niceErrorOnMalformed_inputSpans_PROTO3() {
thrown.expect(IllegalArgumentException.class);
- thrown.expectMessage("Malformed reading List<Span> from proto3");
+ thrown.expectMessage("Truncated: length 101 > bytes remaining 3 reading List<Span> from proto3");
SpanBytesDecoder.PROTO3.decodeList(new byte[] {'h', 'e', 'l', 'l', 'o'});
}
diff --git a/zipkin/src/test/java/zipkin2/codec/V1SpanBytesDecoderTest.java b/zipkin/src/test/java/zipkin2/codec/V1SpanBytesDecoderTest.java
index 5408781..4d66efa 100644
--- a/zipkin/src/test/java/zipkin2/codec/V1SpanBytesDecoderTest.java
+++ b/zipkin/src/test/java/zipkin2/codec/V1SpanBytesDecoderTest.java
@@ -17,6 +17,7 @@
package zipkin2.codec;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import org.junit.Ignore;
@@ -25,7 +26,6 @@ import org.junit.Test;
import org.junit.rules.ExpectedException;
import zipkin2.Endpoint;
import zipkin2.Span;
-import zipkin2.TestObjects;
import static org.assertj.core.api.Assertions.assertThat;
import static zipkin2.TestObjects.BACKEND;
@@ -42,6 +42,31 @@ public class V1SpanBytesDecoderTest {
@Rule public ExpectedException thrown = ExpectedException.none();
+ @Test public void niceErrorOnTruncatedSpans_THRIFT() {
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Truncated: length 8 > bytes remaining 2 reading List<Span> from TBinary");
+
+ byte[] encoded = SpanBytesEncoder.THRIFT.encodeList(TRACE);
+ SpanBytesDecoder.THRIFT.decodeList(Arrays.copyOfRange(encoded, 0, 10));
+ }
+
+ @Test public void niceErrorOnTruncatedSpan_THRIFT() {
+ thrown.expect(IllegalArgumentException.class);
+ thrown.expectMessage("Truncated: length 8 > bytes remaining 7 reading Span from TBinary");
+
+ byte[] encoded = SpanBytesEncoder.THRIFT.encode(SPAN);
+ SpanBytesDecoder.THRIFT.decodeOne(Arrays.copyOfRange(encoded, 0, 10));
+ }
+
+ @Test public void emptyListOk_THRIFT() {
+ assertThat(SpanBytesDecoder.THRIFT.decodeList(new byte[0]))
+ .isEmpty(); // instead of throwing an exception
+
+ byte[] emptyListLiteral = {12 /* TYPE_STRUCT */, 0, 0, 0, 0 /* zero length */};
+ assertThat(SpanBytesDecoder.THRIFT.decodeList(emptyListLiteral))
+ .isEmpty(); // instead of throwing an exception
+ }
+
@Test
public void spanRoundTrip_JSON_V1() {
assertThat(SpanBytesDecoder.JSON_V1.decodeOne(SpanBytesEncoder.JSON_V1.encode(span)))
diff --git a/zipkin/src/test/java/zipkin2/internal/Proto3FieldsTest.java b/zipkin/src/test/java/zipkin2/internal/Proto3FieldsTest.java
index 5e1cef9..388faad 100644
--- a/zipkin/src/test/java/zipkin2/internal/Proto3FieldsTest.java
+++ b/zipkin/src/test/java/zipkin2/internal/Proto3FieldsTest.java
@@ -179,7 +179,7 @@ public class Proto3FieldsTest {
buf.reset();
buf.skip(1); // skip the key
- assertThat(field.readLengthPrefix(buf))
+ assertThat(field.guardLength(buf))
.isEqualTo(10);
}