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

Added metrics to MetricFamily exceptions to fix #362 #364

Merged

Conversation

wolph
Copy link
Contributor

@wolph wolph commented Jan 8, 2019

No description provided.

@wolph wolph force-pushed the improved_metric_family_exceptions branch from 081796c to 30774e6 Compare January 8, 2019 03:39
@wolph wolph force-pushed the improved_metric_family_exceptions branch from 30774e6 to a3fa450 Compare January 8, 2019 03:40
@brian-brazil
Copy link
Contributor

That's a bit more code than I'd expect. We can't use raise from as we support 2.6/7, but https://stackoverflow.com/questions/9157210/how-do-i-raise-the-same-exception-with-a-custom-message-in-python/9157277 should work.

@wolph
Copy link
Contributor Author

wolph commented Jan 8, 2019

That's indeed possible, but the added advantage of this method is that you can catch MetricErrorBase and can be certain it has a metric property :)
Additionally, testing is a bit easier like this and it still works if you're simply trying to catch something like TypeError.

@brian-brazil
Copy link
Contributor

We already know what the metric is, as it's in local scope.

@wolph
Copy link
Contributor Author

wolph commented Jan 8, 2019

Only from within the exposition method, which is what I was trying to mitigate here. This allows for easy exception handling outside of it while still maintaining full backwards compatibility.

Can you provide an example of how you would prefer the handling otherwise? The question you linked has 11 answers so that's a little broad :P

@brian-brazil
Copy link
Contributor

First answer, 2nd example.

@wolph
Copy link
Contributor Author

wolph commented Jan 8, 2019

I've simplified it a bit now, is that better?

Signed-off-by: Rick van Hattem <[email protected]>
@wolph wolph force-pushed the improved_metric_family_exceptions branch from 715fd92 to 6b4592e Compare January 8, 2019 14:17
Copy link
Contributor

@brian-brazil brian-brazil left a comment

Choose a reason for hiding this comment

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

This code should also go in the openmetrics format generator

tests/test_metrics_core.py Outdated Show resolved Hide resolved
@brian-brazil brian-brazil merged commit 4884cb3 into prometheus:master Jan 8, 2019
@brian-brazil
Copy link
Contributor

Thanks!

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.

2 participants