This is an automated email from the ASF dual-hosted git repository. adriancole pushed a commit to branch fix-libthrift in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-reporter-java.git
commit edbfbceab098e0abc4f14fcb2141bba6973f43a2 Author: Adrian Cole <[email protected]> AuthorDate: Wed May 8 21:00:47 2019 +0800 Fixes socket reset problem in libthrift sender Before, we didn't fully read the scribe response, resulting in socket resets. --- .../test/java/zipkin2/reporter/TestObjects.java | 26 ++++++++++++++++++++++ .../reporter/libthrift/InternalScribeCodec.java | 4 +++- .../reporter/libthrift/LibthriftSenderTest.java | 13 +++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/zipkin2/reporter/TestObjects.java b/core/src/test/java/zipkin2/reporter/TestObjects.java index 4a5d921..0cfe894 100644 --- a/core/src/test/java/zipkin2/reporter/TestObjects.java +++ b/core/src/test/java/zipkin2/reporter/TestObjects.java @@ -18,11 +18,13 @@ package zipkin2.reporter; import java.nio.charset.Charset; import java.util.Calendar; +import java.util.Random; import java.util.TimeZone; import java.util.concurrent.TimeUnit; import zipkin2.Endpoint; import zipkin2.Span; +// TODO: replace with zipkin-tests jar! public final class TestObjects { public static final Charset UTF_8 = Charset.forName("UTF-8"); /** Notably, the cassandra implementation has day granularity */ @@ -67,4 +69,28 @@ public final class TestObjects { .putTag("http.path", "/api") .putTag("clnt/finagle.version", "6.45.0") .build(); + + static final Span.Builder spanBuilder = spanBuilder(); + + /** Reuse a builder as it is significantly slows tests to create 100000 of these! */ + static Span.Builder spanBuilder() { + return Span.newBuilder() + .name("get /foo") + .timestamp(System.currentTimeMillis() * 1000) + .duration(1000) + .kind(Span.Kind.SERVER) + .localEndpoint(BACKEND) + .putTag("http.method", "GET"); + } + + /** + * Zipkin trace ids are random 64bit numbers. This creates a relatively large input to avoid + * flaking out due to PRNG nuance. + */ + public static final Span[] LOTS_OF_SPANS = + new Random().longs(100_000).mapToObj(TestObjects::span).toArray(Span[]::new); + + public static Span span(long traceId) { + return spanBuilder.traceId(Long.toHexString(traceId)).id(traceId).build(); + } } diff --git a/libthrift/src/main/java/zipkin2/reporter/libthrift/InternalScribeCodec.java b/libthrift/src/main/java/zipkin2/reporter/libthrift/InternalScribeCodec.java index eb6199d..6e643e4 100644 --- a/libthrift/src/main/java/zipkin2/reporter/libthrift/InternalScribeCodec.java +++ b/libthrift/src/main/java/zipkin2/reporter/libthrift/InternalScribeCodec.java @@ -78,15 +78,17 @@ public final class InternalScribeCodec { // public for zipkin-finagle } static boolean parseResponse(TBinaryProtocol iprot) throws TException { + Boolean result = null; iprot.readStructBegin(); TField schemeField; while ((schemeField = iprot.readFieldBegin()).type != TType.STOP) { if (schemeField.id == 0 /* SUCCESS */ && schemeField.type == TType.I32) { - return iprot.readI32() == 0; + result = iprot.readI32() == 0; } else { TProtocolUtil.skip(iprot, schemeField.type); } } + if (result != null) return result; throw new TApplicationException(MISSING_RESULT, "Log failed: unknown result"); } diff --git a/libthrift/src/test/java/zipkin2/reporter/libthrift/LibthriftSenderTest.java b/libthrift/src/test/java/zipkin2/reporter/libthrift/LibthriftSenderTest.java index c84490b..a2f3b6f 100644 --- a/libthrift/src/test/java/zipkin2/reporter/libthrift/LibthriftSenderTest.java +++ b/libthrift/src/test/java/zipkin2/reporter/libthrift/LibthriftSenderTest.java @@ -17,6 +17,7 @@ package zipkin2.reporter.libthrift; import java.io.IOException; +import java.util.Arrays; import java.util.List; import java.util.stream.Stream; import org.junit.After; @@ -35,6 +36,7 @@ import static java.util.stream.Collectors.toList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; import static zipkin2.reporter.TestObjects.CLIENT_SPAN; +import static zipkin2.reporter.TestObjects.LOTS_OF_SPANS; public class LibthriftSenderTest { @Rule public ExpectedException thrown = ExpectedException.none(); @@ -68,6 +70,17 @@ public class LibthriftSenderTest { assertThat(storage.spanStore().getTraces()).containsExactly(asList(CLIENT_SPAN)); } + /** This will help verify sequence ID and response parsing logic works */ + @Test + public void sendsSpans_multipleTimes() throws Exception { + for (int i = 0; i < 5; i++) { // Have client send 5 messages + send(Arrays.copyOfRange(LOTS_OF_SPANS, i, (i * 10) + 10)); + } + + assertThat(storage.getTraces()).flatExtracting(l -> l) + .contains(Arrays.copyOfRange(LOTS_OF_SPANS, 0, 50)); + } + @Test public void sendsSpansExpectedMetrics() throws Exception { send(CLIENT_SPAN, CLIENT_SPAN);
