prom-client icon indicating copy to clipboard operation
prom-client copied to clipboard

Labels values containing a " are not escaped, leading to "no token found" scraping error

Open WalexMerapar opened this issue 6 years ago • 3 comments

Hello

I encountered an issue while implementing a new metric with the nodejs client lib. The prometheus server was not able to scrape my metrics and the target was reported down with error "no token found".

After some search and investigation, I finally found the issue : one of my label value of my new metric was including a the " character, making the prometheus server unable to parse correctly the fetched metrics.

I suggest that the label value is checked and all its " character escaped properly. This could leads to update both the prometheus server and the client lib.

It worth fixing that because the error reported by the prometheus server was nothing but helpful (and digging on internet about that error only brought me only to false leads). Finding where the issue was not tricky and painfull as I had to closely look at the prometheus endpoint myself to find that only one line (among hundreds) with a " included in a label value caused the error.

WalexMerapar avatar May 24 '18 08:05 WalexMerapar

The code review service LGTM reports a warning related to string escaping, which may be related to this.

https://github.com/siimon/prom-client/blob/24bfd0f57ed7391fc2da2b1aff577e92cd7db6a3/lib/registry.js#L4-L12

They recommend using a sanitization library:

https://lgtm.com/rules/1506222917439/

If there's an easy way to test these edits I'd be happy to make them. Is this code under test?

paulmelnikow avatar Nov 05 '18 16:11 paulmelnikow

Should we escape it, or throw ourselves (with a clear error) and make the consumer escape it?

SimenB avatar Dec 28 '18 13:12 SimenB

It depends I would say.. if we could escape/sanitize it without affecting the performance too much I think we should handle it.

siimon avatar Dec 31 '18 16:12 siimon