Merge pull request #917 from DataDog/tyler/null-span-in-scope

Assert span in scope can't be null and add tests to verify

dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java

@@ -44,6 +44,7 @@ public class ContinuableScope implements Scope, TraceScope {
       final Continuation continuation,
       final DDSpan spanUnderScope,
       final boolean finishOnClose) {
+    assert spanUnderScope != null : "span must not be null";
     this.scopeManager = scopeManager;
     this.openCount = openCount;
     this.continuation = continuation;

dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java

@@ -15,10 +15,11 @@ public class SimpleScope implements Scope {
       final ContextualScopeManager scopeManager,
       final Span spanUnderScope,
       final boolean finishOnClose) {
+    assert spanUnderScope != null : "span must not be null";
     this.scopeManager = scopeManager;
     this.spanUnderScope = spanUnderScope;
     this.finishOnClose = finishOnClose;
-    this.toRestore = scopeManager.tlsScope.get();
+    toRestore = scopeManager.tlsScope.get();
     scopeManager.tlsScope.set(this);
     for (final ScopeListener listener : scopeManager.scopeListeners) {
       listener.afterScopeActivated();

dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy

@@ -6,6 +6,8 @@ import datadog.trace.api.Config
 import datadog.trace.api.DDTags
 import datadog.trace.api.sampling.PrioritySampling
 import datadog.trace.common.writer.ListWriter
+import io.opentracing.Scope
+import io.opentracing.noop.NoopSpan
 import spock.lang.Specification
 
 import static datadog.opentracing.DDSpanContext.ORIGIN_KEY
@@ -173,6 +175,33 @@ class DDSpanBuilderTest extends Specification {
     actualContext.getTraceId() == spanId
   }
 
+  def "should link to parent span implicitly"() {
+    setup:
+    final Scope parent = noopParent ?
+      tracer.scopeManager().activate(NoopSpan.INSTANCE, false) :
+      tracer.buildSpan("parent")
+        .startActive(false)
+
+    final String expectedParentId = noopParent ? "0" : parent.span().context().getSpanId()
+
+    final String expectedName = "fakeName"
+
+    final DDSpan span = tracer
+      .buildSpan(expectedName)
+      .start()
+
+    final DDSpanContext actualContext = span.context()
+
+    expect:
+    actualContext.getParentId() == expectedParentId
+
+    cleanup:
+    parent.close()
+
+    where:
+    noopParent << [false, true]
+  }
+
   def "should inherit the DD parent attributes"() {
     setup:
     def expectedName = "fakeName"

dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy

@@ -96,19 +96,19 @@ class ScopeManagerTest extends Specification {
   def "sets parent as current upon close"() {
     setup:
     def parentScope = tracer.buildSpan("parent").startActive(finishSpan)
-    def childScope = tracer.buildSpan("parent").startActive(finishSpan)
+    def childScope = noopChild ? tracer.scopeManager().activate(NoopSpan.INSTANCE, finishSpan) : tracer.buildSpan("parent").startActive(finishSpan)
 
     expect:
     scopeManager.active() == childScope
-    childScope.span().context().parentId == parentScope.span().context().spanId
-    childScope.span().context().trace == parentScope.span().context().trace
+    noopChild || childScope.span().context().parentId == parentScope.span().context().spanId
+    noopChild || childScope.span().context().trace == parentScope.span().context().trace
 
     when:
     childScope.close()
 
     then:
     scopeManager.active() == parentScope
-    spanFinished(childScope.span()) == finishSpan
+    noopChild || spanFinished(childScope.span()) == finishSpan
     !spanFinished(parentScope.span())
     writer == []
 
@@ -116,13 +116,17 @@ class ScopeManagerTest extends Specification {
     parentScope.close()
 
     then:
-    spanFinished(childScope.span()) == finishSpan
+    noopChild || spanFinished(childScope.span()) == finishSpan
     spanFinished(parentScope.span()) == finishSpan
-    writer == [[parentScope.span(), childScope.span()]] || !finishSpan
+    writer == [[parentScope.span(), childScope.span()]] || !finishSpan || noopChild
     scopeManager.active() == null
 
     where:
-    finishSpan << [true, false]
+    finishSpan | noopChild
+    true       | false
+    false      | false
+    true       | true
+    false      | true
   }
 
   def "ContinuableScope only creates continuations when propagation is set"() {
@@ -568,7 +572,7 @@ class ScopeManagerTest extends Specification {
   }
 
   boolean spanFinished(Span span) {
-    return ((DDSpan) span).isFinished()
+    return ((DDSpan) span)?.isFinished()
   }
 
   class AtomicReferenceScope extends AtomicReference<Span> implements ScopeContext, Scope {