Timer helpers

[cl-bvl]

Hello.

I'm proposing to add several helper functions to make working with timers more convenient.
I haven’t brought this up on the mailing list yet, but I’d like to hear your feedback.
The code is still a draft; if you like the approach, I’ll add more detailed comments, tests, and documentation.

Here are a few examples of how the new timers could be used:

// Counter with SUM of all measurements
timerCounter := NewTimerCounter(Opts{Name: "timer_counter"})
go func() {
	defer timerCounter.Observe()()
	time.Sleep(2 * time.Second)
}()

// Histogram with all measurements
timerHistogram := NewTimerHistogram(HistogramOpts{Name: "timer_histogram", Buckets: []float64{1, 2, 3, 4, 5, 6}})
timerHistogram.Wrap(func() {
	time.Sleep(time.Duration(rand.Intn(5000)) * time.Millisecond)
})

// Vector histogram. Most other types of timers also have a vector version.
timerHistogramVec := NewTimerHistogramVec(HistogramOpts{Name: "timer_histogram_vec", Buckets: []float64{1, 2, 3, 4, 5, 6}}, []string{"name"})
go func() {
	defer timerHistogramVec.Observe(map[string]string{"name": "timer1"})()
	time.Sleep(time.Duration(rand.Intn(5000)) * time.Millisecond)
}()
timerHistogramVec.WrapLabelValues([]string{"timer2"}, func() {
	time.Sleep(time.Duration(rand.Intn(5000)) * time.Millisecond)
})

// Counter with SUM of all measurements
// But the value also shows the sum of running timers. 
// It is well suited for long-running tasks - then the metric shows their running time until their completion.
timerContinuous := NewTimerContinuous(Opts{Name: "timer_continuous"}, 0)
timerContinuous.Wrap(func() {
	time.Sleep(5 * time.Minute)
})

[bwplotka]

Thanks for this proposal!

BTW: Likely here or Slack is a better place for those discussions.

I have some questions around benefit vs cons (code to maintain, 2 ways of doing same thing).

Correct if I'm wrong, but I see you proposed:

1) Wrap methods

So instead of:

timer := NewTimer(myHistogram)
func() {
  defer timer.ObserveDuration()
  // Do actual work.
}()

one could do:

timer := NewTimerHistogram(myHistogramOpts)
timer.Wrap( func() {
  // Do actual work.
})

Is it really that beneficial? 🤔

2) TimerHistogram type

So instead of:

timer := NewTimer(myHistogram)
defer timer.ObserveDuration()
// Do actual work.

one could do:

timer := NewTimerHistogram(myHistogramOpts)
defer timer.ObserveDuration()
// Do actual work.

What this type gives us that's not possible now?

3) TimerHistogramVec type

So (conceptually) instead of:

timer := NewTimer(myHistogram.WithLabelValues("200"))
timerErr := NewTimer(myHistogram.WithLabelValues("500"))

defer func() { 
  if err != {
    timerErr.ObserveDuration()
  } else {
    timer.ObserveDuration()
  }
}()
// Do actual work, return err

one could do:

timer := NewTimerHistogram(myHistogramOpts, "code")
defer func() { 
  if err != {
    timerErr.WithLabelValues("500").ObserveDuration()
  } else {
    timerErr.WithLabelValues("200").ObserveDuration()
  }
}()
// Do actual work, return err

This might indeed has some benefits, I think I like it

I only wonder if some generic function would not work better. This is because we already have "generic" timer that should work for summary and histogram 🤔

type ObserverVec interface{
  *HistogramVec | *SummaryVec
}
func NewTimerVec[T ObserverVec](o T) *TimerVec {
	return &TimerVec{
		begin:    time.Now(),
		o: o,
	}
}

or

func NewTimerVec(o any) *TimerVec {
	return &TimerVec{
		begin:    time.Now(),
		o: o,
	}
}

4) TimerCounter type

This is interesting, I guess it might make sense to count total time or even gauge the last duration.

I would not call the method Observe though, but rather AddDuration and SetDuration for gauge.

5) TimerContinuous type

That's a fun one! For this one I think one could write a separate function, because you can easily use TimerCounter for this without client_golang changes, no? At least in the beginning I would write an example how to write something like that before considering adding this to v1.

Also I believe there might be better ways to implement it than timer. Why not using CounterFunc to update the time on scrape?

Summary

Generally:

• I don't see wrap being that much helping (1).
• If we add typed functions then (2) and (3), (4) makes sense, but we should likely follow

  client_golang/prometheus/counter.go

  Line 351 in 9e567a7

  func NewCounterFunc(opts CounterOpts, function func() float64) CounterFunc {

  so NewCounterTimer etc. Questions is... should add typed versions OR try write generic ones (Timer and TimerVec only would work for observers, perhaps even for all metrics if we use ObserveDuration for adding and setting logic).
• I would use *Func for (5) and start with example - if the example will be heavily use, we can consider adding as a helper

If we want to make NewTimer work for counters/gauges it could be possible without braking changes with any, so

func NewTimer(o Observer) *Timer {

into

func NewTimer(o any) *Timer {

.. but maybe it's too much -- maybe it's ok to have per metric type functions with corresponding opts, similar to *Func types

[cl-bvl]

Hello.
Thanks for feedback)

1) Wrap methods.

Yes, it does exactly that. But with a few caveats.

1. If you use the existing timer, you need to store the histogram separately and create a new timer each time. This makes the code a little less readable.

type Handler struct {
	reqDuration Histogram
}

func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	timer := NewTimer(h.reqDuration)
	defer timer.ObserveDuration()
}

VS

type Handler2 struct {
	reqDuration *TimerCounter
}

func (h *Handler2) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	defer h.reqDuration.Observe()()
}

I understand that the difference is not very big, but it is there.
Another important nuance is that a histogram is not always required for a timer. Histograms are quite expensive to store. A common case for us is the need to store only a counter of the total elapsed time.

2. Using a separate Wrap function makes the code a little clearer.
   When using the closure option, the person changing the code may want to remove the useless wrapping function, which will break the code

timer := NewTimer(myHistogram)
func() {
  defer timer.ObserveDuration()
  work()
}()

Can be mistakenly converted to:

timer := NewTimer(myHistogram)
defer timer.ObserveDuration()
work()

And although this is 100% the fault of the programmer who did this, but it seems to me that a good API should protect against errors, and not push towards them.

2) TimerHistogram type

You are right, this type is made to maintain a uniform API and does not provide new functionality.
Small bonuses - it's a bit more intuitive to create a HistogramTimer right away, rather than first a Histogram and then a timer on top of it. And it simplifies the code quite a bit.

3) TimerHistogramVec type

Yes, we can do it like this:

type TimerVec struct {
	ObserverVec
}

func NewTimerVec[T HistogramOpts | SummaryOpts](opts T, labelNames []string) *TimerVec {
	t := &TimerVec{}
	switch o := any(opts).(type) {
	case HistogramOpts:
		t.ObserverVec = NewHistogramVec(o, labelNames)
	case SummaryOpts:
		t.ObserverVec = NewSummaryVec(o, labelNames)
         default:
		panic("invalid TimerVec options type")
	}

	return t
}

......

Do you like this?

4) TimerCounter type

Yes, we can call the method AddDuration instead of Observe.
I didn't implement Gauge and it seems to be poorly related to the ideology of Prometheus and timers.
Counter and increase() are great for understanding how much time has been spent since the last measurement.

5) TimerContinuous type

I poked around for 10 minutes and couldn't figure out how to use CounterFunc to implement a ticking timer. I'll think about it some more, but if you have an idea of what it might look like, that would help.