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

Suggestion: Add around iterate callback #445

Merged
merged 1 commit into from
Jan 5, 2024
Merged

Conversation

gustavompo
Copy link
Contributor

@gustavompo gustavompo commented Jan 2, 2024

There has been some circumstances that it would be great to append a callback to job iterations. More noticeably, that adds extensibility to build some generic handlers, such as metrics collection and logging.
At shopify, a particular use case that'll be especially useful is to deal with the canonical logger and long iterating jobs.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@bdewater
Copy link
Contributor

bdewater commented Jan 3, 2024

Since you mention "metrics collection and logging" you might be interested in #338 which added the each_iteration.iteration ActiveSupport::Notifications event to subscribe to specifically for these kinds of things.

You could also bring your own LogSubscriber class for your specific logger needs.

@gustavompo
Copy link
Contributor Author

Since you mention "metrics collection and logging" you might be interested in #338 which added the each_iteration.iteration ActiveSupport::Notifications event to subscribe to specifically for these kinds of things.

You could also bring your own LogSubscriber class for your specific logger needs.

Right, thanks for the comment! Yeah those are certainly useful too, we use some of the current notifications for metrics and logging.

Apart from the main use case I'm trying to address, which re-defining the canonical logger's unit-of-work for each iteration, the PR allows also some advanced instrumentation mechanisms that, similar to the logger, rely on wrapping the execution block, e.g: Database query count and database time, captured in a given context (which under the hood also use ActiveSupport's notifications, but from active record)

@gustavompo gustavompo marked this pull request as ready for review January 4, 2024 18:02
@gustavompo gustavompo merged commit f3a89f8 into main Jan 5, 2024
35 of 39 checks passed
@sambostock sambostock deleted the gus/around_iterate branch February 6, 2024 15:22
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.

None yet

4 participants