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

metrics formatting: preserve trailing whitespace #1320

Merged
merged 4 commits into from
Jul 27, 2021

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Jul 26, 2021

The newline character is required at the end of this string.
Any other whitespace is ignored as per https://prometheus.io/docs/instrumenting/exposition_formats/ so no need to trim
Signed-off-by: Sally MacFarlane [email protected]

I have tested this locally and get the following response from
curl localhost:8581/metrics
Before ie without this change:


tessera_P2P_GET_getPartyInfo_MinTime_ms 0
tessera_P2P_GET_getPartyInfo_MaxTime_ms 0
...
tessera_THIRD_PARTY_GET_getVersion_RequestRate_requestsPerSeconds 0.0
tessera_THIRD_PARTY_GET_getVersion_RequestCount 0%

^ no new line at end of string

After ie with this change:

tessera_P2P_GET_getPartyInfo_MinTime_ms 0
tessera_P2P_GET_getPartyInfo_MaxTime_ms 0
...
tessera_THIRD_PARTY_GET_getVersion_RequestRate_requestsPerSeconds 0.0
tessera_THIRD_PARTY_GET_getVersion_RequestCount 0

so there is a newline at the end. Prometheus serves up the list of metrics.

Signed-off-by: Sally MacFarlane <[email protected]>
Copy link
Collaborator

@chris-j-h chris-j-h left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth mentioning that Prometheus does correctly ingest and parse metrics without the trailing newline (checked with v2.28.1). However, as the Prometheus spec does state that "The last line must end with a line feed character", third party tools may enforce this (as seen in the linked issue), and future versions of Prometheus may not correctly handle no trailing newline.

macfarla and others added 3 commits July 27, 2021 10:57
Signed-off-by: Sally MacFarlane <[email protected]>
…ry newline from API resp

The Prometheus spec defines that output must end with a newline, this change makes sure this is handled in the Formatter so that the Resource doesn't have to be concerned about the Prometheus spec
@macfarla macfarla merged commit e83782c into Consensys:master Jul 27, 2021
@macfarla macfarla deleted the newline branch April 1, 2022 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants