Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builder-like pattern to simplify creation of metrics containers #945

Closed
rfratto opened this issue Dec 9, 2021 · 3 comments
Closed

Builder-like pattern to simplify creation of metrics containers #945

rfratto opened this issue Dec 9, 2021 · 3 comments
Labels

Comments

@rfratto
Copy link
Member

rfratto commented Dec 9, 2021

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 
@vtolstov
Copy link

vtolstov commented Mar 7, 2022

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)

@stale
Copy link

stale bot commented Sep 20, 2022

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 stale bot added the stale label Sep 20, 2022
@rfratto
Copy link
Member Author

rfratto commented Sep 20, 2022

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

@rfratto rfratto closed this as completed Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants