Skip to content

Added metrics to MetricFamily exceptions to fix #362 - #364

Merged
brian-brazil merged 4 commits into
prometheus:masterfrom
wolph:improved_metric_family_exceptions
Jan 8, 2019
Merged

brian-brazil merged 4 commits into
prometheus:masterfrom
wolph:improved_metric_family_exceptions

Conversation

@wolph

@wolph wolph commented Jan 8, 2019

Copy link
Copy Markdown
Contributor

No description provided.

@wolph
wolph force-pushed the improved_metric_family_exceptions branch from 081796c to 30774e6 Compare January 8, 2019 03:39
Signed-off-by: Rick van Hattem <Wolph@wol.ph>
@wolph
wolph force-pushed the improved_metric_family_exceptions branch from 30774e6 to a3fa450 Compare January 8, 2019 03:40
@brian-brazil

Copy link
Copy Markdown
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

wolph commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor

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

@wolph

wolph commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor

First answer, 2nd example.

@wolph

wolph commented Jan 8, 2019

Copy link
Copy Markdown
Contributor Author

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

Signed-off-by: Rick van Hattem <Wolph@wol.ph>
@wolph
wolph force-pushed the improved_metric_family_exceptions branch from 715fd92 to 6b4592e Compare January 8, 2019 14:17

@brian-brazil brian-brazil left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread tests/test_metrics_core.py Outdated
wolph added 2 commits January 8, 2019 15:51
Signed-off-by: Rick van Hattem <Wolph@wol.ph>
Signed-off-by: Rick van Hattem <Wolph@wol.ph>
@brian-brazil
brian-brazil merged commit 4884cb3 into prometheus:master Jan 8, 2019
@brian-brazil

Copy link
Copy Markdown
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