Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Commit

Permalink
Fixes dynamic registration of views bug (#456)
Browse files Browse the repository at this point in the history
Prometheus's default registry appears to block registration of any other
metrics based on there being >= 1 duplicate in the set of metrics returned
from Describe to register. While Describe's API seems to indicate that a
collector should return a superset of metrics, and while this makes perfect
sense with the Unregister logic, it does not seem to play nicely with us
running register on the same collector repeatedly with different metrics, nor
with calling unregister and then attempting to register everything once again.
that is to say, Prometheus client seems to explicitly disallow the thing we're
seemingly trying to do. upstream: prometheus/client_golang#47

This patch will:
* No longer call Unregister and then Register, as this is a case handled by
prometheus: https://github.com/prometheus/client_golang/blob/fcc130e101e76c5d303513d0e28f4b6d732845c7/prometheus/registry.go#L112-L117
* Only return metrics that are not yet registered to Prometheus registry
  • Loading branch information
Reed Allman authored and JBD committed Feb 20, 2018
1 parent ba5c8f1 commit 30bf88b
Showing 1 changed file with 28 additions and 14 deletions.
42 changes: 28 additions & 14 deletions exporter/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,9 @@ func (c *collector) registerViews(views ...*view.View) {
count := 0
for _, view := range views {
sig := viewSignature(c.opts.Namespace, view)
c.registeredViewsMu.Lock()
c.viewsMu.Lock()
_, ok := c.registeredViews[sig]
c.registeredViewsMu.Unlock()
c.viewsMu.Unlock()

if !ok {
desc := prometheus.NewDesc(
Expand All @@ -103,17 +103,16 @@ func (c *collector) registerViews(views ...*view.View) {
tagKeysToLabels(view.TagKeys()),
nil,
)
c.registeredViewsMu.Lock()
c.registeredViews[sig] = desc
c.registeredViewsMu.Unlock()
c.viewsMu.Lock()
c.newViews[sig] = desc
c.viewsMu.Unlock()
count++
}
}
if count == 0 {
return
}

c.reg.Unregister(c)
if err := c.reg.Register(c); err != nil {
c.opts.onError(fmt.Errorf("cannot register the collector: %v", err))
}
Expand Down Expand Up @@ -160,9 +159,14 @@ type collector struct {
// Collect is invoked and the cycle is repeated.
viewData map[string]*view.Data

registeredViewsMu sync.Mutex
// protects registeredViews and newViews
viewsMu sync.Mutex
// registeredViews maps a view to a prometheus desc.
registeredViews map[string]*prometheus.Desc

// newViews is the set of views that are
// queued to be registered with Prometheus.
newViews map[string]*prometheus.Desc
}

func (c *collector) addViewData(vd *view.Data) {
Expand All @@ -175,12 +179,21 @@ func (c *collector) addViewData(vd *view.Data) {
}

func (c *collector) Describe(ch chan<- *prometheus.Desc) {
c.registeredViewsMu.Lock()
registered := make(map[string]*prometheus.Desc)
for k, desc := range c.registeredViews {
registered[k] = desc
// NOTE that this will not work properly if calling
// prometheus.Registerer.Unregister, but the semantics around prometheus client
// do not seem to lend to us being able to register all our views repeatedly.
// this does not return the super set of desc, just the subset that we have not
// yet registered with prometheus (it yells about dupes, and explicitly
// disallows unregistering all metrics and then registering them again).

c.viewsMu.Lock()
registered := make([]*prometheus.Desc, 0, len(c.newViews))
for k, desc := range c.newViews {
registered = append(registered, desc)
c.registeredViews[k] = desc
delete(c.newViews, k)
}
c.registeredViewsMu.Unlock()
c.viewsMu.Unlock()

for _, desc := range registered {
ch <- desc
Expand All @@ -197,9 +210,9 @@ func (c *collector) Collect(ch chan<- prometheus.Metric) {

for _, vd := range viewData {
sig := viewSignature(c.opts.Namespace, vd.View)
c.registeredViewsMu.Lock()
c.viewsMu.Lock()
desc := c.registeredViews[sig]
c.registeredViewsMu.Unlock()
c.viewsMu.Unlock()

for _, row := range vd.Rows {
metric, err := c.toMetric(desc, vd.View, row)
Expand Down Expand Up @@ -260,6 +273,7 @@ func newCollector(opts Options, registrar *prometheus.Registry) *collector {
reg: registrar,
opts: opts,
registeredViews: make(map[string]*prometheus.Desc),
newViews: make(map[string]*prometheus.Desc),
viewData: make(map[string]*view.Data),
}
}
Expand Down

0 comments on commit 30bf88b

Please sign in to comment.