Add instrumentation for Concurrent Ruby #1682
Conversation
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.
kaylareopelle
force-pushed
the
concurrent_ruby_promises
branch
from
c40a516 to
d0791ff
Compare
kaylareopelle
changed the title
|
The changes I pushed on 12/14 change the following:
We're getting segments nested within a transaction that have the Next steps:
|
|
It also looks like I may have some syntax that isn't compatible with Ruby 2.2: https://github.com/newrelic/newrelic-ruby-agent/actions/runs/3699041227/jobs/6265975231#step:11:3068 |
| if use_prepend? | ||
| prepend_instrument(Concurrent::ThreadPoolExecutor, NewRelic::Agent::Instrumentation::ConcurrentRuby::Prepend) | ||
| else | ||
| chain_instrument NewRelic::Agent::Instrumentation::ConcurrentRuby::Chain |
There was a problem hiding this comment.
The name of this class has to have a ::Chain suffix to have the log messages for what was instrumented show up correctly. This method looks for the second-to-last namespace and puts that in the logs. Without the ::Chain suffix, "Instrumentation" is logged instead of "ConcurrentRuby". I'll make a separate PR to fix this in the instrumentation generator.
test/multiverse/suites/concurrent_ruby/concurrent_ruby_instrumentation_test.rb
Outdated
Show resolved
Hide resolved
| # raise future rescue $! => rescues the error, returns inspected version | ||
|
|
||
| def test_promises_future_captures_segment_error | ||
| skip "future doesn't raise errors, so they can't be captured" |
There was a problem hiding this comment.
This is a different problem someone could tackle. See comments above the test for background. Essentially, the return value if an error is raised is nil. You have to call a different method to get the error. What should we do to capture errors automatically within calls to Concurrent::Promises#future?
Turns out we do need to copy the current transaction over every time. Concurrent ruby can create a thread prior to the transaction beginning or during a different transaction, but continue to use the same thread. This ensures the agent is always using the most accurate source for the current transaction
Concurrent ruby will reuse threads created before a transaction began, which causes the thread creation segment to not appear in the expected transaction.
…ic-ruby-agent into concurrent_ruby_promises
A 12-year old typo! :D
…ic-ruby-agent into concurrent_ruby_promises
This also removes the inclusion of parent in the thread_block_with_current_transaction calls made in Thread and TracedThread instrumentation
Co-authored-by: James Bunch <fallwith@gmail.com>
| end | ||
|
|
||
| def test_task_segment_has_correct_parent | ||
| txn = future_in_transaction { 'are you my mother?' } |
There was a problem hiding this comment.
There's a movie that has a character deliver this line and follow it up with "Am I part of you both?" but I now I can't remember it's name. Something about a garden I think.
There was a problem hiding this comment.
Interesting! I was thinking Dr. Seuss when I wrote this one. If you think of the title, let me know!
There was a problem hiding this comment.
Ah, the word "garden" was in the quote itself and it's from a book - not a movie.
“Are these Gardens my mother?” she asked suddenly, and he told her they were. “Am I a part of you both?” she asked, and he told her yes.
Now I'm wondering why I thought it was a movie. I definitely have an old (15+ years?) audio clip of the quote. Maybe there was an audio book? Weird.
SimpleCov Report
|
Overview
This PR adds automatic instrumentation for the
concurrent-rubygem to the Ruby agent.We've instrumented
Concurrent::ThreadPoolExecutor#post, which is called by many methods within concurrent-ruby including, but not limited to:Concurrent::Promises.futureConcurrent::Future.executeNow, what happens within the blocks passed to these methods will be visible to the agent.
Concurrent::ThreadPoolExecutor#postwill add a segment to an existing transaction calledConcurrent/Taskand create nested segments for calls to libraries the agent instruments within the passed-in block, such asNet::HTTPrequests.Furthermore, a new Supportability metric will be generated the first time the agent fields a call to
Concurrent::ThreadPoolExecutor#post, helping us better understand which customers use theconcurrent-rubyso that we can improve our instrumentation in the future.Also, we've updated the
Ruby/Threadinstrumentation omit the Thread id from the metric name only whenthread_id_enabledis set to true. It is false and private by default. We believe Thread ids in metrics are valuable for debugging the agent, so we want to have this option easily available if support tickets arise.Closes #1601
Closes #1705
Submitter Checklist:
Testing
The agent includes a suite of unit and functional tests which should be used to
verify your changes don't break existing functionality. These tests will run with
GitHub Actions when a pull request is made. More details on running the tests locally can be found
here for our unit tests,
and here for our functional tests.
For most contributions it is strongly recommended to add additional tests which
exercise your changes.
Reviewer Checklist