Merge pull request #129 from FFMMM/describe-descriptors

actually describe PrometheusSink descriptors

prometheus/prometheus.go

@@ -20,6 +20,7 @@ var (
 	// PrometheusSink.
 	DefaultPrometheusOpts = PrometheusOpts{
 		Expiration: 60 * time.Second,
+		Name: "default_prometheus_sink",
 	}
 )
 
@@ -48,6 +49,7 @@ type PrometheusOpts struct {
 	GaugeDefinitions   []GaugeDefinition
 	SummaryDefinitions []SummaryDefinition
 	CounterDefinitions []CounterDefinition
+	Name               string
 }
 
 type PrometheusSink struct {
@@ -57,6 +59,7 @@ type PrometheusSink struct {
 	counters   sync.Map
 	expiration time.Duration
 	help       map[string]string
+	name       string
 }
 
 // GaugeDefinition can be provided to PrometheusOpts to declare a constant gauge that is not deleted on expiry.
@@ -106,12 +109,17 @@ func NewPrometheusSink() (*PrometheusSink, error) {
 
 // NewPrometheusSinkFrom creates a new PrometheusSink using the passed options.
 func NewPrometheusSinkFrom(opts PrometheusOpts) (*PrometheusSink, error) {
+	name := opts.Name
+	if name == "" {
+		name = "default_prometheus_sink"
+	}
 	sink := &PrometheusSink{
 		gauges:     sync.Map{},
 		summaries:  sync.Map{},
 		counters:   sync.Map{},
 		expiration: opts.Expiration,
 		help:       make(map[string]string),
+		name:       name,
 	}
 
 	initGauges(&sink.gauges, opts.GaugeDefinitions, sink.help)
@@ -126,11 +134,15 @@ func NewPrometheusSinkFrom(opts PrometheusOpts) (*PrometheusSink, error) {
 	return sink, reg.Register(sink)
 }
 
-// Describe is needed to meet the Collector interface.
+// Describe sends a Collector.Describe value from the descriptor created around PrometheusSink.Name
+// Note that we cannot describe all the metrics (gauges, counters, summaries) in the sink as
+// metrics can be added at any point during the lifecycle of the sink, which does not respect
+// the idempotency aspect of the Collector.Describe() interface
 func (p *PrometheusSink) Describe(c chan<- *prometheus.Desc) {
-	// We must emit some description otherwise an error is returned. This
-	// description isn't shown to the user!
-	prometheus.NewGauge(prometheus.GaugeOpts{Name: "Dummy", Help: "Dummy"}).Describe(c)
+	// dummy value to be able to register and unregister "empty" sinks
+	// Note this is not actually retained in the PrometheusSink so this has no side effects
+	// on the caller's sink. So it shouldn't show up to any of its consumers.
+	prometheus.NewGauge(prometheus.GaugeOpts{Name: p.name, Help: p.name}).Describe(c)
 }
 
 // Collect meets the collection interface and allows us to enforce our expiration
@@ -398,6 +410,7 @@ func NewPrometheusPushSink(address string, pushInterval time.Duration, name stri
 		summaries:  sync.Map{},
 		counters:   sync.Map{},
 		expiration: 60 * time.Second,
+		name:       "default_prometheus_sink",
 	}
 
 	pusher := push.New(address, name).Collector(promSink)

prometheus/prometheus_test.go

@@ -54,6 +54,61 @@ func TestNewPrometheusSink(t *testing.T) {
 		t.Fatalf("Unregister(sink) = false, want true")
 	}
 }
+// TestMultiplePrometheusSink tests registering multiple sinks on the same registerer with different descriptors
+func TestMultiplePrometheusSink(t *testing.T) {
+	gaugeDef := GaugeDefinition{
+		Name: []string{"my", "test", "gauge"},
+		Help: "A gauge for testing? How helpful!",
+	}
+
+	cfg := PrometheusOpts{
+		Expiration:         5 * time.Second,
+		GaugeDefinitions:   append([]GaugeDefinition{}, gaugeDef),
+		SummaryDefinitions: append([]SummaryDefinition{}),
+		CounterDefinitions: append([]CounterDefinition{}),
+		Name: "sink1",
+	}
+
+	sink1, err := NewPrometheusSinkFrom(cfg)
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+
+	reg := prometheus.DefaultRegisterer
+	if reg == nil {
+		t.Fatalf("Expected default register to be non nil, got nil.")
+	}
+
+	gaugeDef2 := GaugeDefinition{
+		Name: []string{"my2", "test", "gauge"},
+		Help: "A gauge for testing? How helpful!",
+	}
+
+	cfg2 := PrometheusOpts{
+		Expiration:         15 * time.Second,
+		GaugeDefinitions:   append([]GaugeDefinition{}, gaugeDef2),
+		SummaryDefinitions: append([]SummaryDefinition{}),
+		CounterDefinitions: append([]CounterDefinition{}),
+		// commenting out the name to point out that the default name will be used here instead
+		// Name:               "sink2",
+	}
+
+	sink2, err := NewPrometheusSinkFrom(cfg2)
+	if err != nil {
+		t.Fatalf("err = %v, want nil", err)
+	}
+	//check if register has a sink by unregistering it.
+	ok := reg.Unregister(sink1)
+	if !ok {
+		t.Fatalf("Unregister(sink) = false, want true")
+	}
+
+	//check if register has a sink by unregistering it.
+	ok = reg.Unregister(sink2)
+	if !ok {
+		t.Fatalf("Unregister(sink) = false, want true")
+	}
+}
 
 func TestDefinitions(t *testing.T) {
 	gaugeDef := GaugeDefinition{
@@ -65,7 +120,7 @@ func TestDefinitions(t *testing.T) {
 		Help: "A summary for testing? How helpful!",
 	}
 	counterDef := CounterDefinition{
-		Name: []string{"my", "test", "summary"},
+		Name: []string{"my", "test", "counter"},
 		Help: "A counter for testing? How helpful!",
 	}
 
@@ -78,7 +133,7 @@ func TestDefinitions(t *testing.T) {
 	}
 	sink, err := NewPrometheusSinkFrom(cfg)
 	if err != nil {
-		t.Fatalf("err = #{err}, want nil")
+		t.Fatalf("err = %v, want nil", err)
 	}
 	defer prometheus.Unregister(sink)
 
@@ -94,7 +149,6 @@ func TestDefinitions(t *testing.T) {
 	})
 	sink.summaries.Range(func(key, value interface{}) bool {
 		name, _ := flattenKey(summaryDef.Name, summaryDef.ConstLabels)
-		fmt.Printf("k: %+v, v: %+v", key, value)
 		if name != key {
 			t.Fatalf("expected my_test_summary, got #{name}")
 		}
@@ -252,7 +306,7 @@ func TestDefinitionsWithLabels(t *testing.T) {
 		Help: "A summary for testing? How helpful!",
 	}
 	counterDef := CounterDefinition{
-		Name: []string{"my", "test", "summary"},
+		Name: []string{"my", "test", "counter"},
 		Help: "A counter for testing? How helpful!",
 	}