[#6632] improvement(core): Add timestamp information to metrics

[Abyss-lord]

What changes were proposed in this pull request?

Add timestamp information to metrics

Why are the changes needed?

Fix: #6632

Does this PR introduce any user-facing change?

no

How was this patch tested?

local test.

[Abyss-lord]

Hi @FANNG1 , could you please review this PR when you have time? I’d really appreciate your feedback.

[jerqi]

@FANNG1 Would you like to take an another look? If no another suggestion, I will merge this PR tomorrow.

[jerryshao]

@FANNG1 please help to review this PR.

[FANNG1]

This PR adds extra metrics name to current guaga metrics like adding gravitino-server.http-server.idle-thread.num.timestamp corresponding to current gravitino-server.http-server.idle-thread.num metrics. what we expect may include timestamp to metric values, like metrics-name timestamp metrics-value, but this may break existing Prometheus metrics format, is there any way to extend the Prometheus format? @jerryshao WDYT?

[jerryshao]

I'm not so sure about Prometheus metrics, but I think to keep the compatibility is the important thing. Can you please investigate other projects to see how they add timestamp to the metrics?

[Abyss-lord]

This PR adds extra metrics name to current guaga metrics like adding gravitino-server.http-server.idle-thread.num.timestamp corresponding to current gravitino-server.http-server.idle-thread.num metrics. what we expect may include timestamp to metric values, like metrics-name timestamp metrics-value, but this may break existing Prometheus metrics format, is there any way to extend the Prometheus format?

@FANNG1 Thanks for the feedback! what we expect ideally is to have a single metric line with explicit timestamps, like:

gravitino_server_http_server_idle_thread_num 42 1713085340000

But if I remember correctly, in the standard /metrics endpoint, Prometheus itself ignores any explicit timestamps even if they are appended.

[jerqi]

This PR adds extra metrics name to current guaga metrics like adding gravitino-server.http-server.idle-thread.num.timestamp corresponding to current gravitino-server.http-server.idle-thread.num metrics. what we expect may include timestamp to metric values, like metrics-name timestamp metrics-value, but this may break existing Prometheus metrics format, is there any way to extend the Prometheus format?

@FANNG1 Thanks for the feedback! what we expect ideally is to have a single metric line with explicit timestamps, like:

gravitino_server_http_server_idle_thread_num 42 1713085340000

But if I remember correctly, in the standard /metrics endpoint, Prometheus itself ignores any explicit timestamps even if they are appended.

@FANNG1 Gently ping.

[FANNG1]

seems we could close this PR?

[jerqi]

seems we could close this PR?

Why？

[FANNG1]

seems we could close this PR?

Why？

Prometheus doesn't support timestamps in metrics.

[jerqi]

seems we could close this PR?

Why？

Prometheus doesn't support timestamps in metrics.

@Abyss-lord WDYT?

[Abyss-lord]

seems we could close this PR?

Why？

Prometheus doesn't support timestamps in metrics.

@Abyss-lord WDYT?

@jerqi @FANNG1 Thanks for the clarification! Would this kind of .timestamp suffix be considered acceptable in general? Could this pattern still be useful in some cases — for example, if we just want to know when a particular metric was last updated?

Maybe it's not suitable for all metrics, but could it be acceptable to apply this suffix to a small number of selected metrics where update time is meaningful?