-
Notifications
You must be signed in to change notification settings - Fork 602
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
Enable instrumentation.thread.tracing
by default
#1767
Conversation
enabled/disabled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is so exciting, Tanna! Could you run a workflow dispatch on JRuby and/or earlier Ruby versions? I feel like JRuby especially may reveal something to us, but I don't think it has the workflow dispatch set up yet.
CHANGELOG.md
Outdated
|
||
- **Enabled Thread Instrumentation by default** | ||
|
||
The configuration option `instrumentation.thread.tracing` is now enabled by default. Allowing the agent to properly monitor code that occurs inside of threads. If you are currently using custom instrumentation to start a new transaction inside of threads, this may be a breaking change, as it will no longer start a new transaction if one already exists. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sentence "Allowing the agent to properly monitor code that occurs inside of threads" seems like it's missing something? Did you mean to put a comma after default? Or could you something to the beginning, like "This instrumentation allows..."?
Also, could you start the description under the version number as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yes, thank you! I missed that. I've changed both of these things in a33b7e9
Jruby does have it set up! 🎉 I included it when I first set it up because I figured there'd be situations like this that come up. They are both running right now |
disabling tracing correctly
concurrent_invocation to work with thread tracing
prereqs = self.prerequisite_tasks.map(&:name).join(", ") | ||
if txn = ::NewRelic::Agent::Tracer.current_transaction | ||
txn.current_segment.params[:statement] = NewRelic::Agent::Database.truncate_query("Couldn't trace concurrent prereq tasks: #{prereqs}") | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was able to be removed because this code is adding statements to the transaction saying that we can't trace the stuff happening in other threads. Since thread tracing is enabled by default, this is no longer true! We can now trace these tasks just like any other task.
to outside the if statement
SimpleCov Report
|
Before contributing, please read our contributing guidelines and code of conduct.
Overview
Enabled the config option
instrumentation.thread.tracing
in the default config.Updates tests that aren't expecting this config to be enabled.
closes #1089
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