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/06/15 13:23:29 UTC
[incubator-zipkin-brave] branch mirror-equals created (now d6f6c49)
This is an automated email from the ASF dual-hosted git repository.
adriancole pushed a change to branch mirror-equals
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git.
at d6f6c49 Fixes assertion by making equals the same between lazy and real span
This branch includes the following new commits:
new d6f6c49 Fixes assertion by making equals the same between lazy and real span
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-brave] 01/01: Fixes assertion by making equals
the same between lazy and real span
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adriancole pushed a commit to branch mirror-equals
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git
commit d6f6c49ff0a84765d407a0fb28a643d347bb8f1c
Author: Adrian Cole <ac...@pivotal.io>
AuthorDate: Sat Jun 15 21:22:08 2019 +0800
Fixes assertion by making equals the same between lazy and real span
We had an assertion in `ThreadLocalSpan` which broke because our equals
comparison was one way. This fixes it.
---
brave/src/main/java/brave/LazySpan.java | 9 +--
brave/src/main/java/brave/RealSpan.java | 24 +++++--
brave/src/test/java/brave/LazySpanTest.java | 82 ++++++++++++++++++++++
brave/src/test/java/brave/RealSpanTest.java | 73 ++++++++++++++-----
.../brave/propagation/ThreadLocalSpanTest.java | 48 +++++++++++++
5 files changed, 207 insertions(+), 29 deletions(-)
diff --git a/brave/src/main/java/brave/LazySpan.java b/brave/src/main/java/brave/LazySpan.java
index 90aae26..bbf6a45 100644
--- a/brave/src/main/java/brave/LazySpan.java
+++ b/brave/src/main/java/brave/LazySpan.java
@@ -16,6 +16,8 @@ package brave;
import brave.handler.MutableSpan;
import brave.propagation.TraceContext;
+import static brave.RealSpan.isEqualToRealOrLazySpan;
+
/**
* This defers creation of a span until first public method call.
*
@@ -112,12 +114,7 @@ final class LazySpan extends Span {
*/
@Override public boolean equals(Object o) {
if (o == this) return true;
- if (o instanceof LazySpan) {
- return context.equals(((LazySpan) o).context);
- } else if (o instanceof RealSpan) {
- return context.equals(((RealSpan) o).context);
- }
- return false;
+ return isEqualToRealOrLazySpan(context, o);
}
/**
diff --git a/brave/src/main/java/brave/RealSpan.java b/brave/src/main/java/brave/RealSpan.java
index 1764517..b827ac4 100644
--- a/brave/src/main/java/brave/RealSpan.java
+++ b/brave/src/main/java/brave/RealSpan.java
@@ -28,10 +28,10 @@ final class RealSpan extends Span {
final FinishedSpanHandler finishedSpanHandler;
RealSpan(TraceContext context,
- PendingSpans pendingSpans,
- MutableSpan state,
- Clock clock,
- FinishedSpanHandler finishedSpanHandler
+ PendingSpans pendingSpans,
+ MutableSpan state,
+ Clock clock,
+ FinishedSpanHandler finishedSpanHandler
) {
this.context = context;
this.pendingSpans = pendingSpans;
@@ -164,10 +164,22 @@ final class RealSpan extends Span {
return "RealSpan(" + context + ")";
}
+ /**
+ * This also matches equals against a lazy span. The rationale is least surprise to the user, as
+ * code should not act differently given an instance of lazy or {@link RealSpan}.
+ */
@Override public boolean equals(Object o) {
if (o == this) return true;
- if (!(o instanceof RealSpan)) return false;
- return context.equals(((RealSpan) o).context);
+ return isEqualToRealOrLazySpan(context, o);
+ }
+
+ static boolean isEqualToRealOrLazySpan(TraceContext context, Object o) {
+ if (o instanceof LazySpan) {
+ return context.equals(((LazySpan) o).context);
+ } else if (o instanceof RealSpan) {
+ return context.equals(((RealSpan) o).context);
+ }
+ return false;
}
@Override public int hashCode() {
diff --git a/brave/src/test/java/brave/LazySpanTest.java b/brave/src/test/java/brave/LazySpanTest.java
new file mode 100644
index 0000000..ce41044
--- /dev/null
+++ b/brave/src/test/java/brave/LazySpanTest.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright 2013-2019 The OpenZipkin Authors
+ *
+ * Licensed 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;
+
+import brave.propagation.CurrentTraceContext.Scope;
+import brave.propagation.ThreadLocalCurrentTraceContext;
+import brave.propagation.TraceContext;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.After;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class LazySpanTest {
+ List<zipkin2.Span> spans = new ArrayList();
+ Tracing tracing = Tracing.newBuilder()
+ .currentTraceContext(ThreadLocalCurrentTraceContext.create())
+ .spanReporter(spans::add)
+ .build();
+
+ TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(1L).sampled(true).build();
+ TraceContext context2 = TraceContext.newBuilder().traceId(1L).spanId(2L).sampled(true).build();
+
+ @After public void close() {
+ tracing.close();
+ }
+
+ @Test public void equals_sameContext() {
+ Span current1, current2;
+ try (Scope ws = tracing.currentTraceContext().newScope(context)) {
+ current1 = tracing.tracer().currentSpan();
+ current2 = tracing.tracer().currentSpan();
+ }
+
+ assertThat(current1)
+ .isInstanceOf(LazySpan.class)
+ .isNotSameAs(current2)
+ .isEqualTo(current2);
+ }
+
+ @Test public void equals_notSameContext() {
+ Span current1, current2;
+ try (Scope ws = tracing.currentTraceContext().newScope(context)) {
+ current1 = tracing.tracer().currentSpan();
+ }
+ try (Scope ws = tracing.currentTraceContext().newScope(context2)) {
+ current2 = tracing.tracer().currentSpan();
+ }
+
+ assertThat(current1).isNotEqualTo(current2);
+ }
+
+ @Test public void equals_realSpan_sameContext() {
+ Span current;
+ try (Scope ws = tracing.currentTraceContext().newScope(context)) {
+ current = tracing.tracer().currentSpan();
+ }
+
+ assertThat(current).isEqualTo(tracing.tracer().toSpan(context));
+ }
+
+ @Test public void equals_realSpan_notSameContext() {
+ Span current;
+ try (Scope ws = tracing.currentTraceContext().newScope(context)) {
+ current = tracing.tracer().currentSpan();
+ }
+
+ assertThat(current).isNotEqualTo(tracing.tracer().toSpan(context2));
+ }
+}
diff --git a/brave/src/test/java/brave/RealSpanTest.java b/brave/src/test/java/brave/RealSpanTest.java
index 5d6d143..eb024e0 100644
--- a/brave/src/test/java/brave/RealSpanTest.java
+++ b/brave/src/test/java/brave/RealSpanTest.java
@@ -13,7 +13,9 @@
*/
package brave;
+import brave.propagation.CurrentTraceContext;
import brave.propagation.ThreadLocalCurrentTraceContext;
+import brave.propagation.TraceContext;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
@@ -27,9 +29,13 @@ import static org.assertj.core.api.Assertions.entry;
public class RealSpanTest {
List<zipkin2.Span> spans = new ArrayList();
Tracing tracing = Tracing.newBuilder()
- .currentTraceContext(ThreadLocalCurrentTraceContext.create())
- .spanReporter(spans::add)
- .build();
+ .currentTraceContext(ThreadLocalCurrentTraceContext.create())
+ .spanReporter(spans::add)
+ .build();
+
+ TraceContext context = TraceContext.newBuilder().traceId(1L).spanId(1L).sampled(true).build();
+ TraceContext context2 = TraceContext.newBuilder().traceId(1L).spanId(2L).sampled(true).build();
+
Span span = tracing.tracer().newTrace();
@After public void close() {
@@ -53,8 +59,8 @@ public class RealSpanTest {
span.flush();
assertThat(spans).hasSize(1).first()
- .extracting(zipkin2.Span::timestamp)
- .isNotNull();
+ .extracting(zipkin2.Span::timestamp)
+ .isNotNull();
}
@Test public void start_timestamp() {
@@ -62,8 +68,8 @@ public class RealSpanTest {
span.flush();
assertThat(spans).hasSize(1).first()
- .extracting(zipkin2.Span::timestamp)
- .isEqualTo(2L);
+ .extracting(zipkin2.Span::timestamp)
+ .isEqualTo(2L);
}
@Test public void finish() {
@@ -71,8 +77,8 @@ public class RealSpanTest {
span.finish();
assertThat(spans).hasSize(1).first()
- .extracting(zipkin2.Span::duration)
- .isNotNull();
+ .extracting(zipkin2.Span::duration)
+ .isNotNull();
}
@Test public void finish_timestamp() {
@@ -80,8 +86,8 @@ public class RealSpanTest {
span.finish(5);
assertThat(spans).hasSize(1).first()
- .extracting(zipkin2.Span::duration)
- .isEqualTo(3L);
+ .extracting(zipkin2.Span::duration)
+ .isEqualTo(3L);
}
@Test public void abandon() {
@@ -96,8 +102,8 @@ public class RealSpanTest {
span.flush();
assertThat(spans).flatExtracting(zipkin2.Span::annotations)
- .extracting(Annotation::value)
- .containsExactly("foo");
+ .extracting(Annotation::value)
+ .containsExactly("foo");
}
@Test public void remoteEndpoint_nulls() {
@@ -112,7 +118,7 @@ public class RealSpanTest {
span.flush();
assertThat(spans).flatExtracting(zipkin2.Span::annotations)
- .containsExactly(Annotation.create(2L, "foo"));
+ .containsExactly(Annotation.create(2L, "foo"));
}
@Test public void tag() {
@@ -120,7 +126,7 @@ public class RealSpanTest {
span.flush();
assertThat(spans).flatExtracting(s -> s.tags().entrySet())
- .containsExactly(entry("foo", "bar"));
+ .containsExactly(entry("foo", "bar"));
}
@Test public void finished_client_annotation() {
@@ -173,7 +179,7 @@ public class RealSpanTest {
span.flush();
assertThat(spans).flatExtracting(s -> s.tags().entrySet())
- .containsExactly(entry("error", "this cake is a lie"));
+ .containsExactly(entry("error", "this cake is a lie"));
}
@Test public void error_noMessage() {
@@ -181,6 +187,39 @@ public class RealSpanTest {
span.flush();
assertThat(spans).flatExtracting(s -> s.tags().entrySet())
- .containsExactly(entry("error", "RuntimeException"));
+ .containsOnly(entry("error", "RuntimeException"));
+ }
+
+ @Test public void equals_sameContext() {
+ Span one = tracing.tracer().toSpan(context), two = tracing.tracer().toSpan(context);
+
+ assertThat(one)
+ .isInstanceOf(RealSpan.class)
+ .isNotSameAs(two)
+ .isEqualTo(two);
+ }
+
+ @Test public void equals_notSameContext() {
+ Span one = tracing.tracer().toSpan(context), two = tracing.tracer().toSpan(context2);
+
+ assertThat(one).isNotEqualTo(two);
+ }
+
+ @Test public void equals_realSpan_sameContext() {
+ Span current;
+ try (CurrentTraceContext.Scope ws = tracing.currentTraceContext().newScope(context)) {
+ current = tracing.tracer().currentSpan();
+ }
+
+ assertThat(tracing.tracer().toSpan(context)).isEqualTo(current);
+ }
+
+ @Test public void equals_realSpan_notSameContext() {
+ Span current;
+ try (CurrentTraceContext.Scope ws = tracing.currentTraceContext().newScope(context2)) {
+ current = tracing.tracer().currentSpan();
+ }
+
+ assertThat(tracing.tracer().toSpan(context)).isNotEqualTo(current);
}
}
diff --git a/brave/src/test/java/brave/propagation/ThreadLocalSpanTest.java b/brave/src/test/java/brave/propagation/ThreadLocalSpanTest.java
new file mode 100644
index 0000000..ae9b58f
--- /dev/null
+++ b/brave/src/test/java/brave/propagation/ThreadLocalSpanTest.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2013-2019 The OpenZipkin Authors
+ *
+ * Licensed 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.propagation;
+
+import brave.Tracing;
+import java.util.ArrayList;
+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 ThreadLocalSpanTest {
+
+ List<Span> spans = new ArrayList<>();
+ Tracing tracing = Tracing.newBuilder()
+ .currentTraceContext(ThreadLocalCurrentTraceContext.create())
+ .spanReporter(spans::add)
+ .build();
+
+ ThreadLocalSpan threadLocalSpan = ThreadLocalSpan.create(tracing.tracer());
+
+ @After public void close() {
+ tracing.close();
+ }
+
+ @Test public void next() {
+ assertThat(threadLocalSpan.next())
+ .isEqualTo(threadLocalSpan.remove());
+ }
+
+ @Test public void next_extracted() {
+ assertThat(threadLocalSpan.next(TraceContextOrSamplingFlags.DEBUG))
+ .isEqualTo(threadLocalSpan.remove());
+ }
+}