Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: context only parenting #398

Merged
merged 10 commits into from
Sep 29, 2020
8 changes: 2 additions & 6 deletions api/benchmarks/tracer_bench.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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

Expand Down
23 changes: 15 additions & 8 deletions api/lib/opentelemetry/trace/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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+.
mwear marked this conversation as resolved.
Show resolved Hide resolved
# @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
Expand Down
35 changes: 19 additions & 16 deletions api/test/opentelemetry/trace/tracer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion examples/http/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 7 additions & 4 deletions exporter/otlp/test/opentelemetry/exporter/otlp/exporter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 4 additions & 2 deletions sdk/lib/opentelemetry/sdk/trace/multi_span_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?}
Expand Down
4 changes: 3 additions & 1 deletion sdk/lib/opentelemetry/sdk/trace/noop_span_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions sdk/lib/opentelemetry/sdk/trace/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
12 changes: 7 additions & 5 deletions sdk/lib/opentelemetry/sdk/trace/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions sdk/test/integration/api_trace_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion sdk/test/integration/global_tracer_configurations_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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/]
Expand Down
7 changes: 4 additions & 3 deletions sdk/test/opentelemetry/sdk/trace/multi_span_processor_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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
Expand Down
Loading