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

Catch panic in executeHook #4025

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

linux019
Copy link
Contributor

@linux019 linux019 commented Oct 30, 2024

@linux019 linux019 changed the title #3737 catch panic in executeHook Catch panic in executeHook Oct 30, 2024
@SyntaxNode
Copy link
Contributor

Good addition. Please add extra context including the module and hook which caused a panic, similar to the adapter panic error information. Might also be a good idea to add a module panic metric to mirror the adapter panic metric.

@bsardo
Copy link
Collaborator

bsardo commented Dec 12, 2024

@linux019 are you able to add the module panic metric as @SyntaxNode suggested?

@linux019
Copy link
Contributor Author

linux019 commented Dec 14, 2024

I don't think that we need this metric. We will have in this case new endpoints X module_name X stage_name time series because metrics engine preloads all labels. Panic in module is very rare case developers can check PBS logs or set up alert rules in logs scraper

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.

3 participants