Skip to content

Commit

Permalink
Use the same Help for metrics with and without labels
Browse files Browse the repository at this point in the history 
When metrics are pre-registered with Help and without labels,
when issuing metrics with labels, the help was filled with
the key_name.

But prometheus client enforces help to be consistent accross
several elements, so, it can break at Runtime if a metric
has been declared without its labels and later issued this metric
with labels.

In order to fix this, we store the help declared at startup for
each key, so, when declaring a metric with some labels, we try
to re-used pre-declared help for the metric.

It should fix the issue that appeared with Consul 1.9.x and
fix hashicorp/consul#9471
  • Loading branch information
@pierresouchay
pierresouchay committed Dec 29, 2020
1 parent e640e04 commit 923ff58
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 30 deletions.
68 changes: 44 additions & 24 deletions prometheus/prometheus.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,14 @@ type PrometheusSink struct {
summaries sync.Map
counters sync.Map
expiration time.Duration
help map[string]string
}

// GaugeDefinition can be provided to PrometheusOpts to declare a constant gauge that is not deleted on expiry.
type GaugeDefinition struct {
Name []string
Name []string
ConstLabels []metrics.Label
Help string
Help string
}

type gauge struct {
Expand All @@ -76,9 +77,9 @@ type gauge struct {

// SummaryDefinition can be provided to PrometheusOpts to declare a constant summary that is not deleted on expiry.
type SummaryDefinition struct {
Name []string
Name []string
ConstLabels []metrics.Label
Help string
Help string
}

type summary struct {
Expand All @@ -89,9 +90,9 @@ type summary struct {

// CounterDefinition can be provided to PrometheusOpts to declare a constant counter that is not deleted on expiry.
type CounterDefinition struct {
Name []string
Name []string
ConstLabels []metrics.Label
Help string
Help string
}

type counter struct {
Expand All @@ -112,11 +113,12 @@ func NewPrometheusSinkFrom(opts PrometheusOpts) (*PrometheusSink, error) {
summaries: sync.Map{},
counters: sync.Map{},
expiration: opts.Expiration,
help: make(map[string]string),
}

initGauges(&sink.gauges, opts.GaugeDefinitions)
initSummaries(&sink.summaries, opts.SummaryDefinitions)
initCounters(&sink.counters, opts.CounterDefinitions)
initGauges(&sink.gauges, opts.GaugeDefinitions, sink.help)
initSummaries(&sink.summaries, opts.SummaryDefinitions, sink.help)
initCounters(&sink.counters, opts.CounterDefinitions, sink.help)

reg := opts.Registerer
if reg == nil {
Expand Down Expand Up @@ -191,43 +193,46 @@ func (p *PrometheusSink) Collect(c chan<- prometheus.Metric) {
})
}

func initGauges(m *sync.Map, gauges []GaugeDefinition) {
func initGauges(m *sync.Map, gauges []GaugeDefinition, help map[string]string) {
for _, g := range gauges {
key, hash := flattenKey(g.Name, g.ConstLabels)
help[fmt.Sprintf("gauge.%s", key)] = g.Help
pG := prometheus.NewGauge(prometheus.GaugeOpts{
Name: key,
Help: g.Help,
ConstLabels: prometheusLabels(g.ConstLabels),
})
m.Store(hash, &gauge{ Gauge: pG })
m.Store(hash, &gauge{Gauge: pG})
}
return
}

func initSummaries(m *sync.Map, summaries []SummaryDefinition) {
func initSummaries(m *sync.Map, summaries []SummaryDefinition, help map[string]string) {
for _, s := range summaries {
key, hash := flattenKey(s.Name, s.ConstLabels)
help[fmt.Sprintf("summary.%s", key)] = s.Help
pS := prometheus.NewSummary(prometheus.SummaryOpts{
Name: key,
Help: s.Help,
MaxAge: 10 * time.Second,
ConstLabels: prometheusLabels(s.ConstLabels),
Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001},
})
m.Store(hash, &summary{ Summary: pS })
m.Store(hash, &summary{Summary: pS})
}
return
}

func initCounters(m *sync.Map, counters []CounterDefinition) {
func initCounters(m *sync.Map, counters []CounterDefinition, help map[string]string) {
for _, c := range counters {
key, hash := flattenKey(c.Name, c.ConstLabels)
help[fmt.Sprintf("counter.%s", key)] = c.Help
pC := prometheus.NewCounter(prometheus.CounterOpts{
Name: key,
Help: c.Help,
ConstLabels: prometheusLabels(c.ConstLabels),
})
m.Store(hash, &counter{ Counter: pC })
m.Store(hash, &counter{Counter: pC})
}
return
}
Expand Down Expand Up @@ -274,16 +279,21 @@ func (p *PrometheusSink) SetGaugeWithLabels(parts []string, val float32, labels
localGauge.updatedAt = time.Now()
p.gauges.Store(hash, &localGauge)

// The gauge does not exist, create the gauge and allow it to be deleted
// The gauge does not exist, create the gauge and allow it to be deleted
} else {
help := key
existingHelp, ok := p.help[fmt.Sprintf("gauge.%s", key)]
if ok {
help = existingHelp
}
g := prometheus.NewGauge(prometheus.GaugeOpts{
Name: key,
Help: key,
Help: help,
ConstLabels: prometheusLabels(labels),
})
g.Set(float64(val))
pg = &gauge{
Gauge: g,
Gauge: g,
updatedAt: time.Now(),
canDelete: true,
}
Expand All @@ -306,18 +316,23 @@ func (p *PrometheusSink) AddSampleWithLabels(parts []string, val float32, labels
localSummary.updatedAt = time.Now()
p.summaries.Store(hash, &localSummary)

// The summary does not exist, create the Summary and allow it to be deleted
// The summary does not exist, create the Summary and allow it to be deleted
} else {
help := key
existingHelp, ok := p.help[fmt.Sprintf("summary.%s", key)]
if ok {
help = existingHelp
}
s := prometheus.NewSummary(prometheus.SummaryOpts{
Name: key,
Help: key,
Help: help,
MaxAge: 10 * time.Second,
ConstLabels: prometheusLabels(labels),
Objectives: map[float64]float64{0.5: 0.05, 0.9: 0.01, 0.99: 0.001},
})
s.Observe(float64(val))
ps = &summary{
Summary: s,
Summary: s,
updatedAt: time.Now(),
canDelete: true,
}
Expand Down Expand Up @@ -346,16 +361,21 @@ func (p *PrometheusSink) IncrCounterWithLabels(parts []string, val float32, labe
localCounter.updatedAt = time.Now()
p.counters.Store(hash, &localCounter)

// The counter does not exist yet, create it and allow it to be deleted
// The counter does not exist yet, create it and allow it to be deleted
} else {
help := key
existingHelp, ok := p.help[fmt.Sprintf("counter.%s", key)]
if ok {
help = existingHelp
}
c := prometheus.NewCounter(prometheus.CounterOpts{
Name: key,
Help: key,
Help: help,
ConstLabels: prometheusLabels(labels),
})
c.Add(float64(val))
pc = &counter{
Counter: c,
Counter: c,
updatedAt: time.Now(),
canDelete: true,
}
Expand Down
78 changes: 72 additions & 6 deletions prometheus/prometheus_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http/httptest"
"net/url"
"reflect"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -56,16 +57,16 @@ func TestNewPrometheusSink(t *testing.T) {

func TestDefinitions(t *testing.T) {
gaugeDef := GaugeDefinition{
Name: []string{"my", "test", "gauge"},
Help: "A gauge for testing? How helpful!",
Name: []string{"my", "test", "gauge"},
Help: "A gauge for testing? How helpful!",
}
summaryDef := SummaryDefinition{
Name: []string{"my", "test", "summary"},
Help: "A summary for testing? How helpful!",
Name: []string{"my", "test", "summary"},
Help: "A summary for testing? How helpful!",
}
counterDef := CounterDefinition{
Name: []string{"my", "test", "summary"},
Help: "A counter for testing? How helpful!",
Name: []string{"my", "test", "summary"},
Help: "A counter for testing? How helpful!",
}

// PrometheusSink config w/ definitions for each metric type
Expand All @@ -79,6 +80,7 @@ func TestDefinitions(t *testing.T) {
if err != nil {
t.Fatalf("err = #{err}, want nil")
}
defer prometheus.Unregister(sink)

// We can't just len(x) where x is a sync.Map, so we range over the single item and assert the name in our metric
// definition matches the key we have for the map entry. Should fail if any metrics exist that aren't defined, or if
Expand Down Expand Up @@ -168,3 +170,67 @@ func TestSetGauge(t *testing.T) {
t.Fatal(response)
}
}

func TestDefinitionsWithLabels(t *testing.T) {
gaugeDef := GaugeDefinition{
Name: []string{"my", "test", "gauge"},
Help: "A gauge for testing? How helpful!",
}
summaryDef := SummaryDefinition{
Name: []string{"my", "test", "summary"},
Help: "A summary for testing? How helpful!",
}
counterDef := CounterDefinition{
Name: []string{"my", "test", "summary"},
Help: "A counter for testing? How helpful!",
}

// PrometheusSink config w/ definitions for each metric type
cfg := PrometheusOpts{
Expiration: 5 * time.Second,
GaugeDefinitions: append([]GaugeDefinition{}, gaugeDef),
SummaryDefinitions: append([]SummaryDefinition{}, summaryDef),
CounterDefinitions: append([]CounterDefinition{}, counterDef),
}
sink, err := NewPrometheusSinkFrom(cfg)
if err != nil {
t.Fatalf("err =%#v, want nil", err)
}
defer prometheus.Unregister(sink)
if len(sink.help) != 3 {
t.Fatalf("Expected len(sink.help) to be 3, was %d: %#v", len(sink.help), sink.help)
}

sink.SetGaugeWithLabels(gaugeDef.Name, 42.0, []metrics.Label{
{Name: "version", Value: "some info"},
})
sink.gauges.Range(func(key, value interface{}) bool {
localGauge := *value.(*gauge)
if !strings.Contains(localGauge.Desc().String(), gaugeDef.Help) {
t.Fatalf("expected gauge to include correct help=%s, but was %s", gaugeDef.Help, localGauge.Desc().String())
}
return true
})

sink.AddSampleWithLabels(summaryDef.Name, 42.0, []metrics.Label{
{Name: "version", Value: "some info"},
})
sink.summaries.Range(func(key, value interface{}) bool {
metric := *value.(*summary)
if !strings.Contains(metric.Desc().String(), summaryDef.Help) {
t.Fatalf("expected gauge to include correct help=%s, but was %s", summaryDef.Help, metric.Desc().String())
}
return true
})

sink.IncrCounterWithLabels(counterDef.Name, 42.0, []metrics.Label{
{Name: "version", Value: "some info"},
})
sink.counters.Range(func(key, value interface{}) bool {
metric := *value.(*counter)
if !strings.Contains(metric.Desc().String(), counterDef.Help) {
t.Fatalf("expected gauge to include correct help=%s, but was %s", counterDef.Help, metric.Desc().String())
}
return true
})
}

0 comments on commit 923ff58

Please sign in to comment.