[3.0] Correct dashboard "Failed Jobs Past 7 Days" metric #633
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DashboardStatsController returns a count
filtered by config('horizon.trim.recent')
[default 1 hour] but the duration label is
using config('horizon.trim.failed')
[default 7 days].
Change the count to cover 7 days,
matching total rows under tab
"Failed Jobs".
|
@derekmd feel free to add the deprecated annotations |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #621
The dashboard "Failed Jobs Past 7 Days" metric almost always shows zero because out-of-the-box the counter is actually from the past 1 hour of failed jobs. The time period label is calculated from
config('horizon.trim.failed')but the Redis count query is filtered byconfig('horizon.trim.recent')minutes ago, same as the "Jobs Past Hour" label in the above screenshot.This change makes the metric match the time period label so the count matches the number of results under the "Failed Jobs" page at URI /horizon/failed.
Other possible implementations
I described two other possible approaches in the issue thread: #621 (comment)
Another config/horizon.php option could be added to give devs more control over the failed job count filtered by timestamp.
Possible deprecations for Laravel Horizon 4.0+
With this merged,
RedisJobRepository::countRecentlyFailed()and Redis set'recent_failed_jobs'would no longer be used in Horizon. They could be earmarked for removal in 4.0? In the 3.0 branch, it's possible userland calls these in custom queue dashboards.