refactor:! context only parenting (#398)

* refactor: remove with_parent (span) option to start_span (and friends)

* refactor: rename with_parent_context to with_parent (context)

* docs: restore with_parent tracer docs

* fix: resurrect tracer benchmarks

* feat: add Tracer#context_with_span convenience method

* refactor: update tests to use context_with_span convenience method

* feat: pass parent context to span processors

* refactor: use named parameter for context in context_with_span method

api/benchmarks/tracer_bench.rb

@@ -10,6 +10,7 @@
 tracer = OpenTelemetry::Trace::Tracer.new
 
 parent_span = tracer.start_span('parent')
+parent_context = tracer.context_with_span(parent_span)
 
 attributes = {
   'component' => 'rack',
@@ -31,13 +32,8 @@
     span.finish
   end
 
-  x.report 'start span with parent' do
-    span = tracer.start_span('test_span', with_parent: parent_span)
-    span.finish
-  end
-
   x.report 'start span with parent context' do
-    span = tracer.start_span('test_span', with_parent_context: parent_span.context)
+    span = tracer.start_span('test_span', with_parent: parent_context)
     span.finish
   end
 

api/lib/opentelemetry/trace/tracer.rb

@@ -21,6 +21,15 @@ def current_span(context = nil)
         context.value(CURRENT_SPAN_KEY) || Span::INVALID
       end
 
+      # Returns a context containing the span, derived from the optional parent
+      # context, or the current context if one was not provided.
+      #
+      # @param [optional Context] context The context to use as the parent for
+      #  the returned context
+      def context_with_span(span, parent_context: Context.current)
+        parent_context.set_value(CURRENT_SPAN_KEY, span)
+      end
+
       # This is a helper for the default use-case of extending the current trace with a span.
       #
       # With this helper:
@@ -36,9 +45,9 @@ def current_span(context = nil)
       # span and reraised.
       # @yield [span, context] yields the newly created span and a context containing the
       #   span to the block.
-      def in_span(name, attributes: nil, links: nil, start_timestamp: nil, kind: nil, with_parent: nil, with_parent_context: nil)
+      def in_span(name, attributes: nil, links: nil, start_timestamp: nil, kind: nil, with_parent: nil)
         span = nil
-        span = start_span(name, attributes: attributes, links: links, start_timestamp: start_timestamp, kind: kind, with_parent: with_parent, with_parent_context: with_parent_context)
+        span = start_span(name, attributes: attributes, links: links, start_timestamp: start_timestamp, kind: kind, with_parent: with_parent)
         with_span(span) { |s, c| yield s, c }
       rescue Exception => e # rubocop:disable Lint/RescueException
         span&.record_exception(e)
@@ -69,14 +78,12 @@ def start_root_span(name, attributes: nil, links: nil, start_timestamp: nil, kin
       #
       # Parent context can be either passed explicitly, or inferred from currently activated span.
       #
-      # @param [optional Span] with_parent Explicitly managed parent Span, overrides
-      #   +with_parent_context+.
-      # @param [optional Context] with_parent_context Explicitly managed. Overridden by
-      #   +with_parent+.
+      # @param [optional Context] with_parent Explicitly managed parent context
       #
       # @return [Span]
-      def start_span(name, with_parent: nil, with_parent_context: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil)
-        span_context = with_parent&.context || current_span(with_parent_context).context
+      def start_span(name, with_parent: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil)
+        span_context = current_span(with_parent).context
+
         if span_context.valid?
           Span.new(span_context: span_context)
         else

api/test/opentelemetry/trace/tracer_test.rb

@@ -91,15 +91,8 @@ def start_span(*)
       mock_span.verify
     end
 
-    it 'yields a span with the same context as the parent' do
-      tracer.in_span('op', with_parent: parent) do |span|
-        _(span.context).must_be :valid?
-        _(span.context).must_equal(parent.context)
-      end
-    end
-
     it 'yields a span with the parent context' do
-      tracer.in_span('op', with_parent_context: parent_context) do |span|
+      tracer.in_span('op', with_parent: parent_context) do |span|
         _(span.context).must_be :valid?
         _(span.context).must_equal(parent_span_context)
       end
@@ -151,7 +144,7 @@ def start_span(*)
     let(:parent) { tracer.start_span('parent') }
 
     it 'returns a valid span with the parent context' do
-      span = tracer.start_span('op', with_parent_context: parent_context)
+      span = tracer.start_span('op', with_parent: parent_context)
       _(span.context).must_be :valid?
       _(span.context).must_equal(parent_span_context)
     end
@@ -162,16 +155,26 @@ def start_span(*)
       _(span.context).wont_equal(tracer.current_span.context)
     end
 
-    it 'returns a span with the same context as the parent' do
-      span = tracer.start_span('op', with_parent: parent)
-      _(span.context).must_be :valid?
-      _(span.context).must_equal(parent.context)
-    end
-
     it 'returns a span with a new context when passed an invalid context' do
-      span = tracer.start_span('op', with_parent_context: invalid_parent_context)
+      span = tracer.start_span('op', with_parent: invalid_parent_context)
       _(span.context).must_be :valid?
       _(span.context).wont_equal(invalid_span_context)
     end
   end
+
+  describe '#context_with_span' do
+    it 'returns a context containing span' do
+      span = tracer.start_span('test')
+      ctx = tracer.context_with_span(span)
+      _(tracer.current_span(ctx)).must_equal(span)
+    end
+
+    it 'returns a context containing span' do
+      parent_ctx = OpenTelemetry::Context.empty.set_value('foo', 'bar')
+      span = tracer.start_span('test')
+      ctx = tracer.context_with_span(span, parent_context: parent_ctx)
+      _(tracer.current_span(ctx)).must_equal(span)
+      _(ctx.value('foo')).must_equal('bar')
+    end
+  end
 end

examples/http/server.rb

@@ -45,7 +45,7 @@ def call(env)
         'http.url' => env['REQUEST_URI'],
       },
       kind: :server,
-      with_parent_context: context
+      with_parent: context
     ) do |span|
       # Run application stack
       status, headers, response_body = @app.call(env)

exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb

@@ -154,17 +154,20 @@
 
       OpenTelemetry.tracer_provider.add_span_processor(processor)
       root = with_ids(trace_id, root_span_id) { tracer.start_root_span('root', kind: :internal, start_timestamp: start_timestamp).finish(end_timestamp: end_timestamp) }
-      span = with_ids(trace_id, child_span_id) { tracer.start_span('child', with_parent: root, kind: :producer, start_timestamp: start_timestamp + 1, links: [OpenTelemetry::Trace::Link.new(root.context, 'attr' => 4)]) }
+      root_ctx = tracer.context_with_span(root)
+      span = with_ids(trace_id, child_span_id) { tracer.start_span('child', with_parent: root_ctx, kind: :producer, start_timestamp: start_timestamp + 1, links: [OpenTelemetry::Trace::Link.new(root.context, 'attr' => 4)]) }
       span['b'] = true
       span['f'] = 1.1
       span['i'] = 2
       span['s'] = 'val'
       span['a'] = [3, 4]
       span.status = OpenTelemetry::Trace::Status.new(OpenTelemetry::Trace::Status::ERROR)
-      client = with_ids(trace_id, client_span_id) { tracer.start_span('client', with_parent: span, kind: :client, start_timestamp: start_timestamp + 2).finish(end_timestamp: end_timestamp) }
-      with_ids(trace_id, server_span_id) { other_tracer.start_span('server', with_parent: client, kind: :server, start_timestamp: start_timestamp + 3).finish(end_timestamp: end_timestamp) }
+      child_ctx = tracer.context_with_span(span)
+      client = with_ids(trace_id, client_span_id) { tracer.start_span('client', with_parent: child_ctx, kind: :client, start_timestamp: start_timestamp + 2).finish(end_timestamp: end_timestamp) }
+      client_ctx = tracer.context_with_span(client)
+      with_ids(trace_id, server_span_id) { other_tracer.start_span('server', with_parent: client_ctx, kind: :server, start_timestamp: start_timestamp + 3).finish(end_timestamp: end_timestamp) }
       span.add_event('event', attributes: { 'attr' => 42 }, timestamp: start_timestamp + 4)
-      with_ids(trace_id, consumer_span_id) { tracer.start_span('consumer', with_parent: span, kind: :consumer, start_timestamp: start_timestamp + 5).finish(end_timestamp: end_timestamp) }
+      with_ids(trace_id, consumer_span_id) { tracer.start_span('consumer', with_parent: child_ctx, kind: :consumer, start_timestamp: start_timestamp + 5).finish(end_timestamp: end_timestamp) }
       span.finish(end_timestamp: end_timestamp)
       OpenTelemetry.tracer_provider.shutdown
 

instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb

@@ -81,7 +81,7 @@ def create_frontend_span(env, extracted_context)
             return unless config[:record_frontend_span] && !request_start_time.nil?
 
             span = tracer.start_span('http_server.proxy',
-                                     with_parent_context: extracted_context,
+                                     with_parent: extracted_context,
                                      attributes: {
                                        'start_time' => request_start_time.to_f
                                      },

instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/server/tracer_middleware.rb

@@ -20,7 +20,7 @@ def call(_worker, msg, _queue)
                   'messaging.message_id' => msg['jid'],
                   'messaging.destination' => msg['queue']
                 },
-                with_parent_context: parent_context,
+                with_parent: parent_context,
                 kind: :consumer
               ) do |span|
                 span.add_event('created_at', timestamp: msg['created_at'])

instrumentation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb

@@ -22,7 +22,7 @@ def call(env)
               attributes: { 'http.method' => env['REQUEST_METHOD'],
                             'http.url' => env['PATH_INFO'] },
               kind: :server,
-              with_parent_context: parent_context(env)
+              with_parent: parent_context(env)
             ) do |span|
               app.call(env).tap { |resp| trace_response(span, env, resp) }
             end

sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb

@@ -62,7 +62,7 @@ def initialize(exporter:,
           end
 
           # does nothing for this processor
-          def on_start(span)
+          def on_start(span, parent_context)
             # noop
           end
 

sdk/lib/opentelemetry/sdk/trace/export/simple_span_processor.rb

@@ -33,7 +33,9 @@ def initialize(span_exporter)
           # not throw or block the execution thread.
           #
           # @param [Span] span the {Span} that just started.
-          def on_start(span)
+          # @param [Context] parent_context the parent {Context} of the newly
+          #  started span.
+          def on_start(span, parent_context)
             # Do nothing.
           end
 

sdk/lib/opentelemetry/sdk/trace/multi_span_processor.rb

@@ -26,8 +26,10 @@ def initialize(span_processors)
         # not throw or block the execution thread.
         #
         # @param [Span] span the {Span} that just started.
-        def on_start(span)
-          @span_processors.each { |processor| processor.on_start(span) }
+        # @param [Context] parent_context the parent {Context} of the newly
+        #  started span.
+        def on_start(span, parent_context)
+          @span_processors.each { |processor| processor.on_start(span, parent_context) }
         end
 
         # Called when a {Span} is ended, if the {Span#recording?}

sdk/lib/opentelemetry/sdk/trace/noop_span_processor.rb

@@ -22,7 +22,9 @@ class NoopSpanProcessor
         # not throw or block the execution thread.
         #
         # @param [Span] span the {Span} that just started.
-        def on_start(span); end
+        # @param [Context] parent_context the parent {Context} of the newly
+        #  started span.
+        def on_start(span, parent_context); end
 
         # Called when a {Span} is ended, if the {Span#recording?}
         # returns true.

sdk/lib/opentelemetry/sdk/trace/span.rb

@@ -241,7 +241,7 @@ def to_span_data
         end
 
         # @api private
-        def initialize(context, name, kind, parent_span_id, trace_config, span_processor, attributes, links, start_timestamp, resource, instrumentation_library) # rubocop:disable Metrics/AbcSize
+        def initialize(context, parent_context, name, kind, parent_span_id, trace_config, span_processor, attributes, links, start_timestamp, resource, instrumentation_library) # rubocop:disable Metrics/AbcSize
           super(span_context: context)
           @mutex = Mutex.new
           @name = name
@@ -262,7 +262,7 @@ def initialize(context, name, kind, parent_span_id, trace_config, span_processor
           trim_span_attributes(@attributes)
           @events = nil
           @links = trim_links(links, trace_config.max_links_count, trace_config.max_attributes_per_link)
-          @span_processor.on_start(self)
+          @span_processor.on_start(self, parent_context)
         end
 
         # TODO: Java implementation overrides finalize to log if a span isn't finished.

sdk/lib/opentelemetry/sdk/trace/tracer.rb

@@ -30,32 +30,34 @@ def initialize(name, version, tracer_provider)
         end
 
         def start_root_span(name, attributes: nil, links: nil, start_timestamp: nil, kind: nil)
-          start_span(name, with_parent_context: Context.empty, attributes: attributes, links: links, start_timestamp: start_timestamp, kind: kind)
+          start_span(name, with_parent: Context.empty, attributes: attributes, links: links, start_timestamp: start_timestamp, kind: kind)
         end
 
-        def start_span(name, with_parent: nil, with_parent_context: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil)
+        def start_span(name, with_parent: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil) # rubocop:disable Metrics/AbcSize
           name ||= 'empty'
 
-          parent_span_context = with_parent&.context || current_span(with_parent_context).context
+          with_parent ||= Context.current
+          parent_span_context = current_span(with_parent).context
           parent_span_context = nil unless parent_span_context.valid?
           parent_span_id = parent_span_context&.span_id
           tracestate = parent_span_context&.tracestate
           trace_id = parent_span_context&.trace_id
           trace_id ||= OpenTelemetry::Trace.generate_trace_id
           sampler = tracer_provider.active_trace_config.sampler
           result = sampler.should_sample?(trace_id: trace_id, parent_context: parent_span_context, links: links, name: name, kind: kind, attributes: attributes)
-          internal_create_span(result, name, kind, trace_id, parent_span_id, attributes, links, start_timestamp, tracestate)
+          internal_create_span(result, name, kind, trace_id, parent_span_id, attributes, links, start_timestamp, tracestate, with_parent)
         end
 
         private
 
-        def internal_create_span(result, name, kind, trace_id, parent_span_id, attributes, links, start_timestamp, tracestate) # rubocop:disable Metrics/AbcSize
+        def internal_create_span(result, name, kind, trace_id, parent_span_id, attributes, links, start_timestamp, tracestate, parent_context) # rubocop:disable Metrics/AbcSize
           if result.recording? && !tracer_provider.stopped?
             trace_flags = result.sampled? ? OpenTelemetry::Trace::TraceFlags::SAMPLED : OpenTelemetry::Trace::TraceFlags::DEFAULT
             context = OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, trace_flags: trace_flags, tracestate: tracestate)
             attributes = attributes&.merge(result.attributes) || result.attributes
             Span.new(
               context,
+              parent_context,
               name,
               kind,
               parent_span_id,

sdk/test/integration/api_trace_test.rb

@@ -57,8 +57,8 @@
     end
 
     before do
-      @remote_span = tracer.start_span('remote', with_parent_context: context_with_remote_parent)
-      @child_of_remote = tracer.start_span('child1', with_parent: @remote_span)
+      @remote_span = tracer.start_span('remote', with_parent: context_with_remote_parent)
+      @child_of_remote = tracer.start_span('child1', with_parent: tracer.context_with_span(@remote_span))
     end
 
     it 'has a child' do
@@ -69,9 +69,9 @@
   describe 'tracing child-of-local spans' do
     before do
       @local_parent_span = tracer.start_span('local')
-
-      tracer.in_span('child1', with_parent: @local_parent_span) do |span|
-        @child_of_local = span
+      parent_ctx = tracer.context_with_span(@local_parent_span)
+      tracer.in_span('child1', with_parent: parent_ctx) do |child|
+        @child_of_local = child
       end
     end
 

sdk/test/integration/global_tracer_configurations_test.rb

@@ -20,7 +20,7 @@
   let(:finished_spans) { exporter.finished_spans }
 
   before do
-    tracer.in_span('root', with_parent_context: parent_context) do
+    tracer.in_span('root', with_parent: parent_context) do
       tracer.in_span('child1') {}
       tracer.in_span('child2') {}
     end

sdk/test/opentelemetry/sdk/trace/export/simple_span_processor_test.rb

@@ -30,10 +30,11 @@ def mock.nil?
 
   let(:stub_span_unrecorded) { stub_span_builder(recording: false) }
   let(:stub_span_recorded)   { stub_span_builder(recording: true) }
+  let(:parent_context) { OpenTelemetry::Context.empty }
   let(:processor) { export::SimpleSpanProcessor.new(mock_span_exporter) }
 
   it 'accepts calls to #on_start' do
-    processor.on_start(stub_span_recorded)
+    processor.on_start(stub_span_recorded, parent_context)
   end
 
   it 'accepts calls to #force_flush' do
@@ -43,13 +44,13 @@ def mock.nil?
   it 'forwards recorded spans from #on_finish' do
     mock_span_exporter.expect :export, export::SUCCESS, [Array]
 
-    processor.on_start(stub_span_recorded)
+    processor.on_start(stub_span_recorded, parent_context)
     processor.on_finish(stub_span_recorded)
     mock_span_exporter.verify
   end
 
   it 'ignores unrecorded spans in #on_finish' do
-    processor.on_start(stub_span_unrecorded)
+    processor.on_start(stub_span_unrecorded, parent_context)
     processor.on_finish(stub_span_unrecorded)
     mock_span_exporter.verify
   end
@@ -67,7 +68,7 @@ def mock.nil?
     mock_span.expect :context, mock_span_context
     mock_span.expect :to_span_data, nil
 
-    processor_noop.on_start(mock_span)
+    processor_noop.on_start(mock_span, parent_context)
     processor_noop.on_finish(mock_span)
     mock_span.verify
   end
@@ -83,7 +84,7 @@ def raising_exporter.export(_)
       raising_exporter
     )
 
-    processor_with_raising_exporter.on_start(stub_span_recorded)
+    processor_with_raising_exporter.on_start(stub_span_recorded, parent_context)
 
     logger_mock = Minitest::Mock.new
     logger_mock.expect :error, nil, [/ArgumentError/]

sdk/test/opentelemetry/sdk/trace/multi_span_processor_test.rb

@@ -10,6 +10,7 @@
   let(:mock_processor1)    { Minitest::Mock.new }
   let(:mock_processor2)    { Minitest::Mock.new }
   let(:span)               { 'dummy span' }
+  let(:context)            { 'dummy context' }
 
   let(:processor) do
     OpenTelemetry::SDK::Trace::MultiSpanProcessor.new(
@@ -18,10 +19,10 @@
   end
 
   it 'implements #on_start' do
-    mock_processor1.expect :on_start, nil, [span]
-    mock_processor2.expect :on_start, nil, [span]
+    mock_processor1.expect :on_start, nil, [span, context]
+    mock_processor2.expect :on_start, nil, [span, context]
 
-    processor.on_start(span)
+    processor.on_start(span, context)
 
     mock_processor1.verify
     mock_processor2.verify