diff options
15 files changed, 155 insertions, 117 deletions
diff --git a/api/src/main/java/io/opencensus/trace/SpanBuilder.java b/api/src/main/java/io/opencensus/trace/SpanBuilder.java index f573d9a6..be55906f 100644 --- a/api/src/main/java/io/opencensus/trace/SpanBuilder.java +++ b/api/src/main/java/io/opencensus/trace/SpanBuilder.java @@ -86,14 +86,14 @@ import javax.annotation.Nullable; * <pre>{@code * class MyClass { * private static final Tracer tracer = Tracing.getTracer(); - * void DoWork() { - * Span span = tracer.spanBuilder(null, "MyRootSpan").startSpan(); - * span.addAnnotation("my annotation"); + * void DoWork(Span parent) { + * Span childSpan = tracer.spanBuilderWithExplicitParent("MyChildSpan", parent).startSpan(); + * childSpan.addAnnotation("my annotation"); * try { - * doSomeWork(span); // Manually propagate the new span down the stack. + * doSomeWork(childSpan); // Manually propagate the new span down the stack. * } finally { * // To make sure we end the span even in case of an exception. - * span.end(); // Manually end the span. + * childSpan.end(); // Manually end the span. * } * } * } @@ -145,14 +145,14 @@ public abstract class SpanBuilder { * <pre>{@code * class MyClass { * private static final Tracer tracer = Tracing.getTracer(); - * void DoWork() { - * Span span = tracer.spanBuilder(null, "MyRootSpan").startSpan(); - * span.addAnnotation("my annotation"); + * void DoWork(Span parent) { + * Span childSpan = tracer.spanBuilderWithExplicitParent("MyChildSpan", parent).startSpan(); + * childSpan.addAnnotation("my annotation"); * try { - * doSomeWork(span); // Manually propagate the new span down the stack. + * doSomeWork(childSpan); // Manually propagate the new span down the stack. * } finally { * // To make sure we end the span even in case of an exception. - * span.end(); // Manually end the span. + * childSpan.end(); // Manually end the span. * } * } * } @@ -222,13 +222,13 @@ public abstract class SpanBuilder { } static final class NoopSpanBuilder extends SpanBuilder { - NoopSpanBuilder(@Nullable Span parentSpan, String name) { - checkNotNull(name, "name"); + static NoopSpanBuilder createWithParent(String spanName, @Nullable Span parent) { + return new NoopSpanBuilder(spanName); } - NoopSpanBuilder(SpanContext remoteParentSpanContext, String name) { - checkNotNull(remoteParentSpanContext, "remoteParentSpanContext"); - checkNotNull(name, "name"); + static NoopSpanBuilder createWithRemoteParent( + String spanName, @Nullable SpanContext remoteParentSpanContext) { + return new NoopSpanBuilder(spanName); } @Override @@ -250,5 +250,9 @@ public abstract class SpanBuilder { public SpanBuilder setRecordEvents(boolean recordEvents) { return this; } + + private NoopSpanBuilder(String name) { + checkNotNull(name, "name"); + } } } diff --git a/api/src/main/java/io/opencensus/trace/Tracer.java b/api/src/main/java/io/opencensus/trace/Tracer.java index ef5616a9..97cd0998 100644 --- a/api/src/main/java/io/opencensus/trace/Tracer.java +++ b/api/src/main/java/io/opencensus/trace/Tracer.java @@ -16,6 +16,7 @@ package io.opencensus.trace; import static com.google.common.base.Preconditions.checkNotNull; import io.opencensus.common.NonThrowingCloseable; +import io.opencensus.trace.SpanBuilder.NoopSpanBuilder; import javax.annotation.Nullable; /** @@ -49,15 +50,14 @@ import javax.annotation.Nullable; * <pre>{@code * class MyClass { * private static final Tracer tracer = Tracing.getTracer(); - * void doWork() { - * Span span = tracer.spanBuilder(null, "MyRootSpan").startSpan(); - * span.addAnnotation("Starting the work."); + * void doWork(Span parent) { + * Span childSpan = tracer.spanBuilderWithExplicitParent("MyChildSpan", parent).startSpan(); + * childSpan.addAnnotation("Starting the work."); * try { - * doSomeWork(span); // Manually propagate the new span down the stack. + * doSomeWork(childSpan); // Manually propagate the new span down the stack. * } finally { - * span.addAnnotation("Finished working."); * // To make sure we end the span even in case of an exception. - * span.end(); // Manually end the span. + * childSpan.end(); // Manually end the span. * } * } * } @@ -139,7 +139,7 @@ public abstract class Tracer { * @param span The {@link Span} to be set to the current Context. * @return an object that defines a scope where the given {@link Span} will be set to the current * Context. - * @throws NullPointerException if {@code span} is null. + * @throws NullPointerException if {@code span} is {@code null}. */ public final NonThrowingCloseable withSpan(Span span) { return ContextUtils.withSpan(checkNotNull(span, "span")); @@ -154,12 +154,18 @@ public abstract class Tracer { * <p>This <b>must</b> be used to create a {@code Span} when automatic Context propagation is * used. * - * @param name The name of the returned Span. + * <p>This is equivalent with: + * + * <pre>{@code + * tracer.spanBuilderWithExplicitParent("MySpanName",tracer.getCurrentSpan()); + * }</pre> + * + * @param spanName The name of the returned Span. * @return a {@code SpanBuilder} to create and start a new {@code Span}. - * @throws NullPointerException if {@code name} is null. + * @throws NullPointerException if {@code spanName} is {@code null}. */ - public final SpanBuilder spanBuilder(String name) { - return spanBuilder(ContextUtils.getCurrentSpan(), name); + public final SpanBuilder spanBuilder(String spanName) { + return spanBuilderWithExplicitParent(spanName, ContextUtils.getCurrentSpan()); } /** @@ -169,51 +175,50 @@ public abstract class Tracer { * * <p>See {@link SpanBuilder} for usage examples. * - * <p>This <b>must</b> be used to create a {@code Span} when manual Context propagation is used - * OR when creating a root {@code Span} with a {@code null} parent. + * <p>This <b>must</b> be used to create a {@code Span} when manual Context propagation is used OR + * when creating a root {@code Span} with a {@code null} parent. * - * @param parent The parent of the returned Span. If null the {@code SpanBuilder} will build a - * root {@code Span}. - * @param name The name of the returned Span. + * @param spanName The name of the returned Span. + * @param parent The parent of the returned Span. If {@code null} the {@code SpanBuilder} will + * build a root {@code Span}. * @return a {@code SpanBuilder} to create and start a new {@code Span}. - * @throws NullPointerException if {@code name} is null. + * @throws NullPointerException if {@code spanName} is {@code null}. */ - public abstract SpanBuilder spanBuilder(@Nullable Span parent, String name); + public abstract SpanBuilder spanBuilderWithExplicitParent(String spanName, @Nullable Span parent); /** * Returns a {@link SpanBuilder} to create and start a new child {@link Span} (or root if parent - * is an invalid {@link SpanContext}), with parent being the {@link Span} designated by the {@link - * SpanContext}. + * is {@link SpanContext#INVALID} or {@code null}), with parent being the remote {@link Span} + * designated by the {@link SpanContext}. * * <p>See {@link SpanBuilder} for usage examples. * * <p>This <b>must</b> be used to create a {@code Span} when the parent is in a different process. * This is only intended for use by RPC systems or similar. * - * <p>If no {@link SpanContext} OR fail to parse the {@link SpanContext} on the server side, - * users must call this method with an {@link SpanContext#INVALID} remote parent {@code - * SpanContext}. + * <p>If no {@link SpanContext} OR fail to parse the {@link SpanContext} on the server side, users + * must call this method with a {@code null} remote parent {@code SpanContext}. * + * @param spanName The name of the returned Span. * @param remoteParentSpanContext The remote parent of the returned Span. - * @param name The name of the returned Span. * @return a {@code SpanBuilder} to create and start a new {@code Span}. - * @throws NullPointerException if {@code name} or {@code remoteParentSpanContext} are null. + * @throws NullPointerException if {@code spanName} is {@code null}. */ public abstract SpanBuilder spanBuilderWithRemoteParent( - SpanContext remoteParentSpanContext, String name); + String spanName, @Nullable SpanContext remoteParentSpanContext); // No-Op implementation of the Tracer. private static final class NoopTracer extends Tracer { @Override - public SpanBuilder spanBuilder(@Nullable Span parent, String name) { - return new SpanBuilder.NoopSpanBuilder(parent, name); + public SpanBuilder spanBuilderWithExplicitParent(String spanName, @Nullable Span parent) { + return NoopSpanBuilder.createWithParent(spanName, parent); } @Override public SpanBuilder spanBuilderWithRemoteParent( - SpanContext remoteParentSpanContext, String name) { - return new SpanBuilder.NoopSpanBuilder(remoteParentSpanContext, name); + String spanName, @Nullable SpanContext remoteParentSpanContext) { + return NoopSpanBuilder.createWithRemoteParent(spanName, remoteParentSpanContext); } private NoopTracer() {} diff --git a/api/src/main/java/io/opencensus/trace/propagation/BinaryFormat.java b/api/src/main/java/io/opencensus/trace/propagation/BinaryFormat.java index d34f69f9..284576ec 100644 --- a/api/src/main/java/io/opencensus/trace/propagation/BinaryFormat.java +++ b/api/src/main/java/io/opencensus/trace/propagation/BinaryFormat.java @@ -52,7 +52,7 @@ import java.text.ParseException; * // Maybe log the exception. * } * try (NonThrowingCloseable ss = - * tracer.spanBuilderWithRemoteParent(spanContext, "Recv.MyRequest").startScopedSpan()) { + * tracer.spanBuilderWithRemoteParent("Recv.MyRequest", spanContext).startScopedSpan()) { * // Handle request and send response back. * } * } diff --git a/api/src/test/java/io/opencensus/trace/TracerTest.java b/api/src/test/java/io/opencensus/trace/TracerTest.java index 3c69068b..a0830428 100644 --- a/api/src/test/java/io/opencensus/trace/TracerTest.java +++ b/api/src/test/java/io/opencensus/trace/TracerTest.java @@ -101,12 +101,13 @@ public class TracerTest { @Test(expected = NullPointerException.class) public void spanBuilderWithParentAndName_NullName() { - noopTracer.spanBuilder(null, null); + noopTracer.spanBuilderWithExplicitParent(null, null); } @Test public void defaultSpanBuilderWithParentAndName() { - assertThat(noopTracer.spanBuilder(null, SPAN_NAME).startSpan()).isSameAs(BlankSpan.INSTANCE); + assertThat(noopTracer.spanBuilderWithExplicitParent(SPAN_NAME, null).startSpan()) + .isSameAs(BlankSpan.INSTANCE); } @Test(expected = NullPointerException.class) @@ -114,14 +115,15 @@ public class TracerTest { noopTracer.spanBuilderWithRemoteParent(null, null); } - @Test(expected = NullPointerException.class) - public void defaultSpanBuilderWitRemoteParent_NullParent() { - noopTracer.spanBuilderWithRemoteParent(null, SPAN_NAME); + @Test + public void defaultSpanBuilderWithRemoteParent_NullParent() { + assertThat(noopTracer.spanBuilderWithRemoteParent(SPAN_NAME, null).startSpan()) + .isSameAs(BlankSpan.INSTANCE); } @Test public void defaultSpanBuilderWithRemoteParent() { - assertThat(noopTracer.spanBuilderWithRemoteParent(SpanContext.INVALID, SPAN_NAME).startSpan()) + assertThat(noopTracer.spanBuilderWithRemoteParent(SPAN_NAME, SpanContext.INVALID).startSpan()) .isSameAs(BlankSpan.INSTANCE); } @@ -130,7 +132,8 @@ public class TracerTest { NonThrowingCloseable ws = tracer.withSpan(span); try { assertThat(tracer.getCurrentSpan()).isSameAs(span); - when(tracer.spanBuilder(same(span), same(SPAN_NAME))).thenReturn(spanBuilder); + when(tracer.spanBuilderWithExplicitParent(same(SPAN_NAME), same(span))) + .thenReturn(spanBuilder); assertThat(tracer.spanBuilder(SPAN_NAME)).isSameAs(spanBuilder); } finally { ws.close(); @@ -142,7 +145,8 @@ public class TracerTest { NonThrowingCloseable ws = tracer.withSpan(BlankSpan.INSTANCE); try { assertThat(tracer.getCurrentSpan()).isSameAs(BlankSpan.INSTANCE); - when(tracer.spanBuilder(same(BlankSpan.INSTANCE), same(SPAN_NAME))).thenReturn(spanBuilder); + when(tracer.spanBuilderWithExplicitParent(same(SPAN_NAME), same(BlankSpan.INSTANCE))) + .thenReturn(spanBuilder); assertThat(tracer.spanBuilder(SPAN_NAME)).isSameAs(spanBuilder); } finally { ws.close(); diff --git a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java index 06a1412f..ebc30072 100644 --- a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java +++ b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsNonSampledSpanBenchmark.java @@ -36,9 +36,15 @@ public class RecordTraceEventsNonSampledSpanBenchmark { private static final String ATTRIBUTE_KEY = "MyAttributeKey"; private static final String ATTRIBUTE_VALUE = "MyAttributeValue"; private Span linkedSpan = - tracer.spanBuilder(null, SPAN_NAME).setSampler(Samplers.neverSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, null) + .setSampler(Samplers.neverSample()) + .startSpan(); private Span span = - tracer.spanBuilder(null, SPAN_NAME).setSampler(Samplers.neverSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, null) + .setSampler(Samplers.neverSample()) + .startSpan(); /** TearDown method. */ @TearDown diff --git a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java index 5773923f..6a80a26d 100644 --- a/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java +++ b/benchmarks/src/jmh/java/io/opencensus/trace/RecordTraceEventsSampledSpanBenchmark.java @@ -36,9 +36,15 @@ public class RecordTraceEventsSampledSpanBenchmark { private static final String ATTRIBUTE_KEY = "MyAttributeKey"; private static final String ATTRIBUTE_VALUE = "MyAttributeValue"; private Span linkedSpan = - tracer.spanBuilder(null, SPAN_NAME).setSampler(Samplers.alwaysSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, null) + .setSampler(Samplers.alwaysSample()) + .startSpan(); private Span span = - tracer.spanBuilder(null, SPAN_NAME).setSampler(Samplers.alwaysSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, null) + .setSampler(Samplers.alwaysSample()) + .startSpan(); /** TearDown method. */ @TearDown diff --git a/benchmarks/src/jmh/java/io/opencensus/trace/StartEndSpanBenchmark.java b/benchmarks/src/jmh/java/io/opencensus/trace/StartEndSpanBenchmark.java index 8418184f..6827c097 100644 --- a/benchmarks/src/jmh/java/io/opencensus/trace/StartEndSpanBenchmark.java +++ b/benchmarks/src/jmh/java/io/opencensus/trace/StartEndSpanBenchmark.java @@ -29,7 +29,10 @@ public class StartEndSpanBenchmark { private static final Tracer tracer = Tracing.getTracer(); private static final String SPAN_NAME = "MySpanName"; private Span rootSpan = - tracer.spanBuilder(null, SPAN_NAME).setSampler(Samplers.neverSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, null) + .setSampler(Samplers.neverSample()) + .startSpan(); @TearDown public void doTearDown() { @@ -45,7 +48,10 @@ public class StartEndSpanBenchmark { @OutputTimeUnit(TimeUnit.NANOSECONDS) public Span startEndNonSampledRootSpan() { Span span = - tracer.spanBuilder(null, SPAN_NAME).setSampler(Samplers.neverSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, null) + .setSampler(Samplers.neverSample()) + .startSpan(); span.end(); return span; } @@ -60,7 +66,7 @@ public class StartEndSpanBenchmark { public Span startEndRecordEventsRootSpan() { Span span = tracer - .spanBuilder(null, SPAN_NAME) + .spanBuilderWithExplicitParent(SPAN_NAME, null) .setSampler(Samplers.neverSample()) .setRecordEvents(true) .startSpan(); @@ -89,7 +95,10 @@ public class StartEndSpanBenchmark { @OutputTimeUnit(TimeUnit.NANOSECONDS) public Span startEndNonSampledChildSpan() { Span span = - tracer.spanBuilder(rootSpan, SPAN_NAME).setSampler(Samplers.neverSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, rootSpan) + .setSampler(Samplers.neverSample()) + .startSpan(); span.end(); return span; } @@ -104,7 +113,7 @@ public class StartEndSpanBenchmark { public Span startEndRecordEventsChildSpan() { Span span = tracer - .spanBuilder(rootSpan, SPAN_NAME) + .spanBuilderWithExplicitParent(SPAN_NAME, rootSpan) .setSampler(Samplers.neverSample()) .setRecordEvents(true) .startSpan(); @@ -120,7 +129,10 @@ public class StartEndSpanBenchmark { @OutputTimeUnit(TimeUnit.NANOSECONDS) public Span startEndSampledChildSpan() { Span span = - tracer.spanBuilder(rootSpan, SPAN_NAME).setSampler(Samplers.alwaysSample()).startSpan(); + tracer + .spanBuilderWithExplicitParent(SPAN_NAME, rootSpan) + .setSampler(Samplers.alwaysSample()) + .startSpan(); span.end(); return span; } diff --git a/benchmarks/src/jmh/java/io/opencensus/trace/propagation/BinaryPropagationImplBenchmark.java b/benchmarks/src/jmh/java/io/opencensus/trace/propagation/BinaryPropagationImplBenchmark.java index 4a49b9c8..d99f0dbd 100644 --- a/benchmarks/src/jmh/java/io/opencensus/trace/propagation/BinaryPropagationImplBenchmark.java +++ b/benchmarks/src/jmh/java/io/opencensus/trace/propagation/BinaryPropagationImplBenchmark.java @@ -38,8 +38,7 @@ public class BinaryPropagationImplBenchmark { private static final TraceOptions traceOptions = TraceOptions.fromBytes(traceOptionsBytes); private static final SpanContext spanContext = SpanContext.create(traceId, spanId, traceOptions); private static final BinaryFormat BINARY_PROPAGATION = new BinaryFormatImpl(); - private static final byte[] spanContextBinary = - BINARY_PROPAGATION.toBinaryValue(spanContext); + private static final byte[] spanContextBinary = BINARY_PROPAGATION.toBinaryValue(spanContext); /** * This benchmark attempts to measure performance of {@link @@ -72,7 +71,6 @@ public class BinaryPropagationImplBenchmark { @BenchmarkMode(Mode.SampleTime) @OutputTimeUnit(TimeUnit.NANOSECONDS) public SpanContext toFromBinarySpanContext() throws ParseException { - return BINARY_PROPAGATION.fromBinaryValue( - BINARY_PROPAGATION.toBinaryValue(spanContext)); + return BINARY_PROPAGATION.fromBinaryValue(BINARY_PROPAGATION.toBinaryValue(spanContext)); } } diff --git a/examples/src/main/java/io/opencensus/examples/trace/MultiSpansContextTracing.java b/examples/src/main/java/io/opencensus/examples/trace/MultiSpansContextTracing.java index c6fa0317..b59ba01e 100644 --- a/examples/src/main/java/io/opencensus/examples/trace/MultiSpansContextTracing.java +++ b/examples/src/main/java/io/opencensus/examples/trace/MultiSpansContextTracing.java @@ -49,7 +49,7 @@ public final class MultiSpansContextTracing { /** Main method. */ public static void main(String[] args) { LoggingHandler.register(Tracing.getExportComponent().getSpanExporter()); - Span span = tracer.spanBuilder(null,"MyRootSpan").startSpan(); + Span span = tracer.spanBuilderWithExplicitParent("MyRootSpan", null).startSpan(); try (NonThrowingCloseable ws = tracer.withSpan(span)) { doWork(); } diff --git a/examples/src/main/java/io/opencensus/examples/trace/MultiSpansScopedTracing.java b/examples/src/main/java/io/opencensus/examples/trace/MultiSpansScopedTracing.java index 547403bb..c73c3cc2 100644 --- a/examples/src/main/java/io/opencensus/examples/trace/MultiSpansScopedTracing.java +++ b/examples/src/main/java/io/opencensus/examples/trace/MultiSpansScopedTracing.java @@ -48,7 +48,7 @@ public final class MultiSpansScopedTracing { public static void main(String[] args) { LoggingHandler.register(Tracing.getExportComponent().getSpanExporter()); try (NonThrowingCloseable ss = - tracer.spanBuilder(null,"MyRootSpan").startScopedSpan()) { + tracer.spanBuilderWithExplicitParent("MyRootSpan", null).startScopedSpan()) { doWork(); } } diff --git a/examples/src/main/java/io/opencensus/examples/trace/MultiSpansTracing.java b/examples/src/main/java/io/opencensus/examples/trace/MultiSpansTracing.java index 6b8f3ed6..25f71973 100644 --- a/examples/src/main/java/io/opencensus/examples/trace/MultiSpansTracing.java +++ b/examples/src/main/java/io/opencensus/examples/trace/MultiSpansTracing.java @@ -24,9 +24,9 @@ public final class MultiSpansTracing { private static final Tracer tracer = Tracing.getTracer(); private static void doWork() { - Span rootSpan = tracer.spanBuilder(null, "MyRootSpan").startSpan(); + Span rootSpan = tracer.spanBuilderWithExplicitParent("MyRootSpan", null).startSpan(); rootSpan.addAnnotation("Annotation to the root Span before child is created."); - Span childSpan = tracer.spanBuilder(rootSpan, "MyChildSpan").startSpan(); + Span childSpan = tracer.spanBuilderWithExplicitParent("MyChildSpan", rootSpan).startSpan(); childSpan.addAnnotation("Annotation to the child Span"); childSpan.end(); rootSpan.addAnnotation("Annotation to the root Span after child is ended."); diff --git a/impl_core/src/main/java/io/opencensus/trace/SpanBuilderImpl.java b/impl_core/src/main/java/io/opencensus/trace/SpanBuilderImpl.java index 74a4ae34..f9a0bf0c 100644 --- a/impl_core/src/main/java/io/opencensus/trace/SpanBuilderImpl.java +++ b/impl_core/src/main/java/io/opencensus/trace/SpanBuilderImpl.java @@ -37,13 +37,13 @@ final class SpanBuilderImpl extends SpanBuilder { private final Options options; private final String name; - private final Span parentSpan; + private final Span parent; private final SpanContext remoteParentSpanContext; private Sampler sampler; private List<Span> parentLinks = Collections.<Span>emptyList(); private Boolean recordEvents; - private Span startSpanInternal( + private SpanImpl startSpanInternal( @Nullable SpanContext parent, boolean hasRemoteParent, String name, @@ -80,7 +80,7 @@ final class SpanBuilderImpl extends SpanBuilder { if (traceOptions.isSampled() || Boolean.TRUE.equals(recordEvents)) { spanOptions.add(Span.Options.RECORD_EVENTS); } - Span span = + SpanImpl span = SpanImpl.startSpan( SpanContext.create(traceId, spanId, traceOptions), spanOptions, @@ -105,36 +105,35 @@ final class SpanBuilderImpl extends SpanBuilder { } } - static SpanBuilder createBuilder(@Nullable Span parentSpan, String name, Options options) { - return new SpanBuilderImpl(parentSpan, null, name, options); + static SpanBuilderImpl createWithParent(String spanName, @Nullable Span parent, Options options) { + return new SpanBuilderImpl(spanName, null, parent, options); } - static SpanBuilder createBuilderWithRemoteParent( - SpanContext remoteParentSpanContext, String name, Options options) { - return new SpanBuilderImpl(null, checkNotNull(remoteParentSpanContext, - "remoteParentSpanContext"), name, options); + static SpanBuilderImpl createWithRemoteParent( + String spanName, @Nullable SpanContext remoteParentSpanContext, Options options) { + return new SpanBuilderImpl(spanName, remoteParentSpanContext, null, options); } private SpanBuilderImpl( - @Nullable Span parentSpan, - @Nullable SpanContext remoteParentSpanContext, String name, + @Nullable SpanContext remoteParentSpanContext, + @Nullable Span parent, Options options) { this.name = checkNotNull(name, "name"); - this.parentSpan = parentSpan; + this.parent = parent; this.remoteParentSpanContext = remoteParentSpanContext; this.options = options; } @Override - public Span startSpan() { + public SpanImpl startSpan() { SpanContext parentContext = remoteParentSpanContext; boolean hasRemoteParent = parentContext != null; TimestampConverter timestampConverter = null; if (!hasRemoteParent) { // This is not a child of a remote Span. Get the parent SpanContext from the parent Span if // any. - Span parent = parentSpan; + Span parent = this.parent; if (parent != null) { parentContext = parent.getContext(); // Pass the timestamp converter from the parent to ensure that the recorded events are in @@ -173,19 +172,19 @@ final class SpanBuilderImpl extends SpanBuilder { } @Override - public SpanBuilder setSampler(Sampler sampler) { + public SpanBuilderImpl setSampler(Sampler sampler) { this.sampler = checkNotNull(sampler, "sampler"); return this; } @Override - public SpanBuilder setParentLinks(List<Span> parentLinks) { + public SpanBuilderImpl setParentLinks(List<Span> parentLinks) { this.parentLinks = checkNotNull(parentLinks, "parentLinks"); return this; } @Override - public SpanBuilder setRecordEvents(boolean recordEvents) { + public SpanBuilderImpl setRecordEvents(boolean recordEvents) { this.recordEvents = recordEvents; return this; } diff --git a/impl_core/src/main/java/io/opencensus/trace/TracerImpl.java b/impl_core/src/main/java/io/opencensus/trace/TracerImpl.java index b15a8b99..0dfdf361 100644 --- a/impl_core/src/main/java/io/opencensus/trace/TracerImpl.java +++ b/impl_core/src/main/java/io/opencensus/trace/TracerImpl.java @@ -32,14 +32,14 @@ final class TracerImpl extends Tracer { } @Override - public SpanBuilder spanBuilder(@Nullable Span parent, String name) { - return SpanBuilderImpl.createBuilder(parent, name, spanBuilderOptions); + public SpanBuilder spanBuilderWithExplicitParent(String spanName, @Nullable Span parent) { + return SpanBuilderImpl.createWithParent(spanName, parent, spanBuilderOptions); } @Override public SpanBuilder spanBuilderWithRemoteParent( - SpanContext remoteParentSpanContext, String name) { - return SpanBuilderImpl.createBuilderWithRemoteParent( - remoteParentSpanContext, name, spanBuilderOptions); + String spanName, @Nullable SpanContext remoteParentSpanContext) { + return SpanBuilderImpl.createWithRemoteParent( + spanName, remoteParentSpanContext, spanBuilderOptions); } } diff --git a/impl_core/src/test/java/io/opencensus/trace/SpanBuilderImplTest.java b/impl_core/src/test/java/io/opencensus/trace/SpanBuilderImplTest.java index afcd3b8b..3e827485 100644 --- a/impl_core/src/test/java/io/opencensus/trace/SpanBuilderImplTest.java +++ b/impl_core/src/test/java/io/opencensus/trace/SpanBuilderImplTest.java @@ -57,12 +57,12 @@ public class SpanBuilderImplTest { @Test public void startSpanNullParent() { - Span span = SpanBuilderImpl.createBuilder(null, SPAN_NAME, spanBuilderOptions).startSpan(); + SpanImpl span = + SpanBuilderImpl.createWithParent(SPAN_NAME, null, spanBuilderOptions).startSpan(); assertThat(span.getContext().isValid()).isTrue(); assertThat(span.getOptions().contains(Options.RECORD_EVENTS)).isTrue(); assertThat(span.getContext().getTraceOptions().isSampled()).isTrue(); - assertThat(span instanceof SpanImpl).isTrue(); - SpanData spanData = ((SpanImpl) span).toSpanData(); + SpanData spanData = span.toSpanData(); assertThat(spanData.getParentSpanId()).isNull(); assertThat(spanData.getHasRemoteParent()).isFalse(); assertThat(spanData.getStartTimestamp()).isEqualTo(testClock.now()); @@ -71,16 +71,15 @@ public class SpanBuilderImplTest { @Test public void startSpanNullParentWithRecordEvents() { - Span span = - SpanBuilderImpl.createBuilder(null, SPAN_NAME, spanBuilderOptions) + SpanImpl span = + SpanBuilderImpl.createWithParent(SPAN_NAME, null, spanBuilderOptions) .setSampler(Samplers.neverSample()) .setRecordEvents(true) .startSpan(); assertThat(span.getContext().isValid()).isTrue(); assertThat(span.getOptions().contains(Options.RECORD_EVENTS)).isTrue(); assertThat(span.getContext().getTraceOptions().isSampled()).isFalse(); - assertThat(span instanceof SpanImpl).isTrue(); - SpanData spanData = ((SpanImpl) span).toSpanData(); + SpanData spanData = span.toSpanData(); assertThat(spanData.getParentSpanId()).isNull(); assertThat(spanData.getHasRemoteParent()).isFalse(); } @@ -88,7 +87,7 @@ public class SpanBuilderImplTest { @Test public void startSpanNullParentNoRecordOptions() { Span span = - SpanBuilderImpl.createBuilder(null, SPAN_NAME, spanBuilderOptions) + SpanBuilderImpl.createWithParent(SPAN_NAME, null, spanBuilderOptions) .setSampler(Samplers.neverSample()) .startSpan(); assertThat(span.getContext().isValid()).isTrue(); @@ -98,12 +97,13 @@ public class SpanBuilderImplTest { @Test public void startChildSpan() { - Span rootSpan = SpanBuilderImpl.createBuilder(null, SPAN_NAME, spanBuilderOptions).startSpan(); + Span rootSpan = + SpanBuilderImpl.createWithParent(SPAN_NAME, null, spanBuilderOptions).startSpan(); assertThat(rootSpan.getContext().isValid()).isTrue(); assertThat(rootSpan.getOptions().contains(Options.RECORD_EVENTS)).isTrue(); assertThat(rootSpan.getContext().getTraceOptions().isSampled()).isTrue(); Span childSpan = - SpanBuilderImpl.createBuilder(rootSpan, SPAN_NAME, spanBuilderOptions).startSpan(); + SpanBuilderImpl.createWithParent(SPAN_NAME, rootSpan, spanBuilderOptions).startSpan(); assertThat(childSpan.getContext().isValid()).isTrue(); assertThat(childSpan.getContext().getTraceId()).isEqualTo(rootSpan.getContext().getTraceId()); assertThat(((SpanImpl) childSpan).toSpanData().getParentSpanId()) @@ -112,22 +112,27 @@ public class SpanBuilderImplTest { .isEqualTo(((SpanImpl) rootSpan).getTimestampConverter()); } - @Test(expected = NullPointerException.class) + @Test public void startRemoteSpan_NullParent() { - SpanBuilderImpl.createBuilderWithRemoteParent(null, SPAN_NAME, spanBuilderOptions); + SpanImpl span = + SpanBuilderImpl.createWithRemoteParent(SPAN_NAME, null, spanBuilderOptions).startSpan(); + assertThat(span.getContext().isValid()).isTrue(); + assertThat(span.getOptions().contains(Options.RECORD_EVENTS)).isTrue(); + assertThat(span.getContext().getTraceOptions().isSampled()).isTrue(); + SpanData spanData = span.toSpanData(); + assertThat(spanData.getParentSpanId()).isNull(); + assertThat(spanData.getHasRemoteParent()).isFalse(); } @Test public void startRemoteSpanInvalidParent() { - Span span = - SpanBuilderImpl.createBuilderWithRemoteParent( - SpanContext.INVALID, SPAN_NAME, spanBuilderOptions) + SpanImpl span = + SpanBuilderImpl.createWithRemoteParent(SPAN_NAME, SpanContext.INVALID, spanBuilderOptions) .startSpan(); assertThat(span.getContext().isValid()).isTrue(); assertThat(span.getOptions().contains(Options.RECORD_EVENTS)).isTrue(); assertThat(span.getContext().getTraceOptions().isSampled()).isTrue(); - assertThat(span instanceof SpanImpl).isTrue(); - SpanData spanData = ((SpanImpl) span).toSpanData(); + SpanData spanData = span.toSpanData(); assertThat(spanData.getParentSpanId()).isNull(); assertThat(spanData.getHasRemoteParent()).isFalse(); } @@ -139,14 +144,13 @@ public class SpanBuilderImplTest { TraceId.generateRandomId(randomHandler.current()), SpanId.generateRandomId(randomHandler.current()), TraceOptions.DEFAULT); - Span span = - SpanBuilderImpl.createBuilderWithRemoteParent(spanContext, SPAN_NAME, spanBuilderOptions) + SpanImpl span = + SpanBuilderImpl.createWithRemoteParent(SPAN_NAME, spanContext, spanBuilderOptions) .startSpan(); assertThat(span.getContext().isValid()).isTrue(); assertThat(span.getContext().getTraceId()).isEqualTo(spanContext.getTraceId()); assertThat(span.getContext().getTraceOptions().isSampled()).isTrue(); - assertThat(span instanceof SpanImpl).isTrue(); - SpanData spanData = ((SpanImpl) span).toSpanData(); + SpanData spanData = span.toSpanData(); assertThat(spanData.getParentSpanId()).isEqualTo(spanContext.getSpanId()); assertThat(spanData.getHasRemoteParent()).isTrue(); } diff --git a/impl_core/src/test/java/io/opencensus/trace/TracerImplTest.java b/impl_core/src/test/java/io/opencensus/trace/TracerImplTest.java index 9ee70c98..1cb27e20 100644 --- a/impl_core/src/test/java/io/opencensus/trace/TracerImplTest.java +++ b/impl_core/src/test/java/io/opencensus/trace/TracerImplTest.java @@ -43,13 +43,13 @@ public class TracerImplTest { @Test public void createSpanBuilder() { - SpanBuilder spanBuilder = tracer.spanBuilder(BlankSpan.INSTANCE, SPAN_NAME); + SpanBuilder spanBuilder = tracer.spanBuilderWithExplicitParent(SPAN_NAME, BlankSpan.INSTANCE); assertThat(spanBuilder).isInstanceOf(SpanBuilderImpl.class); } @Test public void createSpanBuilderWithRemoteParet() { - SpanBuilder spanBuilder = tracer.spanBuilderWithRemoteParent(SpanContext.INVALID, SPAN_NAME); + SpanBuilder spanBuilder = tracer.spanBuilderWithRemoteParent(SPAN_NAME, SpanContext.INVALID); assertThat(spanBuilder).isInstanceOf(SpanBuilderImpl.class); } } |