Would this make it so any invocation of the related Job class, including invocations triggered by sidekiq-cron, and invocations that the user did manually, with just HappyWorkerJob.perform_async? It looks like it would, right?

I wonder if there are situations when users want to kick the same job for a one-off thing that they don't want to be reported to Sentry. I, personally, think that since the defined sidekiq-cron job just wraps a Sidekiq job, than whenever that job was triggered, it should be reported.

Do we want to add this to docs? Is this behavior consistent with other languages and frameworks Sentry supports?

/cc @sl0thentr0py

I will add docs for all of this around releasing.

Would this make it so any invocation of the related Job class, including invocations triggered by sidekiq-cron, and invocations that the user did manually, with just HappyWorkerJob.perform_async? It looks like it would, right?

yes

I wonder if there are situations when users want to kick the same job for a one-off thing that they don't want to be reported to Sentry. I, personally, think that since the defined sidekiq-cron job just wraps a Sidekiq job, than whenever that job was triggered, it should be reported.

This is a new feature, we will add special config once people use it and request it. This is fine as a first version, it's an opt-in patch either way.

It looks like there could be multiple ways to invoke save in addition to the initial creation. Although the current implementation doesn't seem to have obvious performance impact, I feel a better place to inject such one-off activation logic is load_from_array instead.
I understand that it's an experimental feature for now, so I'm not suggesting an immediate refactor. Just want to point it out for future improvements.

if people are scheduling cron jobs with Sidekiq::Cron::Job.create, we patch those too automatically, either way shouldn't matter, we can react to feedback from users later depending on usage.

Honest question so I can draft the docs PR:

• to enable sidekiq-cron and/or sidekiq-scheduler monitoring, the users will have to manually add config.enabled_patches += [:sidekiq_cron], right?
• When this patch is applied, and as long as sidekiq-cron is indeed available, we aim to instrument all jobs that it runs, all the time. The current logic that hooks into save does that. Correct?
• One side-iffect is that we're instrumenting the job class itself, so whether it was the sidekiq-cron schedule that invoked the job on a timer, or the user's code invoked perform_* on that job, we will monitor that too. That's by design for now, but we'll keep an eye out for feedback. Right?

yes to all

It looks like there could be multiple ways to invoke save in addition to the initial creation. Although the current implementation doesn't seem to have obvious performance impact, I feel a better place to inject such one-off activation logic is load_from_array instead.
I understand that it's an experimental feature for now, so I'm not suggesting an immediate refactor. Just want to point it out for future improvements.