Avoid writer reinitialization during shutdown (#1248)

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

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

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

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.

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