proposal: Update Created Timestamps syntax for OpenMetrics #43

Maniktherana · 2024-12-14T16:17:26Z

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

Aims to address prometheus/prometheus#14823

Signed-off-by: Manik Rana <manikrana54@gmail.com>

Maniktherana · 2024-12-14T17:47:35Z

@bwplotka @ArthurSens

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

ArthurSens

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

ArthurSens · 2024-12-17T00:03:11Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+
+### Pitfalls of the current solution
+
+As stated above, `_created`  lines can appear anywhere after its associated metric line. This means parser is required to store the current position of the lexer before we start searching for the created timestamp. Currently, we cache the timestamp and minimize data stored in memory. Before, we used to make a deep copy of the parser at every line whenever the `CreatedTimestamp`  function was called which lead to [Prometheus consuming Gigabytes more memory](https://github.com/prometheus/prometheus/issues/14808) than needed.


There's no need to mention how the parser used to look like. That implementation was not OM's fault; it was just us iterating over the implementation until we found a more efficient solution 🤔

How about

Suggested change

As stated above, `_created` lines can appear anywhere after its associated metric line. This means parser is required to store the current position of the lexer before we start searching for the created timestamp. Currently, we cache the timestamp and minimize data stored in memory. Before, we used to make a deep copy of the parser at every line whenever the `CreatedTimestamp` function was called which lead to [Prometheus consuming Gigabytes more memory](https://github.com/prometheus/prometheus/issues/14808) than needed.

As stated above, `_created` lines can appear anywhere after its associated metric line. This means parser is required to store the current position of the lexer before we start searching for the created timestamp. Currently, we cache the timestamp and minimize data stored in memory but there is still significant CPU overhead.

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

ArthurSens · 2024-12-17T00:18:29Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+foo_total{le="1"} 10.0
+```
+
+### Summaries and Histograms


Do you have any preference between the two proposals?

For proposal 1: Do we know if that's a complicated thing for SDKs?
For proposal 2: What should the parser do if it sees different CTs for the same histogram/summary?

I actually haven't thought of this
For 1: I may need to dig into how CTs are applied.
For 2: Good question. Not sure if there's a right answer here besides going with 1

Let's mention what you propose, and move other to alternatives. I think I would put it on every line - simpler and we are chatting about the idea of single line for each complex type too: prometheus/OpenMetrics#283

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Manik Rana <Manikrana54@gmail.com>

I should use grammarly Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Manik Rana <Manikrana54@gmail.com>

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

bwplotka

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!

bwplotka · 2024-12-18T14:54:34Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+## Goals
+
+* Prometheus can parse Created Timestamps without holding state between lines.
+* OpenMetrics has an updated specification with a clear and precise syntax.


What about "We don't produce artificial metrics for CT semantics."

bwplotka · 2024-12-18T14:57:03Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+
+## How
+
+We store the Created Timestamp inline with the attached metric and remove `_created` lines. This will allow the parser to immediately associate the CT with the metric without having to search for it. For counters, it's straightforward, as we can just add the timestamp after the metric value like an exemplar. To separate it from a traditional timestamp, we can prefix the created timestamp with something like `ct@`, although this portion of the syntax is not final and can be changed. Furthermore, we can order the created timestamp such that it is after the metric value + timestamp and before an exemplar.


Suggested change

We store the Created Timestamp inline with the attached metric and remove `_created` lines. This will allow the parser to immediately associate the CT with the metric without having to search for it. For counters, it's straightforward, as we can just add the timestamp after the metric value like an exemplar. To separate it from a traditional timestamp, we can prefix the created timestamp with something like `ct@`, although this portion of the syntax is not final and can be changed. Furthermore, we can order the created timestamp such that it is after the metric value + timestamp and before an exemplar.

We store the Created Timestamp inline with the attached metric and remove `_created` lines. This will allow the parser to immediately associate the CT with the metric without having to search for it. For counters, it's straightforward, as we can just add the timestamp after the metric value like an exemplar. To separate it from a traditional timestamp, we can prefix the created timestamp with something like `ct@`, although this portion of the syntax is not final and can be changed. We propose to force a certain order of the created timestamp such that it is after the metric value and the timestamp and before an exemplar.

Should we mention the timestamp format/unit (that it is the same as the OM timestamp).

bwplotka · 2024-12-18T15:00:16Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+
+## Goals
+
+* Prometheus can parse Created Timestamps without holding state between lines.


We could put less detailed goal e.g. parsing is efficient

This is because it's fine to have some state e.g. HELP and TYPE, UNIT are generally ok because it's clear when they change (no need to recompute or compare series)

bwplotka · 2024-12-18T15:02:08Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+foo_total{le="1"} 10.0
+```
+
+### Summaries and Histograms


Let's mention what you propose, and move other to alternatives. I think I would put it on every line - simpler and we are chatting about the idea of single line for each complex type too: prometheus/OpenMetrics#283

bwplotka · 2024-12-18T15:20:29Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+```
+# 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


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

bwplotka · 2024-12-18T15:23:15Z

proposals/2024-12-14_update-created-timestamps-for-openmetrics-text.md

+    * Arthur Silva Sens [@ArthurSens](https://github.com/ArthurSens)
+    * Bartłomiej Płotka [@bwplotka](https://github.com/bwplotka)


Suggested change

* Arthur Silva Sens [@ArthurSens](https://github.com/ArthurSens)

* Bartłomiej Płotka [@bwplotka](https://github.com/bwplotka)

**Contributors:**

* Arthur Silva Sens [@ArthurSens](https://github.com/ArthurSens)

* Bartłomiej Płotka [@bwplotka](https://github.com/bwplotka)

Maniktherana force-pushed the update-cts branch from 715ab7f to 55f3ad3 Compare December 14, 2024 16:24

Maniktherana added 4 commits December 14, 2024 22:16

feat: init ct proposal

1d313cc

Signed-off-by: Manik Rana <manikrana54@gmail.com>

chore: minor updates to content before "How" section

9531e66

Signed-off-by: Manik Rana <manikrana54@gmail.com>

feat: add an "Alternatives" section

768d1f8

Signed-off-by: Manik Rana <manikrana54@gmail.com>

feat: add action plan

be917f1

Signed-off-by: Manik Rana <manikrana54@gmail.com>

Maniktherana force-pushed the update-cts branch from 70bd50a to be917f1 Compare December 14, 2024 16:46

Maniktherana added 4 commits December 14, 2024 22:18

chore: update audience

6f7889d

Signed-off-by: Manik Rana <manikrana54@gmail.com>

fix: alternative example

e24e60d

Signed-off-by: Manik Rana <manikrana54@gmail.com>

feat: add a how section

73a57fa

Signed-off-by: Manik Rana <manikrana54@gmail.com>

feat: updates and add a backwards compatibilty section

696b466

Signed-off-by: Manik Rana <manikrana54@gmail.com>

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

Maniktherana changed the title ~~feat: Update Created Timestamps syntax for OpenMetrics~~ proposal: Update Created Timestamps syntax for OpenMetrics Dec 14, 2024

Maniktherana added 2 commits December 14, 2024 23:25

chore: minor fixes

bbf069f

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

chore: add an assumption

5f78950

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

ArthurSens reviewed Dec 17, 2024

View reviewed changes

Maniktherana and others added 7 commits December 17, 2024 22:45

fix: punctuation

f77c828

Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Manik Rana <Manikrana54@gmail.com>

fix: wording

9b87407

Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Manik Rana <Manikrana54@gmail.com>

fix: wording

a34f609

Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Manik Rana <Manikrana54@gmail.com>

fix: grammar

aea32d8

I should use grammarly Co-authored-by: Arthur Silva Sens <arthursens2005@gmail.com> Signed-off-by: Manik Rana <Manikrana54@gmail.com>

fix: example

3a4bc9c

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

fix: order action plan

8f730b7

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

fix: why section

509eb0a

Signed-off-by: Manik Rana <Manikrana54@gmail.com>

bwplotka mentioned this pull request Dec 18, 2024

OM 2.0: Created Timestamps text exposition format prometheus/OpenMetrics#285

Open

bwplotka approved these changes Dec 18, 2024

View reviewed changes

bwplotka reviewed Dec 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

proposal: Update Created Timestamps syntax for OpenMetrics #43

proposal: Update Created Timestamps syntax for OpenMetrics #43

Maniktherana commented Dec 14, 2024 •

edited

Loading

Maniktherana commented Dec 14, 2024

ArthurSens left a comment

ArthurSens Dec 17, 2024

Maniktherana Dec 17, 2024

ArthurSens Dec 17, 2024

Maniktherana Dec 17, 2024

bwplotka Dec 18, 2024

bwplotka left a comment

bwplotka Dec 18, 2024

bwplotka Dec 18, 2024

bwplotka Dec 18, 2024

bwplotka Dec 18, 2024

bwplotka Dec 18, 2024

bwplotka Dec 18, 2024

bwplotka Dec 18, 2024 •

edited

Loading


		### Pitfalls of the current solution

		As stated above, `_created` lines can appear anywhere after its associated metric line. This means parser is required to store the current position of the lexer before we start searching for the created timestamp. Currently, we cache the timestamp and minimize data stored in memory. Before, we used to make a deep copy of the parser at every line whenever the `CreatedTimestamp` function was called which lead to [Prometheus consuming Gigabytes more memory](https://github.com/prometheus/prometheus/issues/14808) than needed.


		## How

		We store the Created Timestamp inline with the attached metric and remove `_created` lines. This will allow the parser to immediately associate the CT with the metric without having to search for it. For counters, it's straightforward, as we can just add the timestamp after the metric value like an exemplar. To separate it from a traditional timestamp, we can prefix the created timestamp with something like `ct@`, although this portion of the syntax is not final and can be changed. Furthermore, we can order the created timestamp such that it is after the metric value + timestamp and before an exemplar.


		## Goals

		* Prometheus can parse Created Timestamps without holding state between lines.

		* Arthur Silva Sens [@ArthurSens](https://github.com/ArthurSens)
		* Bartłomiej Płotka [@bwplotka](https://github.com/bwplotka)

proposal: Update Created Timestamps syntax for OpenMetrics #43

Are you sure you want to change the base?

proposal: Update Created Timestamps syntax for OpenMetrics #43

Conversation

Maniktherana commented Dec 14, 2024 • edited Loading

Maniktherana commented Dec 14, 2024

ArthurSens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bwplotka Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Maniktherana commented Dec 14, 2024 •

edited

Loading

bwplotka Dec 18, 2024 •

edited

Loading