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/12 11:11:40 UTC
[incubator-zipkin-brave] branch lazy-customizer created (now
32d2c83)
This is an automated email from the ASF dual-hosted git repository.
adriancole pushed a change to branch lazy-customizer
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git.
at 32d2c83 Makes Tracer.currentSpanCustomizer() lazy
This branch includes the following new commits:
new 32d2c83 Makes Tracer.currentSpanCustomizer() lazy
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: Makes
Tracer.currentSpanCustomizer() lazy
Posted by ad...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
adriancole pushed a commit to branch lazy-customizer
in repository https://gitbox.apache.org/repos/asf/incubator-zipkin-brave.git
commit 32d2c835d7a439c3767fd1bed6acde4d6ad360f1
Author: Adrian Cole <ac...@pivotal.io>
AuthorDate: Wed Jun 12 19:07:17 2019 +0800
Makes Tracer.currentSpanCustomizer() lazy
@lambcode noticed that calling `Tracer.currentSpan()` without using the
result can lead to orphaned spans. The simplest way to prevent this is
to defer overhead until data is added with the result.
Fixes #920
---
brave/src/main/java/brave/LazySpan.java | 126 +++++++++++++++++++++
brave/src/main/java/brave/RealSpan.java | 4 +-
...anCustomizer.java => SpanCustomizerShield.java} | 32 ++----
brave/src/main/java/brave/Tracer.java | 17 ++-
brave/src/test/java/brave/RealSpanTest.java | 2 +-
brave/src/test/java/brave/TracerTest.java | 2 +-
.../src/main/java/brave/TracerBenchmarks.java | 24 ++++
7 files changed, 175 insertions(+), 32 deletions(-)
diff --git a/brave/src/main/java/brave/LazySpan.java b/brave/src/main/java/brave/LazySpan.java
new file mode 100644
index 0000000..13744cc
--- /dev/null
+++ b/brave/src/main/java/brave/LazySpan.java
@@ -0,0 +1,126 @@
+/*
+ * 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;
+
+import brave.propagation.TraceContext;
+
+/** This wraps the public api and guards access to a mutable span. */
+final class LazySpan extends Span {
+
+ final Tracer tracer;
+ final TraceContext context;
+ volatile Span delegate;
+
+ LazySpan(Tracer tracer, TraceContext context) {
+ this.tracer = tracer;
+ this.context = context;
+ }
+
+ @Override public boolean isNoop() {
+ return false;
+ }
+
+ @Override public TraceContext context() {
+ return context;
+ }
+
+ @Override public SpanCustomizer customizer() {
+ return new SpanCustomizerShield(this);
+ }
+
+ @Override public Span start() {
+ return span().start();
+ }
+
+ @Override public Span start(long timestamp) {
+ return span().start(timestamp);
+ }
+
+ @Override public Span name(String name) {
+ return span().name(name);
+ }
+
+ @Override public Span kind(Kind kind) {
+ return span().kind(kind);
+ }
+
+ @Override public Span annotate(String value) {
+ return span().annotate(value);
+ }
+
+ @Override public Span annotate(long timestamp, String value) {
+ return span().annotate(timestamp, value);
+ }
+
+ @Override public Span tag(String key, String value) {
+ return span().tag(key, value);
+ }
+
+ @Override public Span error(Throwable throwable) {
+ return span().error(throwable);
+ }
+
+ @Override public Span remoteServiceName(String remoteServiceName) {
+ return span().remoteServiceName(remoteServiceName);
+ }
+
+ @Override public boolean remoteIpAndPort(String remoteIp, int remotePort) {
+ return span().remoteIpAndPort(remoteIp, remotePort);
+ }
+
+ @Override public void finish() {
+ span().finish();
+ }
+
+ @Override public void finish(long timestamp) {
+ span().finish(timestamp);
+ }
+
+ @Override public void abandon() {
+ if (delegate == null) return; // prevent resurrection
+ span().abandon();
+ }
+
+ @Override public void flush() {
+ if (delegate == null) return; // prevent resurrection
+ span().flush();
+ }
+
+ @Override public String toString() {
+ return "LazySpan(" + context + ")";
+ }
+
+ @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;
+ }
+
+ Span span() {
+ Span result = delegate;
+ if (result != null) return result;
+ return delegate = tracer.toSpan(context);
+ }
+
+ @Override public int hashCode() {
+ return context.hashCode();
+ }
+}
diff --git a/brave/src/main/java/brave/RealSpan.java b/brave/src/main/java/brave/RealSpan.java
index 0a900e4..99cbe00 100644
--- a/brave/src/main/java/brave/RealSpan.java
+++ b/brave/src/main/java/brave/RealSpan.java
@@ -29,7 +29,6 @@ final class RealSpan extends Span {
final MutableSpan state;
final Clock clock;
final FinishedSpanHandler finishedSpanHandler;
- final RealSpanCustomizer customizer;
RealSpan(TraceContext context,
PendingSpans pendingSpans,
@@ -41,7 +40,6 @@ final class RealSpan extends Span {
this.pendingSpans = pendingSpans;
this.state = state;
this.clock = clock;
- this.customizer = new RealSpanCustomizer(context, state, clock);
this.finishedSpanHandler = finishedSpanHandler;
}
@@ -54,7 +52,7 @@ final class RealSpan extends Span {
}
@Override public SpanCustomizer customizer() {
- return new RealSpanCustomizer(context, state, clock);
+ return new SpanCustomizerShield(this);
}
@Override public Span start() {
diff --git a/brave/src/main/java/brave/RealSpanCustomizer.java b/brave/src/main/java/brave/SpanCustomizerShield.java
similarity index 60%
rename from brave/src/main/java/brave/RealSpanCustomizer.java
rename to brave/src/main/java/brave/SpanCustomizerShield.java
index 0cb00a5..1a51777 100644
--- a/brave/src/main/java/brave/RealSpanCustomizer.java
+++ b/brave/src/main/java/brave/SpanCustomizerShield.java
@@ -16,46 +16,32 @@
*/
package brave;
-import brave.handler.MutableSpan;
-import brave.propagation.TraceContext;
+/** This reduces exposure of methods on {@link Span} to those exposed on {@link SpanCustomizer}. */
+final class SpanCustomizerShield implements SpanCustomizer {
-/** This wraps the public api and guards access to a mutable span. */
-final class RealSpanCustomizer implements SpanCustomizer {
+ final Span delegate;
- final TraceContext context;
- final MutableSpan state;
- final Clock clock;
-
- RealSpanCustomizer(TraceContext context, MutableSpan state, Clock clock) {
- this.context = context;
- this.state = state;
- this.clock = clock;
+ SpanCustomizerShield(Span delegate) {
+ this.delegate = delegate;
}
@Override public SpanCustomizer name(String name) {
- synchronized (state) {
- state.name(name);
- }
+ delegate.name(name);
return this;
}
@Override public SpanCustomizer annotate(String value) {
- long timestamp = clock.currentTimeMicroseconds();
- synchronized (state) {
- state.annotate(timestamp, value);
- }
+ delegate.annotate(value);
return this;
}
@Override public SpanCustomizer tag(String key, String value) {
- synchronized (state) {
- state.tag(key, value);
- }
+ delegate.tag(key, value);
return this;
}
@Override
public String toString() {
- return "RealSpanCustomizer(" + context + ")";
+ return "SpanCustomizer(" + delegate.customizer() + ")";
}
}
diff --git a/brave/src/main/java/brave/Tracer.java b/brave/src/main/java/brave/Tracer.java
index 05f5ec2..d722e96 100644
--- a/brave/src/main/java/brave/Tracer.java
+++ b/brave/src/main/java/brave/Tracer.java
@@ -348,6 +348,10 @@ public class Tracer {
/** Converts the context to a Span object after decorating it for propagation */
public Span toSpan(TraceContext context) {
+ return _toSpan(decorateExternal(context));
+ }
+
+ TraceContext decorateExternal(TraceContext context) {
if (context == null) throw new NullPointerException("context == null");
if (alwaysSampleLocal) {
int flags = InternalPropagation.instance.flags(context);
@@ -356,7 +360,7 @@ public class Tracer {
}
}
// decorating here addresses join, new traces or children and ad-hoc trace contexts
- return _toSpan(propagationFactory.decorate(context));
+ return propagationFactory.decorate(context);
}
Span _toSpan(TraceContext decorated) {
@@ -426,8 +430,7 @@ public class Tracer {
// note: we don't need to decorate the context for propagation as it is only used for toString
TraceContext context = currentTraceContext.get();
if (context == null || isNoop(context)) return NoopSpanCustomizer.INSTANCE;
- PendingSpan pendingSpan = pendingSpans.getOrCreate(context, false);
- return new RealSpanCustomizer(context, pendingSpan.state(), pendingSpan.clock());
+ return new SpanCustomizerShield(toSpan(context));
}
/**
@@ -438,7 +441,13 @@ public class Tracer {
*/
@Nullable public Span currentSpan() {
TraceContext currentContext = currentTraceContext.get();
- return currentContext != null ? toSpan(currentContext) : null;
+ if (currentContext == null) return null;
+ TraceContext decorated = decorateExternal(currentContext);
+ if (isNoop(decorated)) return new NoopSpan(decorated);
+
+ // Returns a lazy span to reduce overhead when tracer.currentSpan() is invoked just to see if
+ // one exists, or when the result is never used.
+ return new LazySpan(this, decorated);
}
/**
diff --git a/brave/src/test/java/brave/RealSpanTest.java b/brave/src/test/java/brave/RealSpanTest.java
index 5ffd55b..6040b00 100644
--- a/brave/src/test/java/brave/RealSpanTest.java
+++ b/brave/src/test/java/brave/RealSpanTest.java
@@ -48,7 +48,7 @@ public class RealSpanTest {
}
@Test public void hasRealCustomizer() {
- assertThat(span.customizer()).isInstanceOf(RealSpanCustomizer.class);
+ assertThat(span.customizer()).isInstanceOf(SpanCustomizerShield.class);
}
@Test public void start() {
diff --git a/brave/src/test/java/brave/TracerTest.java b/brave/src/test/java/brave/TracerTest.java
index 353f275..c204228 100644
--- a/brave/src/test/java/brave/TracerTest.java
+++ b/brave/src/test/java/brave/TracerTest.java
@@ -396,7 +396,7 @@ public class TracerTest {
try {
assertThat(tracer.currentSpanCustomizer())
- .isInstanceOf(RealSpanCustomizer.class);
+ .isInstanceOf(SpanCustomizerShield.class);
} finally {
parent.finish();
}
diff --git a/instrumentation/benchmarks/src/main/java/brave/TracerBenchmarks.java b/instrumentation/benchmarks/src/main/java/brave/TracerBenchmarks.java
index d0619d5..2ea962e 100644
--- a/instrumentation/benchmarks/src/main/java/brave/TracerBenchmarks.java
+++ b/instrumentation/benchmarks/src/main/java/brave/TracerBenchmarks.java
@@ -19,6 +19,7 @@ package brave;
import brave.handler.FinishedSpanHandler;
import brave.handler.MutableSpan;
import brave.propagation.B3Propagation;
+import brave.propagation.CurrentTraceContext;
import brave.propagation.ExtraFieldPropagation;
import brave.propagation.Propagation;
import brave.propagation.SamplingFlags;
@@ -226,6 +227,29 @@ public class TracerBenchmarks {
}
}
+ @Benchmark public void currentSpan() {
+ currentSpan(tracer, extracted.context(), false);
+ }
+
+ @Benchmark public void currentSpan_unsampled() {
+ currentSpan(tracer, unsampledExtracted.context(), false);
+ }
+
+ @Benchmark public void currentSpan_tag() {
+ currentSpan(tracer, extracted.context(), true);
+ }
+
+ @Benchmark public void currentSpan_tag_unsampled() {
+ currentSpan(tracer, unsampledExtracted.context(), true);
+ }
+
+ void currentSpan(Tracer tracer, TraceContext context, boolean tag) {
+ try (CurrentTraceContext.Scope scope = tracer.currentTraceContext.newScope(context)) {
+ Span span = tracer.currentSpan();
+ if (tag) span.tag("customer.id", "1234");
+ }
+ }
+
// Convenience main entry-point
public static void main(String[] args) throws Exception {
Options opt = new OptionsBuilder()