8000 Fix error when initializing histogram with empty labels list by savoie · Pull Request #147 · prometheus/client_python · GitHub
[go: up one dir, main page]
Skip to content

Fix error when initializing histogram with empty labels list #147

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Mar 20, 2017
Merged

Fix error when initializing histogram with empty labels list #147

merged 1 commit into from
Mar 20, 2017

Conversation

savoie
Copy link
Contributor
@savoie savoie commented Mar 20, 2017

Docs demonstrate initializing a metric's labels with a list, but labelnames is internally represented as a tuple. If labelnames is truthy, it gets cast to a tuple (and a reasonable error is thrown if it can't be cast), but _MetricWrapper simply forwards labelnames as-is if it is falsy.

This is normally not a problem, but when initializing a histogram with an empty labels list:

from prometheus_client import Histogram
Histogram('h', 'help', [])

this error occurs:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/prometheus_client/core.py", line 539, in init
    collector = cls(name, labelnames, (), **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/prometheus_client/core.py", line 835, in __init__
    bucket_labelnames = labelnames + ('le',)
TypeError: can only concatenate list (not "tuple") to list

I see no reason why any iterable that can be cast to a tuple shouldn't reasonably be expected to work here (use case for the empty list: building up a list of labels conditionally; it's inconvenient/inconsistent to check if empty, or to cast to tuple when lists are used for all other cases).

I think a reasonable fix is to set labelnames to an empty tuple if it evaluates to falsy. (Possible alternative: casting even the falsy value to tuple, in case you would like to throw a clear error if someone calls the function with a falsy non-iterable?)

The added test might be a bit simplistic, but it passes with this change and fails without it.

@brian-brazil brian-brazil merged commit e5e73e5 into prometheus:master Mar 20, 2017
@brian-brazil
Copy link
Contributor

Thanks!

@savoie savoie deleted the empty-labelnames-tuple branch March 20, 2017 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@savoie @brian-brazil
0