diff --git a/lib/ddtrace/opentracer/tracer.rb b/lib/ddtrace/opentracer/tracer.rb index 75f5cd7efbf..e61320fcca2 100644 --- a/lib/ddtrace/opentracer/tracer.rb +++ b/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 diff --git a/spec/ddtrace/opentracer/tracer_integration_spec.rb b/spec/ddtrace/opentracer/tracer_integration_spec.rb index 38f9ed51c31..4a7849e8a2b 100644 --- a/spec/ddtrace/opentracer/tracer_integration_spec.rb +++ b/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