From 948b0cbb528c09f0595e797ff72992d8a9f46be0 Mon Sep 17 00:00:00 2001 From: rghetia Date: Mon, 1 Apr 2019 10:08:52 -0700 Subject: [PATCH] Refactor gauge api with options. (#1086) * Refactor gauge api with options. * fixed review comments. --- metric/examples_test.go | 5 +- metric/gauge_test.go | 108 ++++++++++++++++++++++------- metric/metricexport/reader_test.go | 5 +- metric/registry.go | 66 ++++++++++++++---- 4 files changed, 145 insertions(+), 39 deletions(-) diff --git a/metric/examples_test.go b/metric/examples_test.go index cc39571ba..c4b2e0aa9 100644 --- a/metric/examples_test.go +++ b/metric/examples_test.go @@ -25,7 +25,10 @@ func ExampleRegistry_AddInt64Gauge() { r := metric.NewRegistry() // TODO: allow exporting from a registry - g, _ := r.AddInt64Gauge("active_request", "Number of active requests, per method.", metricdata.UnitDimensionless, "method") + g, _ := r.AddInt64Gauge("active_request", + metric.WithDescription("Number of active requests, per method."), + metric.WithUnit(metricdata.UnitDimensionless), + metric.WithLabelKeys("method")) http.HandleFunc("/", func(writer http.ResponseWriter, request *http.Request) { e, _ := g.GetEntry(metricdata.NewLabelValue(request.Method)) diff --git a/metric/gauge_test.go b/metric/gauge_test.go index b451f5714..8ed358b1e 100644 --- a/metric/gauge_test.go +++ b/metric/gauge_test.go @@ -26,7 +26,9 @@ import ( func TestGauge(t *testing.T) { r := NewRegistry() - f, _ := r.AddFloat64Gauge("TestGauge", "", "", "k1", "k2") + + f, _ := r.AddFloat64Gauge("TestGauge", + WithLabelKeys("k1", "k2")) e, _ := f.GetEntry(metricdata.LabelValue{}, metricdata.LabelValue{}) e.Set(5) e, _ = f.GetEntry(metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) @@ -81,16 +83,15 @@ func TestGauge(t *testing.T) { } func TestGaugeMetricDescriptor(t *testing.T) { - unit := metricdata.UnitDimensionless r := NewRegistry() - gf, _ := r.AddFloat64Gauge("float64_gauge", "", unit) + gf, _ := r.AddFloat64Gauge("float64_gauge") compareType(gf.g.desc.Type, metricdata.TypeGaugeFloat64, t) - gi, _ := r.AddInt64Gauge("int64_gauge", "", unit) + gi, _ := r.AddInt64Gauge("int64_gauge") compareType(gi.g.desc.Type, metricdata.TypeGaugeInt64, t) - dgf, _ := r.AddFloat64DerivedGauge("derived_float64_gauge", "", unit) + dgf, _ := r.AddFloat64DerivedGauge("derived_float64_gauge") compareType(dgf.g.desc.Type, metricdata.TypeGaugeFloat64, t) - dgi, _ := r.AddInt64DerivedGauge("derived_int64_gauge", "", unit) + dgi, _ := r.AddInt64DerivedGauge("derived_int64_gauge") compareType(dgi.g.desc.Type, metricdata.TypeGaugeInt64, t) } @@ -100,9 +101,68 @@ func compareType(got, want metricdata.Type, t *testing.T) { } } +func TestGaugeMetricOptionDesc(t *testing.T) { + r := NewRegistry() + name := "testOptDesc" + gf, _ := r.AddFloat64Gauge(name, WithDescription("test")) + want := metricdata.Descriptor{ + Name: name, + Description: "test", + Type: metricdata.TypeGaugeFloat64, + } + got := gf.g.desc + if !cmp.Equal(got, want) { + t.Errorf("metric option description: got %v, want %v\n", got, want) + } +} + +func TestGaugeMetricOptionUnit(t *testing.T) { + r := NewRegistry() + name := "testOptUnit" + gf, _ := r.AddFloat64Gauge(name, WithUnit(metricdata.UnitMilliseconds)) + want := metricdata.Descriptor{ + Name: name, + Unit: metricdata.UnitMilliseconds, + Type: metricdata.TypeGaugeFloat64, + } + got := gf.g.desc + if !cmp.Equal(got, want) { + t.Errorf("metric descriptor: got %v, want %v\n", got, want) + } +} + +func TestGaugeMetricOptionLabelKeys(t *testing.T) { + r := NewRegistry() + name := "testOptUnit" + gf, _ := r.AddFloat64Gauge(name, WithLabelKeys("k1", "k3")) + want := metricdata.Descriptor{ + Name: name, + LabelKeys: []string{"k1", "k3"}, + Type: metricdata.TypeGaugeFloat64, + } + got := gf.g.desc + if !cmp.Equal(got, want) { + t.Errorf("metric descriptor: got %v, want %v\n", got, want) + } +} + +func TestGaugeMetricOptionDefault(t *testing.T) { + r := NewRegistry() + name := "testOptUnit" + gf, _ := r.AddFloat64Gauge(name) + want := metricdata.Descriptor{ + Name: name, + Type: metricdata.TypeGaugeFloat64, + } + got := gf.g.desc + if !cmp.Equal(got, want) { + t.Errorf("metric descriptor: got %v, want %v\n", got, want) + } +} + func TestFloat64Entry_Add(t *testing.T) { r := NewRegistry() - g, _ := r.AddFloat64Gauge("g", "", metricdata.UnitDimensionless) + g, _ := r.AddFloat64Gauge("g") e, _ := g.GetEntry() e.Add(0) ms := r.Read() @@ -125,7 +185,7 @@ func TestFloat64Entry_Add(t *testing.T) { func TestFloat64Gauge_Add_NegativeTotals(t *testing.T) { r := NewRegistry() - g, _ := r.AddFloat64Gauge("g", "", metricdata.UnitDimensionless) + g, _ := r.AddFloat64Gauge("g") e, _ := g.GetEntry() e.Add(-1.0) ms := r.Read() @@ -136,7 +196,7 @@ func TestFloat64Gauge_Add_NegativeTotals(t *testing.T) { func TestInt64GaugeEntry_Add(t *testing.T) { r := NewRegistry() - g, _ := r.AddInt64Gauge("g", "", metricdata.UnitDimensionless) + g, _ := r.AddInt64Gauge("g") e, _ := g.GetEntry() e.Add(0) ms := r.Read() @@ -153,7 +213,7 @@ func TestInt64GaugeEntry_Add(t *testing.T) { func TestInt64Gauge_Add_NegativeTotals(t *testing.T) { r := NewRegistry() - g, _ := r.AddInt64Gauge("g", "", metricdata.UnitDimensionless) + g, _ := r.AddInt64Gauge("g") e, _ := g.GetEntry() e.Add(-1) ms := r.Read() @@ -164,16 +224,16 @@ func TestInt64Gauge_Add_NegativeTotals(t *testing.T) { func TestGaugeWithSameNameDiffType(t *testing.T) { r := NewRegistry() - r.AddInt64Gauge("g", "", metricdata.UnitDimensionless) - _, gotErr := r.AddFloat64Gauge("g", "", metricdata.UnitDimensionless) + r.AddInt64Gauge("g") + _, gotErr := r.AddFloat64Gauge("g") if gotErr == nil { t.Errorf("got: nil, want error: %v", errGaugeExistsWithDiffType) } - _, gotErr = r.AddInt64DerivedGauge("g", "", metricdata.UnitDimensionless) + _, gotErr = r.AddInt64DerivedGauge("g") if gotErr == nil { t.Errorf("got: nil, want error: %v", errGaugeExistsWithDiffType) } - _, gotErr = r.AddFloat64DerivedGauge("g", "", metricdata.UnitDimensionless) + _, gotErr = r.AddFloat64DerivedGauge("g") if gotErr == nil { t.Errorf("got: nil, want error: %v", errGaugeExistsWithDiffType) } @@ -181,7 +241,7 @@ func TestGaugeWithSameNameDiffType(t *testing.T) { func TestGaugeWithLabelMismatch(t *testing.T) { r := NewRegistry() - g, _ := r.AddInt64Gauge("g", "", metricdata.UnitDimensionless, "k1") + g, _ := r.AddInt64Gauge("g", WithLabelKeys("k1")) _, gotErr := g.GetEntry(metricdata.NewLabelValue("k1v2"), metricdata.NewLabelValue("k2v2")) if gotErr == nil { t.Errorf("got: nil, want error: %v", errKeyValueMismatch) @@ -222,7 +282,7 @@ func TestRaceCondition(t *testing.T) { for i := 0; i < 5; i++ { go func(k int) { for j := 0; j < 5; j++ { - g, _ := r.AddInt64Gauge(fmt.Sprintf("g%d%d", k, j), "", metricdata.UnitDimensionless) + g, _ := r.AddInt64Gauge(fmt.Sprintf("g%d%d", k, j)) e, _ := g.GetEntry() e.Add(1) } @@ -272,7 +332,7 @@ func (q *queueInt64) ToInt64() int64 { func TestInt64DerivedGaugeEntry_Add(t *testing.T) { r := NewRegistry() q := &queueInt64{3} - g, _ := r.AddInt64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddInt64DerivedGauge("g", WithLabelKeys("k1", "k2")) err := g.UpsertEntry(q.ToInt64, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) if err != nil { t.Errorf("want: nil, got: %v", err) @@ -290,7 +350,7 @@ func TestInt64DerivedGaugeEntry_Add(t *testing.T) { func TestInt64DerivedGaugeEntry_AddWithNilObj(t *testing.T) { r := NewRegistry() - g, _ := r.AddInt64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddInt64DerivedGauge("g", WithLabelKeys("k1", "k2")) gotErr := g.UpsertEntry(nil, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) if gotErr == nil { t.Errorf("expected error but got nil") @@ -300,7 +360,7 @@ func TestInt64DerivedGaugeEntry_AddWithNilObj(t *testing.T) { func TestInt64DerivedGaugeEntry_AddWithInvalidLabels(t *testing.T) { r := NewRegistry() q := &queueInt64{3} - g, _ := r.AddInt64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddInt64DerivedGauge("g", WithLabelKeys("k1", "k2")) gotErr := g.UpsertEntry(q.ToInt64, metricdata.NewLabelValue("k1v1")) if gotErr == nil { t.Errorf("expected error but got nil") @@ -311,7 +371,7 @@ func TestInt64DerivedGaugeEntry_Update(t *testing.T) { r := NewRegistry() q := &queueInt64{3} q2 := &queueInt64{5} - g, _ := r.AddInt64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddInt64DerivedGauge("g", WithLabelKeys("k1", "k2")) g.UpsertEntry(q.ToInt64, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) gotErr := g.UpsertEntry(q2.ToInt64, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) if gotErr != nil { @@ -334,7 +394,7 @@ func (q *queueFloat64) ToFloat64() float64 { func TestFloat64DerivedGaugeEntry_Add(t *testing.T) { r := NewRegistry() q := &queueFloat64{5.0} - g, _ := r.AddFloat64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddFloat64DerivedGauge("g", WithLabelKeys("k1", "k2")) err := g.UpsertEntry(q.ToFloat64, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) if err != nil { t.Errorf("want: nil, got: %v", err) @@ -352,7 +412,7 @@ func TestFloat64DerivedGaugeEntry_Add(t *testing.T) { func TestFloat64DerivedGaugeEntry_AddWithNilObj(t *testing.T) { r := NewRegistry() - g, _ := r.AddFloat64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddFloat64DerivedGauge("g", WithLabelKeys("k1", "k2")) gotErr := g.UpsertEntry(nil, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) if gotErr == nil { t.Errorf("expected error but got nil") @@ -362,7 +422,7 @@ func TestFloat64DerivedGaugeEntry_AddWithNilObj(t *testing.T) { func TestFloat64DerivedGaugeEntry_AddWithInvalidLabels(t *testing.T) { r := NewRegistry() q := &queueFloat64{3} - g, _ := r.AddFloat64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddFloat64DerivedGauge("g", WithLabelKeys("k1", "k2")) gotErr := g.UpsertEntry(q.ToFloat64, metricdata.NewLabelValue("k1v1")) if gotErr == nil { t.Errorf("expected error but got nil") @@ -373,7 +433,7 @@ func TestFloat64DerivedGaugeEntry_Update(t *testing.T) { r := NewRegistry() q := &queueFloat64{3.0} q2 := &queueFloat64{5.0} - g, _ := r.AddFloat64DerivedGauge("g", "", metricdata.UnitDimensionless, "k1", "k2") + g, _ := r.AddFloat64DerivedGauge("g", WithLabelKeys("k1", "k2")) g.UpsertEntry(q.ToFloat64, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) gotErr := g.UpsertEntry(q2.ToFloat64, metricdata.NewLabelValue("k1v1"), metricdata.LabelValue{}) if gotErr != nil { diff --git a/metric/metricexport/reader_test.go b/metric/metricexport/reader_test.go index a043530e8..756792486 100644 --- a/metric/metricexport/reader_test.go +++ b/metric/metricexport/reader_test.go @@ -53,7 +53,10 @@ func (e *metricExporter) ExportMetrics(ctx context.Context, metrics []*metricdat func init() { r := metric.NewRegistry() metricproducer.GlobalManager().AddProducer(r) - g, _ := r.AddInt64Gauge("active_request", "Number of active requests, per method.", metricdata.UnitDimensionless, "method") + g, _ := r.AddInt64Gauge("active_request", + metric.WithDescription("Number of active requests, per method."), + metric.WithUnit(metricdata.UnitDimensionless), + metric.WithLabelKeys("method")) gaugeEntry, _ = g.GetEntry(metricdata.NewLabelValue("foo")) } diff --git a/metric/registry.go b/metric/registry.go index 6181f45f3..80df54229 100644 --- a/metric/registry.go +++ b/metric/registry.go @@ -37,19 +37,50 @@ const ( derivedGaugeFloat64 ) +//TODO: [rghetia] add constant labels. +type metricOptions struct { + unit metricdata.Unit + labelkeys []string + desc string +} + +// Options apply changes to metricOptions. +type Options func(*metricOptions) + +// WithDescription applies provided description. +func WithDescription(desc string) Options { + return func(mo *metricOptions) { + mo.desc = desc + } +} + +// WithUnit applies provided unit. +func WithUnit(unit metricdata.Unit) Options { + return func(mo *metricOptions) { + mo.unit = unit + } +} + +// WithLabelKeys applies provided label. +func WithLabelKeys(labelKeys ...string) Options { + return func(mo *metricOptions) { + mo.labelkeys = labelKeys + } +} + // NewRegistry initializes a new Registry. func NewRegistry() *Registry { return &Registry{} } // AddFloat64Gauge creates and adds a new float64-valued gauge to this registry. -func (r *Registry) AddFloat64Gauge(name, description string, unit metricdata.Unit, labelKeys ...string) (*Float64Gauge, error) { +func (r *Registry) AddFloat64Gauge(name string, mos ...Options) (*Float64Gauge, error) { f := &Float64Gauge{ g: gauge{ gType: gaugeFloat64, }, } - _, err := r.initGauge(&f.g, labelKeys, name, description, unit) + _, err := r.initGauge(&f.g, name, mos...) if err != nil { return nil, err } @@ -57,13 +88,13 @@ func (r *Registry) AddFloat64Gauge(name, description string, unit metricdata.Uni } // AddInt64Gauge creates and adds a new int64-valued gauge to this registry. -func (r *Registry) AddInt64Gauge(name, description string, unit metricdata.Unit, labelKeys ...string) (*Int64Gauge, error) { +func (r *Registry) AddInt64Gauge(name string, mos ...Options) (*Int64Gauge, error) { i := &Int64Gauge{ g: gauge{ gType: gaugeInt64, }, } - _, err := r.initGauge(&i.g, labelKeys, name, description, unit) + _, err := r.initGauge(&i.g, name, mos...) if err != nil { return nil, err } @@ -73,13 +104,13 @@ func (r *Registry) AddInt64Gauge(name, description string, unit metricdata.Unit, // AddInt64DerivedGauge creates and adds a new derived int64-valued gauge to this registry. // A derived gauge is convenient form of gauge where the object associated with the gauge // provides its value by implementing func() int64. -func (r *Registry) AddInt64DerivedGauge(name, description string, unit metricdata.Unit, labelKeys ...string) (*Int64DerivedGauge, error) { +func (r *Registry) AddInt64DerivedGauge(name string, mos ...Options) (*Int64DerivedGauge, error) { i := &Int64DerivedGauge{ g: gauge{ gType: derivedGaugeInt64, }, } - _, err := r.initGauge(&i.g, labelKeys, name, description, unit) + _, err := r.initGauge(&i.g, name, mos...) if err != nil { return nil, err } @@ -89,13 +120,13 @@ func (r *Registry) AddInt64DerivedGauge(name, description string, unit metricdat // AddFloat64DerivedGauge creates and adds a new derived float64-valued gauge to this registry. // A derived gauge is convenient form of gauge where the object associated with the gauge // provides its value by implementing func() float64. -func (r *Registry) AddFloat64DerivedGauge(name, description string, unit metricdata.Unit, labelKeys ...string) (*Float64DerivedGauge, error) { +func (r *Registry) AddFloat64DerivedGauge(name string, mos ...Options) (*Float64DerivedGauge, error) { f := &Float64DerivedGauge{ g: gauge{ gType: derivedGaugeFloat64, }, } - _, err := r.initGauge(&f.g, labelKeys, name, description, unit) + _, err := r.initGauge(&f.g, name, mos...) if err != nil { return nil, err } @@ -117,7 +148,15 @@ func gTypeToMetricType(g *gauge) metricdata.Type { } } -func (r *Registry) initGauge(g *gauge, labelKeys []string, name string, description string, unit metricdata.Unit) (*gauge, error) { +func createMetricOption(mos ...Options) *metricOptions { + o := &metricOptions{} + for _, mo := range mos { + mo(o) + } + return o +} + +func (r *Registry) initGauge(g *gauge, name string, mos ...Options) (*gauge, error) { val, ok := r.gauges.Load(name) if ok { existing := val.(*gauge) @@ -125,13 +164,14 @@ func (r *Registry) initGauge(g *gauge, labelKeys []string, name string, descript return nil, errGaugeExistsWithDiffType } } - g.keys = labelKeys g.start = time.Now() + o := createMetricOption(mos...) + g.keys = o.labelkeys g.desc = metricdata.Descriptor{ Name: name, - Description: description, - Unit: unit, - LabelKeys: labelKeys, + Description: o.desc, + Unit: o.unit, + LabelKeys: o.labelkeys, Type: gTypeToMetricType(g), } r.gauges.Store(name, g)