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

rescue all exceptions for metrics #32

Merged
merged 3 commits into from
Sep 9, 2024

Conversation

orioldsm
Copy link
Contributor

@orioldsm orioldsm commented Sep 6, 2024

Currently because the rescue only captures StandardError type errors, we're not getting into the rescue block for all of the issues Sidekiq workers are experiencing.

This PR:

  • Changes rescue StandardError => e to now be rescue Exception => e to properly handle gathering stats for all potential error types. The error is re-raised at the end of the rescue so there is no need for concern in widening the umbrella of what we're catching.
  • Adds an error tag to the DataDog stats reported so we can get granular data on what error types our worker's are experiencing

lib/sidekiq/instrument/middleware/server.rb Show resolved Hide resolved
lib/sidekiq/instrument/middleware/server.rb Outdated Show resolved Hide resolved
lib/sidekiq/instrument/middleware/server.rb Outdated Show resolved Hide resolved
spec/sidekiq-instrument/server_middleware_spec.rb Outdated Show resolved Hide resolved
Copy link

@hkim3163 hkim3163 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 cool with rescuing exception & raising

let's give this a shot in prod

@kaisensan kaisensan merged commit b76e5b4 into enova:main Sep 9, 2024
13 checks passed
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.

4 participants