Skip to content
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

[NO-TICKET] Remove profiler support for Ruby 2.3 and 2.4 #3621

Merged
merged 2 commits into from
May 7, 2024

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented May 1, 2024

What does this PR do?

This PR removes the code for supporting Ruby 2.3 and 2.4 from the profiler.

This matches the minimum version for the datadog gem which is now Ruby 2.5 (e.g. this was dead code -- you couldn't actually use it anymore).

Motivation:

We opted not to do this earlier on in the 2.x release cycle, but as we are getting close to the final release, and given that the diff is not that big, I decided to go ahead and do it now.

Additional Notes:

In some cases, because a conditional was removed, indentation was adjusted. I suggest reviewing this diff with "Hide whitespace" turned on (or -w in the git cli).

In ddtrace 1.x, the gem supported Ruby 2.1+ whereas the profiler was supporting 2.3+. We still had some awkward support code to deal with this difference, which I also went ahead and removed.

It's cool in particular to see private_vm_api_access.c shrinking. I'm looking forward to this trend continuing, and maybe one day we'll even be able to remove it completely :)

How to test the change?

Validate that CI is still green on supported Rubies.

**What does this PR do?**

This PR removes the code for supporting Ruby 2.3 and 2.4 from the
profiler.

This matches the minimum version for the datadog gem which is now
Ruby 2.5 (e.g. this was dead code -- you couldn't actually use it
anymore).

**Motivation:**

We opted not to do this earlier on in the 2.x release cycle,
but as we are getting close to the final release, and given that the
diff is not that big, I decided to go ahead and do it now.

**Additional Notes:**

In some cases, because a conditional was removed, indentation
was adjusted. I suggest reviewing this diff with "Hide whitespace"
turned on (or `-w` in the git cli).

In ddtrace 1.x, the gem supported Ruby 2.1+ whereas the profiler
was supporting 2.3+. We still had some awkward support code to
deal with this difference, which I also went ahead and removed.

It's cool in particular to see `private_vm_api_access.c` shrinking.
I'm looking forward to this trend continuing, and maybe one day we'll
even be able to remove it completely :)

**How to test the change?**

Validate that CI is still green on supported Rubies.
@ivoanjo ivoanjo requested review from a team as code owners May 1, 2024 15:48
@github-actions github-actions bot added the profiling Involves Datadog profiling label May 1, 2024
@ivoanjo ivoanjo added this to the 2.0.0 milestone May 1, 2024
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.13%. Comparing base (2be8d89) to head (17ec5fb).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3621   +/-   ##
=======================================
  Coverage   98.12%   98.13%           
=======================================
  Files        1223     1223           
  Lines       72136    72121   -15     
  Branches     3425     3419    -6     
=======================================
- Hits        70787    70775   -12     
+ Misses       1349     1346    -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@marcotc marcotc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot 2024-05-01 at 11 07 38 AM

@ivoanjo ivoanjo merged commit d1cbfff into master May 7, 2024
166 checks passed
@ivoanjo ivoanjo deleted the ivoanjo/drop-profiler-support-for-ruby-2.3-and-2.4 branch May 7, 2024 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
profiling Involves Datadog profiling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants