Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix keyword arguments for MonitorCheckIns monkeypatch #2199

Merged
merged 4 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
## Unreleased
## 5.15.1

### Features

- Expose `configuration.background_worker_max_queue` to control thread pool queue size [#2195](https://github.com/getsentry/sentry-ruby/pull/2195)

### Bug Fixes

- Fix `Sentry::Cron::MonitorCheckIns` monkeypatch keyword arguments [#2199](https://github.com/getsentry/sentry-ruby/pull/2199)
- Fixes [#2198](https://github.com/getsentry/sentry-ruby/issues/2198)

## 5.15.0

### Features
Expand Down
2 changes: 2 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ ruby_version = Gem::Version.new(RUBY_VERSION)
if ruby_version >= Gem::Version.new("2.7.0")
gem "debug", github: "ruby/debug", platform: :ruby
gem "irb"
# new release breaks on jruby
gem "io-console", "0.6.0"

if ruby_version >= Gem::Version.new("3.0.0")
gem "ruby-lsp-rspec"
Expand Down
5 changes: 3 additions & 2 deletions sentry-ruby/lib/sentry/cron/monitor_check_ins.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
MAX_SLUG_LENGTH = 50

module Patch
def perform(*args)
def perform(*args, **opts)
slug = self.class.sentry_monitor_slug
monitor_config = self.class.sentry_monitor_config

Expand All @@ -13,7 +13,8 @@
monitor_config: monitor_config)

start = Sentry.utc_now.to_i
ret = super
# need to do this on ruby <= 2.6 sadly
ret = method(:perform).super_method.arity == 0 ? super() : super

Check warning on line 17 in sentry-ruby/lib/sentry/cron/monitor_check_ins.rb

View check run for this annotation

Codecov / codecov/patch

sentry-ruby/lib/sentry/cron/monitor_check_ins.rb#L17

Added line #L17 was not covered by tests
duration = Sentry.utc_now.to_i - start

Sentry.capture_check_in(slug,
Expand Down
90 changes: 69 additions & 21 deletions sentry-ruby/spec/sentry/cron/monitor_check_ins_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,22 @@
RSpec.describe Sentry::Cron::MonitorCheckIns do
before { perform_basic_setup }

shared_examples 'original_job' do
it 'does the work' do
expect(job).to receive(:work).with(1, 42, 99).and_call_original
expect(job.perform(1)).to eq(142)
end

it 'does the work with args' do
expect(job).to receive(:work).with(1, 43, 100).and_call_original
expect(job.perform(1, 43, c: 100)).to eq(144)
end
end

context 'without including mixin' do
before do
job_class = Class.new do
def work(a, b, c); end
def work(a, b, c); a + b + c end

def perform(a, b = 42, c: 99)
work(a, b, c)
Expand All @@ -18,10 +30,7 @@ def perform(a, b = 42, c: 99)

let(:job) { Job.new }

it 'does the work' do
expect(job).to receive(:work).with(1, 42, 99)
job.perform(1)
end
it_behaves_like 'original_job'

it 'does not call capture_check_in' do
expect(Sentry).not_to receive(:capture_check_in)
Expand All @@ -37,7 +46,7 @@ def perform(a, b = 42, c: 99)
job_class = Class.new do
include mod

def work(a, b, c); end
def work(a, b, c); a + b + c end

def perform(a, b = 42, c: 99)
work(a, b, c)
Expand All @@ -49,10 +58,7 @@ def perform(a, b = 42, c: 99)

let(:job) { Job.new }

it 'does the work' do
expect(job).to receive(:work).with(1, 42, 99)
job.perform(1)
end
it_behaves_like 'original_job'

it 'does not prepend the patch' do
expect(Job.ancestors.first).not_to eq(described_class::Patch)
Expand Down Expand Up @@ -81,7 +87,7 @@ def perform(a, b = 42, c: 99)

sentry_monitor_check_ins

def work(a, b, c); end
def work(a, b, c); a + b + c end

def perform(a, b = 42, c: 99)
work(a, b, c)
Expand All @@ -93,15 +99,55 @@ def perform(a, b = 42, c: 99)

let(:job) { Job.new }

it 'does the work' do
expect(job).to receive(:work).with(1, 42, 99)
it_behaves_like 'original_job'

it 'prepends the patch' do
expect(Job.ancestors.first).to eq(described_class::Patch)
end

it 'calls capture_check_in twice' do
expect(Sentry).to receive(:capture_check_in).with(
'job',
:in_progress,
hash_including(monitor_config: nil)
).ordered.and_call_original

expect(Sentry).to receive(:capture_check_in).with(
'job',
:ok,
hash_including(:check_in_id, monitor_config: nil, duration: 0)
).ordered.and_call_original

job.perform(1)
end
end

context 'patched perform with arity 0 with default options' do
before do
mod = described_class

job_class = Class.new do
include mod
sentry_monitor_check_ins

def work; 42 end
def perform; work end
end

stub_const('Job', job_class)
end

let(:job) { Job.new }

it 'prepends the patch' do
expect(Job.ancestors.first).to eq(described_class::Patch)
end

it 'does the work' do
expect(job).to receive(:work).and_call_original
expect(job.perform).to eq(42)
end

it 'calls capture_check_in twice' do
expect(Sentry).to receive(:capture_check_in).with(
'job',
Expand All @@ -128,7 +174,7 @@ def perform(a, b = 42, c: 99)

sentry_monitor_check_ins

def work(a, b, c); end
def work(a, b, c); a + b + c end

def perform(a, b = 42, c: 99)
work(a, b, c)
Expand Down Expand Up @@ -156,7 +202,7 @@ def perform(a, b = 42, c: 99)

sentry_monitor_check_ins slug: 'custom_slug', monitor_config: conf

def work(a, b, c); end
def work(a, b, c); a + b + c end

def perform(a, b = 42, c: 99)
work(a, b, c)
Expand All @@ -168,10 +214,7 @@ def perform(a, b = 42, c: 99)

let(:job) { Job.new }

it 'does the work' do
expect(job).to receive(:work).with(1, 42, 99)
job.perform(1)
end
it_behaves_like 'original_job'

it 'prepends the patch' do
expect(Job.ancestors.first).to eq(described_class::Patch)
Expand Down Expand Up @@ -226,8 +269,13 @@ def perform(a, b = 42, c: 99)
let(:job) { Job.new }

it 'does the work' do
expect(job).to receive(:work).with(1, 42, 99)
job.perform(1)
expect(job).to receive(:work).with(1, 42, 99).and_call_original
expect { job.perform(1) }.to raise_error(ZeroDivisionError)
end

it 'does the work with args' do
expect(job).to receive(:work).with(1, 43, 100).and_call_original
expect { job.perform(1, 43, c: 100) }.to raise_error(ZeroDivisionError)
end

it 'prepends the patch' do
Expand Down