[OpenTracing] Fix context propagation between OpenTracing and Datadog

lib/ddtrace/opentracer/tracer.rb

@@ -56,6 +56,28 @@ def start_active_span(operation_name,
                             tags: nil,
                             ignore_active_scope: false,
                             finish_on_close: true)
+
+        # When meant to automatically determine the parent,
+        # Use the active scope first, otherwise fall back to any
+        # context generated by Datadog, so as to append to it and gain
+        # the benefit of any out-of-the-box tracing from Datadog preceding
+        # the OpenTracer::Tracer.
+        #
+        # We do this here instead of in #start_span because #start_span generates
+        # spans that are not assigned to a scope, a.k.a not supposed to be used by
+        # subsequent spans implicitly. By using the existing Datadog context, the span
+        # effectively ends up "assigned to a scope", by virtue of being added to the
+        # Context. Hence, it would behave more like an active span, which is why it
+        # should only be here.
+        unless child_of || ignore_active_scope
+          child_of = if scope_manager.active
+                       scope_manager.active.span.context
+                     else
+                       SpanContextFactory.build(datadog_context: datadog_tracer.call_context)
+                     end
+        end
+
+        # Create the span, and auto-add it to the Datadog context.
         span = start_span(
           operation_name,
           child_of: child_of,
@@ -65,6 +87,12 @@ def start_active_span(operation_name,
           ignore_active_scope: ignore_active_scope
         )
 
+        # Overwrite the tracer context with the OpenTracing managed context.
+        # This is mostly for the benefit of any out-of-the-box tracing from Datadog,
+        # such that spans generated by that tracing will be attached to the OpenTracer
+        # parent span.
+        datadog_tracer.provider.context = span.datadog_span.context
+
         scope_manager.activate(span, finish_on_close: finish_on_close).tap do |scope|
           if block_given?
             begin
@@ -101,20 +129,16 @@ def start_span(operation_name,
                      tags: nil,
                      ignore_active_scope: false)
 
-        # Derive the OpenTracer::SpanContext to inherit from
-        parent_span_context = case child_of
-                              when Span
-                                child_of.context
-                              when SpanContext
-                                child_of
-                              else
-                                ignore_active_scope ? nil : scope_manager.active && scope_manager.active.span.context
-                              end
+        # Derive the OpenTracer::SpanContext to inherit from.
+        parent_span_context = inherited_span_context(child_of, ignore_active_scope: ignore_active_scope)
+
+        # Retrieve Datadog::Context from parent SpanContext.
+        datadog_context = parent_span_context.nil? ? nil : parent_span_context.datadog_context
 
         # Build the new Datadog span
         datadog_span = datadog_tracer.start_span(
           operation_name,
-          child_of: parent_span_context && parent_span_context.datadog_context,
+          child_of: datadog_context,
           start_time: start_time,
           tags: tags || {}
         )
@@ -166,6 +190,19 @@ def extract(format, carrier)
           nil
         end
       end
+
+      private
+
+      def inherited_span_context(parent, ignore_active_scope: false)
+        case parent
+        when Span
+          parent.context
+        when SpanContext
+          parent
+        else
+          ignore_active_scope ? nil : scope_manager.active && scope_manager.active.span.context
+        end
+      end
     end
   end
 end

spec/ddtrace/opentracer/tracer_integration_spec.rb

@@ -51,6 +51,30 @@ def current_trace_for(object)
       end
 
       context 'for a nested span' do
+        context 'when there is no active scope' do
+          before(:each) do
+            tracer.start_span('operation.outer').tap do |_outer_span|
+              tracer.start_span('operation.inner').tap do |inner_span|
+                # Assert Datadog context integrity
+                # 1 item because they each should get their own context.
+                expect(current_trace_for(inner_span)).to have(1).items
+                expect(current_trace_for(inner_span)).to include(inner_span.datadog_span)
+              end.finish
+            end.finish
+          end
+
+          let(:outer_datadog_span) { datadog_spans.last }
+          let(:inner_datadog_span) { datadog_spans.first }
+
+          it { expect(datadog_spans).to have(2).items }
+          it { expect(outer_datadog_span.name).to eq('operation.outer') }
+          it { expect(outer_datadog_span.parent_id).to eq(0) }
+          it { expect(outer_datadog_span.finished?).to be true }
+          it { expect(inner_datadog_span.name).to eq('operation.inner') }
+          it { expect(inner_datadog_span.parent_id).to eq(0) }
+          it { expect(inner_datadog_span.finished?).to be true }
+        end
+
         context 'when there is an active scope' do
           context 'which is used' do
             before(:each) do
@@ -204,6 +228,58 @@ def current_trace_for(object)
             end
           end
         end
+
+        context 'preceded by a Datadog span' do
+          let(:parent_span_name) { 'operation.bar' }
+          let(:options) { { finish_on_close: true } }
+
+          before(:each) do
+            datadog_tracer.trace(parent_span_name) do |span|
+              @parent_span = span
+              tracer.start_active_span(span_name, **options) do |scope|
+                @scope = scope
+              end
+            end
+          end
+
+          let(:parent_datadog_span) { datadog_spans.first }
+          let(:child_datadog_span) { datadog_spans.last }
+
+          it { expect(datadog_spans).to have(2).items }
+          it { expect(parent_datadog_span.name).to eq(parent_span_name) }
+          it { expect(parent_datadog_span.parent_id).to eq(0) }
+          it { expect(parent_datadog_span.finished?).to be(true) }
+          it { expect(child_datadog_span.name).to eq(span_name) }
+          it { expect(child_datadog_span.parent_id).to eq(parent_datadog_span.span_id) }
+          it { expect(child_datadog_span.finished?).to be(true) }
+          it { expect(child_datadog_span.trace_id).to eq(parent_datadog_span.trace_id) }
+        end
+
+        context 'followed by a Datadog span' do
+          let(:child_span_name) { 'operation.bar' }
+          let(:options) { { finish_on_close: true } }
+
+          before(:each) do
+            tracer.start_active_span(span_name, **options) do |scope|
+              @scope = scope
+              datadog_tracer.trace(child_span_name) do |span|
+                @child_span = span
+              end
+            end
+          end
+
+          let(:parent_datadog_span) { datadog_spans.last }
+          let(:child_datadog_span) { datadog_spans.first }
+
+          it { expect(datadog_spans).to have(2).items }
+          it { expect(parent_datadog_span.name).to eq(span_name) }
+          it { expect(parent_datadog_span.parent_id).to eq(0) }
+          it { expect(parent_datadog_span.finished?).to be(true) }
+          it { expect(child_datadog_span.name).to eq(child_span_name) }
+          it { expect(child_datadog_span.parent_id).to eq(parent_datadog_span.span_id) }
+          it { expect(child_datadog_span.finished?).to be(true) }
+          it { expect(child_datadog_span.trace_id).to eq(parent_datadog_span.trace_id) }
+        end
       end
 
       context 'for a nested span' do
@@ -258,7 +334,7 @@ def current_trace_for(object)
         context 'manually associated with child_of' do
           before(:each) do
             tracer.start_span('operation.parent').tap do |parent_span|
-              tracer.start_active_span('operation.fake_parent') do
+              tracer.start_active_span('operation.fake_parent') do |_fake_parent_span|
                 tracer.start_active_span('operation.child', child_of: parent_span) do |scope|
                   # Assert Datadog context integrity
                   expect(current_trace_for(scope)).to have(2).items
@@ -279,6 +355,9 @@ def current_trace_for(object)
           it { expect(child_datadog_span.name).to eq('operation.child') }
           it { expect(child_datadog_span.parent_id).to eq(parent_datadog_span.span_id) }
           it { expect(child_datadog_span.finished?).to be true }
+          it { expect(fake_parent_datadog_span.name).to eq('operation.fake_parent') }
+          it { expect(fake_parent_datadog_span.parent_id).to eq(0) }
+          it { expect(fake_parent_datadog_span.finished?).to be true }
         end
       end