[PROF-7625] Add workaround for legacy profiler incompatibility with ruby-cloud-profiler gem

[ivoanjo]

What does this PR do?:

This PR changes the datadog/profiling.rb top-level file to not load the datadog/profiling/pprof/pprof_pb.rb file unless the legacy profiler is in use (aka don't load it when the new CPU Profiling 2.0 is in use).

Motivation:

The legacy profiler's pprof support conflicts with the ruby-cloud-profiler gem.

This is not a problem for almost all customers, since we now default everyone to use the new CPU Profiling 2.0 profiler. But the issue was still triggered, because currently we still load both the legacy and new profiling code paths.

To work around this issue, and because we plan on deleting the old profiler soon, rather than poking at the pprof support code, we only load the conflicting file when the old profiler is in use. This way customers using the new profiler (almost all) will not be affected by the issue any longer.

Additional Notes:

The ruby-cloud-profiler gem is not even on rubygems but it's used by some releases of GitLab and thus makes it harder for us to use GitLab for correctness testing.

How to test the change?:

Here's a quick test case:

require 'bundler/inline'

ENV['DD_TRACE_DEBUG'] = 'true'
ENV['DD_PROFILING_ENABLED'] = 'true'

gemfile do
  source 'https://rubygems.org'
  gem 'ddtrace', (ENV['USE_LOCAL'] == 'true' ? {path: '.'} : '= 1.11.1')
  gem 'cloud_profiler_agent', git: 'https://github.com/remind101/ruby-cloud-profiler.git'
end

Datadog::Profiling.start_if_enabled

and how it works:

 # Triggering issue on current dd-trace-rb:

$ USE_LOCAL=false ruby trigger.rb
Traceback (most recent call last):
  31: from trigger.rb:6:in `<main>'
  30: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/inline.rb:42:in `gemfile'
  29: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler.rb:418:in `with_unbundled_env'
  28: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler.rb:664:in `with_env'
  27: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler.rb:418:in `block in with_unbundled_env'
  26: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/inline.rb:51:in `block in gemfile'
  25: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/settings.rb:131:in `temporary'
  24: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/inline.rb:66:in `block (2 levels) in gemfile'
  23: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/runtime.rb:44:in `require'
  22: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/runtime.rb:44:in `each'
  21: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/runtime.rb:55:in `block in require'
  20: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/runtime.rb:55:in `each'
  19: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/runtime.rb:60:in `block (2 levels) in require'
  18: from .rvm/gems/ruby-2.7.7/gems/bundler-2.4.6/lib/bundler/runtime.rb:60:in `require'
  17: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/ddtrace.rb:8:in `<top (required)>'
  16: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/ddtrace.rb:8:in `require_relative'
  15: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling.rb:5:in `<top (required)>'
  14: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling.rb:7:in `<module:Datadog>'
  13: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling.rb:209:in `<module:Profiling>'
  12: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling.rb:200:in `load_profiling'
  11: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling.rb:200:in `require_relative'
  10: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling/pprof/pprof_pb.rb:70:in `<top (required)>'
   9: from .rvm/gems/ruby-2.7.7/gems/ddtrace-1.11.1/lib/datadog/profiling/pprof/pprof_pb.rb:71:in `<module:Perftools>'
   8: from .rvm/rubies/ruby-2.7.7/lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
   7: from .rvm/rubies/ruby-2.7.7/lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
   6: from .rvm/gems/ruby-2.7.7/bundler/gems/ruby-cloud-profiler-7faeb201b99e/lib/profile_pb.rb:6:in `<top (required)>'
   5: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:460:in `build'
   4: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:460:in `instance_eval'
   3: from .rvm/gems/ruby-2.7.7/bundler/gems/ruby-cloud-profiler-7faeb201b99e/lib/profile_pb.rb:7:in `block in <top (required)>'
   2: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:43:in `add_file'
   1: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:65:in `internal_add_file'
.rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:65:in `add_serialized_file': Unable to build file to DescriptorPool: duplicate symbol 'perftools.profiles.Profile' (Google::Protobuf::TypeError)

 # Using this branch:

$ USE_LOCAL=true ruby trigger.rb
D, [2023-06-01T18:07:32.516725 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/core/workers/async.rb:134:in `start_worker') Starting thread for: #<Datadog::Core::Telemetry::Heartbeat:0x00005afb4fe59458>
D, [2023-06-01T18:07:32.516779 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/core/configuration/components.rb:98:in `startup!') Profiling started
D, [2023-06-01T18:07:32.516800 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb:60:in `block in start') Starting thread for: #<Datadog::Profiling::Collectors::CpuAndWallTimeWorker:0x00005afb4fe5d9e0>
D, [2023-06-01T18:07:32.516815 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/collectors/idle_sampling_helper.rb:24:in `block in start') Starting thread for: #<Datadog::Profiling::Collectors::IdleSamplingHelper:0x00005afb4fe5d918>
D, [2023-06-01T18:07:32.516840 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/core/workers/async.rb:134:in `start_worker') Starting thread for: #<Datadog::Profiling::Scheduler:0x00005afb4fe5c428>
D, [2023-06-01T18:07:32.517003 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/profiler.rb:27:in `shutdown!') Shutting down profiler
D, [2023-06-01T18:07:32.517027 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb:90:in `block in stop') Requesting CpuAndWallTimeWorker thread shut down
D, [2023-06-01T18:07:32.517038 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/collectors/idle_sampling_helper.rb:52:in `block in stop') Requesting IdleSamplingHelper thread shut down
D, [2023-06-01T18:07:32.517120 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/collectors/idle_sampling_helper.rb:36:in `block (2 levels) in start') IdleSamplingHelper thread stopping cleanly
D, [2023-06-01T18:07:32.527251 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/profiling/collectors/cpu_and_wall_time_worker.rb:70:in `block (2 levels) in start') CpuAndWallTimeWorker thread stopping cleanly
D, [2023-06-01T18:07:32.527668 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/core/workers/polling.rb:31:in `stop') Timeout while waiting for worker to finish gracefully, forcing termination for: #<Datadog::Core::Telemetry::Heartbeat:0x00005afb4fe59458>
D, [2023-06-01T18:07:32.527722 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/core/workers/async.rb:46:in `terminate') Forcibly terminating worker thread for: #<Datadog::Core::Telemetry::Heartbeat:0x00005afb4fe59458>
D, [2023-06-01T18:07:32.528059 #855995] DEBUG -- ddtrace: [ddtrace] (lib/datadog/core/telemetry/http/adapters/net.rb:45:in `rescue in post') Unable to send telemetry event to agent: Failed to open TCP connection to 127.0.0.1:8126 (Connection refused - connect(2) for "127.0.0.1" port 8126)

 # Using this branch + legacy profiler: (Still expected to fail)

$ USE_LOCAL=true DD_PROFILING_FORCE_ENABLE_LEGACY=true ruby trigger.rb
W, [2023-06-01T18:07:46.414800 #856348]  WARN -- ddtrace: [ddtrace] (lib/datadog/core/configuration/settings.rb:291:in `block (4 levels) in <class:Settings>') The profiling.advanced.force_enable_legacy_profiler setting has been deprecated for removal. Do not use unless instructed to by support. If you needed to use it due to incompatibilities with the CPU Profiling 2.0 profiler, consider using the profiling.advanced.no_signals_workaround_enabled setting instead. See <https://dtdg.co/ruby-profiler-troubleshooting> for details.
W, [2023-06-01T18:07:46.414833 #856348]  WARN -- ddtrace: [ddtrace] (lib/datadog/profiling/component.rb:147:in `enable_new_profiler?') Legacy profiler has been force-enabled via configuration. Do not use unless instructed to by support.
Traceback (most recent call last):
  24: from trigger.rb:12:in `<main>'
  23: from lib/datadog/profiling.rb:35:in `start_if_enabled'
  22: from lib/datadog/core/configuration.rb:195:in `components'
  21: from lib/datadog/core/configuration.rb:228:in `safely_synchronize'
  20: from lib/datadog/core/configuration.rb:228:in `synchronize'
  19: from lib/datadog/core/configuration.rb:230:in `block in safely_synchronize'
  18: from lib/datadog/core/configuration.rb:196:in `block in components'
  17: from lib/datadog/core/configuration.rb:248:in `build_components'
  16: from lib/datadog/core/configuration.rb:248:in `new'
  15: from lib/datadog/ci/configuration/components.rb:15:in `initialize'
  14: from lib/datadog/core/configuration/components.rb:83:in `initialize'
  13: from lib/datadog/profiling/component.rb:82:in `build_profiler_component'
  12: from lib/datadog/profiling/component.rb:271:in `load_pprof_support'
  11: from lib/datadog/profiling/component.rb:271:in `require_relative'
  10: from lib/datadog/profiling/pprof/pprof_pb.rb:70:in `<top (required)>'
   9: from lib/datadog/profiling/pprof/pprof_pb.rb:71:in `<module:Perftools>'
   8: from .rvm/rubies/ruby-2.7.7/lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
   7: from .rvm/rubies/ruby-2.7.7/lib/ruby/site_ruby/2.7.0/rubygems/core_ext/kernel_require.rb:37:in `require'
   6: from .rvm/gems/ruby-2.7.7/bundler/gems/ruby-cloud-profiler-7faeb201b99e/lib/profile_pb.rb:6:in `<top (required)>'
   5: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:460:in `build'
   4: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:460:in `instance_eval'
   3: from .rvm/gems/ruby-2.7.7/bundler/gems/ruby-cloud-profiler-7faeb201b99e/lib/profile_pb.rb:7:in `block in <top (required)>'
   2: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:43:in `add_file'
   1: from .rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:65:in `internal_add_file'
.rvm/gems/ruby-2.7.7/gems/google-protobuf-3.23.2-x86_64-linux/lib/google/protobuf/descriptor_dsl.rb:65:in `add_serialized_file': Unable to build file to DescriptorPool: duplicate symbol 'perftools.profiles.Profile' (Google::Protobuf::TypeError)

[lloeki]

LGTM

[lloeki]

Does it make sense to keep this commented require line (and comment)? It seems like it's only there now because before this commit it used to be there.

Is this specific spot supposed to require everything under datadog/profiling and this is an exception? Is it the only exception?

[ivoanjo]

Is this specific spot supposed to require everything under datadog/profiling and this is an exception? Is it the only exception?

Yup, that's what I'm going for.

I went and checked and this is not the only exception -- right now there's still some other files belonging to the old profiler that get required by other parts of the old profiler, instead of being required from here.

(From the new profiler there's only 2 files that aren't loaded from here; I expect to fix this after removing the old profiler)

Does it make sense to keep this commented require line (and comment)? It seems like it's only there now because before this commit it used to be there.

I decided to leave it as documentation that there's something important about how we load that file, and that differs from everything else that is loaded eagerly here, directly or indirectly.

That is, I wanted to highlight that after Datadog::Profiling.load_profiling finishes all of profiling is loaded -- both old and new -- except that one file, which we load in a very specific situation/way.

[lloeki]

That is, I wanted to highlight that after Datadog::Profiling.load_profiling finishes all of profiling is loaded -- both old and new --

I guess this was a bit ambiguous to me if that was the intent...

except that one file, which we load in a very specific situation/way.

... which in turn made that part ambiguous.

May I suggest adding a comment like # we load everything just before the require list would clarify that, and then # ... except these which we load upon dynamic conditions, with the intentionally missing requires thoroughly listed?

Tangent: I think we should do something similar for lib/datadog/appsec as well.

[ivoanjo]

Sounds good! I'll open a PR with that.

[ivoanjo]

Added in #2904