Skip to content

Commit b2111be

Browse files
authored
Improve default slug generation for sidekiq-scheduler (#2184)
`sidekiq-scheduler` allows to use a class name as a schedule name directly as follows. ``` Namspeced::CancelAbandonedOrders: cron: '0 */5 * * * *' ``` In this case, a slug is shown as `namspecedcancelabandonedorders` on Sentry. I think this isn't good for readability. This PR converts `::` to `-` as well as the cron default behavior of Crons mixin. Related with: #2138.
1 parent 4c8110c commit b2111be

File tree

6 files changed

+21
-5
lines changed

6 files changed

+21
-5
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
### Features
44

5+
- Improve default slug generation for `sidekiq-scheduler` [#2184](https://github.com/getsentry/sentry-ruby/pull/2184)
6+
57
### Bug Fixes
68

79
- Network errors raised in `Sentry::HTTPTransport` will no longer be reported to Sentry [#2178](https://github.com/getsentry/sentry-ruby/pull/2178)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ def sentry_monitor_check_ins(slug: nil, monitor_config: nil)
4444
prepend Patch
4545
end
4646

47-
def sentry_monitor_slug
47+
def sentry_monitor_slug(name: self.name)
4848
@sentry_monitor_slug ||= begin
4949
slug = name.gsub('::', '-').downcase
5050
slug[-MAX_SLUG_LENGTH..-1] || slug

sentry-sidekiq/lib/sentry/sidekiq-scheduler/scheduler.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ def new_job(name, interval_type, config, schedule, options)
5050
# only patch if not explicitly included in job by user
5151
unless klass_const.send(:ancestors).include?(Sentry::Cron::MonitorCheckIns)
5252
klass_const.send(:include, Sentry::Cron::MonitorCheckIns)
53+
slug = klass_const.send(:sentry_monitor_slug, name: name)
5354
klass_const.send(:sentry_monitor_check_ins,
54-
slug: name,
55+
slug: slug,
5556
monitor_config: monitor_config)
5657

5758
::Sidekiq.logger.info "Injected Sentry Crons monitor checkins into #{klass}"

sentry-sidekiq/spec/fixtures/sidekiq-scheduler-schedule.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,6 @@
1111
reports:
1212
in: "2 hours"
1313
class: "ReportingWorker"
14+
VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job:
15+
cron: "* * * * *"
1416

sentry-sidekiq/spec/sentry/sidekiq-scheduler/scheduler_spec.rb

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,12 @@
4949
expect(EveryHappyWorker.sentry_monitor_config.schedule.to_hash).to eq({value: 10, type: :interval, unit: :minute})
5050
end
5151

52-
it "does not add monitors for a one-off job" do
53-
expect(ReportingWorker.ancestors).not_to include(Sentry::Cron::MonitorCheckIns)
54-
end
52+
it 'truncates from the beginning and parameterizes slug' do
53+
expect(VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job.ancestors).to include(Sentry::Cron::MonitorCheckIns)
54+
expect(VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job.sentry_monitor_slug).to eq('ongoutermodule-veryveryveryverylonginnermodule-job')
55+
expect(VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job.sentry_monitor_config).to be_a(Sentry::Cron::MonitorConfig)
56+
expect(VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job.sentry_monitor_config.schedule).to be_a(Sentry::Cron::MonitorSchedule::Crontab)
57+
expect(VeryLongOuterModule::VeryVeryVeryVeryLongInnerModule::Job.sentry_monitor_config.schedule.value).to eq('* * * * *')
58+
end
5559
end
5660

sentry-sidekiq/spec/spec_helper.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,13 @@ def [](key)
208208
end
209209
end
210210

211+
module VeryLongOuterModule
212+
module VeryVeryVeryVeryLongInnerModule
213+
class Job
214+
end
215+
end
216+
end
217+
211218
# Sidekiq 7 has a Config class, but for Sidekiq 6, we'll mock it.
212219
def sidekiq_config(opts)
213220
WITH_SIDEKIQ_7 ? ::Sidekiq::Config.new(opts) : SidekiqConfigMock.new(opts)

0 commit comments

Comments
 (0)