From e08e3a787ce53ff27f6155b09dcd4ce3796dbe30 Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Mon, 15 Jul 2019 19:11:51 -0700 Subject: [PATCH] Allow null to be set in Scope for Span This is useful to temporarily remove a trace from scope for a defined period. --- .../java/datadog/opentracing/DDTracer.java | 2 +- .../scopemanager/ContinuableScope.java | 2 +- .../opentracing/scopemanager/SimpleScope.java | 4 +-- .../opentracing/DDSpanBuilderTest.groovy | 28 +++++++++++++++++++ .../scopemanager/ScopeManagerTest.groovy | 22 +++++++++------ 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java index 74bf6129e7..487f53be66 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/DDTracer.java @@ -619,7 +619,7 @@ public class DDTracer implements io.opentracing.Tracer, Closeable, datadog.trace if (parentContext == null && !ignoreScope) { // use the Scope as parent unless overridden or ignored. final Scope scope = scopeManager.active(); - if (scope != null) { + if (scope != null && scope.span() != null) { parentContext = scope.span().context(); } } diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java index a40126b343..53a788dd9e 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/ContinuableScope.java @@ -48,7 +48,7 @@ public class ContinuableScope implements Scope, TraceScope { this.openCount = openCount; this.continuation = continuation; this.spanUnderScope = spanUnderScope; - this.finishOnClose = finishOnClose; + this.finishOnClose = finishOnClose && spanUnderScope != null; toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); for (final ScopeListener listener : scopeManager.scopeListeners) { diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java index fc43e92fe8..d11194a9c9 100644 --- a/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java +++ b/dd-trace-ot/src/main/java/datadog/opentracing/scopemanager/SimpleScope.java @@ -17,8 +17,8 @@ public class SimpleScope implements Scope { final boolean finishOnClose) { this.scopeManager = scopeManager; this.spanUnderScope = spanUnderScope; - this.finishOnClose = finishOnClose; - this.toRestore = scopeManager.tlsScope.get(); + this.finishOnClose = finishOnClose && spanUnderScope != null; + toRestore = scopeManager.tlsScope.get(); scopeManager.tlsScope.set(this); for (final ScopeListener listener : scopeManager.scopeListeners) { listener.afterScopeActivated(); diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy index f158c3845b..e7a6da24e2 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy +++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/DDSpanBuilderTest.groovy @@ -6,6 +6,7 @@ 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 spock.lang.Specification import static datadog.opentracing.DDSpanContext.ORIGIN_KEY @@ -173,6 +174,33 @@ class DDSpanBuilderTest extends Specification { actualContext.getTraceId() == spanId } + def "should link to parent span implicitly"() { + setup: + final Scope parent = nullParent ? + tracer.scopeManager().activate(null, false) : + tracer.buildSpan("parent") + .startActive(false) + + final String expectedParentId = nullParent ? "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: + nullParent << [false, true] + } + def "should inherit the DD parent attributes"() { setup: def expectedName = "fakeName" diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy index 4ff52ea087..54afaade40 100644 --- a/dd-trace-ot/src/test/groovy/datadog/opentracing/scopemanager/ScopeManagerTest.groovy +++ b/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 = nullChild ? tracer.scopeManager().activate(null, 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 + nullChild || childScope.span().context().parentId == parentScope.span().context().spanId + nullChild || childScope.span().context().trace == parentScope.span().context().trace when: childScope.close() then: scopeManager.active() == parentScope - spanFinished(childScope.span()) == finishSpan + spanFinished(childScope.span()) == (nullChild ? null : finishSpan) !spanFinished(parentScope.span()) writer == [] @@ -116,13 +116,17 @@ class ScopeManagerTest extends Specification { parentScope.close() then: - spanFinished(childScope.span()) == finishSpan + spanFinished(childScope.span()) == (nullChild ? null : finishSpan) spanFinished(parentScope.span()) == finishSpan - writer == [[parentScope.span(), childScope.span()]] || !finishSpan + writer == [[parentScope.span(), childScope.span()]] || !finishSpan || nullChild scopeManager.active() == null where: - finishSpan << [true, false] + finishSpan | nullChild + true | false + false | false + true | true + false | true } def "ContinuableScope only creates continuations when propagation is set"() { @@ -567,8 +571,8 @@ class ScopeManagerTest extends Specification { closedCount.get() == 4 } - boolean spanFinished(Span span) { - return ((DDSpan) span).isFinished() + Boolean spanFinished(Span span) { + return ((DDSpan) span)?.isFinished() } class AtomicReferenceScope extends AtomicReference implements ScopeContext, Scope {