[PROF-2585] Warn on incompatible rollbar version

[ivoanjo]

Rollbar gem versions <= 3.1.1 have an incompatibility with ddtrace's
CPU profiling. This was ack'd and fixed upstream in
rollbar/rollbar-gem#1018.

This commit adds code to detect when customers are still using a legacy
version of rollbar, printing a warning to ask them to upgrade.

Note: Because the fixed version of rollbar has not yet been released,
the tests that validate that the warning is not shown on unpatched
versions are disabled and tagged with a FIXME.

See the "FIXME NEW ROLLBAR NEEDED" note in the setup_spec.rb for more
details on what's disabled and what steps we need to take once the
new version gets released.

First commit @ Datadog 🎉

[codecov-io]

Codecov Report

Merging #1300 (91b5b32) into feature/profiling (6f2f1ce) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@                Coverage Diff                 @@
##           feature/profiling    #1300   +/-   ##
==================================================
  Coverage              97.91%   97.92%           
==================================================
  Files                    848      848           
  Lines                  39745    39779   +34     
==================================================
+ Hits                   38918    38952   +34     
  Misses                   827      827           

Impacted Files	Coverage Δ
lib/ddtrace/profiling/tasks/setup.rb	95.91% <100.00%> (+0.68%)	⬆️
spec/ddtrace/profiling/tasks/setup_spec.rb	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f2f1ce...91b5b32. Read the comment docs.

[ericmustin]

not super familiar with the profiling branch yet but generally lgtm, deferring to @delner here though

[delner]

I think the actual warning itself looks good! However, I think we could leverage a test helper to greatly simplify the tests for this, and eliminate much of the appraisal related changes.

We did this in another file, which tested for behavior based on the presence or non-presence of a gem (and its version):

RSpec.describe Datadog::Contrib::Faraday::Integration do
  extend ConfigurationHelpers

  describe '.version' do
    subject(:version) { described_class.version }

    context 'when the "faraday" gem is loaded' do
      include_context 'loaded gems', faraday: described_class::MINIMUM_VERSION
      it { is_expected.to be_a_kind_of(Gem::Version) }
    end

    context 'when "faraday" gem is not loaded' do
      include_context 'loaded gems', faraday: nil
      it { is_expected.to be nil }
    end
  end

I think you could use this, and remove all the appraisal changes. What do you think @ivoanjo ?

[ivoanjo]

Ah, interesting. It took me a few minutes to wrap my head around what the helper was doing ;)

If I understand correctly, the loaded gems helper simulates looking up the info on a gem that is loaded (e.g. the response from Gem.loaded_specs).

I think the general idea would work for this PR. The tests in my first attempt are more realistic, but they do introduce a number of extra steps (and make testing slower), as well as making it hard for testing all variants in one run (you always need to restart your ruby with different environments).

I can't quite use the helper directly since I'm using Gem::Specification.find_all_by_name which differs from Gem.loaded_specs because it includes gems that may not be loaded:

$ pry
[1] pry(main)> Gem.loaded_specs.keys.include?("rollbar")
=> false
[2] pry(main)> Gem::Specification.find_all_by_name("rollbar").map { |it| [it.name, it.version] }
=> [["rollbar", Gem::Version.new("3.1.1")], ["rollbar", Gem::Version.new("2.27.1")]]

...but the approach is the same -- I shall mock the other method instead.

Now I shall quote Arnie:

[delner]

I think the use of mocks here is appropriate (Gem::Specification is a stable API) and serves us well in simplifying the test code; it's now much easier to reason about its mechanisms.

Nice work @ivoanjo ! Thank you :)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.92%. Comparing base (6f2f1ce) to head (91b5b32).

Additional details and impacted files

@@                Coverage Diff                 @@
##           feature/profiling    #1300   +/-   ##
==================================================
  Coverage              97.91%   97.92%           
==================================================
  Files                    848      848           
  Lines                  39745    39779   +34     
==================================================
+ Hits                   38918    38952   +34     
  Misses                   827      827           

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.