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

proposal: Update Created Timestamps syntax for OpenMetrics #43

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

Maniktherana
Copy link

@Maniktherana Maniktherana commented Dec 14, 2024

This PR adds a proposal to update the syntax of Created Timestamps in the OpenMetrics text exposition format.

Aims to address prometheus/prometheus#14823

@Maniktherana Maniktherana marked this pull request as ready for review December 14, 2024 17:47
@Maniktherana
Copy link
Author

@bwplotka @ArthurSens

@Maniktherana Maniktherana changed the title feat: Update Created Timestamps syntax for OpenMetrics proposal: Update Created Timestamps syntax for OpenMetrics Dec 14, 2024
Signed-off-by: Manik Rana <[email protected]>
Signed-off-by: Manik Rana <[email protected]>
Copy link
Member

@ArthurSens ArthurSens left a 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

Maniktherana and others added 7 commits December 17, 2024 22:45
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]>
Copy link
Member

@bwplotka bwplotka left a 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!

```
# 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
Copy link
Member

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

Copy link
Author

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)?

Maniktherana and others added 4 commits December 19, 2024 22:24
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]>
@Maniktherana
Copy link
Author

just saw the tagged issue

CT would be ingested as a metric and could lead to a 50% increase in metric ingestion if the target exposes mainly counters.

This is something I overlooked and could be worth adding to the proposal

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

Successfully merging this pull request may close these issues.

3 participants