Skip to content

Commit

Permalink
Remove DefaultSpan from public API. (#1791)
Browse files Browse the repository at this point in the history
* Cleanup

* Fix comment

* isValid not valid
  • Loading branch information
Anuraag Agrawal authored Oct 15, 2020
1 parent 0ceff17 commit 2d0f91b
Show file tree
Hide file tree
Showing 42 changed files with 137 additions and 163 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator.Setter;
import io.opentelemetry.trace.DefaultSpan;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.TraceFlags;
import io.opentelemetry.trace.TraceState;
Expand Down Expand Up @@ -75,7 +75,7 @@ private static SpanContext createTestSpanContext(String traceId, String spanId)
private static List<Context> createContexts(List<SpanContext> spanContexts) {
List<Context> contexts = new ArrayList<>();
for (SpanContext context : spanContexts) {
contexts.add(TracingContextUtils.withSpan(DefaultSpan.create(context), Context.root()));
contexts.add(TracingContextUtils.withSpan(Span.wrap(context), Context.root()));
}
return contexts;
}
Expand Down
4 changes: 1 addition & 3 deletions api/src/main/java/io/opentelemetry/trace/DefaultTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ public Span startSpan() {
spanContext = TracingContextUtils.getCurrentSpan().getContext();
}

return spanContext != null && !SpanContext.getInvalid().equals(spanContext)
? new DefaultSpan(spanContext)
: DefaultSpan.getInvalid();
return Span.wrap(spanContext);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,15 @@
/**
* The {@code DefaultSpan} is the default {@link Span} that is used when no {@code Span}
* implementation is available. All operations are no-op except context propagation.
*
* <p>Used also to stop tracing, see {@link Tracer#withSpan}.
*/
@Immutable
public final class DefaultSpan implements Span {
final class PropagatedSpan implements Span {

private static final DefaultSpan INVALID = new DefaultSpan(SpanContext.getInvalid());

/**
* Returns a {@link DefaultSpan} with an invalid {@link SpanContext}.
*
* @return a {@code DefaultSpan} with an invalid {@code SpanContext}.
*/
public static Span getInvalid() {
return INVALID;
}

/**
* Creates an instance of this class with the {@link SpanContext}.
*
* @param spanContext the {@code SpanContext}.
* @return a {@link DefaultSpan}.
*/
public static Span create(SpanContext spanContext) {
return new DefaultSpan(spanContext);
}
static final PropagatedSpan INVALID = new PropagatedSpan(SpanContext.getInvalid());

private final SpanContext spanContext;

DefaultSpan(SpanContext spanContext) {
PropagatedSpan(SpanContext spanContext) {
this.spanContext = spanContext;
}

Expand Down
20 changes: 20 additions & 0 deletions api/src/main/java/io/opentelemetry/trace/Span.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,26 @@
@ThreadSafe
public interface Span {

/**
* Returns an invalid {@link Span}. An invalid {@link Span} is used when tracing is disabled,
* usually because there is no OpenTelemetry SDK installed.
*/
static Span getInvalid() {
return PropagatedSpan.INVALID;
}

/**
* Returns a non-recording {@link Span} that holds the provided {@link SpanContext} but has no
* functionality. It will not be exported and all tracing operations are no-op, but it can be used
* to propagate a valid {@link SpanContext} downstream.
*/
static Span wrap(SpanContext spanContext) {
if (spanContext == null || !spanContext.isValid()) {
return getInvalid();
}
return new PropagatedSpan(spanContext);
}

/**
* Type of span. Can be used to specify additional relationships between spans in addition to a
* parent/child relationship.
Expand Down
3 changes: 2 additions & 1 deletion api/src/main/java/io/opentelemetry/trace/Tracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ public interface Tracer {
*
* <p>Supports try-with-resource idiom.
*
* <p>Can be called with {@link DefaultSpan} to enter a scope of code where tracing is stopped.
* <p>Can be called with {@code Span.getPropagated(span.getContext())} to enter a scope of code
* where tracing is stopped.
*
* <p>Example of usage:
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static Span getCurrentSpan() {
*/
public static Span getSpan(Context context) {
Span span = context.getValue(CONTEXT_SPAN_KEY);
return span == null ? DefaultSpan.getInvalid() : span;
return span == null ? Span.getInvalid() : span;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.internal.TemporaryBuffers;
import io.opentelemetry.trace.DefaultSpan;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.SpanId;
import io.opentelemetry.trace.TraceFlags;
Expand Down Expand Up @@ -149,7 +149,7 @@ public <C> void inject(Context context, C carrier, Setter<C> setter) {
return context;
}

return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context);
return TracingContextUtils.withSpan(Span.wrap(spanContext), context);
}

private static <C> SpanContext extractImpl(C carrier, Getter<C> getter) {
Expand Down
30 changes: 14 additions & 16 deletions api/src/test/java/io/opentelemetry/trace/DefaultTracerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,16 @@ class DefaultTracerTest {

@Test
void defaultGetCurrentSpan() {
assertThat(defaultTracer.getCurrentSpan()).isInstanceOf(DefaultSpan.class);
assertThat(defaultTracer.getCurrentSpan().getContext().isValid()).isFalse();
}

@Test
void getCurrentSpan_WithSpan() {
assertThat(defaultTracer.getCurrentSpan()).isInstanceOf(DefaultSpan.class);
try (Scope ws = defaultTracer.withSpan(DefaultSpan.getInvalid())) {
assertThat(defaultTracer.getCurrentSpan()).isInstanceOf(DefaultSpan.class);
assertThat(defaultTracer.getCurrentSpan().getContext().isValid()).isFalse();
try (Scope ws = defaultTracer.withSpan(Span.getInvalid())) {
assertThat(defaultTracer.getCurrentSpan().getContext().isValid()).isFalse();
}
assertThat(defaultTracer.getCurrentSpan()).isInstanceOf(DefaultSpan.class);
assertThat(defaultTracer.getCurrentSpan().getContext().isValid()).isFalse();
}

@Test
Expand All @@ -50,7 +50,7 @@ void spanBuilderWithName_NullName() {

@Test
void defaultSpanBuilderWithName() {
assertThat(defaultTracer.spanBuilder(SPAN_NAME).startSpan()).isInstanceOf(DefaultSpan.class);
assertThat(defaultTracer.spanBuilder(SPAN_NAME).startSpan().getContext().isValid()).isFalse();
}

@Test
Expand All @@ -65,23 +65,22 @@ void testInProcessContext() {
assertThat(defaultTracer.getCurrentSpan()).isEqualTo(span);
}
}
assertThat(defaultTracer.getCurrentSpan()).isInstanceOf(DefaultSpan.class);
assertThat(defaultTracer.getCurrentSpan().getContext().isValid()).isFalse();
}

@Test
void testSpanContextPropagationExplicitParent() {
Span span =
defaultTracer
.spanBuilder(SPAN_NAME)
.setParent(
TracingContextUtils.withSpan(DefaultSpan.create(spanContext), Context.root()))
.setParent(TracingContextUtils.withSpan(Span.wrap(spanContext), Context.root()))
.startSpan();
assertThat(span.getContext()).isSameAs(spanContext);
}

@Test
void testSpanContextPropagation() {
DefaultSpan parent = new DefaultSpan(spanContext);
Span parent = Span.wrap(spanContext);

Span span =
defaultTracer
Expand All @@ -106,31 +105,31 @@ void testSpanContextPropagation_nullContext() {

@Test
void testSpanContextPropagation_fromContext() {
Context context = TracingContextUtils.withSpan(new DefaultSpan(spanContext), Context.current());
Context context = TracingContextUtils.withSpan(Span.wrap(spanContext), Context.current());

Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(context).startSpan();
assertThat(span.getContext()).isSameAs(spanContext);
}

@Test
void testSpanContextPropagation_fromContextAfterNoParent() {
Context context = TracingContextUtils.withSpan(new DefaultSpan(spanContext), Context.current());
Context context = TracingContextUtils.withSpan(Span.wrap(spanContext), Context.current());

Span span = defaultTracer.spanBuilder(SPAN_NAME).setNoParent().setParent(context).startSpan();
assertThat(span.getContext()).isSameAs(spanContext);
}

@Test
void testSpanContextPropagation_fromContextThenNoParent() {
Context context = TracingContextUtils.withSpan(new DefaultSpan(spanContext), Context.current());
Context context = TracingContextUtils.withSpan(Span.wrap(spanContext), Context.current());

Span span = defaultTracer.spanBuilder(SPAN_NAME).setParent(context).setNoParent().startSpan();
assertThat(span.getContext()).isEqualTo(SpanContext.getInvalid());
}

@Test
void testSpanContextPropagationCurrentSpan() {
DefaultSpan parent = new DefaultSpan(spanContext);
Span parent = Span.wrap(spanContext);
try (Scope scope = defaultTracer.withSpan(parent)) {
Span span = defaultTracer.spanBuilder(SPAN_NAME).startSpan();
assertThat(span.getContext()).isSameAs(spanContext);
Expand All @@ -139,8 +138,7 @@ void testSpanContextPropagationCurrentSpan() {

@Test
void testSpanContextPropagationCurrentSpanContext() {
Context context =
TracingContextUtils.withSpan(DefaultSpan.create(spanContext), Context.current());
Context context = TracingContextUtils.withSpan(Span.wrap(spanContext), Context.current());
try (Scope scope = context.makeCurrent()) {
Span span = defaultTracer.spanBuilder(SPAN_NAME).startSpan();
assertThat(span.getContext()).isSameAs(spanContext);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,18 @@
import io.opentelemetry.common.Attributes;
import org.junit.jupiter.api.Test;

/** Unit tests for {@link DefaultSpan}. */
class DefaultSpanTest {
class PropagatedSpanTest {

@Test
void hasInvalidContextAndDefaultSpanOptions() {
SpanContext context = DefaultSpan.getInvalid().getContext();
SpanContext context = Span.getInvalid().getContext();
assertThat(context.getTraceFlags()).isEqualTo(TraceFlags.getDefault());
assertThat(context.getTraceState()).isEqualTo(TraceState.getDefault());
}

@Test
void doNotCrash() {
Span span = DefaultSpan.getInvalid();
Span span = Span.getInvalid();
span.setAttribute(stringKey("MyStringAttributeKey"), "MyStringAttributeValue");
span.setAttribute(booleanKey("MyBooleanAttributeKey"), true);
span.setAttribute(longKey("MyLongAttributeKey"), 123L);
Expand All @@ -56,7 +55,7 @@ void doNotCrash() {

@Test
void defaultSpan_ToString() {
Span span = DefaultSpan.getInvalid();
Span span = Span.getInvalid();
assertThat(span.toString()).isEqualTo("DefaultSpan");
}
}
8 changes: 4 additions & 4 deletions api/src/test/java/io/opentelemetry/trace/SpanBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,18 @@ class SpanBuilderTest {
void doNotCrash_NoopImplementation() {
Span.Builder spanBuilder = tracer.spanBuilder("MySpanName");
spanBuilder.setSpanKind(Kind.SERVER);
spanBuilder.setParent(TracingContextUtils.withSpan(DefaultSpan.create(null), Context.root()));
spanBuilder.setParent(TracingContextUtils.withSpan(Span.wrap(null), Context.root()));
spanBuilder.setParent(Context.root());
spanBuilder.setNoParent();
spanBuilder.addLink(DefaultSpan.getInvalid().getContext());
spanBuilder.addLink(DefaultSpan.getInvalid().getContext(), Attributes.empty());
spanBuilder.addLink(Span.getInvalid().getContext());
spanBuilder.addLink(Span.getInvalid().getContext(), Attributes.empty());
spanBuilder.setAttribute("key", "value");
spanBuilder.setAttribute("key", 12345L);
spanBuilder.setAttribute("key", .12345);
spanBuilder.setAttribute("key", true);
spanBuilder.setAttribute(stringKey("key"), "value");
spanBuilder.setStartTimestamp(12345L);
assertThat(spanBuilder.startSpan()).isInstanceOf(DefaultSpan.class);
assertThat(spanBuilder.startSpan().getContext().isValid()).isFalse();
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ class TracingContextUtilsTest {
@Test
void testGetCurrentSpan_Default() {
Span span = TracingContextUtils.getCurrentSpan();
assertThat(span).isSameAs(DefaultSpan.getInvalid());
assertThat(span).isSameAs(Span.getInvalid());
}

@Test
void testGetCurrentSpan_SetSpan() {
Span span = DefaultSpan.create(SpanContext.getInvalid());
Span span = Span.wrap(SpanContext.getInvalid());
try (Scope ignored = TracingContextUtils.withSpan(span, Context.current()).makeCurrent()) {
assertThat(TracingContextUtils.getCurrentSpan()).isSameAs(span);
}
Expand All @@ -30,12 +30,12 @@ void testGetCurrentSpan_SetSpan() {
@Test
void testGetSpan_DefaultContext() {
Span span = TracingContextUtils.getSpan(Context.current());
assertThat(span).isSameAs(DefaultSpan.getInvalid());
assertThat(span).isSameAs(Span.getInvalid());
}

@Test
void testGetSpan_ExplicitContext() {
Span span = DefaultSpan.create(SpanContext.getInvalid());
Span span = Span.wrap(SpanContext.getInvalid());
Context context = TracingContextUtils.withSpan(span, Context.current());
assertThat(TracingContextUtils.getSpan(context)).isSameAs(span);
}
Expand All @@ -48,7 +48,7 @@ void testGetSpanWithoutDefault_DefaultContext() {

@Test
void testGetSpanWithoutDefault_ExplicitContext() {
Span span = DefaultSpan.create(SpanContext.getInvalid());
Span span = Span.wrap(SpanContext.getInvalid());
Context context = TracingContextUtils.withSpan(span, Context.current());
assertThat(TracingContextUtils.getSpanWithoutDefault(context)).isSameAs(span);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator.Getter;
import io.opentelemetry.context.propagation.TextMapPropagator.Setter;
import io.opentelemetry.trace.DefaultSpan;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.SpanId;
import io.opentelemetry.trace.TraceFlags;
Expand Down Expand Up @@ -52,7 +52,7 @@ private static SpanContext getSpanContext(Context context) {
}

private static Context withSpanContext(SpanContext spanContext, Context context) {
return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context);
return TracingContextUtils.withSpan(Span.wrap(spanContext), context);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.trace.DefaultSpan;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.TraceFlags;
import io.opentelemetry.trace.TraceState;
Expand Down Expand Up @@ -62,8 +62,7 @@ public abstract static class AbstractContextInjectBenchmark {
@BenchmarkMode(Mode.AverageTime)
@Fork(1)
public Map<String, String> measureInject() {
Context context =
TracingContextUtils.withSpan(DefaultSpan.create(contextToTest), Context.current());
Context context = TracingContextUtils.withSpan(Span.wrap(contextToTest), Context.current());
doInject(context, carrier);
return carrier;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import io.opentelemetry.context.Context;
import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.trace.DefaultSpan;
import io.opentelemetry.trace.Span;
import io.opentelemetry.trace.SpanContext;
import io.opentelemetry.trace.SpanId;
Expand Down Expand Up @@ -128,7 +127,7 @@ public <C> Context extract(Context context, C carrier, Getter<C> getter) {
return context;
}

return TracingContextUtils.withSpan(DefaultSpan.create(spanContext), context);
return TracingContextUtils.withSpan(Span.wrap(spanContext), context);
}

private static <C> SpanContext getSpanContextFromHeader(C carrier, Getter<C> getter) {
Expand Down
Loading

0 comments on commit 2d0f91b

Please sign in to comment.