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

Allow timestamp in metric samples #64

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Aug 8, 2021

Metric line can also contain optional timestamp.

http_requests_total{method="post",code="200"}  0
http_requests_total{method="post",code="400"}  3   1395066363000

Should I move forward with this?

  • support of timestamp in gauge
  • support of timestamp in counter
  • support of timestamp in histogram (should we have this)
  • support in InMemory adapter
  • support in Redis adapter
  • support in APC adapter

@simPod simPod marked this pull request as draft August 8, 2021 12:26
@pachico
Copy link
Contributor

pachico commented Oct 28, 2022

Can you share the rationale behind this?
Although it's an optional valid value, in my experience, I hardly saw any reason to use it, which is why most of the clients in other languages I use tend to get rid of it or to disable it by default.

Prometheus might have problems ingesting metrics if timestamps are either in the future (clocks might be misaligned) and in the case of distributed applications, timestamps might be inconsistent too, which is why to unify them as "when things were observed", which is the result of not having a timestamp in the exposed metrics.

@rs-orlov
Copy link

@pachico, if you don't mind I will share mine.

We have following setup: multiple nodes serving http-requests using php-fpm. Each node has its own local instance of redis storing metrics, each node scraped separately.

Once written, any metric is stored in redis forever. So even if part reporting that metric is dead or removed, metric itself continues being exported. And you can't tell if it has been updated right now or a long time ago.

In opposite to manual housekeeping exporting with timestamp could help prometheus mark metric as staled.

@pachico
Copy link
Contributor

pachico commented Nov 10, 2022

@rs-orlov

You seem to be describing a common issue with Prometheus exporters, in general.
I was facing the same issue not long ago with Vector where the problem wasn't to know when a metric actually changed but an always increasing cardinality that was indeed causing ingestion problems.

The way they solved it is, in my opinion, better than adding a timestamp (https://vector.dev/docs/reference/configuration/global-options/#expire_metrics_secs): metrics that didn't change within a certain interval were not exported.

Having Redis a TTL mechanism I might be in favour of using a SETEX every time there is an update to a given metric and letting it, instead, expire after a certain amount of time.

In addition, you would have smaller exported payloads and a smaller memory usage footprint in Redis.

@rs-orlov
Copy link

@pachico

Agree, seems like a better option. But \Prometheus\Storage\Redis uses HSET for storing data. As I understand, that means that you can't set an expiration to particular series, only whole key. "inside" one metric name all staled series will continue to be reported as long as there is at least one alive.

@rs-orlov
Copy link

Btw, doc says

incrementing the value of a key with INCR, pushing a new value into a list with LPUSH, or altering the field value of a hash with HSET are all operations that will leave the timeout untouched

@simPod
Copy link
Contributor Author

simPod commented Nov 11, 2022

We're using victoriametrics that support openmetrics format.

@johannesx75
Copy link

Any chance that this gets implemented? Would love to be able to add timestamps.
Use case is similar to @rs-orlov. Basically to provide context on how old the metric is.

@rs-orlov
Copy link

@johannesx75, we switched to telegraf as a local aggregator. Each php-node have its local telegraf instance, application uses statsd to report metrics. On telegraf side we use prometheus plugin to export metrics.

Works fine, there are settings for obsolescence and you also can export update timestamp

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.

4 participants