From 32722dfc5791c2e932d5913c8f9be91a8d6fd44f Mon Sep 17 00:00:00 2001
From: Tyler Benson <tyler.benson@datadoghq.com>
Date: Mon, 4 May 2020 15:09:15 -0400
Subject: [PATCH] Fix null span/scope handling for OpenTracing Bridge

The OT API expects null to be returned in certain cases, so we shouldn't returned a wrapped null.
---
 .../datadog/opentracing/TypeConverter.java    | 20 ++++++++++++++-----
 .../opentracing/OpenTracingAPITest.groovy     |  8 ++++++++
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java b/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java
index 9c6c98e8a80..a5a6a58696e 100644
--- a/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java
+++ b/dd-trace-ot/src/main/java/datadog/opentracing/TypeConverter.java
@@ -24,7 +24,9 @@ public TypeConverter(final LogHandler logHandler) {
   }
 
   public AgentSpan toAgentSpan(final Span span) {
-    if (span instanceof OTSpan) {
+    if (span == null) {
+      return null;
+    } else if (span instanceof OTSpan) {
       return ((OTSpan) span).getDelegate();
     } else {
       // NOOP Span
@@ -33,7 +35,9 @@ public AgentSpan toAgentSpan(final Span span) {
   }
 
   public Span toSpan(final AgentSpan agentSpan) {
-    if (agentSpan instanceof DDSpan) {
+    if (agentSpan == null) {
+      return null;
+    } else if (agentSpan instanceof DDSpan) {
       return new OTSpan((DDSpan) agentSpan, this, logHandler);
     } else {
       // NOOP AgentSpans
@@ -45,7 +49,9 @@ public Span toSpan(final AgentSpan agentSpan) {
   // That fact that some methods return AgentScope and other TraceScope even though its the same
   // underlying object needs to be cleaned up
   public Scope toScope(final Object scope) {
-    if (scope instanceof CustomScopeManagerWrapper.CustomScopeManagerScope) {
+    if (scope == null) {
+      return null;
+    } else if (scope instanceof CustomScopeManagerWrapper.CustomScopeManagerScope) {
       return ((CustomScopeManagerWrapper.CustomScopeManagerScope) scope).getDelegate();
     } else if (scope instanceof TraceScope) {
       return new OTScopeManager.OTTraceScope((TraceScope) scope, this);
@@ -59,7 +65,9 @@ public SpanContext toSpanContext(final DDSpanContext context) {
   }
 
   public SpanContext toSpanContext(final TagContext tagContext) {
-    if (tagContext instanceof ExtractedContext) {
+    if (tagContext == null) {
+      return null;
+    } else if (tagContext instanceof ExtractedContext) {
       return new OTExtractedContext((ExtractedContext) tagContext);
     } else {
       return new OTTagContext(tagContext);
@@ -69,7 +77,9 @@ public SpanContext toSpanContext(final TagContext tagContext) {
   public AgentSpan.Context toContext(final SpanContext spanContext) {
     // FIXME: [API] DDSpanContext, ExtractedContext, TagContext, AgentSpan.Context
     // don't share a meaningful hierarchy
-    if (spanContext instanceof OTGenericContext) {
+    if (spanContext == null) {
+      return null;
+    } else if (spanContext instanceof OTGenericContext) {
       return ((OTGenericContext) spanContext).getDelegate();
     } else if (spanContext instanceof OTExtractedContext) {
       return ((OTExtractedContext) spanContext).getDelegate();
diff --git a/dd-trace-ot/src/test/groovy/datadog/opentracing/OpenTracingAPITest.groovy b/dd-trace-ot/src/test/groovy/datadog/opentracing/OpenTracingAPITest.groovy
index 655f52dd324..dfe9ce78e49 100644
--- a/dd-trace-ot/src/test/groovy/datadog/opentracing/OpenTracingAPITest.groovy
+++ b/dd-trace-ot/src/test/groovy/datadog/opentracing/OpenTracingAPITest.groovy
@@ -24,10 +24,18 @@ class OpenTracingAPITest extends DDSpecification {
   def scopeListener = Mock(ScopeListener)
 
   def setup() {
+    assert tracer.scopeManager().active() == null
     tracer.addTraceInterceptor(traceInterceptor)
     tracer.addScopeListener(scopeListener)
   }
 
+  def "tracer/scopeManager returns null for no active span"() {
+    expect:
+    tracer.activeSpan() == null
+    tracer.scopeManager().active() == null
+    tracer.scopeManager().activeSpan() == null
+  }
+
   def "single span"() {
     when:
     Scope scope