PromPHP / prometheus_client_php

Prometheus instrumentation library for PHP applications
https://prometheus.io/docs/concepts/metric_types/
Apache License 2.0
423 stars 93 forks source link

Allow timestamp in metric samples #64

Open simPod opened 3 years ago

simPod commented 3 years ago

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?

pachico commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

We're using victoriametrics that support openmetrics format.

johannesx75 commented 11 months ago

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 commented 11 months ago

@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