From 3bd111e0e936e61388ae4cf356657c96b8255a20 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Sat, 5 May 2018 16:57:08 -0400 Subject: [PATCH 1/7] silently ignore certain unsupported metric types Instead of logging a warning for the stats types which are known to be incompatible with prometheus (e.g. String, Rates, etc), just silently skip them in the publish phase. At the same time remove the throttled logger and replace with a warning log in case new metric types are created which aren't covered as part of the backend. Signed-off-by: Michael Demmer --- go/stats/prometheusbackend/prometheusbackend.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/go/stats/prometheusbackend/prometheusbackend.go b/go/stats/prometheusbackend/prometheusbackend.go index 00a3b55222b..c73ddad79c4 100644 --- a/go/stats/prometheusbackend/prometheusbackend.go +++ b/go/stats/prometheusbackend/prometheusbackend.go @@ -4,12 +4,11 @@ import ( "expvar" "net/http" "strings" - "time" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" "vitess.io/vitess/go/stats" - "vitess.io/vitess/go/vt/logutil" + "vitess.io/vitess/go/vt/log" ) // PromBackend implements PullBackend using Prometheus as the backing metrics storage. @@ -18,15 +17,13 @@ type PromBackend struct { } var ( - be PromBackend - logUnsupported *logutil.ThrottledLogger + be PromBackend ) // Init initializes the Prometheus be with the given namespace. func Init(namespace string) { http.Handle("/metrics", promhttp.Handler()) be.namespace = namespace - logUnsupported = logutil.NewThrottledLogger("PrometheusUnsupportedMetricType", 1*time.Minute) stats.Register(be.publishPrometheusMetric) } @@ -67,8 +64,11 @@ func (be PromBackend) publishPrometheusMetric(name string, v expvar.Var) { newMultiTimingsCollector(st, be.buildPromName(name)) case *stats.Histogram: newHistogramCollector(st, be.buildPromName(name)) + case *stats.String, stats.StringFunc, stats.StringMapFunc, *stats.Rates: + // silently ignore these types since they don't make sense to + // export to prometheus' data model default: - logUnsupported.Infof("Not exporting to Prometheus an unsupported metric type of %T: %s", st, name) + log.Warningf("Not exporting to Prometheus an unsupported metric type of %T: %s", st, name) } } From 86de973c7de9a64ab6fe6c6573668470bd191feb Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Mon, 7 May 2018 06:32:54 -0700 Subject: [PATCH 2/7] change the unsupported metric type case to log.Fatalf Also minor comment changes as suggested in PR feedback. Signed-off-by: Michael Demmer --- go/stats/prometheusbackend/prometheusbackend.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/go/stats/prometheusbackend/prometheusbackend.go b/go/stats/prometheusbackend/prometheusbackend.go index c73ddad79c4..fe4ed8ef47f 100644 --- a/go/stats/prometheusbackend/prometheusbackend.go +++ b/go/stats/prometheusbackend/prometheusbackend.go @@ -65,10 +65,10 @@ func (be PromBackend) publishPrometheusMetric(name string, v expvar.Var) { case *stats.Histogram: newHistogramCollector(st, be.buildPromName(name)) case *stats.String, stats.StringFunc, stats.StringMapFunc, *stats.Rates: - // silently ignore these types since they don't make sense to - // export to prometheus' data model + // Silently ignore these types since they don't make sense to + // export to Prometheus' data model. default: - log.Warningf("Not exporting to Prometheus an unsupported metric type of %T: %s", st, name) + log.Fatalf("prometheus: not exporting unsupported metric type %T: %s", st, name) } } From 4648dedcff9af307fcfb53ff6b4fba565ff5f047 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Wed, 9 May 2018 13:07:16 -0700 Subject: [PATCH 3/7] tweak log message as per PR suggestion Signed-off-by: Michael Demmer --- go/stats/prometheusbackend/prometheusbackend.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/stats/prometheusbackend/prometheusbackend.go b/go/stats/prometheusbackend/prometheusbackend.go index fe4ed8ef47f..0224fbb9c42 100644 --- a/go/stats/prometheusbackend/prometheusbackend.go +++ b/go/stats/prometheusbackend/prometheusbackend.go @@ -68,7 +68,7 @@ func (be PromBackend) publishPrometheusMetric(name string, v expvar.Var) { // Silently ignore these types since they don't make sense to // export to Prometheus' data model. default: - log.Fatalf("prometheus: not exporting unsupported metric type %T: %s", st, name) + log.Fatalf("prometheus: Metric type %T (seen for variable: %s) is not covered by type switch. Add it there and to all other plugins which register a NewVarHook.", st, name) } } From f29adf48daa3f63fc10ee0a4a085f75b6fc20875 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Thu, 26 Jul 2018 15:27:53 -0400 Subject: [PATCH 4/7] remove the unused JSONFunc stat type Signed-off-by: Michael Demmer --- go/stats/export.go | 15 --------------- go/stats/export_test.go | 12 +++++++++--- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/go/stats/export.go b/go/stats/export.go index 6239cf6cb1a..3aac7583b03 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -240,21 +240,6 @@ func (f StringFunc) String() string { return strconv.Quote(f()) } -// JSONFunc is the public type for a single function that returns json directly. -type JSONFunc func() string - -// String is the implementation of expvar.var -func (f JSONFunc) String() string { - return f() -} - -// PublishJSONFunc publishes any function that returns -// a JSON string as a variable. The string is sent to -// expvar as is. -func PublishJSONFunc(name string, f func() string) { - publish(name, JSONFunc(f)) -} - // StringMap is a map of string -> string type StringMap struct { mu sync.Mutex diff --git a/go/stats/export_test.go b/go/stats/export_test.go index 32d71057c40..15acd66e803 100644 --- a/go/stats/export_test.go +++ b/go/stats/export_test.go @@ -129,15 +129,21 @@ func f() string { return "abcd" } +type expvarFunc func() string + +func (f expvarFunc) String() string { + return f() +} + func TestPublishFunc(t *testing.T) { var gotname string - var gotv JSONFunc + var gotv expvarFunc clear() Register(func(name string, v expvar.Var) { gotname = name - gotv = v.(JSONFunc) + gotv = v.(expvarFunc) }) - PublishJSONFunc("Myfunc", f) + publish("Myfunc", expvarFunc(f)) if gotname != "Myfunc" { t.Errorf("want Myfunc, got %s", gotname) } From 03472f56c1103dcc8e8a3289bd9a7db8b77956d6 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Thu, 26 Jul 2018 15:28:48 -0400 Subject: [PATCH 5/7] remove the unused Float stat type Signed-off-by: Michael Demmer --- go/stats/export.go | 55 ++++++-------------- go/stats/export_test.go | 35 ------------- go/stats/influxdbbackend/influxdb_backend.go | 2 - go/stats/opentsdb/opentsdb.go | 2 - 4 files changed, 15 insertions(+), 79 deletions(-) diff --git a/go/stats/export.go b/go/stats/export.go index 3aac7583b03..4a38640c8fd 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -149,46 +149,6 @@ func emitToBackend(emitPeriod *time.Duration) { } } -// Float is expvar.Float+Get+hook -type Float struct { - mu sync.Mutex - f float64 -} - -// NewFloat creates a new Float and exports it. -func NewFloat(name string) *Float { - v := new(Float) - publish(name, v) - return v -} - -// Add adds the provided value to the Float -func (v *Float) Add(delta float64) { - v.mu.Lock() - v.f += delta - v.mu.Unlock() -} - -// Set sets the value -func (v *Float) Set(value float64) { - v.mu.Lock() - v.f = value - v.mu.Unlock() -} - -// Get returns the value -func (v *Float) Get() float64 { - v.mu.Lock() - f := v.f - v.mu.Unlock() - return f -} - -// String is the implementation of expvar.var -func (v *Float) String() string { - return strconv.FormatFloat(v.Get(), 'g', -1, 64) -} - // FloatFunc converts a function that returns // a float64 as an expvar. type FloatFunc func() float64 @@ -240,6 +200,21 @@ func (f StringFunc) String() string { return strconv.Quote(f()) } +// JSONFunc is the public type for a single function that returns json directly. +type JSONFunc func() string + +// String is the implementation of expvar.var +func (f JSONFunc) String() string { + return f() +} + +// PublishJSONFunc publishes any function that returns +// a JSON string as a variable. The string is sent to +// expvar as is. +func PublishJSONFunc(name string, f func() string) { + publish(name, JSONFunc(f)) +} + // StringMap is a map of string -> string type StringMap struct { mu sync.Mutex diff --git a/go/stats/export_test.go b/go/stats/export_test.go index 15acd66e803..ae4647919c6 100644 --- a/go/stats/export_test.go +++ b/go/stats/export_test.go @@ -35,41 +35,6 @@ func TestNoHook(t *testing.T) { } } -func TestFloat(t *testing.T) { - var gotname string - var gotv *Float - clear() - Register(func(name string, v expvar.Var) { - gotname = name - gotv = v.(*Float) - }) - v := NewFloat("Float") - if gotname != "Float" { - t.Errorf("want Float, got %s", gotname) - } - if gotv != v { - t.Errorf("want %#v, got %#v", v, gotv) - } - v.Set(5.1) - if v.Get() != 5.1 { - t.Errorf("want 5.1, got %v", v.Get()) - } - v.Add(1.0) - if v.Get() != 6.1 { - t.Errorf("want 6.1, got %v", v.Get()) - } - if v.String() != "6.1" { - t.Errorf("want 6.1, got %v", v.Get()) - } - - f := FloatFunc(func() float64 { - return 1.234 - }) - if f.String() != "1.234" { - t.Errorf("want 1.234, got %v", f.String()) - } -} - func TestString(t *testing.T) { var gotname string var gotv *String diff --git a/go/stats/influxdbbackend/influxdb_backend.go b/go/stats/influxdbbackend/influxdb_backend.go index 193522a8f12..54a4e41a820 100644 --- a/go/stats/influxdbbackend/influxdb_backend.go +++ b/go/stats/influxdbbackend/influxdb_backend.go @@ -94,8 +94,6 @@ func (backend *InfluxDBBackend) PushAll() error { // for arbitrary dict values (as tags). func statToValue(v expvar.Var) interface{} { switch v := v.(type) { - case *stats.Float: - return v.Get() case *stats.Counter: return v.Get() case stats.FloatFunc: diff --git a/go/stats/opentsdb/opentsdb.go b/go/stats/opentsdb/opentsdb.go index ccc8cd0e2b5..9b9c1b8fdc4 100644 --- a/go/stats/opentsdb/opentsdb.go +++ b/go/stats/opentsdb/opentsdb.go @@ -181,8 +181,6 @@ func (dc *dataCollector) addFloat(metric string, val float64, tags map[string]st func (dc *dataCollector) addExpVar(kv expvar.KeyValue) { k := kv.Key switch v := kv.Value.(type) { - case *stats.Float: - dc.addFloat(k, v.Get(), nil) case stats.FloatFunc: dc.addFloat(k, v(), nil) case *stats.Counter: From 67144fcdfc1fc95128ef29245badde12ef1c38fc Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Thu, 26 Jul 2018 15:29:29 -0400 Subject: [PATCH 6/7] remove the unused StringMap stat type Signed-off-by: Michael Demmer --- go/stats/export.go | 35 ----------------------------------- go/stats/export_test.go | 23 ----------------------- 2 files changed, 58 deletions(-) diff --git a/go/stats/export.go b/go/stats/export.go index 4a38640c8fd..954ce3b7446 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -215,41 +215,6 @@ func PublishJSONFunc(name string, f func() string) { publish(name, JSONFunc(f)) } -// StringMap is a map of string -> string -type StringMap struct { - mu sync.Mutex - values map[string]string -} - -// NewStringMap returns a new StringMap -func NewStringMap(name string) *StringMap { - v := &StringMap{values: make(map[string]string)} - publish(name, v) - return v -} - -// Set will set a value (existing or not) -func (v *StringMap) Set(name, value string) { - v.mu.Lock() - v.values[name] = value - v.mu.Unlock() -} - -// Get will return the value, or "" f not set. -func (v *StringMap) Get(name string) string { - v.mu.Lock() - s := v.values[name] - v.mu.Unlock() - return s -} - -// String is the implementation of expvar.Var -func (v *StringMap) String() string { - v.mu.Lock() - defer v.mu.Unlock() - return stringMapToString(v.values) -} - // StringMapFunc is the function equivalent of StringMap type StringMapFunc func() map[string]string diff --git a/go/stats/export_test.go b/go/stats/export_test.go index ae4647919c6..792902a2cab 100644 --- a/go/stats/export_test.go +++ b/go/stats/export_test.go @@ -116,26 +116,3 @@ func TestPublishFunc(t *testing.T) { t.Errorf("want %v, got %#v", f(), gotv()) } } - -func TestStringMap(t *testing.T) { - clear() - c := NewStringMap("stringmap1") - c.Set("c1", "val1") - c.Set("c2", "val2") - c.Set("c2", "val3") - want1 := `{"c1": "val1", "c2": "val3"}` - want2 := `{"c2": "val3", "c1": "val1"}` - if s := c.String(); s != want1 && s != want2 { - t.Errorf("want %s or %s, got %s", want1, want2, s) - } - - f := StringMapFunc(func() map[string]string { - return map[string]string{ - "c1": "val1", - "c2": "val3", - } - }) - if s := f.String(); s != want1 && s != want2 { - t.Errorf("want %s or %s, got %s", want1, want2, s) - } -} From 8b73b9c13b7713faf7ab3f4cae1becf6f261d752 Mon Sep 17 00:00:00 2001 From: Michael Demmer Date: Thu, 26 Jul 2018 15:38:40 -0400 Subject: [PATCH 7/7] add prometheus exporter support for FloatFunc Signed-off-by: Michael Demmer --- go/stats/export.go | 5 +++++ go/stats/prometheusbackend/prometheusbackend.go | 2 ++ go/stats/prometheusbackend/prometheusbackend_test.go | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/go/stats/export.go b/go/stats/export.go index 954ce3b7446..b5e516cf975 100644 --- a/go/stats/export.go +++ b/go/stats/export.go @@ -153,6 +153,11 @@ func emitToBackend(emitPeriod *time.Duration) { // a float64 as an expvar. type FloatFunc func() float64 +// Help returns the help string (undefined currently) +func (f FloatFunc) Help() string { + return "help" +} + // String is the implementation of expvar.var func (f FloatFunc) String() string { return strconv.FormatFloat(f(), 'g', -1, 64) diff --git a/go/stats/prometheusbackend/prometheusbackend.go b/go/stats/prometheusbackend/prometheusbackend.go index 0224fbb9c42..961d88fe01a 100644 --- a/go/stats/prometheusbackend/prometheusbackend.go +++ b/go/stats/prometheusbackend/prometheusbackend.go @@ -38,6 +38,8 @@ func (be PromBackend) publishPrometheusMetric(name string, v expvar.Var) { newMetricFuncCollector(st, be.buildPromName(name), prometheus.GaugeValue, func() float64 { return float64(st.Get()) }) case *stats.GaugeFunc: newMetricFuncCollector(st, be.buildPromName(name), prometheus.GaugeValue, func() float64 { return float64(st.F()) }) + case stats.FloatFunc: + newMetricFuncCollector(st, be.buildPromName(name), prometheus.GaugeValue, func() float64 { return (st)() }) case *stats.CountersWithSingleLabel: newCountersWithSingleLabelCollector(st, be.buildPromName(name), st.Label(), prometheus.CounterValue) case *stats.CountersWithMultiLabels: diff --git a/go/stats/prometheusbackend/prometheusbackend_test.go b/go/stats/prometheusbackend/prometheusbackend_test.go index f85febf31af..0fd906a88d3 100644 --- a/go/stats/prometheusbackend/prometheusbackend_test.go +++ b/go/stats/prometheusbackend/prometheusbackend_test.go @@ -59,6 +59,13 @@ func TestPrometheusGaugeFunc(t *testing.T) { checkHandlerForMetrics(t, name, -3) } +func TestPrometheusFloatFunc(t *testing.T) { + name := "blah_floatfunc" + + stats.Publish(name, stats.FloatFunc(func() float64 { return -4 })) + checkHandlerForMetrics(t, name, -4) +} + func TestPrometheusCounterDuration(t *testing.T) { name := "blah_counterduration"