diff --git a/lib/ddtrace/configuration.rb b/lib/ddtrace/configuration.rb index 0aa52a75902..73e42e0b145 100644 --- a/lib/ddtrace/configuration.rb +++ b/lib/ddtrace/configuration.rb @@ -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 diff --git a/lib/ddtrace/writer.rb b/lib/ddtrace/writer.rb index 113c64e5f6d..ea088808c46 100644 --- a/lib/ddtrace/writer.rb +++ b/lib/ddtrace/writer.rb @@ -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 @@ -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 @@ -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 diff --git a/spec/ddtrace/configuration_spec.rb b/spec/ddtrace/configuration_spec.rb index 6120a88421a..632434e0faf 100644 --- a/spec/ddtrace/configuration_spec.rb +++ b/spec/ddtrace/configuration_spec.rb @@ -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 @@ -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 diff --git a/spec/ddtrace/contrib/support/tracer_helpers.rb b/spec/ddtrace/contrib/support/tracer_helpers.rb index 47748b580fa..f06a0aca272 100644 --- a/spec/ddtrace/contrib/support/tracer_helpers.rb +++ b/spec/ddtrace/contrib/support/tracer_helpers.rb @@ -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. diff --git a/spec/ddtrace/writer_spec.rb b/spec/ddtrace/writer_spec.rb index 08f56cff668..70a1babda18 100644 --- a/spec/ddtrace/writer_spec.rb +++ b/spec/ddtrace/writer_spec.rb @@ -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) @@ -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