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

Improve active job context when using Delayed Job or Resque #671

Merged
merged 3 commits into from
Aug 5, 2021

Conversation

imjoehaines
Copy link
Contributor

@imjoehaines imjoehaines commented Aug 5, 2021

Goal

Delayed Job

The current context for Delayed Job uses the display_name method, if implemented. Rails implements display_name internally to help debugging, but this uses the job ID and arguments which makes it unusable as an event's context because the same job will have different contexts between executions

This PR avoids using display_name if it matches the Rails format and will use the same context that we use in other queuing libraries instead (JobName@queue_name)

Resque

The Resque integration reads the class name from the payload data and uses that as context. When run in Active Job the class name is always the Resque wrapper: ActiveJob::QueueAdapters::ResqueAdapter::JobWrapper. This leads to the same context being used across different jobs

We will now "unwrap" the actual job class and use that instead, e.g. SomeJob@some_queue

Testing

Existing tests cover breaks from this change & a new unit test has been added for Resque. We currently don't have unit tests for Delayed Job, but this will be covered by a Maze Runner test in #672

The metadata will now include the original job name which is also
reflected in the context
The default Active Job display_name includes the job ID and arguments

This doesn't really work for context as it will change every execution
and so can't ever be grouped on

We'll now ignore the display_name if it does match the Active Job
format and use the "Job@queue" format instead
@imjoehaines imjoehaines marked this pull request as ready for review August 5, 2021 10:16
Base automatically changed from active-job-support to next August 5, 2021 16:35
@imjoehaines imjoehaines merged commit ef54d7a into next Aug 5, 2021
@imjoehaines imjoehaines deleted the improve-active-job-context branch August 5, 2021 16:36
@imjoehaines imjoehaines mentioned this pull request Aug 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants