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

Incorrect format for Prometheus metrics /_admin/metrics/v2 #18692

Closed
MaxenceAdnot opened this issue Apr 15, 2023 · 7 comments
Closed

Incorrect format for Prometheus metrics /_admin/metrics/v2 #18692

MaxenceAdnot opened this issue Apr 15, 2023 · 7 comments

Comments

@MaxenceAdnot
Copy link

My Environment

  • ArangoDB Version: 3.10.5
  • Deployment Mode: Single Server & Cluster
  • Deployment Strategy: ArangoDB Starter | Kubernetes Operator

Component, Query & Data

Affected feature:

ArangoDB Server Metrics

Problem:

According to
Prometheus Exposition format metrics with labels should be followed by a space.

Based on ArangoDB Server Metrics and my personal tests metrics are returned in this format:
arangodb_agency_cache_callback_number{role="SINGLE"}0

For some reasons Prometheus is accepting the metrics but their Python parser used in multiple tools (such as the Datadog agent) is not supporting this syntax

Expected result:

arangodb_agency_cache_callback_number{role="SINGLE"} 0

@MBkkt
Copy link
Contributor

MBkkt commented Apr 17, 2023

Hello

Prometheus Exposition format metrics with labels should be followed by a space.

I don't see this, please point me

For some reasons Prometheus is accepting the metrics

Because space is not part of format.

Python parser is root of incorrect behavior, btw they fixed this prometheus/client_python#872

Anyway old behavior will be available starting from 3.10.6

@MaxenceAdnot
Copy link
Author

MaxenceAdnot commented Apr 17, 2023

Hello @MBkkt

Thank you for your answer 👍

I don't see this, please point me

From my understanding, this is described here :
https://github.com/prometheus/docs/blob/main/content/docs/instrumenting/exposition_formats.md#line-format
And more specifically with the following statement :
"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). Leading and trailing whitespace is ignored."

Anyway old behavior will be available starting from 3.10.6

Good news ! For now I've decided to rollback to 3.9.10

@Simran-B
Copy link
Contributor

if they would otherwise merge with the previous token

This reads to me that it's only required if it would create an ambiguity, but I don't think that is the case here? Closing curly brace followed by a value seems easily parsable.

@MaxenceAdnot
Copy link
Author

@Simran-B, you're probably right.

I thought it would be helpful to open this issue because in all the examples provided on the Prometheus format page and in the different exporters I have used so far, I have noticed the whitespace separating the labels within braces and their value.

I apologize if I wasted your time, but thank you for submitting a fix so quickly! It reassures us in choosing ArangoDB (we have also been users of ArangoDB Oasis for a few years now).

@mpoeter
Copy link
Member

mpoeter commented Apr 17, 2023

@MaxenceAdnot I also interpret the spec that in case of the curly brace the whitespace is optional, but a space certainly makes it more readable, so I also welcome the fix. Nevertheless it might be worth to also open an issue for the Python parser because ATM it obviously does not follow the spec.

@MaxenceAdnot
Copy link
Author

The issue has been fixed in the Python parser with their 0.16.0 release. You can find the changelog for this release at https://github.com/prometheus/client_python/releases/tag/v0.16.0.

The Datadog integration with ArangoDB is not using this release of the parser, I opened a ticket for them to upgrade the dependency on the Python parser.

@jsteemann
Copy link
Contributor

We are also fixing the issue in ArangoDB, so that whatever version of the Python parser is used, things will work together.
The fix will be released with ArangoDB v3.10.6.

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

No branches or pull requests

5 participants