Add metric description to factory API (#61)

* Add metric description to factory API Signed-off-by: Gary Brown <gary@brownuk.com> * Change to use struct instead of separate parameters Signed-off-by: Gary Brown <gary@brownuk.com> * Rename the types to Options and Scope, and don't embed the Scope inside the Options. Signed-off-by: Gary Brown <gary@brownuk.com> * Rename description to help Signed-off-by: Gary Brown <gary@brownuk.com> * Fix multiline and change Scope to NSOptions Signed-off-by: Gary Brown <gary@brownuk.com>
jaegertracing · Dec 2, 2018 · 3fa9aa0 · 3fa9aa0
1 parent 1fc5c31
commit 3fa9aa0
Show file tree

Hide file tree

Showing 20 changed files with 458 additions and 168 deletions.
diff --git a/metrics/adapters/cache_test.go b/metrics/adapters/cache_test.go
@@ -26,9 +26,9 @@ import (
 
 func TestCache(t *testing.T) {
 	f := metricstest.NewFactory(100 * time.Second)
-	c1 := f.Counter("x", nil)
-	g1 := f.Gauge("y", nil)
-	t1 := f.Timer("z", nil)
+	c1 := f.Counter(metrics.Options{Name: "x"})
+	g1 := f.Gauge(metrics.Options{Name: "y"})
+	t1 := f.Timer(metrics.Options{Name: "z"})
 
 	c := newCache()
 

diff --git a/metrics/adapters/factory.go b/metrics/adapters/factory.go
@@ -20,9 +20,9 @@ import (
 
 // FactoryWithTags creates metrics with fully qualified name and tags.
 type FactoryWithTags interface {
-	Counter(name string, tags map[string]string) metrics.Counter
-	Gauge(name string, tags map[string]string) metrics.Gauge
-	Timer(name string, tags map[string]string) metrics.Timer
+	Counter(name string, tags map[string]string, help string) metrics.Counter
+	Gauge(name string, tags map[string]string, help string) metrics.Gauge
+	Timer(name string, tags map[string]string, help string) metrics.Timer
 }
 
 // Options affect how the adapter factory behaves.
@@ -63,32 +63,32 @@ type factory struct {
 	cache   *cache
 }
 
-func (f *factory) Counter(name string, tags map[string]string) metrics.Counter {
-	fullName, fullTags, key := f.getKey(name, tags)
+func (f *factory) Counter(options metrics.Options) metrics.Counter {
+	fullName, fullTags, key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetCounter(key, func() metrics.Counter {
-		return f.factory.Counter(fullName, fullTags)
+		return f.factory.Counter(fullName, fullTags, options.Help)
 	})
 }
 
-func (f *factory) Gauge(name string, tags map[string]string) metrics.Gauge {
-	fullName, fullTags, key := f.getKey(name, tags)
+func (f *factory) Gauge(options metrics.Options) metrics.Gauge {
+	fullName, fullTags, key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetGauge(key, func() metrics.Gauge {
-		return f.factory.Gauge(fullName, fullTags)
+		return f.factory.Gauge(fullName, fullTags, options.Help)
 	})
 }
 
-func (f *factory) Timer(name string, tags map[string]string) metrics.Timer {
-	fullName, fullTags, key := f.getKey(name, tags)
+func (f *factory) Timer(options metrics.Options) metrics.Timer {
+	fullName, fullTags, key := f.getKey(options.Name, options.Tags)
 	return f.cache.getOrSetTimer(key, func() metrics.Timer {
-		return f.factory.Timer(fullName, fullTags)
+		return f.factory.Timer(fullName, fullTags, options.Help)
 	})
 }
 
-func (f *factory) Namespace(name string, tags map[string]string) metrics.Factory {
+func (f *factory) Namespace(scope metrics.NSOptions) metrics.Factory {
 	return &factory{
 		cache:   f.cache,
-		scope:   f.subScope(name),
-		tags:    f.mergeTags(tags),
+		scope:   f.subScope(scope.Name),
+		tags:    f.mergeTags(scope.Tags),
 		factory: f.factory,
 		Options: f.Options,
 	}

diff --git a/metrics/adapters/factory_test.go b/metrics/adapters/factory_test.go
@@ -80,15 +80,36 @@ func TestFactory(t *testing.T) {
 			ff := &fakeTagless{factory: local}
 			f := WrapFactoryWithoutTags(ff, Options{})
 			if testCase.namespace != "" || testCase.nsTags != nil {
-				f = f.Namespace(testCase.namespace, testCase.nsTags)
+				f = f.Namespace(metrics.NSOptions{
+					Name: testCase.namespace,
+					Tags: testCase.nsTags,
+				})
 			}
-			counter := f.Counter(counterPrefix+testCase.name, testCase.tags)
-			gauge := f.Gauge(gaugePrefix+testCase.name, testCase.tags)
-			timer := f.Timer(timerPrefix+testCase.name, testCase.tags)
+			counter := f.Counter(metrics.Options{
+				Name: counterPrefix + testCase.name,
+				Tags: testCase.tags,
+			})
+			gauge := f.Gauge(metrics.Options{
+				Name: gaugePrefix + testCase.name,
+				Tags: testCase.tags,
+			})
+			timer := f.Timer(metrics.Options{
+				Name: timerPrefix + testCase.name,
+				Tags: testCase.tags,
+			})
 
-			assert.Equal(t, counter, f.Counter(counterPrefix+testCase.name, testCase.tags))
-			assert.Equal(t, gauge, f.Gauge(gaugePrefix+testCase.name, testCase.tags))
-			assert.Equal(t, timer, f.Timer(timerPrefix+testCase.name, testCase.tags))
+			assert.Equal(t, counter, f.Counter(metrics.Options{
+				Name: counterPrefix + testCase.name,
+				Tags: testCase.tags,
+			}))
+			assert.Equal(t, gauge, f.Gauge(metrics.Options{
+				Name: gaugePrefix + testCase.name,
+				Tags: testCase.tags,
+			}))
+			assert.Equal(t, timer, f.Timer(metrics.Options{
+				Name: timerPrefix + testCase.name,
+				Tags: testCase.tags,
+			}))
 
 			assert.Equal(t, fmt.Sprintf(testCase.fullName, counterPrefix), ff.counter)
 			assert.Equal(t, fmt.Sprintf(testCase.fullName, gaugePrefix), ff.gauge)
@@ -104,17 +125,26 @@ type fakeTagless struct {
 	timer   string
 }
 
-func (f *fakeTagless) Counter(name string) metrics.Counter {
+func (f *fakeTagless) Counter(name string, help string) metrics.Counter {
 	f.counter = name
-	return f.factory.Counter(name, nil)
+	return f.factory.Counter(metrics.Options{
+		Name: name,
+		Help: help,
+	})
 }
 
-func (f *fakeTagless) Gauge(name string) metrics.Gauge {
+func (f *fakeTagless) Gauge(name string, help string) metrics.Gauge {
 	f.gauge = name
-	return f.factory.Gauge(name, nil)
+	return f.factory.Gauge(metrics.Options{
+		Name: name,
+		Help: help,
+	})
 }
 
-func (f *fakeTagless) Timer(name string) metrics.Timer {
+func (f *fakeTagless) Timer(name string, help string) metrics.Timer {
 	f.timer = name
-	return f.factory.Timer(name, nil)
+	return f.factory.Timer(metrics.Options{
+		Name: name,
+		Help: help,
+	})
 }
diff --git a/metrics/adapters/tagless.go b/metrics/adapters/tagless.go
@@ -19,9 +19,9 @@ import "github.com/uber/jaeger-lib/metrics"
 // FactoryWithoutTags creates metrics based on name only, without tags.
 // Suitable for integrating with statsd-like backends that don't support tags.
 type FactoryWithoutTags interface {
-	Counter(name string) metrics.Counter
-	Gauge(name string) metrics.Gauge
-	Timer(name string) metrics.Timer
+	Counter(name string, help string) metrics.Counter
+	Gauge(name string, help string) metrics.Gauge
+	Timer(name string, help string) metrics.Timer
 }
 
 // WrapFactoryWithoutTags creates a real metrics.Factory that supports subscopes.
@@ -41,19 +41,19 @@ type tagless struct {
 	factory FactoryWithoutTags
 }
 
-func (f *tagless) Counter(name string, tags map[string]string) metrics.Counter {
+func (f *tagless) Counter(name string, tags map[string]string, help string) metrics.Counter {
 	fullName := f.getFullName(name, tags)
-	return f.factory.Counter(fullName)
+	return f.factory.Counter(fullName, help)
 }
 
-func (f *tagless) Gauge(name string, tags map[string]string) metrics.Gauge {
+func (f *tagless) Gauge(name string, tags map[string]string, help string) metrics.Gauge {
 	fullName := f.getFullName(name, tags)
-	return f.factory.Gauge(fullName)
+	return f.factory.Gauge(fullName, help)
 }
 
-func (f *tagless) Timer(name string, tags map[string]string) metrics.Timer {
+func (f *tagless) Timer(name string, tags map[string]string, help string) metrics.Timer {
 	fullName := f.getFullName(name, tags)
-	return f.factory.Timer(fullName)
+	return f.factory.Timer(fullName, help)
 }
 
 func (f *tagless) getFullName(name string, tags map[string]string) string {

diff --git a/metrics/expvar/factory.go b/metrics/expvar/factory.go
@@ -36,14 +36,14 @@ type factory struct {
 	factory xkit.Factory
 }
 
-func (f *factory) Counter(name string) metrics.Counter {
+func (f *factory) Counter(name string, help string) metrics.Counter {
 	return xkit.NewCounter(f.factory.Counter(name))
 }
 
-func (f *factory) Gauge(name string) metrics.Gauge {
+func (f *factory) Gauge(name string, help string) metrics.Gauge {
 	return xkit.NewGauge(f.factory.Gauge(name))
 }
 
-func (f *factory) Timer(name string) metrics.Timer {
+func (f *factory) Timer(name string, help string) metrics.Timer {
 	return xkit.NewTimer(f.factory.Histogram(name))
 }
diff --git a/metrics/expvar/factory_test.go b/metrics/expvar/factory_test.go
@@ -22,6 +22,7 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/uber/jaeger-lib/metrics"
 )
 
 var (
@@ -63,16 +64,37 @@ func TestFactory(t *testing.T) {
 			}
 			ff := f
 			if testCase.namespace != "" || testCase.nsTags != nil {
-				ff = f.Namespace(testCase.namespace, testCase.nsTags)
+				ff = f.Namespace(metrics.NSOptions{
+					Name: testCase.namespace,
+					Tags: testCase.nsTags,
+				})
 			}
-			counter := ff.Counter(counterPrefix+testCase.name, testCase.tags)
-			gauge := ff.Gauge(gaugePrefix+testCase.name, testCase.tags)
-			timer := ff.Timer(timerPrefix+testCase.name, testCase.tags)
+			counter := ff.Counter(metrics.Options{
+				Name: counterPrefix + testCase.name,
+				Tags: testCase.tags,
+			})
+			gauge := ff.Gauge(metrics.Options{
+				Name: gaugePrefix + testCase.name,
+				Tags: testCase.tags,
+			})
+			timer := ff.Timer(metrics.Options{
+				Name: timerPrefix + testCase.name,
+				Tags: testCase.tags,
+			})
 
 			// register second time, should not panic
-			ff.Counter(counterPrefix+testCase.name, testCase.tags)
-			ff.Gauge(gaugePrefix+testCase.name, testCase.tags)
-			ff.Timer(timerPrefix+testCase.name, testCase.tags)
+			ff.Counter(metrics.Options{
+				Name: counterPrefix + testCase.name,
+				Tags: testCase.tags,
+			})
+			ff.Gauge(metrics.Options{
+				Name: gaugePrefix + testCase.name,
+				Tags: testCase.tags,
+			})
+			ff.Timer(metrics.Options{
+				Name: timerPrefix + testCase.name,
+				Tags: testCase.tags,
+			})
 
 			counter.Inc(42)
 			gauge.Update(42)

diff --git a/metrics/factory.go b/metrics/factory.go
@@ -14,22 +14,41 @@
 
 package metrics
 
+// NSOptions defines the name and tags map associated with a metric
+type NSOptions struct {
+	Name string
+	Tags map[string]string
+}
+
+// Options defines the information associated with a metric
+type Options struct {
+	Name string
+	Tags map[string]string
+	Help string
+}
+
 // Factory creates new metrics
 type Factory interface {
-	Counter(name string, tags map[string]string) Counter
-	Timer(name string, tags map[string]string) Timer
-	Gauge(name string, tags map[string]string) Gauge
+	Counter(metric Options) Counter
+	Timer(metric Options) Timer
+	Gauge(metric Options) Gauge
 
 	// Namespace returns a nested metrics factory.
-	Namespace(name string, tags map[string]string) Factory
+	Namespace(scope NSOptions) Factory
 }
 
 // NullFactory is a metrics factory that returns NullCounter, NullTimer, and NullGauge.
 var NullFactory Factory = nullFactory{}
 
 type nullFactory struct{}
 
-func (nullFactory) Counter(name string, tags map[string]string) Counter   { return NullCounter }
-func (nullFactory) Timer(name string, tags map[string]string) Timer       { return NullTimer }
-func (nullFactory) Gauge(name string, tags map[string]string) Gauge       { return NullGauge }
-func (nullFactory) Namespace(name string, tags map[string]string) Factory { return NullFactory }
+func (nullFactory) Counter(options Options) Counter {
+	return NullCounter
+}
+func (nullFactory) Timer(options Options) Timer {
+	return NullTimer
+}
+func (nullFactory) Gauge(options Options) Gauge {
+	return NullGauge
+}
+func (nullFactory) Namespace(scope NSOptions) Factory { return NullFactory }
diff --git a/metrics/go-kit/factory.go b/metrics/go-kit/factory.go
@@ -103,37 +103,37 @@ func (f *factory) nameAndTagsList(nom string, tags map[string]string) (name stri
 	return
 }
 
-func (f *factory) Counter(name string, tags map[string]string) metrics.Counter {
-	name, tagsList := f.nameAndTagsList(name, tags)
+func (f *factory) Counter(options metrics.Options) metrics.Counter {
+	name, tagsList := f.nameAndTagsList(options.Name, options.Tags)
 	counter := f.factory.Counter(name)
 	if len(tagsList) > 0 {
 		counter = counter.With(tagsList...)
 	}
 	return NewCounter(counter)
 }
 
-func (f *factory) Timer(name string, tags map[string]string) metrics.Timer {
-	name, tagsList := f.nameAndTagsList(name, tags)
+func (f *factory) Timer(options metrics.Options) metrics.Timer {
+	name, tagsList := f.nameAndTagsList(options.Name, options.Tags)
 	hist := f.factory.Histogram(name)
 	if len(tagsList) > 0 {
 		hist = hist.With(tagsList...)
 	}
 	return NewTimer(hist)
 }
 
-func (f *factory) Gauge(name string, tags map[string]string) metrics.Gauge {
-	name, tagsList := f.nameAndTagsList(name, tags)
+func (f *factory) Gauge(options metrics.Options) metrics.Gauge {
+	name, tagsList := f.nameAndTagsList(options.Name, options.Tags)
 	gauge := f.factory.Gauge(name)
 	if len(tagsList) > 0 {
 		gauge = gauge.With(tagsList...)
 	}
 	return NewGauge(gauge)
 }
 
-func (f *factory) Namespace(name string, tags map[string]string) metrics.Factory {
+func (f *factory) Namespace(scope metrics.NSOptions) metrics.Factory {
 	return &factory{
-		scope:    f.subScope(name),
-		tags:     f.mergeTags(tags),
+		scope:    f.subScope(scope.Name),
+		tags:     f.mergeTags(scope.Tags),
 		factory:  f.factory,
 		scopeSep: f.scopeSep,
 		tagsSep:  f.tagsSep,