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

Avoid writer reinitialization during shutdown #1248

Merged
merged 1 commit into from
Nov 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions lib/ddtrace/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,27 @@ def logger
end
end

# Gracefully shuts down all components.
#
# Components will still respond to method calls as usual,
# but might not internally perform their work after shutdown.
#
# This avoids errors being raised across the host application
# during shutdown, while allowing for graceful decommission of resources.
#
# Components won't be automatically reinitialized after a shutdown.
def shutdown!
if instance_variable_defined?(:@components) && @components
components.shutdown!
@components = nil
end
components.shutdown! if instance_variable_defined?(:@components) && @components
end

# Gracefully shuts down the tracer and disposes of component references,
# allowing execution to start anew.
#
# In contrast with +#shutdown!+, components will be automatically
# reinitialized after a reset.
def reset!
shutdown!
@components = nil
end

protected
Expand Down
20 changes: 19 additions & 1 deletion lib/ddtrace/writer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,22 @@ def initialize(options = {})

# one worker for traces
@worker = nil

# Once stopped, this writer instance cannot be restarted.
# This allow for graceful shutdown, while preventing
# the host application from inadvertently start new
# threads during shutdown.
@stopped = false
end

def start
@mutex_after_fork.synchronize do
return false if @stopped

pid = Process.pid
return if @worker && pid == @pid
@pid = pid

start_worker
true
end
Expand All @@ -67,14 +76,23 @@ def start_worker
@worker.start
end

# Gracefully shuts down this writer.
#
# Once stopped methods calls won't fail, but
# no internal work will be performed.
#
# It is not possible to restart a stopped writer instance.
def stop
@mutex_after_fork.synchronize { stop_worker }
end

def stop_worker
@stopped = true

return if @worker.nil?
@worker.stop
@worker = nil

true
end

Expand Down Expand Up @@ -136,7 +154,7 @@ def write(trace, services = nil)

if worker_local
worker_local.enqueue_trace(trace)
else
elsif !@stopped
Datadog.logger.debug('Writer either failed to start or was stopped before #write could complete')
end
end
Expand Down
38 changes: 31 additions & 7 deletions spec/ddtrace/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
before do
allow(Datadog::Configuration::Components).to receive(:new)
.and_wrap_original do |m, *args|
new_components = m.call(*args)
allow(new_components).to receive(:shutdown!)
allow(new_components).to receive(:startup!)
new_components
end
new_components = m.call(*args)
allow(new_components).to receive(:shutdown!)
allow(new_components).to receive(:startup!)
new_components
end
end

context 'and components have been initialized' do
Expand Down Expand Up @@ -376,11 +376,35 @@
describe '#shutdown!' do
subject(:shutdown!) { test_class.shutdown! }

it 'allows for a clean component restart' do
original_components = test_class.send(:components)
let!(:original_components) { test_class.send(:components) }

it 'gracefully shuts down components' do
expect(original_components).to receive(:shutdown!)

shutdown!
end

it 'does not attempt to recreate components' do
shutdown!

expect(test_class.send(:components)).to be(original_components)
end
end

describe '#reset!' do
subject(:reset!) { test_class.reset! }

let!(:original_components) { test_class.send(:components) }

it 'gracefully shuts down components' do
expect(test_class).to receive(:shutdown!)

reset!
end

it 'allows for component re-creation' do
reset!

expect(test_class.send(:components)).to_not be(original_components)
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/ddtrace/contrib/support/tracer_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def clear_spans!
RSpec.configure do |config|
# Capture spans from the global tracer
config.before(:each) do
Datadog.shutdown!
Datadog.reset!

# DEV `*_any_instance_of` has concurrency issues when running with parallelism (e.g. JRuby).
# DEV Single object `allow` and `expect` work as intended with parallelism.
Expand Down
30 changes: 21 additions & 9 deletions spec/ddtrace/writer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -257,17 +257,15 @@
let(:trace) { instance_double(Array) }
let(:services) { nil }

before do
allow_any_instance_of(Datadog::Workers::AsyncTransport)
.to receive(:start)

expect_any_instance_of(Datadog::Workers::AsyncTransport)
.to receive(:enqueue_trace)
.with(trace)
end

context 'when runtime metrics are enabled' do
before do
allow_any_instance_of(Datadog::Workers::AsyncTransport)
.to receive(:start)

expect_any_instance_of(Datadog::Workers::AsyncTransport)
.to receive(:enqueue_trace)
.with(trace)

allow(Datadog.configuration.runtime_metrics)
.to receive(:enabled)
.and_return(true)
Expand All @@ -291,6 +289,20 @@
end
end
end

context 'when tracer has been stopped' do
before { writer.stop }

it 'should not try to record traces' do
expect_any_instance_of(Datadog::Workers::AsyncTransport).to_not receive(:enqueue_trace)

# Ensure clean output, as failing to start the
# worker in this situation is not an error.
expect(Datadog.logger).to_not receive(:debug)

write
end
end
end
end
end
Expand Down