From 99c8a86fa97ba98f7750cedf19570e6f2b475943 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 17 Sep 2020 18:36:50 -0700 Subject: [PATCH 1/8] refactor: remove with_parent (span) option to start_span (and friends) --- api/benchmarks/tracer_bench.rb | 5 ----- api/lib/opentelemetry/trace/tracer.rb | 13 ++++--------- api/test/opentelemetry/trace/tracer_test.rb | 13 ------------- .../opentelemetry/exporter/otlp/exporter_test.rb | 11 +++++++---- sdk/lib/opentelemetry/sdk/trace/tracer.rb | 4 ++-- sdk/test/integration/api_trace_test.rb | 13 ++++++++----- sdk/test/opentelemetry/sdk/trace/tracer_test.rb | 5 +++-- 7 files changed, 24 insertions(+), 40 deletions(-) diff --git a/api/benchmarks/tracer_bench.rb b/api/benchmarks/tracer_bench.rb index 5c152b2e0..f009ba796 100644 --- a/api/benchmarks/tracer_bench.rb +++ b/api/benchmarks/tracer_bench.rb @@ -31,11 +31,6 @@ 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.finish diff --git a/api/lib/opentelemetry/trace/tracer.rb b/api/lib/opentelemetry/trace/tracer.rb index 117f95290..80e1c10fe 100644 --- a/api/lib/opentelemetry/trace/tracer.rb +++ b/api/lib/opentelemetry/trace/tracer.rb @@ -36,9 +36,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_context: 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_context: with_parent_context) with_span(span) { |s, c| yield s, c } rescue Exception => e # rubocop:disable Lint/RescueException span&.record_exception(e) @@ -69,14 +69,9 @@ 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+. - # # @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_context: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil) + span_context = current_span(with_parent_context).context if span_context.valid? Span.new(span_context: span_context) else diff --git a/api/test/opentelemetry/trace/tracer_test.rb b/api/test/opentelemetry/trace/tracer_test.rb index 7c11daff4..1d3991876 100644 --- a/api/test/opentelemetry/trace/tracer_test.rb +++ b/api/test/opentelemetry/trace/tracer_test.rb @@ -91,13 +91,6 @@ 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| _(span.context).must_be :valid? @@ -162,12 +155,6 @@ 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.context).must_be :valid? diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb index e3f645142..72d198752 100644 --- a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb +++ b/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.with_span(root) { |_, ctx| ctx } + span = with_ids(trace_id, child_span_id) { tracer.start_span('child', with_parent_context: 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.with_span(span) { |_, ctx| ctx } + client = with_ids(trace_id, client_span_id) { tracer.start_span('client', with_parent_context: child_ctx, kind: :client, start_timestamp: start_timestamp + 2).finish(end_timestamp: end_timestamp) } + client_ctx = tracer.with_span(client) { |_, ctx| ctx } + with_ids(trace_id, server_span_id) { other_tracer.start_span('server', with_parent_context: 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_context: child_ctx, kind: :consumer, start_timestamp: start_timestamp + 5).finish(end_timestamp: end_timestamp) } span.finish(end_timestamp: end_timestamp) OpenTelemetry.tracer_provider.shutdown diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer.rb b/sdk/lib/opentelemetry/sdk/trace/tracer.rb index 923d567a7..a1340c447 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer.rb @@ -33,10 +33,10 @@ def start_root_span(name, attributes: nil, links: nil, start_timestamp: nil, kin start_span(name, with_parent_context: 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_context: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil) name ||= 'empty' - parent_span_context = with_parent&.context || current_span(with_parent_context).context + parent_span_context = current_span(with_parent_context).context parent_span_context = nil unless parent_span_context.valid? parent_span_id = parent_span_context&.span_id tracestate = parent_span_context&.tracestate diff --git a/sdk/test/integration/api_trace_test.rb b/sdk/test/integration/api_trace_test.rb index 5dc836ec0..c6c0b84cc 100644 --- a/sdk/test/integration/api_trace_test.rb +++ b/sdk/test/integration/api_trace_test.rb @@ -58,7 +58,9 @@ 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) + tracer.with_span(@remote_span) do |_, ctx| + @child_of_remote = tracer.start_span('child1', with_parent_context: ctx) + end end it 'has a child' do @@ -68,10 +70,11 @@ 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 + tracer.in_span('local') do |parent| + @local_parent_span = parent + tracer.in_span('child1') do |child| + @child_of_local = child + end end end diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb index a6cb95838..c18eca574 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb @@ -252,9 +252,10 @@ _(span.attributes).must_equal('1' => 1, '2' => 2) end - it 'uses the context from parent span if supplied' do + it 'uses the context from parent if supplied' do parent = tracer.start_root_span('root') - span = tracer.start_span('child', with_parent: parent) + parent_ctx = tracer.with_span(parent) { |_, ctx| ctx } + span = tracer.start_span('child', with_parent_context: parent_ctx) _(span.parent_span_id).must_equal(parent.context.span_id) _(span.context.trace_id).must_equal(parent.context.trace_id) end From 4e8668a241b38559be4a938bc362f8650b2816aa Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 17 Sep 2020 18:40:52 -0700 Subject: [PATCH 2/8] refactor: rename with_parent_context to with_parent (context) --- api/benchmarks/tracer_bench.rb | 2 +- api/lib/opentelemetry/trace/tracer.rb | 9 +++--- api/test/opentelemetry/trace/tracer_test.rb | 6 ++-- examples/http/server.rb | 2 +- .../exporter/otlp/exporter_test.rb | 8 ++--- .../rack/middlewares/tracer_middleware.rb | 2 +- .../middlewares/server/tracer_middleware.rb | 2 +- .../sinatra/middlewares/tracer_middleware.rb | 2 +- sdk/lib/opentelemetry/sdk/trace/tracer.rb | 6 ++-- sdk/test/integration/api_trace_test.rb | 4 +-- .../global_tracer_configurations_test.rb | 2 +- .../opentelemetry/sdk/trace/tracer_test.rb | 32 +++++++++---------- 12 files changed, 39 insertions(+), 38 deletions(-) diff --git a/api/benchmarks/tracer_bench.rb b/api/benchmarks/tracer_bench.rb index f009ba796..686f1e8f3 100644 --- a/api/benchmarks/tracer_bench.rb +++ b/api/benchmarks/tracer_bench.rb @@ -32,7 +32,7 @@ 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_span.context) span.finish end diff --git a/api/lib/opentelemetry/trace/tracer.rb b/api/lib/opentelemetry/trace/tracer.rb index 80e1c10fe..f1fd19854 100644 --- a/api/lib/opentelemetry/trace/tracer.rb +++ b/api/lib/opentelemetry/trace/tracer.rb @@ -36,9 +36,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_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_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) @@ -70,8 +70,9 @@ 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. # # @return [Span] - def start_span(name, with_parent_context: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil) - span_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 diff --git a/api/test/opentelemetry/trace/tracer_test.rb b/api/test/opentelemetry/trace/tracer_test.rb index 1d3991876..b65b9516d 100644 --- a/api/test/opentelemetry/trace/tracer_test.rb +++ b/api/test/opentelemetry/trace/tracer_test.rb @@ -92,7 +92,7 @@ def start_span(*) 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 @@ -144,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 @@ -156,7 +156,7 @@ def start_span(*) 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 diff --git a/examples/http/server.rb b/examples/http/server.rb index 3b2d0e980..f9abb743f 100755 --- a/examples/http/server.rb +++ b/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) diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb index 72d198752..3cc65cfce 100644 --- a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb +++ b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb @@ -155,7 +155,7 @@ 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) } root_ctx = tracer.with_span(root) { |_, ctx| ctx } - span = with_ids(trace_id, child_span_id) { tracer.start_span('child', with_parent_context: root_ctx, kind: :producer, start_timestamp: start_timestamp + 1, links: [OpenTelemetry::Trace::Link.new(root.context, 'attr' => 4)]) } + 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 @@ -163,11 +163,11 @@ span['a'] = [3, 4] span.status = OpenTelemetry::Trace::Status.new(OpenTelemetry::Trace::Status::ERROR) child_ctx = tracer.with_span(span) { |_, ctx| ctx } - client = with_ids(trace_id, client_span_id) { tracer.start_span('client', with_parent_context: child_ctx, kind: :client, start_timestamp: start_timestamp + 2).finish(end_timestamp: end_timestamp) } + 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.with_span(client) { |_, ctx| ctx } - with_ids(trace_id, server_span_id) { other_tracer.start_span('server', with_parent_context: client_ctx, kind: :server, start_timestamp: start_timestamp + 3).finish(end_timestamp: end_timestamp) } + 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_context: child_ctx, 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 diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb index dfc790c4f..d7273914e 100644 --- a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb +++ b/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 }, diff --git a/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/server/tracer_middleware.rb b/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/server/tracer_middleware.rb index 4cde99b80..e7f116356 100644 --- a/instrumentation/sidekiq/lib/opentelemetry/instrumentation/sidekiq/middlewares/server/tracer_middleware.rb +++ b/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']) diff --git a/instrumentation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb b/instrumentation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb index b4f7b5e66..518c728b3 100644 --- a/instrumentation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb +++ b/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 diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer.rb b/sdk/lib/opentelemetry/sdk/trace/tracer.rb index a1340c447..27743653c 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer.rb @@ -30,13 +30,13 @@ 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_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) name ||= 'empty' - parent_span_context = current_span(with_parent_context).context + 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 diff --git a/sdk/test/integration/api_trace_test.rb b/sdk/test/integration/api_trace_test.rb index c6c0b84cc..122e6a4cd 100644 --- a/sdk/test/integration/api_trace_test.rb +++ b/sdk/test/integration/api_trace_test.rb @@ -57,9 +57,9 @@ end before do - @remote_span = tracer.start_span('remote', with_parent_context: context_with_remote_parent) + @remote_span = tracer.start_span('remote', with_parent: context_with_remote_parent) tracer.with_span(@remote_span) do |_, ctx| - @child_of_remote = tracer.start_span('child1', with_parent_context: ctx) + @child_of_remote = tracer.start_span('child1', with_parent: ctx) end end diff --git a/sdk/test/integration/global_tracer_configurations_test.rb b/sdk/test/integration/global_tracer_configurations_test.rb index a8c6f1c39..ad1526264 100644 --- a/sdk/test/integration/global_tracer_configurations_test.rb +++ b/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 diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb index c18eca574..a958606e2 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_test.rb @@ -155,46 +155,46 @@ end it 'provides a default name' do - _(tracer.start_span(nil, with_parent_context: context).name).wont_be_nil + _(tracer.start_span(nil, with_parent: context).name).wont_be_nil end it 'returns a valid span' do - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context).must_be :valid? end it 'returns a span with the same trace ID as the parent context' do - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context.trace_id).must_equal(span_context.trace_id) end it 'returns a span with the parent context span ID' do - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.parent_span_id).must_equal(span_context.span_id) end it 'returns a span with the parent context tracestate' do - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context.tracestate).must_equal(span_context.tracestate) end it 'returns a no-op span if sampler says do not record events' do activate_trace_config TraceConfig.new(sampler: Samplers::ALWAYS_OFF) - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context.trace_flags).wont_be :sampled? _(span).wont_be :recording? end it 'returns an unsampled span if sampler says record, but do not sample' do activate_trace_config TraceConfig.new(sampler: record_sampler) - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context.trace_flags).wont_be :sampled? _(span).must_be :recording? end it 'returns a sampled span if sampler says sample' do activate_trace_config TraceConfig.new(sampler: Samplers::ALWAYS_ON) - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context.trace_flags).must_be :sampled? _(span).must_be :recording? end @@ -208,20 +208,20 @@ mock_sampler = Minitest::Mock.new mock_sampler.expect(:should_sample?, result, [{ trace_id: span_context.trace_id, parent_context: span_context, links: links, name: name, kind: kind, attributes: attributes }]) activate_trace_config TraceConfig.new(sampler: mock_sampler) - tracer.start_span(name, with_parent_context: context, attributes: attributes, links: links, kind: kind) + tracer.start_span(name, with_parent: context, attributes: attributes, links: links, kind: kind) mock_sampler.verify end it 'returns a no-op span with parent trace ID if tracer has shutdown' do tracer_provider.shutdown - span = tracer.start_span('op', with_parent_context: context) + span = tracer.start_span('op', with_parent: context) _(span.context.trace_flags).wont_be :sampled? _(span).wont_be :recording? _(span.context.trace_id).must_equal(span_context.trace_id) end it 'creates a span with the provided instrumentation library' do - span = tracer.start_span('span', with_parent_context: context) + span = tracer.start_span('span', with_parent: context) _(span.instrumentation_library.name).must_equal('component-tracer') _(span.instrumentation_library.version).must_equal('1.0.0') end @@ -232,7 +232,7 @@ kind = OpenTelemetry::Trace::SpanKind::INTERNAL attributes = { '1' => 1 } start_timestamp = Time.now - span = tracer.start_span(name, with_parent_context: context, attributes: attributes, links: links, kind: kind, start_timestamp: start_timestamp) + span = tracer.start_span(name, with_parent: context, attributes: attributes, links: links, kind: kind, start_timestamp: start_timestamp) _(span.links).must_equal(links) _(span.name).must_equal(name) _(span.kind).must_equal(kind) @@ -248,14 +248,14 @@ result = Result.new(decision: Decision::RECORD_ONLY, attributes: sampler_attributes) mock_sampler.expect(:should_sample?, result, [Hash]) activate_trace_config TraceConfig.new(sampler: mock_sampler) - span = tracer.start_span('op', with_parent_context: context, attributes: { '1' => 0, '2' => 2 }) + span = tracer.start_span('op', with_parent: context, attributes: { '1' => 0, '2' => 2 }) _(span.attributes).must_equal('1' => 1, '2' => 2) end it 'uses the context from parent if supplied' do parent = tracer.start_root_span('root') parent_ctx = tracer.with_span(parent) { |_, ctx| ctx } - span = tracer.start_span('child', with_parent_context: parent_ctx) + span = tracer.start_span('child', with_parent: parent_ctx) _(span.parent_span_id).must_equal(parent.context.span_id) _(span.context.trace_id).must_equal(parent.context.trace_id) end @@ -274,7 +274,7 @@ it 'trims link attributes' do activate_trace_config TraceConfig.new(max_attributes_per_link: 1) link = OpenTelemetry::Trace::Link.new(OpenTelemetry::Trace::SpanContext.new, '1' => 1, '2' => 2) - span = tracer.start_span('op', with_parent_context: context, links: [link]) + span = tracer.start_span('op', with_parent: context, links: [link]) _(span.links.first.attributes.size).must_equal(1) end @@ -282,7 +282,7 @@ activate_trace_config TraceConfig.new(max_links_count: 1) link1 = OpenTelemetry::Trace::Link.new(OpenTelemetry::Trace::SpanContext.new, '1' => 1) link2 = OpenTelemetry::Trace::Link.new(OpenTelemetry::Trace::SpanContext.new, '2' => 2) - span = tracer.start_span('op', with_parent_context: context, links: [link1, link2]) + span = tracer.start_span('op', with_parent: context, links: [link1, link2]) _(span.links.size).must_equal(1) _(span.links.first).must_equal(link2) end From 9071e19bcb4ad697bda8eaf33cb9d66440395798 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 17 Sep 2020 20:10:20 -0700 Subject: [PATCH 3/8] docs: restore with_parent tracer docs --- api/lib/opentelemetry/trace/tracer.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/lib/opentelemetry/trace/tracer.rb b/api/lib/opentelemetry/trace/tracer.rb index f1fd19854..8388d6257 100644 --- a/api/lib/opentelemetry/trace/tracer.rb +++ b/api/lib/opentelemetry/trace/tracer.rb @@ -69,6 +69,8 @@ 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 Context] with_parent Explicitly managed parent context + # # @return [Span] def start_span(name, with_parent: nil, attributes: nil, links: nil, start_timestamp: nil, kind: nil) span_context = current_span(with_parent).context From b53b1ed3e767b0c945d47fcb5276d71078b76588 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 17 Sep 2020 20:20:50 -0700 Subject: [PATCH 4/8] fix: resurrect tracer benchmarks --- api/benchmarks/tracer_bench.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/benchmarks/tracer_bench.rb b/api/benchmarks/tracer_bench.rb index 686f1e8f3..c197237a8 100644 --- a/api/benchmarks/tracer_bench.rb +++ b/api/benchmarks/tracer_bench.rb @@ -10,6 +10,7 @@ tracer = OpenTelemetry::Trace::Tracer.new parent_span = tracer.start_span('parent') +parent_context = tracer.with_span(parent_span) { |_, c| c } attributes = { 'component' => 'rack', @@ -32,7 +33,7 @@ end x.report 'start span with parent context' do - span = tracer.start_span('test_span', with_parent: parent_span.context) + span = tracer.start_span('test_span', with_parent: parent_context) span.finish end From 13ce04cfc894bf92b0ce61a2130cbdcd7b6ec5e1 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 17 Sep 2020 20:41:05 -0700 Subject: [PATCH 5/8] feat: add Tracer#context_with_span convenience method --- api/lib/opentelemetry/trace/tracer.rb | 9 +++++++++ api/test/opentelemetry/trace/tracer_test.rb | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/api/lib/opentelemetry/trace/tracer.rb b/api/lib/opentelemetry/trace/tracer.rb index 8388d6257..addc10e22 100644 --- a/api/lib/opentelemetry/trace/tracer.rb +++ b/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, context = Context.current) + 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: diff --git a/api/test/opentelemetry/trace/tracer_test.rb b/api/test/opentelemetry/trace/tracer_test.rb index b65b9516d..f915d761a 100644 --- a/api/test/opentelemetry/trace/tracer_test.rb +++ b/api/test/opentelemetry/trace/tracer_test.rb @@ -161,4 +161,20 @@ def start_span(*) _(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_ctx) + _(tracer.current_span(ctx)).must_equal(span) + _(ctx.value('foo')).must_equal('bar') + end + end end From d5e20f08974c37ad41e0584e507d3b81dc124b0d Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Thu, 17 Sep 2020 20:52:14 -0700 Subject: [PATCH 6/8] refactor: update tests to use context_with_span convenience method --- api/benchmarks/tracer_bench.rb | 2 +- .../opentelemetry/exporter/otlp/exporter_test.rb | 6 +++--- sdk/test/integration/api_trace_test.rb | 13 +++++-------- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/api/benchmarks/tracer_bench.rb b/api/benchmarks/tracer_bench.rb index c197237a8..5a0f71401 100644 --- a/api/benchmarks/tracer_bench.rb +++ b/api/benchmarks/tracer_bench.rb @@ -10,7 +10,7 @@ tracer = OpenTelemetry::Trace::Tracer.new parent_span = tracer.start_span('parent') -parent_context = tracer.with_span(parent_span) { |_, c| c } +parent_context = tracer.context_with_span(parent_span) attributes = { 'component' => 'rack', diff --git a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb index 3cc65cfce..7ff9b7ef4 100644 --- a/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb +++ b/exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb @@ -154,7 +154,7 @@ 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) } - root_ctx = tracer.with_span(root) { |_, ctx| ctx } + 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 @@ -162,9 +162,9 @@ span['s'] = 'val' span['a'] = [3, 4] span.status = OpenTelemetry::Trace::Status.new(OpenTelemetry::Trace::Status::ERROR) - child_ctx = tracer.with_span(span) { |_, ctx| ctx } + 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.with_span(client) { |_, ctx| ctx } + 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: child_ctx, kind: :consumer, start_timestamp: start_timestamp + 5).finish(end_timestamp: end_timestamp) } diff --git a/sdk/test/integration/api_trace_test.rb b/sdk/test/integration/api_trace_test.rb index 122e6a4cd..c98d86878 100644 --- a/sdk/test/integration/api_trace_test.rb +++ b/sdk/test/integration/api_trace_test.rb @@ -58,9 +58,7 @@ before do @remote_span = tracer.start_span('remote', with_parent: context_with_remote_parent) - tracer.with_span(@remote_span) do |_, ctx| - @child_of_remote = tracer.start_span('child1', with_parent: ctx) - end + @child_of_remote = tracer.start_span('child1', with_parent: tracer.context_with_span(@remote_span)) end it 'has a child' do @@ -70,11 +68,10 @@ describe 'tracing child-of-local spans' do before do - tracer.in_span('local') do |parent| - @local_parent_span = parent - tracer.in_span('child1') do |child| - @child_of_local = child - end + @local_parent_span = tracer.start_span('local') + 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 From 301c4ae7c9777849490df7b05727920818689261 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Tue, 22 Sep 2020 16:29:00 -0700 Subject: [PATCH 7/8] feat: pass parent context to span processors --- .../sdk/trace/export/batch_span_processor.rb | 2 +- .../sdk/trace/export/simple_span_processor.rb | 4 +++- .../sdk/trace/multi_span_processor.rb | 6 ++++-- .../sdk/trace/noop_span_processor.rb | 4 +++- sdk/lib/opentelemetry/sdk/trace/span.rb | 4 ++-- sdk/lib/opentelemetry/sdk/trace/tracer.rb | 8 +++++--- .../trace/export/simple_span_processor_test.rb | 11 ++++++----- .../sdk/trace/multi_span_processor_test.rb | 7 ++++--- .../sdk/trace/noop_span_processor_test.rb | 3 ++- sdk/test/opentelemetry/sdk/trace/span_test.rb | 16 +++++++++------- .../sdk/trace/tracer_provider_test.rb | 4 ++-- 11 files changed, 41 insertions(+), 28 deletions(-) diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb index a67b0e92e..543d95e09 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb +++ b/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 diff --git a/sdk/lib/opentelemetry/sdk/trace/export/simple_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/simple_span_processor.rb index 2150f7ffc..35d9993bc 100644 --- a/sdk/lib/opentelemetry/sdk/trace/export/simple_span_processor.rb +++ b/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 diff --git a/sdk/lib/opentelemetry/sdk/trace/multi_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/multi_span_processor.rb index 2cc585196..282ae5c0f 100644 --- a/sdk/lib/opentelemetry/sdk/trace/multi_span_processor.rb +++ b/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?} diff --git a/sdk/lib/opentelemetry/sdk/trace/noop_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/noop_span_processor.rb index e71d579af..b9d9a60aa 100644 --- a/sdk/lib/opentelemetry/sdk/trace/noop_span_processor.rb +++ b/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. diff --git a/sdk/lib/opentelemetry/sdk/trace/span.rb b/sdk/lib/opentelemetry/sdk/trace/span.rb index 546cfd71c..180206e60 100644 --- a/sdk/lib/opentelemetry/sdk/trace/span.rb +++ b/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. diff --git a/sdk/lib/opentelemetry/sdk/trace/tracer.rb b/sdk/lib/opentelemetry/sdk/trace/tracer.rb index 27743653c..1d85bf743 100644 --- a/sdk/lib/opentelemetry/sdk/trace/tracer.rb +++ b/sdk/lib/opentelemetry/sdk/trace/tracer.rb @@ -33,9 +33,10 @@ def start_root_span(name, attributes: nil, links: nil, start_timestamp: nil, kin 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, 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' + 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 @@ -44,18 +45,19 @@ def start_span(name, with_parent: nil, attributes: nil, links: nil, start_timest 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, diff --git a/sdk/test/opentelemetry/sdk/trace/export/simple_span_processor_test.rb b/sdk/test/opentelemetry/sdk/trace/export/simple_span_processor_test.rb index 65d137af6..37104e5ef 100644 --- a/sdk/test/opentelemetry/sdk/trace/export/simple_span_processor_test.rb +++ b/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/] diff --git a/sdk/test/opentelemetry/sdk/trace/multi_span_processor_test.rb b/sdk/test/opentelemetry/sdk/trace/multi_span_processor_test.rb index df311e04a..65af4c87a 100644 --- a/sdk/test/opentelemetry/sdk/trace/multi_span_processor_test.rb +++ b/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 diff --git a/sdk/test/opentelemetry/sdk/trace/noop_span_processor_test.rb b/sdk/test/opentelemetry/sdk/trace/noop_span_processor_test.rb index 1c83fa547..6318af09f 100644 --- a/sdk/test/opentelemetry/sdk/trace/noop_span_processor_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/noop_span_processor_test.rb @@ -9,9 +9,10 @@ describe OpenTelemetry::SDK::Trace::NoopSpanProcessor do let(:processor) { OpenTelemetry::SDK::Trace::NoopSpanProcessor.instance } let(:span) { nil } + let(:context) { nil } it 'implements #on_start' do - processor.on_start(span) + processor.on_start(span, context) end it 'implements #on_finish' do diff --git a/sdk/test/opentelemetry/sdk/trace/span_test.rb b/sdk/test/opentelemetry/sdk/trace/span_test.rb index c23c902c6..cef4c54ac 100644 --- a/sdk/test/opentelemetry/sdk/trace/span_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/span_test.rb @@ -12,6 +12,7 @@ NoopSpanProcessor = OpenTelemetry::SDK::Trace::NoopSpanProcessor SpanKind = OpenTelemetry::Trace::SpanKind Status = OpenTelemetry::Trace::Status + Context = OpenTelemetry::Context let(:context) { OpenTelemetry::Trace::SpanContext.new } let(:span_processor) { NoopSpanProcessor.instance } @@ -26,7 +27,7 @@ ) end let(:span) do - Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + Span.new(context, Context.empty, 'name', SpanKind::INTERNAL, nil, trace_config, span_processor, nil, nil, Time.now, nil, nil) end @@ -253,7 +254,8 @@ it 'calls the span processor #on_finish callback' do mock_span_processor.expect(:on_start, nil) { |_| true } - span = Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + span = Span.new(context, Context.empty, + 'name', SpanKind::INTERNAL, nil, trace_config, mock_span_processor, nil, nil, Time.now, nil, nil) mock_span_processor.expect(:on_finish, nil, [span]) span.finish @@ -282,7 +284,7 @@ it 'calls the span processor #on_start callback' do yielded_span = nil mock_span_processor.expect(:on_start, nil) { |s| yielded_span = s } - span = Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + span = Span.new(context, Context.empty, 'name', SpanKind::INTERNAL, nil, trace_config, mock_span_processor, nil, nil, Time.now, nil, nil) _(yielded_span).must_equal(span) mock_span_processor.verify @@ -290,7 +292,7 @@ it 'trims excess attributes' do attributes = { 'foo': 'bar', 'other': 'attr' } - span = Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + span = Span.new(context, Context.empty, 'name', SpanKind::INTERNAL, nil, trace_config, span_processor, attributes, nil, Time.now, nil, nil) _(span.to_span_data.total_recorded_attributes).must_equal(2) _(span.attributes.length).must_equal(1) @@ -298,21 +300,21 @@ it 'counts attributes' do attributes = { 'foo': 'bar', 'other': 'attr' } - span = Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + span = Span.new(context, Context.empty, 'name', SpanKind::INTERNAL, nil, trace_config, span_processor, attributes, nil, Time.now, nil, nil) _(span.to_span_data.total_recorded_attributes).must_equal(2) end it 'counts links' do links = [OpenTelemetry::Trace::Link.new(context), OpenTelemetry::Trace::Link.new(context)] - span = Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + span = Span.new(context, Context.empty, 'name', SpanKind::INTERNAL, nil, trace_config, span_processor, nil, links, Time.now, nil, nil) _(span.to_span_data.total_recorded_links).must_equal(2) end it 'trims excess links' do links = [OpenTelemetry::Trace::Link.new(context), OpenTelemetry::Trace::Link.new(context)] - span = Span.new(context, 'name', SpanKind::INTERNAL, nil, trace_config, + span = Span.new(context, Context.empty, 'name', SpanKind::INTERNAL, nil, trace_config, span_processor, nil, links, Time.now, nil, nil) _(span.links.size).must_equal(1) end diff --git a/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb b/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb index 471528180..c139bc6a1 100644 --- a/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb +++ b/sdk/test/opentelemetry/sdk/trace/tracer_provider_test.rb @@ -57,7 +57,7 @@ it 'adds the span processor to the active span processors' do mock_span_processor = Minitest::Mock.new - mock_span_processor.expect(:on_start, nil, [Span]) + mock_span_processor.expect(:on_start, nil, [Span, Context]) mock_span_processor.expect(:on_finish, nil, [Span]) tracer_provider.add_span_processor(mock_span_processor) tracer_provider.tracer.in_span('span') {} @@ -67,7 +67,7 @@ it 'adds multiple span processors to the active span processors' do mock_processors = Array.new(2) { MiniTest::Mock.new } mock_processors.each do |p| - p.expect(:on_start, nil, [Span]) + p.expect(:on_start, nil, [Span, Context]) p.expect(:on_finish, nil, [Span]) tracer_provider.add_span_processor(p) end From 0adf482b3fc1770ea384f7fd832367ed8ee3ac47 Mon Sep 17 00:00:00 2001 From: Matthew Wear Date: Tue, 29 Sep 2020 11:34:35 -0700 Subject: [PATCH 8/8] refactor: use named parameter for context in context_with_span method --- api/lib/opentelemetry/trace/tracer.rb | 4 ++-- api/test/opentelemetry/trace/tracer_test.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/lib/opentelemetry/trace/tracer.rb b/api/lib/opentelemetry/trace/tracer.rb index addc10e22..78d8c0560 100644 --- a/api/lib/opentelemetry/trace/tracer.rb +++ b/api/lib/opentelemetry/trace/tracer.rb @@ -26,8 +26,8 @@ def current_span(context = nil) # # @param [optional Context] context The context to use as the parent for # the returned context - def context_with_span(span, context = Context.current) - context.set_value(CURRENT_SPAN_KEY, span) + 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. diff --git a/api/test/opentelemetry/trace/tracer_test.rb b/api/test/opentelemetry/trace/tracer_test.rb index f915d761a..698a76701 100644 --- a/api/test/opentelemetry/trace/tracer_test.rb +++ b/api/test/opentelemetry/trace/tracer_test.rb @@ -172,7 +172,7 @@ def start_span(*) 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_ctx) + ctx = tracer.context_with_span(span, parent_context: parent_ctx) _(tracer.current_span(ctx)).must_equal(span) _(ctx.value('foo')).must_equal('bar') end