feat: Add Tracer.non_recording_span to API (#799)

* feat: Add Tracer.non_recording_span to API * move non_recording_span to Trace module * do what it says on the tin * Update instrumentation/rack/test/opentelemetry/instrumentation/rack_test.rb Co-authored-by: Robert <robertlaurin@users.noreply.github.com> * Update instrumentation/rack/test/opentelemetry/instrumentation/rack_test.rb Co-authored-by: Robert <robertlaurin@users.noreply.github.com> Co-authored-by: Robert <robertlaurin@users.noreply.github.com>
open-telemetry · Jun 8, 2021 · 2bf0e6a · 2bf0e6a
1 parent 6611787
commit 2bf0e6a
Show file tree

Hide file tree

Showing 28 changed files with 92 additions and 72 deletions.
diff --git a/api/benchmarks/span_bench.rb b/api/benchmarks/span_bench.rb
@@ -5,9 +5,11 @@
 # SPDX-License-Identifier: Apache-2.0
 
 require 'benchmark/ipsa'
-require 'opentelemetry'
+require 'opentelemetry/sdk'
 
-span = OpenTelemetry::Trace::Span.new
+OpenTelemetry::SDK.configure
+tracer = OpenTelemetry.tracer_provider.tracer('bench')
+span = tracer.start_root_span('bench')
 
 attributes = {
   'component' => 'rack',

diff --git a/api/lib/opentelemetry/trace.rb b/api/lib/opentelemetry/trace.rb
@@ -81,6 +81,16 @@ def context_with_span(span, parent_context: Context.current)
     def with_span(span)
       Context.with_value(CURRENT_SPAN_KEY, span) { |c, s| yield s, c }
     end
+
+    # Wraps a SpanContext with an object implementing the Span interface. This is done in order
+    # to expose a SpanContext as a Span in operations such as in-process Span propagation.
+    #
+    # @param [SpanContext] span_context SpanContext to be wrapped
+    #
+    # @return [Span]
+    def non_recording_span(span_context)
+      Span.new(span_context: span_context)
+    end
   end
 end
 

diff --git a/api/lib/opentelemetry/trace/propagation/trace_context/text_map_propagator.rb b/api/lib/opentelemetry/trace/propagation/trace_context/text_map_propagator.rb
@@ -8,7 +8,7 @@ module OpenTelemetry
   module Trace
     module Propagation
       module TraceContext
-        # Propagates baggage using the W3C Trace Context format
+        # Propagates trace context using the W3C Trace Context format
         class TextMapPropagator
           TRACEPARENT_KEY = 'traceparent'
           TRACESTATE_KEY = 'tracestate'
@@ -53,7 +53,7 @@ def extract(carrier, context: Context.current, getter: Context::Propagation.text
                                                   trace_flags: tp.flags,
                                                   tracestate: tracestate,
                                                   remote: true)
-            span = Trace::Span.new(span_context: span_context)
+            span = OpenTelemetry::Trace.non_recording_span(span_context)
             OpenTelemetry::Trace.context_with_span(span)
           rescue OpenTelemetry::Error
             context

diff --git a/api/test/opentelemetry/trace/propagation/trace_context/text_map_propagator_test.rb b/api/test/opentelemetry/trace/propagation/trace_context/text_map_propagator_test.rb
@@ -33,12 +33,12 @@
   let(:context_with_tracestate) do
     span_context = SpanContext.new(trace_id: ("\xff" * 16).b, span_id: ("\x11" * 8).b,
                                    tracestate: tracestate_header)
-    span = Span.new(span_context: span_context)
+    span = OpenTelemetry::Trace.non_recording_span(span_context)
     OpenTelemetry::Trace.context_with_span(span, parent_context: Context.empty)
   end
   let(:context_without_tracestate) do
     span_context = SpanContext.new(trace_id: ("\xff" * 16).b, span_id: ("\x11" * 8).b)
-    span = Span.new(span_context: span_context)
+    span = OpenTelemetry::Trace.non_recording_span(span_context)
     OpenTelemetry::Trace.context_with_span(span, parent_context: Context.empty)
   end
 

diff --git a/api/test/opentelemetry/trace/tracer_test.rb b/api/test/opentelemetry/trace/tracer_test.rb
@@ -31,7 +31,7 @@ def start_span(*)
   let(:parent_span_context) { OpenTelemetry::Trace::SpanContext.new }
   let(:parent_context) do
     OpenTelemetry::Trace.context_with_span(
-      OpenTelemetry::Trace::Span.new(span_context: parent_span_context),
+      OpenTelemetry::Trace.non_recording_span(parent_span_context),
       parent_context: OpenTelemetry::Context.empty
     )
   end

diff --git a/api/test/opentelemetry/trace_test.rb b/api/test/opentelemetry/trace_test.rb
@@ -26,7 +26,7 @@
     end
 
     it 'returns the current span from the provided context' do
-      span = OpenTelemetry::Trace::Span.new(span_context: OpenTelemetry::Trace::SpanContext.new)
+      span = OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new)
       context = OpenTelemetry::Trace.context_with_span(span, parent_context: Context.empty)
       _(OpenTelemetry::Trace.current_span).wont_equal(span)
       _(OpenTelemetry::Trace.current_span(context)).must_equal(span)

diff --git a/api/test/opentelemetry_test.rb b/api/test/opentelemetry_test.rb
@@ -8,6 +8,21 @@
 require 'tempfile'
 
 describe OpenTelemetry do
+  class CustomSpan < OpenTelemetry::Trace::Span
+  end
+
+  class CustomTracer < OpenTelemetry::Trace::Tracer
+    def start_root_span(*)
+      CustomSpan.new
+    end
+  end
+
+  class CustomTracerProvider < OpenTelemetry::Trace::TracerProvider
+    def tracer(name = nil, version = nil)
+      CustomTracer.new
+    end
+  end
+
   describe '.tracer_provider' do
     after do
       # Ensure we don't leak custom tracer factories and tracers to other tests
@@ -24,27 +39,12 @@
     end
 
     it 'returns user specified tracer provider' do
-      custom_tracer_provider = 'a custom tracer provider'
+      custom_tracer_provider = CustomTracerProvider.new
       OpenTelemetry.tracer_provider = custom_tracer_provider
       _(OpenTelemetry.tracer_provider).must_equal(custom_tracer_provider)
     end
   end
 
-  class CustomSpan < OpenTelemetry::Trace::Span
-  end
-
-  class CustomTracer < OpenTelemetry::Trace::Tracer
-    def start_root_span(*)
-      CustomSpan.new
-    end
-  end
-
-  class CustomTracerProvider < OpenTelemetry::Trace::TracerProvider
-    def tracer(name = nil, version = nil)
-      CustomTracer.new
-    end
-  end
-
   describe '.tracer_provider=' do
     after do
       # Ensure we don't leak custom tracer factories and tracers to other tests

diff --git a/common/lib/opentelemetry/common/utilities.rb b/common/lib/opentelemetry/common/utilities.rb
@@ -73,7 +73,7 @@ def truncate(string, size)
       end
 
       def untraced
-        OpenTelemetry::Trace.with_span(OpenTelemetry::Trace::Span.new) { yield }
+        OpenTelemetry::Trace.with_span(OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new)) { yield }
       end
 
       # Returns a URL string with userinfo removed.

diff --git a/instrumentation/rack/test/opentelemetry/instrumentation/rack_test.rb b/instrumentation/rack/test/opentelemetry/instrumentation/rack_test.rb
@@ -10,6 +10,7 @@
 
 describe OpenTelemetry::Instrumentation::Rack do
   let(:instrumentation) { OpenTelemetry::Instrumentation::Rack::Instrumentation.instance }
+  let(:new_span) { OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new) }
 
   it 'has #name' do
     _(instrumentation.name).must_equal 'OpenTelemetry::Instrumentation::Rack'
@@ -32,16 +33,16 @@
     end
 
     it 'returns the span when set' do
-      test_span = OpenTelemetry::Trace::Span.new
+      test_span = new_span
       context = OpenTelemetry::Instrumentation::Rack.context_with_span(test_span)
       _(OpenTelemetry::Instrumentation::Rack.current_span(context)).must_equal(test_span)
     end
   end
 
   describe '#with_span' do
     it 'respects context nesting' do
-      test_span = OpenTelemetry::Trace::Span.new
-      test_span2 = OpenTelemetry::Trace::Span.new
+      test_span = new_span
+      test_span2 = new_span
       OpenTelemetry::Instrumentation::Rack.with_span(test_span) do
         _(OpenTelemetry::Instrumentation::Rack.current_span).must_equal(test_span)
 

diff --git a/propagator/b3/lib/opentelemetry/propagator/b3/text_map_extractor.rb b/propagator/b3/lib/opentelemetry/propagator/b3/text_map_extractor.rb
@@ -82,7 +82,7 @@ def extracted_context(trace_id_hex, span_id_hex, sampled, debug, context)
             remote: true
           )
 
-          span = Trace::Span.new(span_context: span_context)
+          span = OpenTelemetry::Trace.non_recording_span(span_context)
           context = B3.context_with_debug(context) if debug
           Trace.context_with_span(span, parent_context: context)
         end

diff --git a/propagator/b3/test/multi/text_map_propagator_test.rb b/propagator/b3/test/multi/text_map_propagator_test.rb
@@ -199,8 +199,8 @@ def create_context(trace_id:,
                      trace_flags: OpenTelemetry::Trace::TraceFlags::DEFAULT,
                      b3_debug: false)
     context = OpenTelemetry::Trace.context_with_span(
-      OpenTelemetry::Trace::Span.new(
-        span_context: OpenTelemetry::Trace::SpanContext.new(
+      OpenTelemetry::Trace.non_recording_span(
+        OpenTelemetry::Trace::SpanContext.new(
           trace_id: Array(trace_id).pack('H*'),
           span_id: Array(span_id).pack('H*'),
           trace_flags: trace_flags

diff --git a/propagator/b3/test/single/text_map_propagator_test.rb b/propagator/b3/test/single/text_map_propagator_test.rb
@@ -161,8 +161,8 @@ def create_context(trace_id:,
                      trace_flags: OpenTelemetry::Trace::TraceFlags::DEFAULT,
                      b3_debug: false)
     context = OpenTelemetry::Trace.context_with_span(
-      OpenTelemetry::Trace::Span.new(
-        span_context: OpenTelemetry::Trace::SpanContext.new(
+      OpenTelemetry::Trace.non_recording_span(
+        OpenTelemetry::Trace::SpanContext.new(
           trace_id: Array(trace_id).pack('H*'),
           span_id: Array(span_id).pack('H*'),
           trace_flags: trace_flags

diff --git a/propagator/jaeger/lib/opentelemetry/propagator/jaeger/text_map_propagator.rb b/propagator/jaeger/lib/opentelemetry/propagator/jaeger/text_map_propagator.rb
@@ -98,7 +98,7 @@ def build_span(match, sampling_flags)
             trace_flags: to_trace_flags(sampling_flags),
             remote: true
           )
-          Trace::Span.new(span_context: span_context)
+          OpenTelemetry::Trace.non_recording_span(span_context)
         end
 
         def context_with_extracted_baggage(carrier, context, getter)

diff --git a/propagator/jaeger/test/text_map_propagator_test.rb b/propagator/jaeger/test/text_map_propagator_test.rb
@@ -166,8 +166,8 @@ def create_context(trace_id:,
                        trace_flags: OpenTelemetry::Trace::TraceFlags::DEFAULT,
                        jaeger_debug: false)
       context = OpenTelemetry::Trace.context_with_span(
-        OpenTelemetry::Trace::Span.new(
-          span_context: OpenTelemetry::Trace::SpanContext.new(
+        OpenTelemetry::Trace.non_recording_span(
+          OpenTelemetry::Trace::SpanContext.new(
             trace_id: Array(trace_id).pack('H*'),
             span_id: Array(span_id).pack('H*'),
             trace_flags: trace_flags

diff --git a/propagator/ottrace/lib/opentelemetry/propagator/ottrace/text_map_propagator.rb b/propagator/ottrace/lib/opentelemetry/propagator/ottrace/text_map_propagator.rb
@@ -60,7 +60,7 @@ def extract(carrier, context: Context.current, getter: Context::Propagation.text
             remote: true
           )
 
-          span = Trace::Span.new(span_context: span_context)
+          span = OpenTelemetry::Trace.non_recording_span(span_context)
           Trace.context_with_span(span, parent_context: set_baggage(carrier: carrier, context: context, getter: getter))
         end
 

diff --git a/propagator/ottrace/test/opentelemetry/propagator/ottrace/text_map_propagator_test.rb b/propagator/ottrace/test/opentelemetry/propagator/ottrace/text_map_propagator_test.rb
@@ -46,8 +46,8 @@ def set(carrier, key, value)
 
   let(:context) do
     OpenTelemetry::Trace.context_with_span(
-      OpenTelemetry::Trace::Span.new(
-        span_context: OpenTelemetry::Trace::SpanContext.new(
+      OpenTelemetry::Trace.non_recording_span(
+        OpenTelemetry::Trace::SpanContext.new(
           trace_id: Array(trace_id).pack('H*'),
           span_id: Array(span_id).pack('H*'),
           trace_flags: trace_flags

diff --git a/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb b/propagator/xray/lib/opentelemetry/propagator/xray/text_map_propagator.rb
@@ -49,7 +49,7 @@ def extract(carrier, context: Context.current, getter: Context::Propagation.text
             remote: true
           )
 
-          span = Trace::Span.new(span_context: span_context)
+          span = OpenTelemetry::Trace.non_recording_span(span_context)
           context = XRay.context_with_debug(context) if match['sampling_state'] == 'd'
           Trace.context_with_span(span, parent_context: context)
         rescue OpenTelemetry::Error

diff --git a/propagator/xray/test/text_map_propagator_test.rb b/propagator/xray/test/text_map_propagator_test.rb
@@ -156,8 +156,8 @@ def create_context(trace_id:,
                      trace_flags: TraceFlags::DEFAULT,
                      xray_debug: false)
     context = OpenTelemetry::Trace.context_with_span(
-      Span.new(
-        span_context: SpanContext.new(
+      OpenTelemetry::Trace.non_recording_span(
+        SpanContext.new(
           trace_id: Array(trace_id).pack('H*'),
           span_id: Array(span_id).pack('H*'),
           trace_flags: trace_flags

diff --git a/sdk/lib/opentelemetry/sdk/trace/export/multi_span_exporter.rb b/sdk/lib/opentelemetry/sdk/trace/export/multi_span_exporter.rb
@@ -9,7 +9,7 @@ module SDK
     module Trace
       module Export
         # Implementation of the SpanExporter duck type that simply forwards all
-        # received spans to a collection of SpanExporters.
+        # received SpanDatas to a collection of SpanExporters.
         #
         # Can be used to export to multiple backends using the same
         # SpanProcessor like a {SimpleSpanProcessor} or a
@@ -19,9 +19,10 @@ def initialize(span_exporters)
             @span_exporters = span_exporters.clone.freeze
           end
 
-          # Called to export sampled {Span}s.
+          # Called to export sampled {SpanData} structs.
           #
-          # @param [Enumerable<Span>] spans the list of sampled {Span}s to be
+          # @param [Enumerable<SpanData>] span_data the
+          #   list of recorded {SpanData} structs to be
           #   exported.
           # @param [optional Numeric] timeout An optional timeout in seconds.
           # @return [Integer] the result of the export.
@@ -44,11 +45,11 @@ def export(spans, timeout: nil)
           #   non-specific failure occurred, TIMEOUT if a timeout occurred.
           def force_flush(timeout: nil)
             start_time = OpenTelemetry::Common::Utilities.timeout_timestamp
-            results = @span_exporters.map do |processor|
+            results = @span_exporters.map do |span_exporter|
               remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)
               return TIMEOUT if remaining_timeout&.zero?
 
-              processor.force_flush(timeout: remaining_timeout)
+              span_exporter.force_flush(timeout: remaining_timeout)
             end
             results.uniq.max || SUCCESS
           end
@@ -61,11 +62,11 @@ def force_flush(timeout: nil)
           #   non-specific failure occurred, TIMEOUT if a timeout occurred.
           def shutdown(timeout: nil)
             start_time = OpenTelemetry::Common::Utilities.timeout_timestamp
-            results = @span_exporters.map do |processor|
+            results = @span_exporters.map do |span_exporter|
               remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)
               return TIMEOUT if remaining_timeout&.zero?
 
-              processor.shutdown(timeout: remaining_timeout)
+              span_exporter.shutdown(timeout: remaining_timeout)
             end
             results.uniq.max || SUCCESS
           end

diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer.rb b/sdk/lib/opentelemetry/sdk/trace/tracer.rb
@@ -71,7 +71,7 @@ def internal_create_span(result, name, kind, trace_id, parent_span_id, attribute
               @instrumentation_library
             )
           else
-            OpenTelemetry::Trace::Span.new(span_context: OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, tracestate: result.tracestate))
+            OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, tracestate: result.tracestate))
           end
         end
       end

diff --git a/sdk/test/integration/api_trace_test.rb b/sdk/test/integration/api_trace_test.rb
@@ -51,7 +51,7 @@
   describe 'tracing child-of-remote spans' do
     let(:context_with_remote_parent) do
       OpenTelemetry::Trace.context_with_span(
-        OpenTelemetry::Trace::Span.new(span_context: remote_span_context),
+        OpenTelemetry::Trace.non_recording_span(remote_span_context),
         parent_context: OpenTelemetry::Context.empty
       )
     end

diff --git a/sdk/test/integration/global_tracer_configurations_test.rb b/sdk/test/integration/global_tracer_configurations_test.rb
@@ -57,7 +57,7 @@
     let(:parent_span_context) { OpenTelemetry::Trace::SpanContext.new(tracestate: mock_tracestate, trace_flags: OpenTelemetry::Trace::TraceFlags::SAMPLED) }
     let(:parent_context) do
       OpenTelemetry::Trace.context_with_span(
-        OpenTelemetry::Trace::Span.new(span_context: parent_span_context),
+        OpenTelemetry::Trace.non_recording_span(parent_span_context),
         parent_context: OpenTelemetry::Context.empty
       )
     end

diff --git a/sdk/test/opentelemetry/sdk/trace/export/console_span_exporter_test.rb b/sdk/test/opentelemetry/sdk/trace/export/console_span_exporter_test.rb
@@ -10,8 +10,10 @@
   export = OpenTelemetry::SDK::Trace::Export
 
   let(:captured_stdout) { StringIO.new }
-  let(:spans)    { [OpenTelemetry::Trace::Span.new, OpenTelemetry::Trace::Span.new] }
-  let(:exporter) { export::ConsoleSpanExporter.new }
+  let(:span_data1) { OpenTelemetry::SDK::Trace::SpanData.new(name: 'name1') }
+  let(:span_data2) { OpenTelemetry::SDK::Trace::SpanData.new(name: 'name2') }
+  let(:spans)      { [span_data1, span_data2] }
+  let(:exporter)   { export::ConsoleSpanExporter.new }
 
   before do
     @original_stdout = $stdout
@@ -22,11 +24,11 @@
     $stdout = @original_stdout
   end
 
-  it 'accepts an Array of Spans as arg to #export and succeeds' do
+  it 'accepts an Array of SpanData as arg to #export and succeeds' do
     _(exporter.export(spans)).must_equal export::SUCCESS
   end
 
-  it 'accepts an Enumerable of Spans as arg to #export and succeeds' do
+  it 'accepts an Enumerable of SpanData as arg to #export and succeeds' do
     enumerable = Struct.new(:span0, :span1).new(spans[0], spans[1])
 
     _(exporter.export(enumerable)).must_equal export::SUCCESS
@@ -35,7 +37,7 @@
   it 'outputs to console (stdout)' do
     exporter.export(spans)
 
-    _(captured_stdout.string).must_match(/#<OpenTelemetry::Trace::Span:/)
+    _(captured_stdout.string).must_match(/#<struct OpenTelemetry::SDK::Trace::SpanData/)
   end
 
   it 'accepts calls to #force_flush' do
-Original file line number
+Diff line change
@@ Expand Up @@
                   @instrumentation_library
                 )
               else
-                OpenTelemetry::Trace::Span.new(span_context: OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, tracestate: result.tracestate))
+                OpenTelemetry::Trace.non_recording_span(OpenTelemetry::Trace::SpanContext.new(trace_id: trace_id, span_id: span_id, tracestate: result.tracestate))
               end
             end
           end
@@ Expand Down @@