Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add metric description to factory API #61

Merged
merged 5 commits into from
Dec 2, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions metrics/adapters/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,15 @@ 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()

Expand Down
30 changes: 15 additions & 15 deletions metrics/adapters/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.Scope) 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,
}
Expand Down
56 changes: 43 additions & 13 deletions metrics/adapters/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Scope{
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)
Expand All @@ -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,
})
}
18 changes: 9 additions & 9 deletions metrics/adapters/tagless.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions metrics/expvar/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
36 changes: 29 additions & 7 deletions metrics/expvar/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"time"

"github.com/stretchr/testify/assert"
"github.com/uber/jaeger-lib/metrics"
)

var (
Expand Down Expand Up @@ -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.Scope{
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)
Expand Down
35 changes: 27 additions & 8 deletions metrics/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,41 @@

package metrics

// Scope defines the name and tags map associated with a metric
type Scope struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like NSOptions instead? Scope sounds like we’re introducing another concept

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 Scope) 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
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why multi-line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Autoformatting by vscode - but seems to accept manually putting it back to single line.

func (nullFactory) Namespace(scope Scope) Factory { return NullFactory }
18 changes: 9 additions & 9 deletions metrics/go-kit/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.Scope) 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,
Expand Down
Loading