Instrument Concurrent::ThreadPoolExecutor#post

This method is ultimately called by both Concurrent::Promises#future and
Concurrent::Future#execute. By instrumenting here, we'll have more
coverage no matter which syntax applications are using.

lib/new_relic/agent/instrumentation/concurrent_ruby.rb

@@ -10,16 +10,16 @@
   named :'concurrent_ruby'
 
   depends_on do
-    defined?(::Concurrent)
+    defined?(Concurrent)
   end
 
   executes do
-    ::NewRelic::Agent.logger.info('Installing concurrent-ruby instrumentation')
+    NewRelic::Agent.logger.info('Installing concurrent-ruby instrumentation')
 
     if use_prepend?
-      prepend_instrument ::Concurrent::Promises::FactoryMethods, NewRelic::Agent::Instrumentation::ConcurrentRuby::Prepend
+      prepend_instrument(Concurrent::ThreadPoolExecutor, NewRelic::Agent::Instrumentation::ConcurrentRuby::Prepend)
     else
-      chain_instrument NewRelic::Agent::Instrumentation::ConcurrentRuby
+      chain_instrument NewRelic::Agent::Instrumentation::ConcurrentRuby::Chain
     end
   end
 end

lib/new_relic/agent/instrumentation/concurrent_ruby/instrumentation.rb

@@ -4,22 +4,21 @@
 
 module NewRelic::Agent::Instrumentation
   module ConcurrentRuby
-    PROMISES_FUTURE_NAME = 'Concurrent::Promises#future'
+    DEFAULT_NAME = 'Concurrent::ThreadPoolExecutor#post'
 
-    def future_with_new_relic(*args)
-      segment = NewRelic::Agent::Tracer.start_segment(name: segment_name(*args))
+    def post_with_new_relic(*args)
+      return yield unless NewRelic::Agent::Tracer.tracing_enabled?
+
+      current = Thread.current[:newrelic_tracer_state]
+      segment = NewRelic::Agent::Tracer.start_segment(name: DEFAULT_NAME)
       begin
-        NewRelic::Agent::Tracer.capture_segment_error(segment) { yield }
+        NewRelic::Agent::Tracer.capture_segment_error(segment) do
+          Thread.current[:newrelic_tracer_state] = current
+          yield
+        end
       ensure
         ::NewRelic::Agent::Transaction::Segment.finish(segment)
       end
     end
-
-    private
-
-    def segment_name(*args)
-      keyword_args = args[0]
-      keyword_args && keyword_args.key?(:nr_name) ? keyword_args[:nr_name] : PROMISES_FUTURE_NAME
-    end
   end
 end

lib/new_relic/agent/instrumentation/concurrent_ruby/prepend.rb

@@ -6,8 +6,8 @@ module NewRelic::Agent::Instrumentation
   module ConcurrentRuby::Prepend
     include NewRelic::Agent::Instrumentation::ConcurrentRuby
 
-    def future(*args, &task)
-      future_with_new_relic(*args) { super }
+    def post(*args, &task)
+      post_with_new_relic(*args) { super }
     end
   end
 end

test/multiverse/suites/concurrent_ruby/concurrent_ruby_instrumentation_test.rb

@@ -9,27 +9,36 @@ def test_promises_future_creates_segment_with_default_name
     end
     segment = txn.segments[1]
 
-    assert_equal segment.name, NewRelic::Agent::Instrumentation::ConcurrentRuby::PROMISES_FUTURE_NAME
+    assert_equal segment.name, NewRelic::Agent::Instrumentation::ConcurrentRuby::DEFAULT_NAME
   end
 
-  def test_promises_future_creates_segment_with_custom_name
-    name = 'my little pony'
+  def test_promises_future_creates_segments_for_nested_instrumented_calls
+    future = nil
     txn = in_transaction do
-      Concurrent::Promises.future(nr_name: name) { 'hi' }
+      future = Concurrent::Promises.future { Net::HTTP.get(URI('http://www.example.com')) }
     end
-    segment = txn.segments[1]
 
-    assert_equal segment.name, name
+    assert_equal(3, txn.segments.length)
   end
 
   # hmm -- i think I'm not understanding how errors work in this context
   # https://github.com/ruby-concurrency/concurrent-ruby/blob/master/docs-source/promises.in.md?plain=1#L81
+  # Promises.future swallows all errors
+  # they can be raised the result of the future as a variable and calling #value! on it
+  # it also works if you call raise future
+  # future.reason => inspect version of error
+  # future.value => nil if there's an error
+  # future.rejected? => true
+  # future.value! => raises the error
+  # raise future => raises the error
+  # raise future rescue $! => rescues the error, returns inspected version
+
   def test_promises_future_captures_segment_error
-    skip
+    skip "future doesn't raise errors, so they can't be captured"
     txn = nil
-    begin
-      txn = in_transaction('concurrent') do
-        Concurrent::Promises.future { raise 'boom!' }
+    txn = in_transaction('concurrent') do
+      Concurrent::Promises.stub(:future, raise('boom!')) do
+        future = Concurrent::Promises.future { 'hi' }
       end
     rescue StandardError => e
       # NOOP -- allowing span and transaction to notice error