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

Support Sidekiq 8 #1384

Merged
merged 5 commits into from
Mar 6, 2025
Merged

Support Sidekiq 8 #1384

merged 5 commits into from
Mar 6, 2025

Conversation

tombruijn
Copy link
Member

@tombruijn tombruijn commented Mar 6, 2025

Closes #1372

Add Sidekiq 8 to the CI

Make sure we test against both versions of Sidekiq by adding specific gemfiles for both versions.

Includes rails so that we can test the activejob and actionmailer integration for Sidekiq too.

Fix Sidekiq 8 queue time reporting

Sidekiq 8 reports queue time as epoch milliseconds, so we don't need to do the conversion anymore. Check if Sidekiq is version 8 and if so, do not do any conversion.

https://github.com/sidekiq/sidekiq/blob/main/docs/8.0-Upgrade.md

Only run Sidekiq specs when Sidekiq is in bundle

The CI fails on these specs because we now reference the Sidekiq constant and before we didn't. These specs should now only be run when Sidekiq is in the bundle.

Fix Sidekiq ActiveJob specs

Enable Timecop's mock_process_clock behavior. This is disabled by default but needed for Sidekiq 8 because it changed how it fetches the values for enqueued_at and created_at.

Remove Sidekiq ActiveJob case for job arguments

This case statement is never checked. The ActiveJob instrumentation is wrapped around the Sidekiq middleware. This sets the job arguments before Sidekiq can.

It's set with add_params_if_nil helper. Then when the Sidekiq middleware wants to set parameters, they're already set, and the check is skipped.

Remove it to avoid confusion about it. Sidekiq 8 renamed this class to Sidekiq::ActiveJob::Wrapper, but we don't need to check it.

Make sure we test against both versions of Sidekiq by adding specific
gemfiles for both versions.

Includes rails so that we can test the activejob and actionmailer
integration for Sidekiq too.
@tombruijn tombruijn added the bug label Mar 6, 2025
@tombruijn tombruijn self-assigned this Mar 6, 2025
@tombruijn tombruijn changed the title Sidekiq8 Support Sidekiq 8 Mar 6, 2025
Sidekiq 8 reports queue time as epoch milliseconds, so we don't need to
do the conversion anymore.
Check if Sidekiq is version 8 and if so, do not do any conversion.

https://github.com/sidekiq/sidekiq/blob/main/docs/8.0-Upgrade.md
The CI fails on these specs because we now reference the Sidekiq
constant and before we didn't.
These specs should now only be run when Sidekiq is in the bundle.
Enable Timecop's mock_process_clock behavior. This is disabled by
default but needed for Sidekiq 8 because it changed how it fetches the
values for `enqueued_at` and `created_at`.
This case statement is never checked. The ActiveJob instrumentation is
wrapped around the Sidekiq middleware. This sets the job arguments
before Sidekiq can.

It's set with `add_params_if_nil` helper. Then when the Sidekiq
middleware wants to set parameters, they're already set, and the check
is skipped.

Remove it to avoid confusion about it.
Sidekiq 8 renamed this class to `Sidekiq::ActiveJob::Wrapper`, but we
don't need to check it.
@tombruijn tombruijn marked this pull request as ready for review March 6, 2025 14:32
@tombruijn tombruijn merged commit 908df7a into main Mar 6, 2025
159 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test against Sidekiq 8
2 participants