-
Notifications
You must be signed in to change notification settings - Fork 11
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
proposal: Update Created Timestamps syntax for OpenMetrics #43
base: main
Are you sure you want to change the base?
Conversation
715ab7f
to
55f3ad3
Compare
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
70bd50a
to
be917f1
Compare
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great start; thanks for working on this, Manik!
I've left a few questions and stylistic comments
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Arthur Silva Sens <[email protected]> Signed-off-by: Manik Rana <[email protected]>
Co-authored-by: Arthur Silva Sens <[email protected]> Signed-off-by: Manik Rana <[email protected]>
Co-authored-by: Arthur Silva Sens <[email protected]> Signed-off-by: Manik Rana <[email protected]>
I should use grammarly Co-authored-by: Arthur Silva Sens <[email protected]> Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I would go for this. Even in the event of we would do a significant change to the format following the prometheus/OpenMetrics#283 it might be still a relevant and valid direction for CT format.
Also, amazingly well written proposal, thanks for this!
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md
Outdated
Show resolved
Hide resolved
``` | ||
# HELP foo Counter with and without labels to certify CT is parsed for both cases | ||
# TYPE foo counter | ||
# CREATED 1520872607.123; {a="b"} 1520872607.123; {le="c"} 1520872621.123 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is one option, but another option would be to do CREATED <ts>
and then separate "group" when this changes. This is tricky because it in the past the group (metric family) should not duplicate. We try to challenge this in other discussions/proposals though: prometheus/OpenMetrics#280
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont think I follow but are you saying each label within a metricfamily has its own # CREATED
(+ unit and type)?
Co-authored-by: Bartlomiej Plotka <[email protected]> Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
just saw the tagged issue
This is something I overlooked and could be worth adding to the proposal |
|
||
This change is not backwards compatible and would break existing parsers that expect the `_created` line. OpenMetrics 1.x parsers that support `_created` lines would not be able to parse the new syntax. This would require a new major version of the OpenMetrics specification to be released, i.e, OpenMetrics 2.0. Any client libraries or tools that expose OpenMetrics text would also need to be updated to support the new syntax. | ||
|
||
## Alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also the alternative of moving towards fully structured values, where the create timestamp (and potentially the existing timestamp) become fields in the structured point value. This is discussed in https://docs.google.com/document/d/1VbqxXmtZmU9MWvK936uf80ulaR7X9xU_PjrR05avKO8/edit?tab=t.0#heading=h.fgd5g5mmchqj, prometheus/OpenMetrics#283, and elsewhere. It would be nice to avoid having multiple forms of key-value pairs in the exposition in different ways. E.g.
rpc_durations_seconds{service="exponential"} {sum:2.0318666372575776e-05, count:22, quantiles:[0.5:7.689368882420941e-07, 0.9:1.6537614174305048e-06, 0.99:2.0965499063061924e-06], ts:7.689368882420941e-07, ct:1.7268398130168908e+09}
vs
rpc_durations_seconds{service="exponential"} {sum:2.0318666372575776e-05, count:22, quantiles:[0.5:7.689368882420941e-07, 0.9:1.6537614174305048e-06, 0.99:2.0965499063061924e-06]} 7.689368882420941e-07 [email protected]+09
This PR adds a proposal to update the syntax of Created Timestamps in the OpenMetrics text exposition format.
Aims to address prometheus/prometheus#14823