Skip to content

Fix error when initializing histogram with empty labels list#147

Merged
brian-brazil merged 1 commit into
prometheus:masterfrom
savoie:empty-labelnames-tuple
Mar 20, 2017
Merged

Fix error when initializing histogram with empty labels list#147
brian-brazil merged 1 commit into
prometheus:masterfrom
savoie:empty-labelnames-tuple

Conversation

@savoie

@savoie savoie commented Mar 20, 2017

Copy link
Copy Markdown
Contributor

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants