[PROF-8299] Remove legacy profiler codepath #3172
Merged
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
**What does this PR do?** For a while, dd-trace-rb has actually been shipping two profilers. Internally, we've been calling them "Ruby CPU Profiling 2.0" and "legacy profiler". This PR removes the "legacy profiler", leaving the "Ruby CPU Profiling 2.0" profiler as the only option avaialble. **Motivation:** Back in April, the "Ruby CPU Profiling 2.0" went GA and thus became the default profiler that gets used. At the time, I decided to keep the "legacy profiler" around just in case of unknown-unknowns: in case there was a big issue with "CPU Profiling 2.0" profiler that would make it not usable to some of our customers. That was back in April, and here's us at the end of September and we did not see any issues, and adoption of newer versions of the gem (and thus the "CPU Profiling 2.0" profiler) is quite high. So I'm confident we can remove the "legacy profiler", which is quite a nice cleanup to the codebase. **Additional Notes:** * The removed files in `lib/datadog/core` were actually not used by anything else in ddtrace. At some point in the past we thought we may reuse them, but we clearly didn't, so I think it's time to remove them. (These classes were not part of our public API, so they can be removed before 2.0). * The protobuf/pprof decoding code was moved to `spec/` since the profiling specs still make use of it. The encoding part was deleted. * There were a few tiny loose ends that were waiting for the "legacy profiler" to be deleted to be fixed/cleaned up. I've gone ahead and included those as well. * The profiler no longer needs the `google-protobuf` gem (but we used it for the specs). Once this cleanup gets released, I'll do a PR to update https://docs.datadoghq.com/profiler/enabling/ruby/ to reflect this. **How to test the change?** Since the "legacy profiler" was off by default, the only thing to be tested is that the profiler is not affected by these changes.
github-actions
bot
added
core
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?
For a while, dd-trace-rb has actually been shipping two profilers.
Internally, we've been calling them "Ruby CPU Profiling 2.0" and "legacy profiler".
This PR removes the "legacy profiler", leaving the "Ruby CPU Profiling 2.0" profiler as the only option avaialble.
Motivation:
Back in April, the "Ruby CPU Profiling 2.0" went GA and thus became the default profiler that gets used.
At the time, I decided to keep the "legacy profiler" around just in case of unknown-unknowns: in case there was a big issue with "CPU Profiling 2.0" profiler that would make it not usable to some of our customers.
That was back in April, and here's us at the end of September and we did not see any issues, and adoption of newer versions of the gem (and thus the "CPU Profiling 2.0" profiler) is quite high.
So I'm confident we can remove the "legacy profiler", which is quite a nice cleanup to the codebase.
Additional Notes:
The removed files in
lib/datadog/corewere actually not used by anything else in ddtrace. At some point in the past we thought we may reuse them, but we clearly didn't, so I think it's time to remove them. (These classes were not part of our public API, so they can be removed before 2.0).The protobuf/pprof decoding code was moved to
spec/since the profiling specs still make use of it. The encoding part was deleted.There were a few tiny loose ends that were waiting for the "legacy profiler" to be deleted to be fixed/cleaned up. I've gone ahead and included those as well.
The profiler no longer needs the
google-protobufgem (but we used it for the specs). Once this cleanup gets released, I'll do a PR to update https://docs.datadoghq.com/profiler/enabling/ruby/ to reflect this.How to test the change?
Since the "legacy profiler" was off by default, the only thing to be tested is that the profiler is not affected by these changes.
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.