Create runtime.Config struct with metric.Provider and WithMinimumReadMemStatsInterval() configuration options (#224)

* Pass metric.Provider to runtime.Start; make interval param an Option

* Add more tests

* Address feedback

* More comments

* More comments

* Apply instrumentation version

Co-authored-by: Tyler Yahn <[email protected]>

Author: jmacd

instrumentation/runtime/doc.go

@@ -29,4 +29,4 @@
 //   runtime.go.mem.heap_sys      (bytes)    Bytes of heap memory obtained from the OS
 //   runtime.go.mem.live_objects  -          Number of live objects is the number of cumulative Mallocs - Frees
 //   runtime.uptime               (ms)       Milliseconds since application was initialized
-package runtime
+package runtime // import "go.opentelemetry.io/contrib/instrumentation/runtime"

instrumentation/runtime/example/main.go

@@ -21,7 +21,6 @@ import (
 	"syscall"
 	"time"
 
-	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/exporters/stdout"
 	"go.opentelemetry.io/otel/sdk/metric/controller/push"
 
@@ -42,9 +41,11 @@ func initMeter() *push.Controller {
 func main() {
 	defer initMeter().Stop()
 
-	meter := global.Meter("runtime")
-
-	if err := runtime.Start(meter, time.Second); err != nil {
+	if err := runtime.Start(
+		runtime.Configure(
+			runtime.WithMinimumReadMemStatsInterval(time.Second),
+		),
+	); err != nil {
 		panic(err)
 	}
 

instrumentation/runtime/go.mod

@@ -6,6 +6,7 @@ replace go.opentelemetry.io/contrib => ../..
 
 require (
 	github.com/stretchr/testify v1.6.1
+	go.opentelemetry.io/contrib v0.10.1
 	go.opentelemetry.io/otel v0.10.0
 	go.opentelemetry.io/otel/exporters/stdout v0.10.0
 	go.opentelemetry.io/otel/sdk v0.10.0

instrumentation/runtime/go.sum

@@ -90,6 +90,8 @@ google.golang.org/grpc v1.23.0/go.mod h1:Y5yQAOtifL1yxbo5wqy6BxZv8vAUGQwXBOALyac
 google.golang.org/grpc v1.25.1/go.mod h1:c3i+UQWmh7LiEpx4sFZnkU36qjEYZ0imhYfXVyQciAY=
 google.golang.org/grpc v1.30.0 h1:M5a8xTlYTxwMn5ZFkwhRabsygDY5G8TYLyQDBxJNAxE=
 google.golang.org/grpc v1.30.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak=
+google.golang.org/grpc v1.31.0 h1:T7P4R73V3SSDPhH7WW7ATbfViLtmamH0DKrP3f9AuDI=
+google.golang.org/grpc v1.31.0/go.mod h1:N36X2cJ7JwdamYAgDz+s+rVMFjt3numwzf/HckM8pak=
 google.golang.org/protobuf v0.0.0-20200109180630-ec00e32a8dfd/go.mod h1:DFci5gLYBciE7Vtevhsrf46CRTquxDuWsQurQQe4oz8=
 google.golang.org/protobuf v0.0.0-20200221191635-4d8936d0db64/go.mod h1:kwYJMbMJ01Woi6D6+Kah6886xMZcty6N08ah7+eCXa0=
 google.golang.org/protobuf v0.0.0-20200228230310-ab0ca4ff8a60/go.mod h1:cfTl7dwQJ+fmap5saPgwCLgHXTUD7jkjRqWcaiX5VyM=

instrumentation/runtime/runtime.go

@@ -20,24 +20,100 @@ import (
 	"sync"
 	"time"
 
+	"go.opentelemetry.io/contrib"
+	"go.opentelemetry.io/otel/api/global"
 	"go.opentelemetry.io/otel/api/metric"
 	"go.opentelemetry.io/otel/api/unit"
 )
 
 // Runtime reports the work-in-progress conventional runtime metrics specified by OpenTelemetry
 type runtime struct {
-	meter    metric.Meter
-	interval time.Duration
+	config Config
+	meter  metric.Meter
 }
 
-// New returns Runtime, a structure for reporting Go runtime metrics
-// interval is used to limit how often to invoke Go runtime.ReadMemStats() to obtain metric data.
-// If the metric SDK attempts to observe MemStats-derived instruments more frequently than the
-// interval, a cached value will be used.
-func Start(meter metric.Meter, interval time.Duration) error {
+// Config contains optional settings for reporting runtime metrics.
+type Config struct {
+	// MinimumReadMemStatsInterval sets the mininum interval
+	// between calls to runtime.ReadMemStats().  Negative values
+	// are ignored.
+	MinimumReadMemStatsInterval time.Duration
+
+	// MeterProvider sets the metric.Provider.  If nil, the global
+	// Provider will be used.
+	MeterProvider metric.Provider
+}
+
+// Option supports configuring optional settings for runtime metrics.
+type Option interface {
+	// ApplyRuntime updates *Config.
+	ApplyRuntime(*Config)
+}
+
+// DefaultMinimumReadMemStatsInterval is the default minimum interval
+// between calls to runtime.ReadMemStats().  Use the
+// WithMinimumReadMemStatsInterval() option to modify this setting in
+// Start().
+const DefaultMinimumReadMemStatsInterval time.Duration = 15 * time.Second
+
+// WithMinimumReadMemStatsInterval sets a minimum interval between calls to
+// runtime.ReadMemStats(), which is a relatively expensive call to make
+// frequently.  This setting is ignored when `d` is negative.
+func WithMinimumReadMemStatsInterval(d time.Duration) Option {
+	return minimumReadMemStatsIntervalOption(d)
+}
+
+type minimumReadMemStatsIntervalOption time.Duration
+
+// ApplyRuntime implements Option.
+func (o minimumReadMemStatsIntervalOption) ApplyRuntime(c *Config) {
+	if o >= 0 {
+		c.MinimumReadMemStatsInterval = time.Duration(o)
+	}
+}
+
+// WithMeterProvider sets the Metric implementation to use for
+// reporting.  If this option is not used, the global metric.Provider
+// will be used.  `provider` must be non-nil.
+func WithMeterProvider(provider metric.Provider) Option {
+	return metricProviderOption{provider}
+}
+
+type metricProviderOption struct{ metric.Provider }
+
+// ApplyRuntime implements Option.
+func (o metricProviderOption) ApplyRuntime(c *Config) {
+	c.MeterProvider = o.Provider
+}
+
+// Configure computes a Config from the supplied Options.
+func Configure(opts ...Option) Config {
+	c := Config{
+		MeterProvider:               global.MeterProvider(),
+		MinimumReadMemStatsInterval: DefaultMinimumReadMemStatsInterval,
+	}
+	for _, opt := range opts {
+		opt.ApplyRuntime(&c)
+	}
+	return c
+}
+
+// Start initializes reporting of runtime metrics using the supplied Config.
+func Start(c Config) error {
+	if c.MinimumReadMemStatsInterval < 0 {
+		c.MinimumReadMemStatsInterval = DefaultMinimumReadMemStatsInterval
+	}
+	if c.MeterProvider == nil {
+		c.MeterProvider = global.MeterProvider()
+	}
 	r := &runtime{
-		meter:    meter,
-		interval: interval,
+		meter: c.MeterProvider.Meter(
+			// TODO: should library names be qualified?
+			// e.g., contrib/runtime?
+			"runtime",
+			metric.WithInstrumentationVersion(contrib.SemVersion()),
+		),
+		config: c,
 	}
 	return r.register()
 }
@@ -118,7 +194,7 @@ func (r *runtime) registerMemStats() error {
 		defer lock.Unlock()
 
 		now := time.Now()
-		if now.Sub(lastMemStats) >= r.interval {
+		if now.Sub(lastMemStats) >= r.config.MinimumReadMemStatsInterval {
 			goruntime.ReadMemStats(&memStats)
 			lastMemStats = now
 		}

instrumentation/runtime/runtime_test.go

@@ -15,19 +15,87 @@
 package runtime_test
 
 import (
+	goruntime "runtime"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"go.opentelemetry.io/contrib/instrumentation/runtime"
-
-	"go.opentelemetry.io/otel/api/global"
+	"go.opentelemetry.io/contrib/internal/metric"
 )
 
 func TestRuntime(t *testing.T) {
-	meter := global.Meter("test")
-	err := runtime.Start(meter, time.Second)
+	err := runtime.Start(
+		runtime.Configure(
+			runtime.WithMinimumReadMemStatsInterval(time.Second),
+		),
+	)
 	assert.NoError(t, err)
 	time.Sleep(time.Second)
 }
+
+func getGCCount(impl *metric.MeterImpl) int {
+	for _, b := range impl.MeasurementBatches {
+		for _, m := range b.Measurements {
+			if m.Instrument.Descriptor().Name() == "runtime.go.gc.count" {
+				return int(m.Number.CoerceToInt64(m.Instrument.Descriptor().NumberKind()))
+			}
+		}
+	}
+	panic("Could not locate a runtime.go.gc.count metric in test output")
+}
+
+func testMinimumInterval(t *testing.T, shouldHappen bool, opts ...runtime.Option) {
+	goruntime.GC()
+
+	var mstats0 goruntime.MemStats
+	goruntime.ReadMemStats(&mstats0)
+	baseline := int(mstats0.NumGC)
+
+	impl, provider := metric.NewProvider()
+
+	err := runtime.Start(
+		runtime.Configure(
+			append(
+				opts,
+				runtime.WithMeterProvider(provider),
+			)...,
+		),
+	)
+	assert.NoError(t, err)
+
+	goruntime.GC()
+
+	impl.RunAsyncInstruments()
+
+	require.Equal(t, 1, getGCCount(impl)-baseline)
+
+	impl.MeasurementBatches = nil
+
+	extra := 0
+	if shouldHappen {
+		extra = 3
+	}
+
+	goruntime.GC()
+	goruntime.GC()
+	goruntime.GC()
+
+	impl.RunAsyncInstruments()
+
+	require.Equal(t, 1+extra, getGCCount(impl)-baseline)
+}
+
+func TestDefaultMinimumInterval(t *testing.T) {
+	testMinimumInterval(t, false)
+}
+
+func TestNoMinimumInterval(t *testing.T) {
+	testMinimumInterval(t, true, runtime.WithMinimumReadMemStatsInterval(0))
+}
+
+func TestExplicitMinimumInterval(t *testing.T) {
+	testMinimumInterval(t, false, runtime.WithMinimumReadMemStatsInterval(time.Hour))
+}