Skip to content

Commit

Permalink
[PROF-2585] Warn on incompatible rollbar version
Browse files Browse the repository at this point in the history 
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 🎉
  • Loading branch information
@ivoanjo
ivoanjo committed Jan 6, 2021
1 parent 6f2f1ce commit 64a3b6a
Show file tree
Hide file tree
Showing 4 changed files with 208 additions and 0 deletions.
72 changes: 72 additions & 0 deletions Appraisals
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ elsif Gem::Version.new('2.0.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'lograge', '< 0.4'
end

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib-old' do
gem 'active_model_serializers', '~> 0.9.0'
gem 'activerecord', '3.2.22.5'
Expand Down Expand Up @@ -194,6 +203,15 @@ elsif Gem::Version.new('2.1.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'lograge'
end

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib-old' do
gem 'active_model_serializers', '~> 0.9.0'
gem 'activerecord', '3.2.22.5'
Expand Down Expand Up @@ -368,6 +386,15 @@ elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION) \
gem 'lograge'
end

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib' do
gem 'actionpack'
gem 'actionview'
Expand Down Expand Up @@ -561,6 +588,15 @@ elsif Gem::Version.new('2.3.0') <= Gem::Version.new(RUBY_VERSION) \

(3..4).each { |v| gem_cucumber(v) }

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib' do
gem 'actionpack'
gem 'actionview'
Expand Down Expand Up @@ -664,6 +700,15 @@ elsif Gem::Version.new('2.4.0') <= Gem::Version.new(RUBY_VERSION) \

(3..4).each { |v| gem_cucumber(v) }

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib' do
gem 'actionpack'
gem 'actionview'
Expand Down Expand Up @@ -816,6 +861,15 @@ elsif Gem::Version.new('2.5.0') <= Gem::Version.new(RUBY_VERSION) \

(3..5).each { |v| gem_cucumber(v) }

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib' do
gem 'actionpack'
gem 'actionview'
Expand Down Expand Up @@ -959,6 +1013,15 @@ elsif Gem::Version.new('2.6.0') <= Gem::Version.new(RUBY_VERSION) \

(3..5).each { |v| gem_cucumber(v) }

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib' do
gem 'actionpack'
gem 'actionview'
Expand Down Expand Up @@ -1104,6 +1167,15 @@ elsif Gem::Version.new('2.7.0') <= Gem::Version.new(RUBY_VERSION)

(3..5).each { |v| gem_cucumber(v) }

appraise 'incompatible-rollbar' do
gem 'rollbar', '= 3.1.1'
end

appraise 'compatible-rollbar' do
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
#gem 'rollbar', '= 3.1.2'
end

appraise 'contrib' do
gem 'actionpack'
gem 'actionview'
Expand Down
37 changes: 37 additions & 0 deletions Rakefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@ namespace :spec do
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:rollbar) do |t, args|
t.pattern = 'spec/ddtrace/profiling/tasks/setup_spec.rb'
t.rspec_opts = args.to_a.join(' ')
end

RSpec::Core::RakeTask.new(:contrib) do |t, args|
contrib_paths = [
'analytics',
Expand Down Expand Up @@ -259,6 +264,10 @@ task :ci do
declare 'bundle exec appraisal rails32-postgres rake spec:action_view'
declare 'bundle exec appraisal rails32-mysql2 rake spec:active_record'
declare 'bundle exec appraisal rails32-postgres rake spec:active_support'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
elsif Gem::Version.new('2.1.0') <= Gem::Version.new(RUBY_VERSION) \
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.2.0')
Expand Down Expand Up @@ -322,6 +331,10 @@ task :ci do
declare 'bundle exec appraisal rails32-postgres rake spec:action_view'
declare 'bundle exec appraisal rails32-mysql2 rake spec:active_record'
declare 'bundle exec appraisal rails32-postgres rake spec:active_support'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
elsif Gem::Version.new('2.2.0') <= Gem::Version.new(RUBY_VERSION)\
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.3.0')
Expand Down Expand Up @@ -398,6 +411,10 @@ task :ci do
declare 'bundle exec appraisal rails4-postgres rake spec:rails'
declare 'bundle exec appraisal rails5-mysql2 rake spec:rails'
declare 'bundle exec appraisal rails5-postgres rake spec:rails'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
elsif Gem::Version.new('2.3.0') <= Gem::Version.new(RUBY_VERSION) \
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.4.0')
Expand Down Expand Up @@ -479,6 +496,10 @@ task :ci do
# explicitly test resque-2x compatability
declare 'bundle exec appraisal resque2-redis3 rake spec:resque'
declare 'bundle exec appraisal resque2-redis4 rake spec:resque'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
elsif Gem::Version.new('2.4.0') <= Gem::Version.new(RUBY_VERSION) \
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.5.0')
Expand Down Expand Up @@ -550,6 +571,10 @@ task :ci do
# explicitly test cucumber compatibility
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
elsif Gem::Version.new('2.5.0') <= Gem::Version.new(RUBY_VERSION) \
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.6.0')
Expand Down Expand Up @@ -632,6 +657,10 @@ task :ci do
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
declare 'bundle exec appraisal cucumber5 rake spec:cucumber'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
elsif Gem::Version.new('2.6.0') <= Gem::Version.new(RUBY_VERSION) \
&& Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.7.0')
# Main library
Expand Down Expand Up @@ -716,6 +745,10 @@ task :ci do
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
declare 'bundle exec appraisal cucumber5 rake spec:cucumber'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
elsif Gem::Version.new('2.7.0') <= Gem::Version.new(RUBY_VERSION)
# Main library
Expand Down Expand Up @@ -799,6 +832,10 @@ task :ci do
declare 'bundle exec appraisal cucumber3 rake spec:cucumber'
declare 'bundle exec appraisal cucumber4 rake spec:cucumber'
declare 'bundle exec appraisal cucumber5 rake spec:cucumber'

declare 'bundle exec appraisal rollbar-incompatible rake spec:rollbar'
# FIXME: See note under "FIXME NEW ROLLBAR NEEDED" in setup_spec.rb for details
# declare 'bundle exec appraisal rollbar-compatible rake spec:rollbar'
end
end
end
Expand Down
17 changes: 17 additions & 0 deletions lib/ddtrace/profiling/tasks/setup.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module Tasks
# Sets up profiling for the application
class Setup
def run
check_warnings!
activate_main_extensions
autostart_profiler
end
Expand Down Expand Up @@ -79,6 +80,22 @@ def autostart_profiler
log "[DDTRACE] Could not autostart profiling. Cause: #{e.message} Location: #{e.backtrace.first}"
end

def check_warnings!
warn_if_incompatible_rollbar_gem_detected
end

def warn_if_incompatible_rollbar_gem_detected
incompatible_rollbar_versions = Gem::Requirement.new('<= 3.1.1')

if Gem::Specification.find_all_by_name('rollbar', incompatible_rollbar_versions).any?

log "[DDTRACE] Incompatible version of the rollbar gem is installed (#{incompatible_rollbar_versions}). " \
'Loading this version of the rollbar gem will disable ddtrace\'s CPU profiling. ' \
'Please upgrade to the latest rollbar version. ' \
'See https://github.com/rollbar/rollbar-gem/pull/1018 for details.'
end
end

private

def log(message)
Expand Down
82 changes: 82 additions & 0 deletions spec/ddtrace/profiling/tasks/setup_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
subject(:run) { task.run }

it do
expect(task).to receive(:check_warnings!).ordered
expect(task).to receive(:activate_main_extensions).ordered
expect(task).to receive(:autostart_profiler).ordered
run
Expand Down Expand Up @@ -346,4 +347,85 @@
end
end
end

describe '#check_warnings!' do
subject(:check_warnings!) { task.check_warnings! }

it do
expect(task).to receive(:warn_if_incompatible_rollbar_gem_detected)

check_warnings!
end
end

describe '#warn_if_incompatible_rollbar_gem_detected' do
subject(:warn_if_incompatible_rollbar_gem_detected) { task.warn_if_incompatible_rollbar_gem_detected }

# Testing this code is slightly awkward for two reasons:
# 1. We want to avoid using exactly the same code (gem apis) in the tests than we have for production.
# 2. A given Ruby installation can only be in one of the possible states (no rollbar installed, old rollbar
# installed, etc)
#
# To address this, we:
# 1. Shell out to gem to detect if rollbar is installed or not (rather than using the same gem apis we want to test)
# 2. Rely on the Appraisal gem test setup to be able to check the multiple cases in different runs

before(:context) do
@rollbar_versions_installed =
`gem list`.each_line.select { |it| it.start_with?('rollbar ') }.sort.map(&:strip)
end

context 'when rollbar gem is not installed' do
before do
if @rollbar_versions_installed.any?
skip "Current gem environment (#{@rollbar_versions_installed}) not setup for this test"
end
end

it 'does not display a warning to STDOUT' do
expect(STDOUT).to_not receive(:puts)

warn_if_incompatible_rollbar_gem_detected
end
end

context 'when compatible version of rollbar gem is installed' do
before do
skip %q(FIXME NEW ROLLBAR NEEDED: This test cannot be enabled until a new version of rollbar (> 3.1.1) including
"https://github.com/rollbar/rollbar-gem/pull/1018" is released.
Once this new version is released, we need to:
1. Remove this skip
2. Update the `Appraisals` file to enable the new version in the 'compatible-rollbar' appraisals~
3. Enable the 'compatible-rollbar' validations in the `Rakefile`.)

if @rollbar_versions_installed != ['rollbar (3.1.2)']
skip "Current gem environment (#{@rollbar_versions_installed}) not setup for this test"
end
end

it 'does not display a warning to STDOUT' do
expect(STDOUT).to_not receive(:puts)

warn_if_incompatible_rollbar_gem_detected
end
end

context 'when incompatible version of rollbar gem is installed' do
before do
if @rollbar_versions_installed != ['rollbar (3.1.1)']
skip "Current gem environment (#{@rollbar_versions_installed}) not setup for this test"
end
end

it 'displays a warning to STDOUT' do
expect(STDOUT).to receive(:puts) do |message|
STDERR.puts(message)
expect(message).to include('Incompatible version of the rollbar')
end

warn_if_incompatible_rollbar_gem_detected
end
end
end
end

0 comments on commit 64a3b6a

Please sign in to comment.