-
Notifications
You must be signed in to change notification settings - Fork 9
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 PR adds a proposal to update the syntax of Created Timestamps in the OpenMetrics text exposition format.
Aims to address prometheus/prometheus#14823