-
Notifications
You must be signed in to change notification settings - Fork 834
Closed
Description
Label values are not checked for string-likeness, and are not explicitly converted to strings. This causes the following problems:
c = Counter('http_requests_total_by_method', 'Count of requests by HTTP method.', ['method']
def handle_request(request):
c.labels(request.method).inc()to fail silently if request.method is None (or a tuple, or an object, etc.). At export time, the following will happen:
Exception happened during processing of request from ('127.0.0.1', 55703)
Traceback (most recent call last):
File "/usr/lib/python2.7/SocketServer.py", line 295, in _handle_request_noblock
self.process_request(request, client_address)
File "/usr/lib/python2.7/SocketServer.py", line 321, in process_request
self.finish_request(request, client_address)
File "/usr/lib/python2.7/SocketServer.py", line 334, in finish_request
self.RequestHandlerClass(request, client_address, self)
File "/usr/lib/python2.7/SocketServer.py", line 651, in __init__
self.handle()
File "/usr/lib/python2.7/BaseHTTPServer.py", line 340, in handle
self.handle_one_request()
File "/usr/lib/python2.7/BaseHTTPServer.py", line 328, in handle_one_request
method()
File "/.../env/local/lib/python2.7/site-packages/prometheus_client-0.0.7-py2.7.egg/prometheus_client/__init__.py", line 404, in do_GET
self.wfile.write(generate_latest(REGISTRY))
File ".../env/local/lib/python2.7/site-packages/prometheus_client-0.0.7-py2.7.egg/prometheus_client/__init__.py", line 392, in generate_latest
for k, v in labels.items()]))
AttributeError: 'NoneType' object has no attribute 'replace'
There are several ways around this:
- Raise a ValueError in
c.labels()if the label values are not all string-like - In
c.labels()or ingenerate_latest, coerce all label values to the unicode (or string) type - In
generate_latest, drop labels that can't be rendered, but render the other labels for that metric. - In
generate_latest, drop metrics that can't be rendered, but render the rest.
I'm not a fan of the options that drop data, because there's no worse monitoring than monitoring that lies to you. I think coercing to string may work, but I'd prefer if the code could raise. It already raises if the code provides the wrong number of label values, so raising on the wrong type of label values isn't unexpected.
Metadata
Metadata
Assignees
Labels
No labels