Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
80 changes: 5 additions & 75 deletions go/stats/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,50 +149,15 @@ 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

// 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)
Expand Down Expand Up @@ -255,41 +220,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

Expand Down
70 changes: 9 additions & 61 deletions go/stats/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -129,42 +94,25 @@ 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)
}
if gotv.String() != f() {
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)
}
}
2 changes: 0 additions & 2 deletions go/stats/influxdbbackend/influxdb_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 0 additions & 2 deletions go/stats/opentsdb/opentsdb.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
14 changes: 8 additions & 6 deletions go/stats/prometheusbackend/prometheusbackend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}

Expand All @@ -41,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:
Expand All @@ -67,8 +66,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:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing a few:

JSONFunc, Float, FloatFunc, StringMap

FWIW, I just looked up everything that implemented String() inside the stats package, which is what's required for the expvar interface.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh good catch! I only checked for types that were actually used by the vitess code today.

@michael-berlin Since these types aren't used anywhere my instinct is that we should remove them from stats rather than either add them to this list or run the risk that someone starts using them in the future and then it will panic for anyone using prometheus.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmation that the following types are not used internally:

  • JSONFunc
  • Float
  • StringMap

Types which must stay:

  • FloatFunc

// 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.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)
}
}

Expand Down
7 changes: 7 additions & 0 deletions go/stats/prometheusbackend/prometheusbackend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down