Remove registry level labels

Registry level labels are
redundant and can cause
pointless data-series growth.

Author: lukasz-zimnoch

pkg/metrics/gauge.go

@@ -43,7 +43,11 @@ func (g *Gauge) expose() string {
 	}
 	labels := strings.Join(labelsStrings, ",")
 
-	body := fmt.Sprintf("%v{%v} %v %v", g.name, labels, g.value, g.timestamp)
+	if len(labels) > 0 {
+		labels = "{" + labels + "}"
+	}
+
+	body := fmt.Sprintf("%v%v %v %v", g.name, labels, g.value, g.timestamp)
 
 	return fmt.Sprintf("%v\n%v", typeLine, body)
 }

pkg/metrics/gauge_test.go

@@ -59,3 +59,26 @@ func TestGaugeExpose(t *testing.T) {
 		)
 	}
 }
+
+func TestGaugeWithoutLabelsExpose(t *testing.T) {
+	gauge := &Gauge{
+		name:      "test_gauge",
+		labels:    map[string]string{},
+		value:     500,
+		timestamp: 1000,
+	}
+
+	actualText := gauge.expose()
+
+	expectedText := "# TYPE test_gauge gauge\ntest_gauge 500 1000"
+
+	if actualText != expectedText {
+		t.Fatalf(
+			"incorrect gauge expose text:\n"+
+				"expected: [%v]\n"+
+				"actual:   [%v]",
+			expectedText,
+			actualText,
+		)
+	}
+}

pkg/metrics/registry.go

@@ -42,50 +42,17 @@ func NewLabel(name, value string) Label {
 // Registry performs all management of metrics. Specifically, it allows
 // to registering new metrics and exposing them through the metrics server.
 type Registry struct {
-	labels map[string]string
-
 	metrics      map[string]metric
 	metricsMutex sync.RWMutex
 }
 
 // NewRegistry creates a new metrics registry.
-func NewRegistry(
-	application, identifier string,
-	additionalLabels ...Label,
-) *Registry {
-	labels := mergeLabels(
-		map[string]string{
-			"application": application,
-			"identifier":  identifier,
-		},
-		additionalLabels,
-	)
-
+func NewRegistry() *Registry {
 	return &Registry{
-		labels:  labels,
 		metrics: make(map[string]metric),
 	}
 }
 
-func mergeLabels(
-	labels map[string]string,
-	additionalLabels []Label,
-) map[string]string {
-	for _, additionalLabel := range additionalLabels {
-		if additionalLabel.name == "" || additionalLabel.value == "" {
-			continue
-		}
-
-		if _, exists := labels[additionalLabel.name]; exists {
-			continue
-		}
-
-		labels[additionalLabel.name] = additionalLabel.value
-	}
-
-	return labels
-}
-
 // EnableServer enables the metrics server on the given port. Data will
 // be exposed on `/metrics` path.
 func (r *Registry) EnableServer(port int) {
@@ -122,7 +89,7 @@ func (r *Registry) exposeMetrics() string {
 // will be returned.
 func (r *Registry) NewGauge(
 	name string,
-	additionalLabels ...Label,
+	labels ...Label,
 ) (*Gauge, error) {
 	r.metricsMutex.Lock()
 	defer r.metricsMutex.Unlock()
@@ -133,7 +100,7 @@ func (r *Registry) NewGauge(
 
 	gauge := &Gauge{
 		name:   name,
-		labels: mergeLabels(r.labels, additionalLabels),
+		labels: processLabels(labels),
 	}
 
 	r.metrics[name] = gauge
@@ -146,9 +113,9 @@ func (r *Registry) NewGauge(
 func (r *Registry) NewGaugeObserver(
 	name string,
 	input ObserverInput,
-	additionalLabels ...Label,
+	labels ...Label,
 ) (*Observer, error) {
-	gauge, err := r.NewGauge(name, additionalLabels...)
+	gauge, err := r.NewGauge(name, labels...)
 	if err != nil {
 		return nil, err
 	}
@@ -164,7 +131,7 @@ func (r *Registry) NewGaugeObserver(
 // will be returned.
 func (r *Registry) NewInfo(
 	name string,
-	additionalLabels ...Label,
+	labels []Label,
 ) (*Info, error) {
 	r.metricsMutex.Lock()
 	defer r.metricsMutex.Unlock()
@@ -173,11 +140,31 @@ func (r *Registry) NewInfo(
 		return nil, fmt.Errorf("metric [%v] already exists", name)
 	}
 
+	if len(labels) == 0 {
+		return nil, fmt.Errorf("at least one label should be set")
+	}
+
 	info := &Info{
 		name:   name,
-		labels: mergeLabels(r.labels, additionalLabels),
+		labels: processLabels(labels),
 	}
 
 	r.metrics[name] = info
 	return info, nil
 }
+
+func processLabels(
+	labels []Label,
+) map[string]string {
+	result := make(map[string]string)
+
+	for _, label := range labels {
+		if label.name == "" || label.value == "" {
+			continue
+		}
+
+		result[label.name] = label.value
+	}
+
+	return result
+}

pkg/metrics/registry_test.go

@@ -5,7 +5,7 @@ import (
 )
 
 func TestRegistryNewGauge(t *testing.T) {
-	registry := NewRegistry("test-app", "test-id")
+	registry := NewRegistry()
 
 	gauge, err := registry.NewGauge("test-gauge")
 	if err != nil {
@@ -20,3 +20,59 @@ func TestRegistryNewGauge(t *testing.T) {
 		t.Fatalf("metric with name [%v] should exist in the registry", gauge.name)
 	}
 }
+
+func TestRegistryNewGaugeObserver(t *testing.T) {
+	registry := NewRegistry()
+
+	input := func() float64 {
+		return 1
+	}
+
+	_, err := registry.NewGaugeObserver("test-gauge", input)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err = registry.NewGaugeObserver("test-gauge", input); err == nil {
+		t.Fatalf("should fail when creating gauge with the same name")
+	}
+
+	if _, exists := registry.metrics["test-gauge"]; !exists {
+		t.Fatalf("metric with name [%v] should exist in the registry", "test-gauge")
+	}
+}
+
+func TestRegistryNewInfo(t *testing.T) {
+	registry := NewRegistry()
+
+	if _, err := registry.NewInfo("test-info", []Label{}); err == nil {
+		t.Fatalf("should fail when creating info without labels")
+	}
+
+	label := NewLabel("label", "value")
+	info, err := registry.NewInfo("test-info", []Label{label})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err = registry.NewInfo("test-info", []Label{label}); err == nil {
+		t.Fatalf("should fail when creating info with the same name")
+	}
+
+	if _, exists := registry.metrics[info.name]; !exists {
+		t.Fatalf("metric with name [%v] should exist in the registry", info.name)
+	}
+
+	expectedLabelValue, exists := info.labels[label.name]
+	if !exists {
+		t.Fatalf("label with name [%v] should exist", label.name)
+	}
+
+	if expectedLabelValue != label.value {
+		t.Fatalf(
+			"label with name [%v] should have value [%v]",
+			label.name,
+			label.value,
+		)
+	}
+}