Merge pull request #2592 from DataDog/ivoanjo/prof-7145-drop-support-…

…profiling-ruby-2-2

[PROF-7145] Remove support for profiling Ruby 2.2

Rakefile

@@ -21,7 +21,7 @@ namespace :spec do
                         ' spec/**/{auto_instrument,opentelemetry}_spec.rb'
     t.rspec_opts = args.to_a.join(' ')
   end
-  if RUBY_ENGINE == 'ruby' && OS.linux? && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.2.0')
+  if RUBY_ENGINE == 'ruby' && OS.linux? && Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.3.0')
     # "bundle exec rake compile" currently only works on MRI Ruby on Linux
     Rake::Task[:main].enhance([:clean])
     Rake::Task[:main].enhance([:compile])

ddtrace.gemspec

@@ -41,6 +41,7 @@ Gem::Specification.new do |spec|
     ]]
     .select { |fn| File.file?(fn) } # We don't want directories, only files
     .reject { |fn| fn.end_with?('.so', '.bundle') } # Exclude local profiler binary artifacts
+    .reject { |fn| fn.end_with?('skipped_reason.txt') } # Generated by profiler; should never be distributed
 
   spec.executables   = ['ddtracerb']
   spec.require_paths = ['lib']

docs/GettingStarted.md

@@ -126,7 +126,7 @@ To contribute, check out the [contribution guidelines][contribution docs] and [d
 |       |                            | 2.5     | Full                                 | Latest              |
 |       |                            | 2.4     | Full                                 | Latest              |
 |       |                            | 2.3     | Full                                 | Latest              |
-|       |                            | 2.2     | Full                                 | Latest              |
+|       |                            | 2.2     | Full (except for Profiling)          | Latest              |
 |       |                            | 2.1     | Full (except for Profiling)          | Latest              |
 |       |                            | 2.0     | EOL since June 7th, 2021             | < 0.50.0            |
 |       |                            | 1.9.3   | EOL since August 6th, 2020           | < 0.27.0            |

ext/ddtrace_profiling_native_extension/collectors_stack.c

@@ -238,18 +238,7 @@ static void sample_thread_internal(
       filename = rb_profile_frame_path(buffer->stack_buffer[i]);
       line = buffer->lines_buffer[i];
     } else {
-      // **IMPORTANT**: Be very careful when calling any `rb_profile_frame_...` API with a non-Ruby frame, as legacy
-      // Rubies may assume that what's in a buffer will lead to a Ruby frame.
-      //
-      // In particular for Ruby 2.2 the buffer contains a Ruby string (see the notes on our custom
-      // rb_profile_frames for Ruby 2.2) and CALLING **ANY** OF THOSE APIs ON IT WILL CAUSE INSTANT VM CRASHES
-
-#ifndef USE_LEGACY_RB_PROFILE_FRAMES // Modern Rubies
       name = ddtrace_rb_profile_frame_method_name(buffer->stack_buffer[i]);
-#else // Ruby < 2.3
-      name = buffer->stack_buffer[i];
-#endif
-
       filename = NIL_P(last_ruby_frame) ? Qnil : rb_profile_frame_path(last_ruby_frame);
       line = last_ruby_line;
     }

ext/ddtrace_profiling_native_extension/extconf.rb

@@ -125,7 +125,7 @@ def add_compiler_flag(flag)
   # have_library 'pthread'
   # have_func 'pthread_getcpuclockid'
   # ```
-  # but it broke the build on Windows and on older Ruby versions (2.2)
+  # but a) it broke the build on Windows, b) on older Ruby versions (2.2 and below) and c) It's slower to build
   # so instead we just assume that we have the function we need on Linux, and nowhere else
   $defs << '-DHAVE_PTHREAD_GETCPUCLOCKID'
 end
@@ -162,16 +162,6 @@ def add_compiler_flag(flag)
   $defs << '-DUSE_LEGACY_RB_VM_FRAME_METHOD_ENTRY'
 end
 
-# For REALLY OLD Rubies...
-if RUBY_VERSION < '2.3'
-  # ...there was no rb_time_timespec_new function
-  $defs << '-DNO_RB_TIME_TIMESPEC_NEW'
-  # ...the VM changed enough that we need an alternative legacy rb_profile_frames
-  $defs << '-DUSE_LEGACY_RB_PROFILE_FRAMES'
-  # ... you couldn't name threads
-  $defs << '-DNO_THREAD_NAMES'
-end
-
 # If we got here, libdatadog is available and loaded
 ENV['PKG_CONFIG_PATH'] = "#{ENV['PKG_CONFIG_PATH']}:#{Libdatadog.pkgconfig_folder}"
 Logging.message(" [ddtrace] PKG_CONFIG_PATH set to #{ENV['PKG_CONFIG_PATH'].inspect}\n")

ext/ddtrace_profiling_native_extension/native_extension_helpers.rb

@@ -87,8 +87,8 @@ def self.unsupported_reason
             on_windows? ||
             on_macos? ||
             on_unknown_os? ||
-            not_on_amd64_or_arm64? ||
-            on_ruby_2_1? ||
+            on_unsupported_cpu_arch? ||
+            on_unsupported_ruby_version? ||
             expected_to_use_mjit_but_mjit_is_disabled? ||
             libdatadog_not_available? ||
             libdatadog_not_usable?
@@ -251,7 +251,7 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
           unknown_os_not_supported unless RUBY_PLATFORM.include?('darwin') || RUBY_PLATFORM.include?('linux')
         end
 
-        private_class_method def self.not_on_amd64_or_arm64?
+        private_class_method def self.on_unsupported_cpu_arch?
           architecture_not_supported = explain_issue(
             'your CPU architecture is not supported by the Datadog Continuous Profiler.',
             suggested: GET_IN_TOUCH,
@@ -260,13 +260,13 @@ def self.pkg_config_missing?(command: $PKGCONFIG) # rubocop:disable Style/Global
           architecture_not_supported unless RUBY_PLATFORM.start_with?('x86_64', 'aarch64', 'arm64')
         end
 
-        private_class_method def self.on_ruby_2_1?
+        private_class_method def self.on_unsupported_ruby_version?
           ruby_version_not_supported = explain_issue(
-            'the profiler only supports Ruby 2.2 or newer.',
+            'the profiler only supports Ruby 2.3 or newer.',
             suggested: UPGRADE_RUBY,
           )
 
-          ruby_version_not_supported if RUBY_VERSION.start_with?('2.1.')
+          ruby_version_not_supported if RUBY_VERSION.start_with?('2.1.', '2.2.')
         end
 
         # On some Rubies, we require the mjit header to be present. If Ruby was installed without MJIT support, we also skip

ext/ddtrace_profiling_native_extension/private_vm_api_access.c

@@ -247,19 +247,9 @@ bool is_thread_alive(VALUE thread) {
   return thread_struct_from_object(thread)->status != THREAD_KILLED;
 }
 
-// `thread` gets used on all Rubies except 2.2
-// To avoid getting false "unused argument" warnings in setups where it's not used, we need to do this weird dance
-// with diagnostic stuff. See https://nelkinda.com/blog/suppress-warnings-in-gcc-and-clang/#d11e364 for details.
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wunused-parameter"
 VALUE thread_name_for(VALUE thread) {
-  #ifdef NO_THREAD_NAMES
-    return Qnil;
-  #else
-    return thread_struct_from_object(thread)->name;
-  #endif
+  return thread_struct_from_object(thread)->name;
 }
-#pragma GCC diagnostic pop
 
 // -----------------------------------------------------------------------------
 // The sources below are modified versions of code extracted from the Ruby project.
@@ -290,8 +280,6 @@ VALUE thread_name_for(VALUE thread) {
 // OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 // SUCH DAMAGE.
 
-#ifndef USE_LEGACY_RB_PROFILE_FRAMES // Modern Rubies
-
 // Taken from upstream vm_core.h at commit 5f10bd634fb6ae8f74a4ea730176233b0ca96954 (March 2022, Ruby 3.2 trunk)
 // Copyright (C) 2004-2007 Koichi Sasada
 // to support our custom rb_profile_frames (see below)
@@ -392,9 +380,6 @@ calc_lineno(const rb_iseq_t *iseq, const VALUE *pc)
 //   was called from.
 // * Imported fix from https://github.com/ruby/ruby/pull/7116 to avoid sampling threads that are still being created
 //
-// **IMPORTANT: WHEN CHANGING THIS FUNCTION, CONSIDER IF THE SAME CHANGE ALSO NEEDS TO BE MADE TO THE VARIANT FOR
-// RUBY 2.2 AND BELOW WHICH IS ALSO PRESENT ON THIS FILE**
-//
 // What is rb_profile_frames?
 // `rb_profile_frames` is a Ruby VM debug API added for use by profilers for sampling the stack trace of a Ruby thread.
 // Its main other user is the stackprof profiler: https://github.com/tmm1/stackprof .
@@ -722,104 +707,8 @@ check_method_entry(VALUE obj, int can_be_svar)
       return check_method_entry(ep[-1], TRUE);
   }
 #endif // USE_LEGACY_RB_VM_FRAME_METHOD_ENTRY
-
 #endif // RUBY_MJIT_HEADER
 
-#else // USE_LEGACY_RB_PROFILE_FRAMES, Ruby < 2.3
-
-// Taken from upstream vm_backtrace.c at commit bbda1a027475bf7ce5e1a9583a7b55d0be71c8fe (March 2018, ruby_2_2 branch)
-// Copyright (C) 1993-2012 Yukihiro Matsumoto
-// to support our custom rb_profile_frames (see below)
-// Modifications: None
-inline static int
-calc_lineno(const rb_iseq_t *iseq, const VALUE *pc)
-{
-    return rb_iseq_line_no(iseq, pc - iseq->iseq_encoded);
-}
-
-// Taken from upstream vm_backtrace.c at commit bbda1a027475bf7ce5e1a9583a7b55d0be71c8fe (March 2018, ruby_2_2 branch)
-// Copyright (C) 1993-2012 Yukihiro Matsumoto
-// Modifications:
-// * Renamed rb_profile_frames => ddtrace_rb_profile_frames
-// * Add thread argument
-// * Add is_ruby_frame argument
-// * Removed `if (lines)` tests -- require/assume that like `buff`, `lines` is always specified
-// * Added support for getting the name from native methods by getting inspiration from `backtrace_each` in
-//   `vm_backtrace.c`. Note that unlike the `rb_profile_frames` for modern Rubies, this version actually returns the
-//   method name as as `VALUE` containing a Ruby string in the `buff`.
-// * Skip dummy frame that shows up in main thread
-// * Add `end_cfp == NULL` and `end_cfp <= cfp` safety checks. These are used in a bunch of places in
-//   `vm_backtrace.c` (`backtrace_each`, `backtrace_size`, `rb_ec_partial_backtrace_object`) but are conspicuously
-//   absent from `rb_profile_frames`. Oversight?
-// * Check thread status and do not sample if thread has been killed.
-// * Imported fix from https://github.com/ruby/ruby/pull/7116 to avoid sampling threads that are still being created
-//
-// The `rb_profile_frames` function changed quite a bit between Ruby 2.2 and 2.3. Since the change was quite complex
-// I opted not to try to extend support to Ruby 2.2 using the same custom function, and instead I started
-// anew from the Ruby 2.2 version of the function, applying some of the same fixes that we have for the modern version.
-int ddtrace_rb_profile_frames(VALUE thread, int start, int limit, VALUE *buff, int *lines, bool* is_ruby_frame)
-{
-    // **IMPORTANT: THIS IS A CUSTOM RB_PROFILE_FRAMES JUST FOR RUBY 2.2;
-    // SEE ABOVE FOR THE FUNCTION THAT GETS USED FOR MODERN RUBIES**
-
-    int i;
-    rb_thread_t *th = thread_struct_from_object(thread);
-    rb_control_frame_t *cfp = th->cfp, *end_cfp = RUBY_VM_END_CONTROL_FRAME(th);
-
-    // This should not happen for ddtrace (it can only happen when a thread is still being created), but I've imported
-    // it from https://github.com/ruby/ruby/pull/7116 in a "just in case" kind of mindset.
-    if (cfp == NULL) return 0;
-
-    // Avoid sampling dead threads
-    if (th->status == THREAD_KILLED) return 0;
-
-    // `vm_backtrace.c` includes this check in several methods. This happens on newly-created threads, and may
-    // also (not entirely sure) happen on dead threads
-    if (end_cfp == NULL) return PLACEHOLDER_STACK_IN_NATIVE_CODE;
-
-    // Fix: Skip dummy frame that shows up in main thread.
-    //
-    // According to a comment in `backtrace_each` (`vm_backtrace.c`), there's two dummy frames that we should ignore
-    // at the base of every thread's stack.
-    // (see https://github.com/ruby/ruby/blob/4bd38e8120f2fdfdd47a34211720e048502377f1/vm_backtrace.c#L890-L914 )
-    //
-    // One is being pointed to by `RUBY_VM_END_CONTROL_FRAME(ec)`, and so we need to advance to the next one, and
-    // reaching it will be used as a condition to break out of the loop below.
-    //
-    // Note that in `backtrace_each` there's two calls to `RUBY_VM_NEXT_CONTROL_FRAME`, but the loop bounds there
-    // are computed in a different way, so the two calls really are equivalent to one here.
-    end_cfp = RUBY_VM_NEXT_CONTROL_FRAME(end_cfp);
-
-    // See comment on `record_placeholder_stack_in_native_code` for a full explanation of what this means (and why we don't just return 0)
-    if (end_cfp <= cfp) return PLACEHOLDER_STACK_IN_NATIVE_CODE;
-
-    for (i=0; i<limit && cfp != end_cfp;) {
-        if (cfp->iseq && cfp->pc) { /* should be NORMAL_ISEQ */
-            if (start > 0) {
-                start--;
-                continue;
-            }
-
-            /* record frame info */
-            buff[i] = cfp->iseq->self;
-            lines[i] = calc_lineno(cfp->iseq, cfp->pc);
-            is_ruby_frame[i] = true;
-            i++;
-        } else if (RUBYVM_CFUNC_FRAME_P(cfp)) {
-            ID mid = cfp->me->def ? cfp->me->def->original_id : cfp->me->called_id;
-            buff[i] = rb_id2str(mid);
-            lines[i] = 0;
-            is_ruby_frame[i] = false;
-            i++;
-        }
-        cfp = RUBY_VM_PREVIOUS_CONTROL_FRAME(cfp);
-    }
-
-    return i;
-}
-
-#endif // USE_LEGACY_RB_PROFILE_FRAMES
-
 #ifndef NO_RACTORS
   // This API and definition are exported as a public symbol by the VM BUT the function header is not defined in any public header, so we
   // repeat it here to be able to use in our code.

ext/ddtrace_profiling_native_extension/ruby_helpers.h

@@ -12,7 +12,7 @@ static inline VALUE process_pending_interruptions(DDTRACE_UNUSED VALUE _) {
   return Qnil;
 }
 
-// RB_UNLIKELY is not supported on Ruby 2.2 and 2.3
+// RB_UNLIKELY is not supported on Ruby 2.3
 #ifndef RB_UNLIKELY
   #define RB_UNLIKELY(x) x
 #endif

ext/ddtrace_profiling_native_extension/stack_recorder.c

@@ -128,8 +128,6 @@
 static VALUE ok_symbol = Qnil; // :ok in Ruby
 static VALUE error_symbol = Qnil; // :error in Ruby
 
-static ID ruby_time_from_id; // id of :ruby_time_from in Ruby
-
 static VALUE stack_recorder_class = Qnil;
 
 // Contains native state for each instance
@@ -204,7 +202,6 @@ void stack_recorder_init(VALUE profiling_module) {
 
   ok_symbol = ID2SYM(rb_intern_const("ok"));
   error_symbol = ID2SYM(rb_intern_const("error"));
-  ruby_time_from_id = rb_intern_const("ruby_time_from");
 }
 
 // This structure is used to define a Ruby object that stores a pointer to a ddog_prof_Profile instance
@@ -309,13 +306,9 @@ static VALUE _native_serialize(DDTRACE_UNUSED VALUE _self, VALUE recorder_instan
 }
 
 static VALUE ruby_time_from(ddog_Timespec ddprof_time) {
-  #ifndef NO_RB_TIME_TIMESPEC_NEW // Modern Rubies
-    const int utc = INT_MAX - 1; // From Ruby sources
-    struct timespec time = {.tv_sec = ddprof_time.seconds, .tv_nsec = ddprof_time.nanoseconds};
-    return rb_time_timespec_new(&time, utc);
-  #else // Ruby < 2.3
-    return rb_funcall(stack_recorder_class, ruby_time_from_id, 2, LONG2NUM(ddprof_time.seconds), UINT2NUM(ddprof_time.nanoseconds));
-  #endif
+  const int utc = INT_MAX - 1; // From Ruby sources
+  struct timespec time = {.tv_sec = ddprof_time.seconds, .tv_nsec = ddprof_time.nanoseconds};
+  return rb_time_timespec_new(&time, utc);
 }
 
 void record_sample(VALUE recorder_instance, ddog_prof_Sample sample) {

integration/apps/rack/spec/integration/basic_spec.rb

@@ -11,10 +11,9 @@
     it { is_expected.to be_a_kind_of(Net::HTTPOK) }
   end
 
-  let(:expected_profiler_available) { RUBY_VERSION >= '2.2' }
+  let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
 
   let(:expected_profiler_threads) do
-    # NOTE: Threads can't be named on Ruby 2.2
     contain_exactly('Datadog::Profiling::Collectors::OldStack', 'Datadog::Profiling::Scheduler') unless RUBY_VERSION < '2.3'
   end
 

integration/apps/sinatra2-classic/spec/integration/basic_spec.rb

@@ -11,10 +11,9 @@
     it { is_expected.to be_a_kind_of(Net::HTTPOK) }
   end
 
-  let(:expected_profiler_available) { RUBY_VERSION >= '2.2' }
+  let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
 
   let(:expected_profiler_threads) do
-    # NOTE: Threads can't be named on Ruby 2.2
     contain_exactly('Datadog::Profiling::Collectors::OldStack', 'Datadog::Profiling::Scheduler') unless RUBY_VERSION < '2.3'
   end
 

integration/apps/sinatra2-modular/spec/integration/basic_spec.rb

@@ -11,10 +11,9 @@
     it { is_expected.to be_a_kind_of(Net::HTTPOK) }
   end
 
-  let(:expected_profiler_available) { RUBY_VERSION >= '2.2' }
+  let(:expected_profiler_available) { RUBY_VERSION >= '2.3' }
 
   let(:expected_profiler_threads) do
-    # NOTE: Threads can't be named on Ruby 2.2
     contain_exactly('Datadog::Profiling::Collectors::OldStack', 'Datadog::Profiling::Scheduler') unless RUBY_VERSION < '2.3'
   end
 

integration/images/include/build_ddtrace_profiling_native_extension.rb

@@ -1,8 +1,8 @@
 #!/usr/bin/env ruby
 
 if local_gem_path = ENV['DD_DEMO_ENV_GEM_LOCAL_DDTRACE']
-  if RUBY_VERSION.start_with?('2.1.')
-    puts "\n== Skipping build of profiler native extension on Ruby 2.1 =="
+  if RUBY_VERSION.start_with?('2.1.', '2.2.')
+    puts "\n== Skipping build of profiler native extension on Ruby 2.1/2.2 =="
   else
     puts "\n== Building profiler native extension =="
     success =

lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb

@@ -40,7 +40,7 @@ def start
 
             @worker_thread = Thread.new do
               begin
-                Thread.current.name = self.class.name unless Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3')
+                Thread.current.name = self.class.name
 
                 self.class._native_sampling_loop(self)
 

lib/datadog/profiling/collectors/idle_sampling_helper.rb

@@ -31,7 +31,7 @@ def start
 
             @worker_thread = Thread.new do
               begin
-                Thread.current.name = self.class.name unless Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3')
+                Thread.current.name = self.class.name
 
                 self.class._native_idle_sampling_loop(self)
 

lib/datadog/profiling/collectors/old_stack.rb

@@ -75,9 +75,7 @@ def initialize(
           # Cache this buffer, since it's pretty expensive to keep accessing it
           @stack_sample_event_recorder = recorder[Events::StackSample]
           # See below for details on why this is needed
-          @needs_process_waiter_workaround =
-            Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.3') &&
-            Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.7')
+          @needs_process_waiter_workaround = Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.7')
         end
 
         def start

lib/datadog/profiling/stack_recorder.rb

@@ -66,11 +66,6 @@ def clear
         end
       end
 
-      # Used only for Ruby 2.2 which doesn't have the native `rb_time_timespec_new` API; called from native code
-      def self.ruby_time_from(timespec_seconds, timespec_nanoseconds)
-        Time.at(0).utc + timespec_seconds + (timespec_nanoseconds.to_r / 1_000_000_000)
-      end
-
       def reset_after_fork
         self.class._native_reset_after_fork(self)
       end