Support for multiple samples within same metric 'NewMetricWithTimestamp'

[machadovilaca]

This might not even be considered a bug, but I think this is a very annoying behavior.

When using NewMetricWithTimestamp in a Collector as in:

// {
//     Name: "metric_test_1", Description: "metric_test_1 description",
//     Values: [ { value: 1,  timestamp: 1663713926158 }, { value: 2,  timestamp: 1663713937491 } ] 
// } 

desc := prometheus.NewDesc(m.Name, m.Description, nil, nil)

for _, value := range m.Values {
  ch <- prometheus.NewMetricWithTimestamp(
    value.timestamp,
    prometheus.MustNewConstMetric(desc, prometheus.GaugeValue, value.value),
  )
}

The client is running checkMetricConsistency for each of the pushed metrics which is throwing the following error

...
* collected metric "metric_test_1" { gauge:<value:2 > timestamp_ms:1663713937491 } was collected before with the same name and label values
...

I think the validation should be aware of the metrics timestamps and not throw an error in this case because the metrics belong to different point in times, and there are use cases where it makes sense to allow multiple ones to be pushed in the same scrape call.

[bwplotka]

Hey, I think his is not desired functionality, I am afraid. ):

Prometheus clients are optimized to present the current state (or last known state) of data to a potential collector/scraper/Prometheus. Even if we somewhat hack it to put multiple samples of the same series, Prometheus and other scrapers will most likely fail. In other words, you would need to propose a such a change to the https://github.com/OpenObservability/OpenMetrics standard. Only if it's there, will we implement it such extension (which I doubt will be accepted).

Perhaps you might need to look in Otel Go SDK and push the metric paradigm to have functionality like this, but you need to understand the huge impact of buffering multiple samples in your clients.

Hope that makes sense. I will close this issue to avoid confusion but feel free to discuss - maybe I missed something. We can always reopen. Thanks!

[machadovilaca]

@bwplotka I was just checking the OpenMetrics standard specification. In the
Gauge MetricPoint section, isn't what I proposed on of the use cases they show?

An example with a Metric with no labels and two MetricPoints with timestamps:

# TYPE foo gauge
foo 17.0 123
foo 18.0 456

[machadovilaca]

By the way, just tested this with the Python client, and it supports what I suggested

import time
import random
import datetime
from prometheus_client.core import GaugeMetricFamily, REGISTRY
from prometheus_client import start_http_server

class RandomNumberCollector(object):
    def __init__(self):
        pass
    def collect(self):
        gauge = GaugeMetricFamily("random_number", "A random number generator, I have no better idea", labels=["randomNum"])
        gauge.add_metric(['random_num'], random.randint(1, 20), int(datetime.datetime.utcnow().timestamp()))
        time.sleep(1)
        gauge.add_metric(['random_num'], random.randint(1, 20), int(datetime.datetime.utcnow().timestamp()))
        yield gauge

if __name__ == "__main__":
    port = 9000
    frequency = 1
    start_http_server(port)
    REGISTRY.register(RandomNumberCollector())
    while True:
        time.sleep(frequency)

...
# HELP random_number A random number generator, I have no better idea
# TYPE random_number gauge
random_number{randomNum="random_num"} 14.0 1668885744000
random_number{randomNum="random_num"} 18.0 1668885745000

[bwplotka]

Client supports, but how do you want to consume it? Do you plan to use Prometheus or Prometheus-like scraper?

[bwplotka]

You got me with OM statement: https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metric-types-1

..and https://github.com/OpenObservability/OpenMetrics/blob/main/specification/OpenMetrics.md#metricpoint-1

I don't understand yet, why this is there, but you are right, OM supports it. Trying to get more information about it.

BTW Again, what's your use case? Why do you want this done? (:

[machadovilaca]

I was planning to use Prometheus.
My use case is just a tool I am developing to help in creating and testing metrics/alerts. What I was looking for was the ability to quickly create multiple metric values in different timestamps in the last hour/day or so.

[bwplotka]

Please check how this is consumed by Prometheus. It's likely Prometheus just drops other samples, but let's see. Maybe there is some work to be done there. For 2-4 samples it's ok, but more than that on scale, the scrape will be simply too large.

Given OM supports it I think I might change my mind towards implementation here (:

[machadovilaca]

They are

func (s *httpServer) handleMetricReq(w http.ResponseWriter, _ *http.Request) {
	w.WriteHeader(http.StatusOK)

	now := time.Now()

	w.Write([]byte("# HELP random_number A random number generator, I have no better idea\n"))
	w.Write([]byte("# TYPE random_number gauge\n"))

	w.Write([]byte("random_number{randomNum=\"random_num\"} 100.0 " + strconv.FormatInt(now.Add(-60*time.Minute).Unix(), 10) + "000\n"))
	w.Write([]byte("random_number{randomNum=\"random_num\"} 180.0 " + strconv.FormatInt(now.Add(-20*time.Minute).Unix(), 10) + "000\n"))
	w.Write([]byte("random_number{randomNum=\"random_num\"} 120.0 " + strconv.FormatInt(now.Add(-10*time.Minute).Unix(), 10) + "000\n"))
	w.Write([]byte("random_number{randomNum=\"random_num\"} 140.0 " + strconv.FormatInt(now.Add(-5*time.Minute).Unix(), 10) + "000\n"))
}

[bwplotka]

Cool, thanks for checking. I think you convinced me (: We might want to go through other APIs too with @kakkoyun , but we can def reopen your PR if you still want to help make that work! #1142 - Can we please add some unit test too?

[machadovilaca]

I don't know if @Elecon-rou wants to proceed with it, if not I can open a new PR

[Elecon-rou]

@bwplotka @machadovilaca I'll try to add the test, sure

[machadovilaca]

@bwplotka I created a new PR with the fix and a unit test for the metric consistency check