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

metrics: add support for PCP #363

Merged
merged 10 commits into from
Oct 17, 2016
1 change: 1 addition & 0 deletions metrics/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,5 +55,6 @@
// influx n custom custom custom
// prometheus n native native native
// circonus 1 native native native
// pcp 1 native native native
//
package metrics
132 changes: 132 additions & 0 deletions metrics/pcp/pcp.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package pcp

import (
"github.com/performancecopilot/speed"

"github.com/go-kit/kit/metrics"
)
Copy link
Member

Choose a reason for hiding this comment

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

Please group imports according to the canonical style, that is,

import (
    // stdlib

    // third-party, not Go kit

    // Go kit 
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


// Reporter encapsulates a speed client.
type Reporter struct {
c *speed.PCPClient
}

// NewReporter creates a new Reporter instance.
// The first parameter is the application name and is used to create
// the speed client. Hence it should be a valid speed parameter name and
// should not contain spaces or the path separator for your operating system.
func NewReporter(appname string) (*Reporter, error) {
c, err := speed.NewPCPClient(appname)
if err != nil {
return nil, err
}

return &Reporter{c}, nil
}

// Start starts the underlying speed client so it can start reporting registered
// metrics to your PCP installation.
func (r *Reporter) Start() { r.c.MustStart() }

// Stop stops the underlying speed client so it can stop reporting registered
// metrics to your PCP installation.
func (r *Reporter) Stop() { r.c.MustStop() }

// Counter implements metrics.Counter via a single dimensional speed.Counter.
type Counter struct {
c speed.Counter
}

// NewCounter creates a new Counter.
//
// This requires a name parameter and can optionally take a couple of
// description strings, that are used to create the underlying speed.Counter
// and are reported by PCP.
func (r *Reporter) NewCounter(name string, desc ...string) (*Counter, error) {
c, err := speed.NewPCPCounter(0, name, desc...)
if err != nil {
return nil, err
}

r.c.MustRegister(c)
return &Counter{c}, nil
}

// With is a no-op.
func (c *Counter) With(labelValues ...string) metrics.Counter { return c }

// Add increments Counter.
// speed Counters only take int64, so delta is converted to int64 before
// observation. speed Gauges can take float64 and `Add(float64)`
// is implemented by metrics.Gauge.
func (c *Counter) Add(delta float64) { c.c.Inc(int64(delta)) }

// Gauge implements metrics.Gauge via a single dimensional speed.Gauge.
type Gauge struct {
g speed.Gauge
}

// NewGauge creates a new Gauge.
//
// This requires a name parameter, and again, can take a couple of
// optional description strings.
func (r *Reporter) NewGauge(name string, desc ...string) (*Gauge, error) {
g, err := speed.NewPCPGauge(0, name, desc...)
if err != nil {
return nil, err
}

r.c.MustRegister(g)
return &Gauge{g}, nil
}

// With is a no-op.
func (g *Gauge) With(labelValues ...string) metrics.Gauge { return g }

// Set sets the value of the gauge.
func (g *Gauge) Set(value float64) { g.g.Set(value) }

// Add adds a value to the gauge.
func (g *Gauge) Add(value float64) { g.g.Inc(value) }

// Histogram wraps a speed Histogram.
type Histogram struct {
h speed.Histogram
}

// NewHistogram creates a new Histogram.
// Minimum observeable value is 0.
// Maximum observeable value is 3600000000.
//
// The required parameters are a metric name,
// the minimum and maximum observable values,
// and a metric unit for the units of the observed values.
//
// Optionally, it can also take a couple of description strings.
func (r *Reporter) NewHistogram(name string, min, max int64, unit speed.MetricUnit, desc ...string) (*Histogram, error) {
h, err := speed.NewPCPHistogram(name, min, max, 5, unit, desc...)
if err != nil {
return nil, err
}

r.c.MustRegister(h)
return &Histogram{h}, nil
}

// With is a no-op.
func (h *Histogram) With(labelValues ...string) metrics.Histogram { return h }

// Observe observes a value.
//
// This converts float64 value to int64 before observation, as the Histogram in
// speed is backed using codahale/hdrhistogram, which only observes int64
// values. Additionally, the value is interpreted in the metric unit used to
// construct the histogram.
func (h *Histogram) Observe(value float64) { h.h.MustRecord(int64(value)) }
Copy link
Member

Choose a reason for hiding this comment

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

Simply converting to an int64 is going to be problematic, because it is idiomatic to Observe in a histogram in units of seconds, which typically are well less than 1. How should this be supported, do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The histogram implementation isn't native and simply wraps a PCP metric around codahale/hdrhistogram, which only records int64 (https://godoc.org/github.com/codahale/hdrhistogram#Histogram.RecordValue), which in turn is based on a java version, which itself records only long (http://hdrhistogram.github.io/HdrHistogram/JavaDoc/org/HdrHistogram/AbstractHistogram.html#recordValue(long)).

Raw PCP metrics allow setting a unit (https://github.com/performancecopilot/speed#singletonmetric), and I have shorted it to OneUnit at most places as it represents Count unit and most of the times we just want to count stuff. But I can provide a units parameter in histogram, similar to Timer (https://github.com/performancecopilot/speed#timer) so users can customize the unit to Milliseconds, Microseconds, Nanoseconds and report int64.

The other option is to replace the underlying histogram with another implementation. I like hdrhistogram for its ideas on precision and reduced memory footprint, but am open to ideas.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so the normal tests were taking 1.6s but tests with -race enabled were taking 29s on my local machine and 38s on travis, so did a small profile

screenshot from 2016-09-20 23 03 06

from what I can see, the iterator next method on hdrhistogram seems slow, or maybe I am doing something wrong

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with keeping HDRHistogram. I like the Unit idea, it should be settable at metric construction time. Testing with -race will always be much slower than otherwise; I wouldn't worry about profiling it.


// Mean returns the mean of the values observed so far by the Histogram.
func (h *Histogram) Mean() float64 { return h.h.Mean() }

// Percentile returns a percentile value for the given percentile
// between 0 and 100 for all values observed by the histogram.
func (h *Histogram) Percentile(p float64) int64 { return h.h.Percentile(p) }
72 changes: 72 additions & 0 deletions metrics/pcp/pcp_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package pcp

import (
"testing"

"github.com/performancecopilot/speed"

"github.com/go-kit/kit/metrics/teststat"
)

func TestCounter(t *testing.T) {
r, err := NewReporter("test_counter")
if err != nil {
t.Fatal(err)
}

counter, err := r.NewCounter("speed_counter")
if err != nil {
t.Fatal(err)
}

counter = counter.With("label values", "not supported").(*Counter)

value := func() float64 { f := counter.c.Val(); return float64(f) }
if err := teststat.TestCounter(counter, value); err != nil {
t.Fatal(err)
}
}

func TestGauge(t *testing.T) {
r, err := NewReporter("test_gauge")
if err != nil {
t.Fatal(err)
}

gauge, err := r.NewGauge("speed_gauge")
if err != nil {
t.Fatal(err)
}

gauge = gauge.With("label values", "not supported").(*Gauge)

value := func() float64 { f := gauge.g.Val(); return f }
if err := teststat.TestGauge(gauge, value); err != nil {
t.Fatal(err)
}
}

func TestHistogram(t *testing.T) {
r, err := NewReporter("test_histogram")
if err != nil {
t.Fatal(err)
}

histogram, err := r.NewHistogram("speed_histogram", 0, 3600000000, speed.OneUnit)
if err != nil {
t.Fatal(err)
}

histogram = histogram.With("label values", "not supported").(*Histogram)

quantiles := func() (float64, float64, float64, float64) {
p50 := float64(histogram.Percentile(50))
p90 := float64(histogram.Percentile(90))
p95 := float64(histogram.Percentile(95))
p99 := float64(histogram.Percentile(99))
return p50, p90, p95, p99
}
if err := teststat.TestHistogram(histogram, quantiles, 0.01); err != nil {
t.Fatal(err)
}
}