Extend prometheus.Registry to implement Collector

[rfratto]

This change allows Registries to be used as Collectors.

This enables new instances of Registry to be passed to ephemeral subroutines for collecting metrics from subroutines which are still running:

package main

import (
  "fmt"

  "github.com/prometheus/client_golang/prometheus"
)

func main() {
  globalReg := prometheus.NewRegistry()

  for i := 0; i < 100; i++ {
    workerReg := prometheus.WrapRegistererWith(prometheus.Labels{
      // Add an ID label so registered metrics from workers don't
      // collide.
      "worker_id": fmt.Sprintf("%d", i),
    }, prometheus.NewRegistry())

    globalReg.MustRegister(workerReg)

    go func(i int) {
      runWorker(workerReg)

      // Unregister any metrics the worker may have created.
      globalReg.Unregister(workerReg)
    }(i)
  }
}

// runWorker runs a worker, registering worker-specific metrics.
func runWorker(reg *prometheus.Registry) {
  // ... register metrics ...
  // ... do work ...
}

This change makes it easier to avoid leaking metrics from subroutines which do not consistently properly unregister metrics.

This would primarily make things easier for Grafana Agent, where it can spawn different subroutines throughout the process lifetime which have different sets of metrics (i.e., remote_write). Ensuring that metrics from these subroutines consistently get unregistered can be a source of bugs, and something like this would make it easier for us to avoid leaking no-longer-needed metrics.

I'm not completely sure this change works as-is (it seems reasonable), but I wanted to open it as a PR to spawn discussion about the concept.

[rfratto]

This would also make #945 no longer needed, since you could instead pass new Registries to metric containers and easily unregister the container as a whole.

[kakkoyun]

I really like the pattern. I can see the places where I'd use this pattern.
I want to think about it for a second and check if there's any downside (possible abuse?) to complying with Collector interface 🤔

[bwplotka]

Hm, so essentially you want to be able to group collectors better and collect, register and unregister them in one go. I think it's pretty nice characteristics, although at first it the Registry might feel like a separate functionality. The only downside I can see is that we put too many functionalities into one struct which can cause a bit of confusion and many ways of doing same thing.

The alternative is to create dedicated structure MultiRegistry (or multiCollector) attempt I did in https://github.com/prometheus/client_golang/pull/929/files#diff-6f0796727ecbd7f1d226589f6cf66cbb90a2c30d1823ce35d7a480e2561358d6R966 or something like that... But given this code is so little and clean I would be happy with your proposal @rfratto 💪🏽

[bwplotka]

LGMT, I like this logic 👍🏽

A bit more docs would be amazing if you have time (:

[bwplotka]

Do you mind adding a paraph about implementing Collector, it's behavior and what it enables?

Ideally we could add short ExampleRegistryGrouping example in test file too! 🤗

[rfratto]

It's a good thing @bwplotka suggested writing an Example function, because I definitely missed something on my first pass at this :)

The snippet I gave in the PR description doesn't work; prometheus.WrapRegistererWith returns an instance of the Registerer interface, which doesn't implement Collector.

I'm not sure what we should do about this change in that case; it's a lot more limiting than I originally thought since most of the API erases the Collector implementation from the Registry.

There could be a new interface:

type RegistererCollector interface {
  Registerer 
  Collector 
}

which methods like WrapRegistererWith can be updated to return, but I'm not sure if this would bloat the proposal too much.

[bwplotka]

Hmmm what about casting to Collector interface in your code for now?

[rfratto]

Added more docs with an example, PTAL :)

Also, prometheus.WrapRegistererWith would work, it's just not very obvious:

package main

import (
  "fmt"

  "github.com/prometheus/client_golang/prometheus"
)

func main() {
  globalReg := prometheus.NewRegistry()

  for i := 0; i < 100; i++ {
    workerMetrics := prometheus.NewRegistry() 
    globalReg.MustRegister(workerMetrics)

    go func(i int) {
      // Wrap worker metrics with a unique label so workers don't 
      // collide. 
      runWorker(prometheus.WrapRegistererWith(prometheus.Labels{
        "worker_id": fmt.Sprintf("%d", i),
      }, workerMetrics)

      // Unregister any metrics the worker may have created.
      globalReg.Unregister(workerMetrics)
    }(i)
  }
}

// runWorker runs a worker, registering worker-specific metrics.
func runWorker(reg prometheus.Registerer) {
  // ... register metrics ...
  // ... do work ...
}

[bwplotka]

I like it 👍🏽 Again, casting to different interface would also work just fine. Thanks!