client_golang icon indicating copy to clipboard operation
client_golang copied to clipboard

Builder-like pattern to simplify creation of metrics containers

Open rfratto opened this issue 4 years ago • 1 comments

Background

It is common for Prometheus client code to create a container which holds metrics pertaining to a specific subsystem. Custom containers are useful for avoiding pollution of the global registry and allow for tracking independent sets of metrics when multiple instances of a subsystem exist.

Today, I find that creating metrics containers is largely repetitive. A metrics container must always both create the inner metrics and use some mechanism to expose them all.

Ideally, metrics containers expose their inner metrics through the prometheus.Collector interface. However, I find that this is not how most people implement these containers. Instead, developers are more likely to avoid implementing the interface by registering all the metrics at creation time. I view this as an anti-pattern, as the caller potentially loses the ability to unregister those metrics. This has propagated to projects like Grafana Agent, which needed to hackily implement some way of unregistering metrics from packages that do this.

Proposal

I propose a new type, prometheus.Container, to reduce the amount of boilerplate required to create these metric containers. By making it easier to have a metrics container that implements prometheus.Collector, I hope it will also dissuade people from registering metrics from the container's constructor.

The type will track a set of inner Collectors, and expose a similar API to the promauto package:

type Container struct {
  cs []Collector 
}

func (c *Container) NewCounter(opts CounterOpts) Counter {
  m := NewCounter(opts) 
  c.cs = append(c.cs, m)
  return m 
}

// Omitted for brevity: NewCounterVec, NewGauge, etc..

func (c *Container) Describe(ch <-chan *Desc) {
  for _, m := range c.cs {
    m.Describe(ch)
  }
}

func (c *Container) Collect(ch <-chan Metric) {
  for _, m := range c.cs {
    m.Collect(ch)
  }
}

Example Usage

// myCustomMetrics implements prometheus.Collector through usage of the 
// prometheus.Container helper. 
type myCustomMetrics struct {
  prometheus.Container 

  SomeCounter prometheus.Counter   
}

func newMyCustomMetrics() *myCustomMetrics {
  var m myCustomMetrics
  
  // Create and register the counter into our container. 
  m.SomeCounter = m.NewCounter(prometheus.CounterOpts{
    Name: "some_application_counting_something_important_hopefully",
  })

  return &m
}

type MySubsystem struct {
  metrics *myCustomMetrics
}

// Metrics returns the set of metrics for MySubsystem. 
func (s *MySubsystem) Metrics() prometheus.Collector { return s.metrics }

Alternative Implementation

The upcoming support for generics in Go 1.18 would open the possibility for a much slimmer Container implementation:

type Container struct {
  cs []Collector 
}

// Register tracks a Collector. 
// Example usage:
//
//   var c prometheus.Container
//   myCounter := c.Register(prometheus.NewCounter(...))
//   myCounter.Inc() 
// 
func (c *Container) Register[C Collector](c C) C {
  c.cs = append(c.cs, c)
  return c  
}

// Omitted for brevity: Describe, Collect 

rfratto avatar Dec 09 '21 02:12 rfratto

Does this help in this situation:

  1. i have framework that have ability to create meter instance with name and labels
  2. each meter have ability to inc/dec/set
  3. in the code i can create two counter with the same name but different labels like:
  • some_metric1{label1="value1"}
  • some_metric1{label2="value2"}

this is possible with victoriametrics/metrics package, but in promentheus i can't do that (only unregister old metric, create new CounterVec with all labels, and get needed CounterVec based on labels, but unregister removes previous stored values)

vtolstov avatar Mar 07 '22 10:03 vtolstov

Hello 👋 Looks like there was no activity on this issue for the last 3 months. Do you mind updating us on the status? Is this still reproducible or needed? If yes, just comment on this PR or push a commit. Thanks! 🤗 If there will be no activity in the next 4 weeks, this issue will be closed (we can always reopen an issue if we need!).

stale[bot] avatar Sep 20 '22 21:09 stale[bot]

I don't think this is needed anymore since #1103 was merged.

rfratto avatar Sep 20 '22 23:09 rfratto