Skip to content
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.

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

Parser produces invalid metrics for no-space metrics #871

Closed
MatthewScholefield opened this issue Dec 19, 2022 · 2 comments · Fixed by #872
Closed

Parser produces invalid metrics for no-space metrics #871

MatthewScholefield opened this issue Dec 19, 2022 · 2 comments · Fixed by #872
Labels

Comments

@MatthewScholefield
Copy link

MatthewScholefield commented Dec 19, 2022

The prometheus metrics endpoint for an IoT tutorial emits metrics with no space after the metric like this:

# HELP pm02 Particulate Matter PM2.5 value
# TYPE pm02 gauge
pm02{id="airgrad0",mac="FF:FF:FF:FF:FF:FF"}11
# HELP rco2 CO2 value, in ppm
# TYPE rco2 gauge
rco2{id="airgrad0",mac="FF:FF:FF:FF:FF:FF"}532
# HELP atmp Temperature, in degrees Celsius
# TYPE atmp gauge
atmp{id="airgrad0",mac="FF:FF:FF:FF:FF:FF"}22.10
# HELP rhum Relative humidity, in percent
# TYPE rhum gauge
rhum{id="airgrad0",mac="FF:FF:FF:FF:FF:FF"}40

Parsing this didn't error but instead produced metrics with incorrect values:

[Metric(pm02, Particulate Matter PM2.5 value, gauge, , [Sample(name='pm02', labels={'id': 'airgrad0', 'mac': 'FF:FF:FF:FF:FF:FF'}, value=1.0, timestamp=None, exemplar=None)]),
 Metric(rco2, CO2 value, in ppm, gauge, , [Sample(name='rco2', labels={'id': 'airgrad0', 'mac': 'FF:FF:FF:FF:FF:FF'}, value=32.0, timestamp=None, exemplar=None)]),
 Metric(atmp, Temperature, in degrees Celsius, gauge, , [Sample(name='atmp', labels={'id': 'airgrad0', 'mac': 'FF:FF:FF:FF:FF:FF'}, value=2.1, timestamp=None, exemplar=None)]),
 Metric(rhum, Relative humidity, in percent, gauge, , [Sample(name='rhum', labels={'id': 'airgrad0', 'mac': 'FF:FF:FF:FF:FF:FF'}, value=0.0, timestamp=None, exemplar=None)])]

I think ideally perhaps it should raise a ValueError in this circumstance (or alternatively support this if it's considered a valid metrics format; I didn't find a spec on what the exact format is).

@csmarchbanks
Copy link
Member

Interesting, I definitely think this is an issue. The best spec I could find is https://prometheus.io/docs/instrumenting/exposition_formats/, which states

Within a line, tokens can be separated by any number of blanks and/or tabs (and must be separated by at least one if they would otherwise merge with the previous token).

Since in your example the value could not merge with the previous token due to having curly braces for labels I think your example should be valid and parse the full number. I also tested with the Go parser and it properly parsed your examples. I will ask around a little bit to see if there is consensus on that though.

@csmarchbanks
Copy link
Member

I talked about this with a couple other Prometheus maintainers and we think the best path forward is to support the lack of space for the Prometheus format. It works with Prometheus today due to the Go parser accepting the format, so making a space required would break some users. Generally speaking the Prometheus format is a bit under specified, which is part of what OpenMetrics fixes.

Note that in OpenMetrics the space is required by the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants