[PROF-5774] Remove support for profiling Ruby 2.1 #2140

ivoanjo · 2022-07-08T14:07:30Z

What does this PR do?

This PR removes support for profiling Ruby 2.1 using the Continuous Profiling product. Other products shipped in ddtrace (tracing, application security monitoring, CI, etc) are unaffected.

This dropping of support does not impact our customer's ability to install the latest dd-trace-rb on Ruby 2.1. What it does is

Skip compilation of the profiling native extension on Ruby 2.1
Print a warning message when customers try to enable profiling on Ruby 2.1 (but does not otherwise block or break their application)

The warning message shown to customers is:

W, [2022-07-08T14:08:51.295193 #51899] WARN -- ddtrace: [ddtrace] Profiling was requested but is not supported, profiling disabled: Your ddtrace installation is missing support for the Continuous Profiler because
~~your Ruby version (2.1) is too old to be supported~~ the profiler only supports Ruby 2.2 or newer.
Upgrade to a modern Ruby to enable profiling for your app.

Motivation

There is little customer interest on profiling Ruby 2.1, and supporting old Rubies also comes with an extra tax, especially as we build towards the new native profiler.

Additional Notes

I'm opening this PR as a draft not because it's missing anything, but because it MUST only be merged after dd-trace-rb 1.2.0 is released.

How to test the change?

Run test suite on Ruby 2.1
Try enabling profiling on a Ruby 2.1 application

marcotc · 2022-07-08T17:42:54Z

ext/ddtrace_profiling_native_extension/native_extension_helpers.rb

@@ -242,6 +247,15 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
          architecture_not_supported unless RUBY_PLATFORM.start_with?('x86_64', 'aarch64')
        end

+        private_class_method def self.on_ruby_2_1?
+          ruby_version_not_supported = explain_issue(
+            'your Ruby version (2.1) is too old to be supported.',


I think the message is a bit negative, specially for clients that are not happy to be stuck on 2.1 themselves.

How about something like "the profiler only supports Ruby 2.2 or newer".

Seems reasonable, changed in 91eb862 .

marcotc · 2022-07-08T17:43:52Z

ext/ddtrace_profiling_native_extension/native_extension_helpers.rb

@@ -148,6 +149,10 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
          "Get in touch with us if you're interested in profiling your app!"
        ].freeze

+        UPGRADE_RUBY = [
+          'Upgrade to a modern Ruby to enable profiling for your app.'


I think "Upgrade to Ruby 2.2 or newer" is more accurate than modern Ruby.

Hmmm, I think that may be a bit redundant? We always present the two parts of the error message together, e.g. at installation (which is hidden by default by rubygems):

+------------------------------------------------------------------------------+ | Could not compile the Datadog Continuous Profiler because | | the profiler only supports Ruby 2.2 or newer. | | | | The Datadog Continuous Profiler will not be available, | | but all other ddtrace features will work fine! | | | | Upgrade to a modern Ruby to enable profiling for your app. | +------------------------------------------------------------------------------+

and at run-time:

W, [2022-07-11T09:09:22.971998 #20866] WARN -- ddtrace: [ddtrace] Profiling was requested but is not supported, profiling disabled: Your ddtrace installation is missing support for the Continuous Profiler because the profiler only supports Ruby 2.2 or newer. Upgrade to a modern Ruby to enable profiling for your app

So do you think now that I've changed the first part to already state customers need 2.2+, is it worth repeating it again on the second part?

This reads fine now! 👍

ivoanjo · 2022-07-11T15:09:09Z

Update: dd-trace-rb 1.2.0 is out, so this is ready for review. I've noticed there's a conflict because of #2061; I'll push a rebase ASAP to clear it up.

This is the first piece of dropping profiling support for Ruby 2.1. Instead of the extension building, customers will get the following message: ``` +------------------------------------------------------------------------------+ | Could not compile the Datadog Continuous Profiler because | | your Ruby version (2.1) is too old to be supported. | | | | The Datadog Continuous Profiler will not be available, | | but all other ddtrace features will work fine! | | | | Upgrade to a modern Ruby to enable profiling for your app. | +------------------------------------------------------------------------------+ ``` (Note that, as the message says, other dd-trace-rb features are untouched and will still work).

…ative extension Less weird exceptions to maintain, yay!

Message is now going to be > Could not compile the Datadog Continuous Profiler because > the profiler only supports Ruby 2.2 or newer. at installation time, and > Your ddtrace installation is missing support for the Continuous > Profiler because the profiler only supports Ruby 2.2 or newer.

ivoanjo · 2022-07-12T11:17:30Z

Update: Thanks Tony and Marco. I've rebased to solve the conflicts (related to libddprof -> libdatatadog renaming). Going to press the magic button!

We've dropped support for Ruby 2.1 some time ago (<DataDog/dd-trace-rb#2140>) and I spotted that the documentation was outdated.

**What does this PR do?**: This PR removes support for profiling Ruby 2.2 using the Continuous Profiling product. Other products shipped in ddtrace (tracing, application security monitoring, CI, etc) are unaffected. This dropping of support does not impact our customer's ability to install the latest dd-trace-rb on Ruby 2.2. What it does is 1. Skip compilation of the profiling native extension on Ruby 2.2 2. Print a warning message when customers try to enable profiling on Ruby 2.2 (but does not otherwise block or break their application) The warning message shown to customers is: > W, [2023-02-01T11:42:57.022293 #410738] WARN -- ddtrace: [ddtrace] > Profiling was requested but is not supported, profiling disabled: > Your ddtrace installation is missing support for the Continuous > Profiler because the profiler only supports Ruby 2.3 or newer. > Upgrade to a modern Ruby to enable profiling for your app. Customers are welcome to continue using older versions of dd-trace-rb to profile their Ruby 2.2 (as well as 2.1) applications. (This PR is similar to #2140 where we dropped support for Ruby 2.1) **Motivation**: There is little customer interest on profiling Ruby 2.2, and supporting old Rubies also comes with an extra tax -- just look how much code and complexity is being removed by this PR. **Additional Notes**: (N/A) **How to test the change?**: * Validate that test suite still runs successfully on Ruby 2.2 * Validate that you get the above error message when trying to profile a Ruby 2.2 application

ivoanjo requested a review from a team July 8, 2022 14:07

ivoanjo marked this pull request as draft July 8, 2022 14:07

TonyCTHsu approved these changes Jul 8, 2022

View reviewed changes

marcotc reviewed Jul 8, 2022

View reviewed changes

ivoanjo marked this pull request as ready for review July 11, 2022 15:08

marcotc approved these changes Jul 11, 2022

View reviewed changes

ivoanjo added 9 commits July 12, 2022 12:01

Remove hacks and exceptions needed to support Ruby 2.1 in profiling n…

d82d508

…ative extension Less weird exceptions to maintain, yay!

Remove Ruby 2.1-specific checks from profiler tests

dedf521

Remove references to Ruby 2.1 from profiler comments

ac1eb59

Rubocop fixes

992bc20

Skip attempting to compile profiling native extension in CI for Ruby 2.1

003428c

Update rack integration app tests to not test for profiler on Ruby 2.1

d0f91f7

Not in README that profiling is not supported for Ruby 2.1

8d867e9

ivoanjo force-pushed the ivoanjo/prof-5773-drop-support-profiling-ruby-2-1 branch from 91eb862 to cf2330a Compare July 12, 2022 11:04

ivoanjo merged commit 1835642 into master Jul 12, 2022

ivoanjo deleted the ivoanjo/prof-5773-drop-support-profiling-ruby-2-1 branch July 12, 2022 11:17

github-actions bot added this to the 1.3.0 milestone Jul 12, 2022

ivoanjo mentioned this pull request Dec 2, 2022

[PROF-6556] Fix profiler incorrectly detecting if a thread is holding the Global VM Lock #2415

Merged

ivoanjo added a commit to DataDog/documentation that referenced this pull request Dec 13, 2022

Update minimum supported Ruby version for Ruby profiler

81c84cf

We've dropped support for Ruby 2.1 some time ago (<DataDog/dd-trace-rb#2140>) and I spotted that the documentation was outdated.

ivoanjo mentioned this pull request Dec 13, 2022

Update minimum supported Ruby version for Ruby profiler DataDog/documentation#16198

Merged

4 tasks

ivoanjo mentioned this pull request Feb 1, 2023

[PROF-7145] Remove support for profiling Ruby 2.2 #2592

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PROF-5774] Remove support for profiling Ruby 2.1 #2140

[PROF-5774] Remove support for profiling Ruby 2.1 #2140

ivoanjo commented Jul 8, 2022 •

edited

marcotc Jul 8, 2022

ivoanjo Jul 11, 2022

marcotc Jul 8, 2022

ivoanjo Jul 11, 2022

marcotc Jul 11, 2022

ivoanjo commented Jul 11, 2022

ivoanjo commented Jul 12, 2022

[PROF-5774] Remove support for profiling Ruby 2.1 #2140

[PROF-5774] Remove support for profiling Ruby 2.1 #2140

Conversation

ivoanjo commented Jul 8, 2022 • edited

marcotc Jul 8, 2022

Choose a reason for hiding this comment

ivoanjo Jul 11, 2022

Choose a reason for hiding this comment

marcotc Jul 8, 2022

Choose a reason for hiding this comment

ivoanjo Jul 11, 2022

Choose a reason for hiding this comment

marcotc Jul 11, 2022

Choose a reason for hiding this comment

ivoanjo commented Jul 11, 2022

ivoanjo commented Jul 12, 2022

ivoanjo commented Jul 8, 2022 •

edited