From 9d23b6faba2101fd3b7fc81570f73548c5fb131e Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Thu, 22 Aug 2024 13:09:40 +0200 Subject: [PATCH 1/5] Add a generic way for skipping specs (#2385) --- sentry-ruby/spec/sentry/event_spec.rb | 2 +- sentry-ruby/spec/sentry/profiler_spec.rb | 4 +--- .../spec/sentry/rack/capture_exceptions_spec.rb | 4 +--- sentry-ruby/spec/sentry/scope/setters_spec.rb | 2 +- sentry-ruby/spec/sentry/scope_spec.rb | 2 +- sentry-ruby/spec/spec_helper.rb | 16 ++++++++++++++-- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/sentry-ruby/spec/sentry/event_spec.rb b/sentry-ruby/spec/sentry/event_spec.rb index 5eeb232f9..ea59f95e1 100644 --- a/sentry-ruby/spec/sentry/event_spec.rb +++ b/sentry-ruby/spec/sentry/event_spec.rb @@ -66,7 +66,7 @@ end end - context 'rack context specified', rack: true do + context 'rack context specified', when: :rack_available? do require 'stringio' before do diff --git a/sentry-ruby/spec/sentry/profiler_spec.rb b/sentry-ruby/spec/sentry/profiler_spec.rb index 215e739be..e0cba8177 100644 --- a/sentry-ruby/spec/sentry/profiler_spec.rb +++ b/sentry-ruby/spec/sentry/profiler_spec.rb @@ -1,8 +1,6 @@ require "spec_helper" -return unless defined?(StackProf) - -RSpec.describe Sentry::Profiler do +RSpec.describe Sentry::Profiler, when: :stack_prof_installed? do before do perform_basic_setup do |config| config.traces_sample_rate = 1.0 diff --git a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb index e03984cdf..e2a226f77 100644 --- a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb +++ b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb @@ -1,8 +1,6 @@ require 'spec_helper' -return unless defined?(Rack) - -RSpec.describe Sentry::Rack::CaptureExceptions, rack: true do +RSpec.describe Sentry::Rack::CaptureExceptions, when: :rack_available? do let(:exception) { ZeroDivisionError.new("divided by 0") } let(:additional_headers) { {} } let(:env) { Rack::MockRequest.env_for("/test", additional_headers) } diff --git a/sentry-ruby/spec/sentry/scope/setters_spec.rb b/sentry-ruby/spec/sentry/scope/setters_spec.rb index 3ca240488..6a5378194 100644 --- a/sentry-ruby/spec/sentry/scope/setters_spec.rb +++ b/sentry-ruby/spec/sentry/scope/setters_spec.rb @@ -7,7 +7,7 @@ new_breadcrumb end - describe "#set_rack_env", rack: true do + describe "#set_rack_env", when: :rack_available? do let(:env) do Rack::MockRequest.env_for("/test", {}) end diff --git a/sentry-ruby/spec/sentry/scope_spec.rb b/sentry-ruby/spec/sentry/scope_spec.rb index 5de4d8d6d..9b52244a8 100644 --- a/sentry-ruby/spec/sentry/scope_spec.rb +++ b/sentry-ruby/spec/sentry/scope_spec.rb @@ -309,7 +309,7 @@ expect(event.dynamic_sampling_context).to eq(subject.propagation_context.get_dynamic_sampling_context) end - context "with Rack", rack: true do + context "with Rack", when: :rack_available? do let(:env) do Rack::MockRequest.env_for("/test", {}) end diff --git a/sentry-ruby/spec/spec_helper.rb b/sentry-ruby/spec/spec_helper.rb index 611205163..2758a0a1f 100644 --- a/sentry-ruby/spec/spec_helper.rb +++ b/sentry-ruby/spec/spec_helper.rb @@ -46,8 +46,10 @@ ENV.delete('RACK_ENV') end - config.before(:each, rack: true) do - skip("skip rack related tests") unless defined?(Rack) + config.before(:each, when: true) do |example| + meth = example.metadata[:when] + + skip("Skipping because `when: #{meth}` returned false") unless TestHelpers.public_send(meth, example) end RSpec::Matchers.define :have_recorded_lost_event do |reason, data_category, num: 1| @@ -57,6 +59,16 @@ end end +module TestHelpers + def self.stack_prof_installed?(_example) + defined?(StackProf) + end + + def self.rack_available?(_example) + defined?(Rack) + end +end + def fixtures_root @fixtures_root ||= Pathname(__dir__).join("fixtures") end From 51299fa3b188a0fbc2e47cb6f892c91a777ca66e Mon Sep 17 00:00:00 2001 From: Peter Solnica Date: Mon, 26 Aug 2024 10:04:05 +0200 Subject: [PATCH 2/5] Improve CI (#2388) * Remove rubocop from the bundle when running tests on CI * Measure coverage under ruby 3.3 * Test against latest rack and redis-rb * Fix rack spec when rack is not present --- .github/workflows/sentry_ruby_test.yml | 11 +++-- Gemfile | 6 ++- .../sentry/rack/capture_exceptions_spec.rb | 48 +++++++++---------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/.github/workflows/sentry_ruby_test.yml b/.github/workflows/sentry_ruby_test.yml index a154df326..688b54042 100644 --- a/.github/workflows/sentry_ruby_test.yml +++ b/.github/workflows/sentry_ruby_test.yml @@ -29,7 +29,7 @@ jobs: fail-fast: false matrix: ruby_version: ${{ fromJson(needs.ruby-versions.outputs.versions) }} - rack_version: [2.0, 3.0] + rack_version: [2.0, 3.0, 3.1] redis_rb_version: [4.0] include: - { ruby_version: 3.2, rack_version: 0, redis_rb_version: 5.0 } @@ -45,8 +45,12 @@ jobs: } - { ruby_version: 3.2, - rack_version: 3.0, - redis_rb_version: 5.0, + rack_version: 3.0 + } + - { + ruby_version: 3.3, + rack_version: 3.1, + redis_rb_version: 5.3, options: { codecov: 1 }, } steps: @@ -69,6 +73,7 @@ jobs: RACK_VERSION: ${{ matrix.rack_version }} REDIS_RB_VERSION: ${{ matrix.redis_rb_version }} run: | + bundle config set without 'rubocop' bundle install --jobs 4 --retry 3 bundle exec rake diff --git a/Gemfile b/Gemfile index c42bc20c6..c897e543f 100644 --- a/Gemfile +++ b/Gemfile @@ -19,5 +19,7 @@ gem "simplecov" gem "simplecov-cobertura", "~> 1.4" gem "rexml" -gem "rubocop-rails-omakase" -gem "rubocop-packaging" +group :rubocop do + gem "rubocop-rails-omakase" + gem "rubocop-packaging" +end diff --git a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb index e2a226f77..dfd42ab33 100644 --- a/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb +++ b/sentry-ruby/spec/sentry/rack/capture_exceptions_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -RSpec.describe Sentry::Rack::CaptureExceptions, when: :rack_available? do +RSpec.describe 'Sentry::Rack::CaptureExceptions', when: :rack_available? do let(:exception) { ZeroDivisionError.new("divided by 0") } let(:additional_headers) { {} } let(:env) { Rack::MockRequest.env_for("/test", additional_headers) } @@ -12,7 +12,7 @@ it 'captures the exception from direct raise' do app = ->(_e) { raise exception } - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect { stack.call(env) }.to raise_error(ZeroDivisionError) @@ -25,7 +25,7 @@ it 'has the correct mechanism' do app = ->(_e) { raise exception } - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect { stack.call(env) }.to raise_error(ZeroDivisionError) @@ -39,7 +39,7 @@ e['rack.exception'] = exception [200, {}, ['okay']] end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect do stack.call(env) @@ -55,7 +55,7 @@ e['sinatra.error'] = exception [200, {}, ['okay']] end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect do stack.call(env) @@ -70,7 +70,7 @@ e['rack.exception'] = exception [200, {}, ['okay']] end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) stack.call(env) @@ -86,7 +86,7 @@ [200, { 'content-type' => 'text/plain' }, ['OK']] end - stack = described_class.new(Rack::Lint.new(app)) + stack = Sentry::Rack::CaptureExceptions.new(Rack::Lint.new(app)) expect { stack.call(env) }.to_not raise_error expect(env.key?("sentry.error_event_id")).to eq(false) end @@ -109,7 +109,7 @@ a / b end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect { stack.call(env) }.to raise_error(ZeroDivisionError) @@ -133,7 +133,7 @@ def inspect a / b end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect { stack.call(env) }.to raise_error(ZeroDivisionError) @@ -151,7 +151,7 @@ def inspect a / b end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect { stack.call(env) }.to raise_error(ZeroDivisionError) @@ -179,7 +179,7 @@ def inspect [200, {}, ["ok"]] end - app_1 = described_class.new(request_1) + app_1 = Sentry::Rack::CaptureExceptions.new(request_1) app_1.call(env) @@ -193,7 +193,7 @@ def inspect Sentry.capture_message("test") [200, {}, ["ok"]] end - app_1 = described_class.new(request_1) + app_1 = Sentry::Rack::CaptureExceptions.new(request_1) app_1.call(env) @@ -207,7 +207,7 @@ def inspect e['rack.exception'] = Exception.new [200, {}, ["ok"]] end - app_1 = described_class.new(request_1) + app_1 = Sentry::Rack::CaptureExceptions.new(request_1) app_1.call(env) event = last_sentry_event @@ -219,7 +219,7 @@ def inspect e['rack.exception'] = Exception.new [200, {}, ["ok"]] end - app_2 = described_class.new(request_2) + app_2 = Sentry::Rack::CaptureExceptions.new(request_2) app_2.call(env) event = last_sentry_event @@ -248,7 +248,7 @@ def inspect end let(:stack) do - described_class.new( + Sentry::Rack::CaptureExceptions.new( ->(_) do [200, {}, ["ok"]] end @@ -436,7 +436,7 @@ def will_be_sampled_by_sdk [200, {}, ["ok"]] end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) stack.call(env) @@ -460,7 +460,7 @@ def will_be_sampled_by_sdk end end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) stack.call(env) @@ -494,7 +494,7 @@ def will_be_sampled_by_sdk [200, {}, ["ok"]] end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) stack.call(env) @@ -512,7 +512,7 @@ def will_be_sampled_by_sdk raise "foo" end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect do stack.call(env) @@ -539,7 +539,7 @@ def will_be_sampled_by_sdk end let(:stack) do - described_class.new( + Sentry::Rack::CaptureExceptions.new( ->(_) do [200, {}, ["ok"]] end @@ -586,7 +586,7 @@ def will_be_sampled_by_sdk let(:stack) do app = ->(_e) { raise exception } - described_class.new(app) + Sentry::Rack::CaptureExceptions.new(app) end before { perform_basic_setup } @@ -618,7 +618,7 @@ def will_be_sampled_by_sdk expect_any_instance_of(Sentry::Hub).not_to receive(:start_session) expect(Sentry.session_flusher).to be_nil - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) stack.call(env) expect(sentry_envelopes.count).to eq(0) @@ -644,7 +644,7 @@ def will_be_sampled_by_sdk end end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) expect(Sentry.session_flusher).not_to be_nil @@ -707,7 +707,7 @@ def will_be_sampled_by_sdk [200, {}, "ok"] end - stack = described_class.new(app) + stack = Sentry::Rack::CaptureExceptions.new(app) stack.call(env) event = last_sentry_event From adf38eb7545f946118756d5ce5a21818ad29e47d Mon Sep 17 00:00:00 2001 From: y-yagi Date: Tue, 27 Aug 2024 22:34:42 +0900 Subject: [PATCH 3/5] Fix a typo of the issue number in CHANGELOG (#2375) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8937d0134..0aa3ea8c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,7 @@ ``` - Transaction data are now included in the context ([#2365](https://github.com/getsentry/sentry-ruby/pull/2365)) - - Closes [#2364](https://github.com/getsentry/sentry-ruby/issues/2363) + - Closes [#2363](https://github.com/getsentry/sentry-ruby/issues/2363) - Inject Sentry meta tags in the Rails application layout automatically in the generator ([#2369](https://github.com/getsentry/sentry-ruby/pull/2369)) From 7b69425af24c1d70c968b9a460f0b022ac945db8 Mon Sep 17 00:00:00 2001 From: joshuarli Date: Tue, 27 Aug 2024 06:36:05 -0700 Subject: [PATCH 4/5] all-repos: update actions/upload-artifact to v4 (#2359) Committed via https://github.com/asottile/all-repos --- .github/workflows/build_batch_release.yml | 2 +- .github/workflows/build_release.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build_batch_release.yml b/.github/workflows/build_batch_release.yml index ffc30fcbd..ff584e14d 100644 --- a/.github/workflows/build_batch_release.yml +++ b/.github/workflows/build_batch_release.yml @@ -17,7 +17,7 @@ jobs: - name: Build gem source run: ruby .scripts/batch_build.rb - name: Archive Artifacts - uses: actions/upload-artifact@v3.1.1 + uses: actions/upload-artifact@v4 with: name: ${{ github.sha }} path: sentry*/*.gem diff --git a/.github/workflows/build_release.yml b/.github/workflows/build_release.yml index f9cad4c6e..3dfe24f81 100644 --- a/.github/workflows/build_release.yml +++ b/.github/workflows/build_release.yml @@ -27,7 +27,7 @@ jobs: working-directory: ${{env.sdk-directory}} run: make build - name: Archive Artifacts - uses: actions/upload-artifact@v3.1.1 + uses: actions/upload-artifact@v4 with: name: ${{ github.sha }} path: ${{env.sdk-directory}}/*.gem From 4db5f7469cb7b8897e363454a2183a55d08d879d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Zapa=C5=9Bnik?= <9281806+adamzapasnik@users.noreply.github.com> Date: Tue, 27 Aug 2024 15:38:30 +0200 Subject: [PATCH 5/5] Improve sidekiq-cron patch (#2387) --- CHANGELOG.md | 3 +++ sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb | 4 ++-- .../spec/fixtures/sidekiq-cron-schedule.yml | 4 ++++ .../spec/sentry/sidekiq/cron/job_spec.rb | 18 ++++++++++++++++++ sentry-sidekiq/spec/spec_helper.rb | 2 ++ 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aa3ea8c7..4430e9fb7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## Unreleased +### Features + - Add support for $SENTRY_DEBUG and $SENTRY_SPOTLIGHT ([#2374](https://github.com/getsentry/sentry-ruby/pull/2374)) +- Support human readable intervals in `sidekiq-cron` ([#2387](https://github.com/getsentry/sentry-ruby/pull/2387)) ## 5.19.0 diff --git a/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb b/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb index 4098d7564..937fd797c 100644 --- a/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb +++ b/sentry-sidekiq/lib/sentry/sidekiq/cron/job.rb @@ -28,8 +28,8 @@ def save unless klass_const.send(:ancestors).include?(Sentry::Cron::MonitorCheckIns) klass_const.send(:include, Sentry::Cron::MonitorCheckIns) klass_const.send(:sentry_monitor_check_ins, - slug: name, - monitor_config: Sentry::Cron::MonitorConfig.from_crontab(cron)) + slug: name.to_s, + monitor_config: Sentry::Cron::MonitorConfig.from_crontab(parsed_cron.original)) end true diff --git a/sentry-sidekiq/spec/fixtures/sidekiq-cron-schedule.yml b/sentry-sidekiq/spec/fixtures/sidekiq-cron-schedule.yml index 65b5945a6..fe2798262 100644 --- a/sentry-sidekiq/spec/fixtures/sidekiq-cron-schedule.yml +++ b/sentry-sidekiq/spec/fixtures/sidekiq-cron-schedule.yml @@ -6,6 +6,10 @@ manual: cron: "* * * * *" class: "SadWorkerWithCron" +human_readable_cron: + cron: "every 5 minutes" + class: HappyWorkerWithHumanReadableCron + invalid_cron: cron: "not a crontab" class: "ReportingWorker" diff --git a/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb b/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb index 44f02f86e..c0f223096 100644 --- a/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb +++ b/sentry-sidekiq/spec/sentry/sidekiq/cron/job_spec.rb @@ -10,6 +10,7 @@ before do schedule_file = 'spec/fixtures/sidekiq-cron-schedule.yml' schedule = Sidekiq::Cron::Support.load_yaml(ERB.new(IO.read(schedule_file)).result) + schedule = schedule.merge(symbol_name: { cron: '* * * * *', class: HappyWorkerWithSymbolName }) # sidekiq-cron 2.0+ accepts second argument to `load_from_hash!` with options, # such as {source: 'schedule'}, but sidekiq-cron 1.9.1 (last version to support Ruby 2.6) does not. # Since we're not using the source option in our code anyway, it's safe to not pass the 2nd arg. @@ -48,6 +49,23 @@ expect(HappyWorkerForCron.sentry_monitor_config.schedule.value).to eq('* * * * *') end + it 'patches HappyWorkerWithHumanReadableCron' do + expect(HappyWorkerWithHumanReadableCron.ancestors).to include(Sentry::Cron::MonitorCheckIns) + expect(HappyWorkerWithHumanReadableCron.sentry_monitor_slug).to eq('human_readable_cron') + expect(HappyWorkerWithHumanReadableCron.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig) + expect(HappyWorkerWithHumanReadableCron.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab) + expect(HappyWorkerWithHumanReadableCron.sentry_monitor_config.schedule.value).to eq('*/5 * * * *') + end + + it 'patches HappyWorkerWithSymbolName' do + expect(HappyWorkerWithSymbolName.ancestors).to include(Sentry::Cron::MonitorCheckIns) + expect(HappyWorkerWithSymbolName.sentry_monitor_slug).to eq('symbol_name') + expect(HappyWorkerWithSymbolName.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig) + expect(HappyWorkerWithSymbolName.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab) + expect(HappyWorkerWithSymbolName.sentry_monitor_config.schedule.value).to eq('* * * * *') + end + + it 'does not override SadWorkerWithCron manually set values' do expect(SadWorkerWithCron.ancestors).to include(Sentry::Cron::MonitorCheckIns) expect(SadWorkerWithCron.sentry_monitor_slug).to eq('failed_job') diff --git a/sentry-sidekiq/spec/spec_helper.rb b/sentry-sidekiq/spec/spec_helper.rb index 83e55d7da..6b2287957 100644 --- a/sentry-sidekiq/spec/spec_helper.rb +++ b/sentry-sidekiq/spec/spec_helper.rb @@ -139,6 +139,8 @@ class HappyWorkerForCron < HappyWorker; end class HappyWorkerForScheduler < HappyWorker; end class HappyWorkerForSchedulerWithTimezone < HappyWorker; end class EveryHappyWorker < HappyWorker; end +class HappyWorkerWithHumanReadableCron < HappyWorker; end +class HappyWorkerWithSymbolName < HappyWorker; end class HappyWorkerWithCron < HappyWorker include Sentry::Cron::MonitorCheckIns