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

add exception handling for RessourceException #56

Closed
wants to merge 1 commit into from

Conversation

leahoswald
Copy link
Contributor

under some circumstances if a host is reachable but not working properly
a RessourceException can occur if we query the qemu or LXC details.
This can lead to missing metrics for all other nodes so pass forward to
the next node if this happens. should fix #55

under some circumstances if a host is reachable but not working properly
a RessourceException can occur if we query the qemu or LXC details.
This can lead to missing metrics for all other nodes so pass forward to
the next node if this happens.
@znerol
Copy link
Member

znerol commented Feb 19, 2021

Thanks for filing a pull request and sorry for not answering sooner here.

If I'm reading the proposed changes correctly, then this will lead to exceptions being ignored. To be honest, I prefer to have my programs spectacularly go up in flames instead. Because that gives us a chance to detect when things go wrong. If you are interested on reading up on that particular antipattern, real python has a pretty good article on exactly that issue.

@znerol znerol closed this Feb 19, 2021
@leahoswald
Copy link
Contributor Author

I know that it would be an antipattern but I only catch the specific RessourceException to prevent that one failing host is failing the whole scrape. If you want I can add some logging or anything else. But leaving the program crashing if you could handle an exception is also not a good pattern. I would rather get some data than nothing.

@znerol
Copy link
Member

znerol commented Feb 19, 2021

pve_exporter only connects to one single host during a scrape. When that connection fails, then no metrics will be reported no matter whether or not scraping continues after an exception has been caught / ignored. Unless I'm completely mistaken, this PR does not change that situation, instead it introduces risk that exceptions and errors go unnoticed.

I suggest that we continue the discussion in #55 and try to find the root cause over there.

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.

No metrics on authorization failure
2 participants