From commits-return-440-archive-asf-public=cust-asf.ponee.io@zipkin.apache.org Mon Mar 18 05:00:16 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id 30D6C180629 for ; Mon, 18 Mar 2019 06:00:15 +0100 (CET) Received: (qmail 4119 invoked by uid 500); 18 Mar 2019 05:00:14 -0000 Mailing-List: contact commits-help@zipkin.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@zipkin.apache.org Delivered-To: mailing list commits@zipkin.apache.org Received: (qmail 4110 invoked by uid 99); 18 Mar 2019 05:00:14 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 18 Mar 2019 05:00:14 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id 836A98930B; Mon, 18 Mar 2019 05:00:13 +0000 (UTC) Date: Mon, 18 Mar 2019 05:00:13 +0000 To: "commits@zipkin.apache.org" Subject: [incubator-zipkin-brave-cassandra] branch master updated: Fixes integration test setup and propagation bug (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <155288521348.30506.15414144057907747125@gitbox.apache.org> From: adriancole@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: incubator-zipkin-brave-cassandra X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 2f2a7d612a35796b290223300bacc99ae4ca6ace X-Git-Newrev: 228c18f3c93a90a867c2778130359d51ccbcf278 X-Git-Rev: 228c18f3c93a90a867c2778130359d51ccbcf278 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated This is an automated email from the ASF dual-hosted git repository. adriancole pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave-cassandra.git The following commit(s) were added to refs/heads/master by this push: new 228c18f Fixes integration test setup and propagation bug (#20) 228c18f is described below commit 228c18f3c93a90a867c2778130359d51ccbcf278 Author: Adrian Cole AuthorDate: Mon Mar 18 13:00:07 2019 +0800 Fixes integration test setup and propagation bug (#20) There was a propagation bug that resulted in cassandra server not continuing inbound traces. This was hidden by incorrect integration test setup. --- Jenkinsfile | 2 +- .../brave/cassandra/driver/ITTracingSession.java | 23 +++++---- .../src/main/java/brave/cassandra/Tracing.java | 5 +- .../src/test/java/brave/cassandra/ITTracing.java | 5 +- .../src/test/java/brave/cassandra/TracingTest.java | 56 ++++++++++++++++++++++ pom.xml | 44 ++++++++++++++++- 6 files changed, 118 insertions(+), 17 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 599f06d..c107b02 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -69,7 +69,7 @@ pipeline { post { always { - junit '**/target/surefire-reports/*.xml' + junit '**/target/surefire-reports/*.xml **/target/failsafe-reports/*.xml' deleteDir() } diff --git a/cassandra-driver/src/test/java/brave/cassandra/driver/ITTracingSession.java b/cassandra-driver/src/test/java/brave/cassandra/driver/ITTracingSession.java index f28f40e..67d69c3 100644 --- a/cassandra-driver/src/test/java/brave/cassandra/driver/ITTracingSession.java +++ b/cassandra-driver/src/test/java/brave/cassandra/driver/ITTracingSession.java @@ -19,6 +19,7 @@ package brave.cassandra.driver; import brave.SpanCustomizer; import brave.Tracer; import brave.Tracing; +import brave.propagation.B3SingleFormat; import brave.propagation.StrictScopeDecorator; import brave.propagation.ThreadLocalCurrentTraceContext; import brave.sampler.Sampler; @@ -29,8 +30,7 @@ import com.datastax.driver.core.ResultSet; import com.datastax.driver.core.ResultSetFuture; import com.datastax.driver.core.Session; import com.datastax.driver.core.Statement; -import com.datastax.driver.core.exceptions.NoHostAvailableException; -import java.nio.ByteBuffer; +import com.datastax.driver.core.exceptions.DriverInternalError; import java.util.Collections; import java.util.concurrent.ConcurrentLinkedDeque; import org.junit.After; @@ -40,6 +40,7 @@ import org.junit.Test; import zipkin2.Endpoint; import zipkin2.Span; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; @@ -106,8 +107,7 @@ public class ITTracingSession { session.execute("SELECT * from system.schema_keyspaces"); - assertThat(CustomPayloadCaptor.ref.get().keySet()) - .containsExactly("X-B3-SpanId", "X-B3-Sampled", "X-B3-TraceId"); + assertThat(CustomPayloadCaptor.ref.get().keySet()).containsOnly("b3"); } @Test @@ -118,8 +118,7 @@ public class ITTracingSession { invokeBoundStatement(); - assertThat(CustomPayloadCaptor.ref.get().keySet()) - .containsExactly("X-B3-SpanId", "X-B3-Sampled", "X-B3-TraceId"); + assertThat(CustomPayloadCaptor.ref.get().keySet()).containsOnly("b3"); } @Test @@ -141,9 +140,9 @@ public class ITTracingSession { invokeBoundStatement(); - assertThat(CustomPayloadCaptor.ref.get().get("X-B3-Sampled")) - .extracting(ByteBuffer::get) - .isEqualTo((byte) '0'); + assertThat(CustomPayloadCaptor.ref.get().get("b3")) + .extracting(b -> B3SingleFormat.parseB3SingleFormat(UTF_8.decode(b)).sampled()) + .isEqualTo(Boolean.FALSE); } @Test @@ -166,8 +165,8 @@ public class ITTracingSession { try { invokeBoundStatement(); - failBecauseExceptionWasNotThrown(NoHostAvailableException.class); - } catch (NoHostAvailableException e) { + failBecauseExceptionWasNotThrown(DriverInternalError.class); + } catch (DriverInternalError e) { } assertThat(spans).hasSize(1); @@ -179,7 +178,7 @@ public class ITTracingSession { assertThat(spans) .flatExtracting(s -> s.tags().entrySet()) - .containsOnlyOnce(entry("error", "All host(s) tried for query failed (no host was tried)")); + .containsOnlyOnce(entry("error", "Could not send request, session is closed")); } @Test diff --git a/cassandra/src/main/java/brave/cassandra/Tracing.java b/cassandra/src/main/java/brave/cassandra/Tracing.java index 1cb9c1e..224f247 100644 --- a/cassandra/src/main/java/brave/cassandra/Tracing.java +++ b/cassandra/src/main/java/brave/cassandra/Tracing.java @@ -33,6 +33,7 @@ import zipkin2.reporter.AsyncReporter; import zipkin2.reporter.urlconnection.URLConnectionSender; import static brave.Span.Kind.SERVER; +import static java.nio.charset.StandardCharsets.UTF_8; /** * This creates Zipkin server spans for incoming cassandra requests. Spans are created when there's @@ -96,9 +97,9 @@ public class Tracing extends org.apache.cassandra.tracing.Tracing { /** This extracts the RPC span encoded in the custom payload, or starts a new trace */ Span spanFromPayload(Tracer tracer, @Nullable Map payload) { - ByteBuffer b3 = payload.get("b3"); + ByteBuffer b3 = payload != null ? payload.get("b3") : null; if (b3 == null) return tracer.nextSpan(); - TraceContextOrSamplingFlags extracted = B3SingleFormat.parseB3SingleFormat(b3.asCharBuffer()); + TraceContextOrSamplingFlags extracted = B3SingleFormat.parseB3SingleFormat(UTF_8.decode(b3)); if (extracted == null) return tracer.nextSpan(); return tracer.nextSpan(extracted); } diff --git a/cassandra/src/test/java/brave/cassandra/ITTracing.java b/cassandra/src/test/java/brave/cassandra/ITTracing.java index d8e3ce7..e35b51e 100644 --- a/cassandra/src/test/java/brave/cassandra/ITTracing.java +++ b/cassandra/src/test/java/brave/cassandra/ITTracing.java @@ -17,6 +17,7 @@ package brave.cassandra; import brave.ScopedSpan; +import brave.cassandra.driver.CassandraClientTracing; import brave.cassandra.driver.TracingSession; import brave.propagation.StrictScopeDecorator; import brave.propagation.ThreadLocalCurrentTraceContext; @@ -153,11 +154,13 @@ public class ITTracing { } void executeTraced(Function statement) { + CassandraClientTracing withPropagation = CassandraClientTracing.newBuilder(tracing) + .propagationEnabled(true).build(); try (Cluster cluster = Cluster.builder() .addContactPointsWithPorts(Collections.singleton(cassandra.contactPoint())) .build(); - Session session = TracingSession.create(tracing, cluster.connect())) { + Session session = TracingSession.create(withPropagation, cluster.connect())) { session.execute(statement.apply(session)); } } diff --git a/cassandra/src/test/java/brave/cassandra/TracingTest.java b/cassandra/src/test/java/brave/cassandra/TracingTest.java new file mode 100644 index 0000000..b6eb216 --- /dev/null +++ b/cassandra/src/test/java/brave/cassandra/TracingTest.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package brave.cassandra; + +import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.junit.After; +import org.junit.Test; +import zipkin2.Span; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TracingTest { + List spans = new ArrayList<>(); + brave.Tracing tracing = brave.Tracing.newBuilder() + .spanReporter(spans::add) + .build(); + Tracing cassandraTracing = new Tracing(tracing); + + @After public void tearDown() { + tracing.close(); + } + + @Test public void spanFromPayload_startsTraceOnNullPayload() { + assertThat(cassandraTracing.spanFromPayload(tracing.tracer(), null)) + .isNotNull(); + } + + @Test public void spanFromPayload_startsTraceOnAbsentB3SingleEntry() { + assertThat(cassandraTracing.spanFromPayload(tracing.tracer(), Collections.emptyMap())) + .isNotNull(); + } + + @Test public void spanFromPayload_resumesTraceOnB3SingleEntry() { + assertThat(cassandraTracing.spanFromPayload(tracing.tracer(), Collections.singletonMap("b3", + ByteBuffer.wrap(new byte[] {'0'})))) + .extracting(b -> b.isNoop()) + .isEqualTo(Boolean.TRUE); + } +} diff --git a/pom.xml b/pom.xml index 0b4493a..e9a3ad5 100644 --- a/pom.xml +++ b/pom.xml @@ -48,6 +48,8 @@ ${project.basedir} 5.6.3 + 2.11.2 + 2.3.3 @@ -145,6 +147,12 @@ + + + com.github.jbellis + jamm + 0.3.3 + com.datastax.cassandra cassandra-driver-core @@ -160,6 +168,22 @@ assertj-core 3.12.1 + + org.apache.logging.log4j + log4j-core + ${log4j.version} + + + org.apache.logging.log4j + log4j-slf4j-impl + ${log4j.version} + + + org.slf4j + slf4j-api + + + @@ -179,9 +203,13 @@ assertj-core test + + org.apache.logging.log4j + log4j-slf4j-impl + test + - @@ -251,6 +279,20 @@ maven-failsafe-plugin ${maven-failsafe-plugin.version} + + + integration-test + + integration-test + + + + verify + + verify + + +