-
Notifications
You must be signed in to change notification settings - Fork 373
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
[PROF-7409] Do not auto-enable new profiler when rugged gem is detected #2741
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -172,8 +172,19 @@ def self.build_profiler_component(settings:, agent_settings:, optional_tracer:) | |
'library used by the mysql2 gem) have a bug in their signal handling code that the new profiler can trigger. ' \ | ||
'This bug (https://bugs.mysql.com/bug.php?id=83109) is fixed in libmysqlclient versions 8.0.0 and above. ' \ | ||
'If your Linux distribution provides a modern libmysqlclient, you can force-enable the new CPU Profiling 2.0 ' \ | ||
'profiler by using the `DD_PROFILING_FORCE_ENABLE_NEW` or `c.profiling.advanced.force_enable_new_profiler` ' \ | ||
'settings.' | ||
'profiler by using the `DD_PROFILING_FORCE_ENABLE_NEW` environment variable or the ' \ | ||
'`c.profiling.advanced.force_enable_new_profiler` setting.' \ | ||
) | ||
return false | ||
end | ||
|
||
if Gem.loaded_specs['rugged'] | ||
Datadog.logger.warn( | ||
'Falling back to legacy profiler because rugged gem is installed. Some operations on this gem are ' \ | ||
'currently incompatible with the new CPU Profiling 2.0 profiler, as detailed in ' \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a subset of the rugged gem that's safe, like an specific version? I'm asking so users know when it makes sense to try to enable profiling 2.0 anyway. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Currently, there isn't. It's on my next-things-to-do list to report this upstream and even seeing if I can submit a PR with a fix or workaround, which would allow us to indeed make this finer-grained than what this PR does. |
||
'<https://github.com/datadog/dd-trace-rb/issues/2721>. If you still want to try out the new profiler, you ' \ | ||
'can force-enable it by using the `DD_PROFILING_FORCE_ENABLE_NEW` environment variable or the ' \ | ||
'`c.profiling.advanced.force_enable_new_profiler` setting.' | ||
) | ||
return false | ||
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.
I've added this as a useful helper to test in issues like this (see individual commits for details)