[PROF-6900] Fix wrong libdatadog version being picked during profiler build #2531
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `REPORT_ISSUE` string is somewhat specific to the error that was using it, because it mentions "if you needed to use this", where this is the env var to disable building the native extension. So to avoid confusion, I decided to inline it on the check it belongs to.
… build **What does this PR do?**: This PR fixes an issue I spotted recently relating to the libdatadog version that gets picked during profiler build. Specifically, I had the following setup: * Two versions of the libdatadog gem locally installed (1.0.1.1.0, 0.9.0.1.0) * I tried to install ddtrace 1.8.0 from rubygems The ddtrace 1.8.0 release has a dependency on libdatadog 0.9.x. But, what I observed is that building the profiler (and thus installing ddtrace) failed because the profiling native extension was trying to compile and link to libdatadog 1.0.x instead of 0.9.x. Upon investigation, I've discovered that the issue is that the `extconf.rb` script (which gets called by rubygems during installation) is not run with the equivalent of `bundle exec`, e.g. it can access all gems (and versions) that are installed locally, not just the ones that are defined in the dependencies of our gem in the `gemspec`. Thus, at least on my machine, when we did `require 'libdatadog'` to access the `libdatadog` gem, rubygems picked a version which did not match the one that was stated in the `gemspec`. To fix this, I've modified the `native_extension_helpers.rb` (which gets called by the `extconf.rb`) to tell RubyGems to activate the exact version of `libdatadog` that we need. Additionally, I've added error handling so we gracefully recover from any errors that can happen on this step. **Motivation**: Avoid impacting customers that are installing ddtrace (and avoid having them endup having to disable profiling to install ddtrace). **Additional Notes**: Hopefully no customers ever got bitten by this (we had no reports thus far). To reproduce this, they'd need to have a weird combination of having a more modern version of libdatadog than they needed, and then install an older version of ddtrace. **How to test the change?**: See instructions above for the setup I had that triggered this issue.
Codecov Report
@@ Coverage Diff @@
## master #2531 +/- ##
=======================================
Coverage 98.03% 98.03%
=======================================
Files 1124 1124
Lines 60660 60686 +26
Branches 2767 2767
=======================================
+ Hits 59466 59494 +28
+ Misses 1194 1192 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
marcotc
approved these changes
Jan 12, 2023
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?:
This PR fixes an issue I spotted recently relating to the libdatadog version that gets picked during profiler build.
Specifically, I had the following setup:
The ddtrace 1.8.0 release has a dependency on libdatadog 0.9.x.
But, what I observed is that building the profiler (and thus installing ddtrace) failed because the profiling native extension was trying to compile and link to libdatadog 1.0.x instead of 0.9.x.
Upon investigation, I've discovered that the issue is that the
extconf.rbscript (which gets called by rubygems during installation) is not run with the equivalent ofbundle exec, e.g. it can access all gems (and versions) that are installed locally, not just the ones that are defined in the dependencies of our gem in thegemspec.Thus, at least on my machine, when we did
require 'libdatadog'to access thelibdatadoggem, rubygems picked a version which did not match the one that was stated in thegemspec.To fix this, I've modified the
native_extension_helpers.rb(which gets called by theextconf.rb) to tell RubyGems to activate the exact version oflibdatadogthat we need.Additionally, I've added error handling so we gracefully recover from any errors that can happen on this step.
Motivation:
Avoid impacting customers that are installing ddtrace (and avoid having them endup having to disable profiling to install ddtrace).
Additional Notes:
Hopefully no customers ever got bitten by this (we had no reports thus far). To reproduce this, they'd need to have a weird combination of having a more modern version of libdatadog than they needed, and then install an older version of ddtrace.
How to test the change?:
See instructions above for the setup I had that triggered this issue. You'll have to check out the exact commit where the fix happened, because then I had to merge master into this branch, and master brought in support for the latest libdatadog, which will make the issue not trigger again until there's a newer version of libdatadog out there.