Support configurable metrics backend in the agent (#275)

Defaults to expvar. Prometheus metrics currently don't work because
it does not allow dots in the names (jaegertracing/jaeger-lib#20)
and requires pre-declaring all tag keys, which is not supported by
the Jaeger's metrics API.

Author: yurishkuro

cmd/agent/app/agent_test.go

@@ -29,13 +29,12 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"github.com/uber/jaeger-lib/metrics"
 	"go.uber.org/zap"
 )
 
 func TestAgentStartError(t *testing.T) {
 	cfg := &Builder{}
-	agent, err := cfg.CreateAgent(metrics.NullFactory, zap.NewNop())
+	agent, err := cfg.CreateAgent(zap.NewNop())
 	require.NoError(t, err)
 	agent.httpServer.Addr = "bad-address"
 	assert.Error(t, agent.Run())
@@ -53,7 +52,7 @@ func TestAgentStartStop(t *testing.T) {
 			},
 		},
 	}
-	agent, err := cfg.CreateAgent(metrics.NullFactory, zap.NewNop())
+	agent, err := cfg.CreateAgent(zap.NewNop())
 	require.NoError(t, err)
 	ch := make(chan error, 2)
 	go func() {

cmd/agent/app/builder.go

@@ -36,6 +36,7 @@ import (
 	tchreporter "github.com/uber/jaeger/cmd/agent/app/reporter/tchannel"
 	"github.com/uber/jaeger/cmd/agent/app/servers"
 	"github.com/uber/jaeger/cmd/agent/app/servers/thriftudp"
+	jmetrics "github.com/uber/jaeger/pkg/metrics"
 	zipkinThrift "github.com/uber/jaeger/thrift-gen/agent"
 	jaegerThrift "github.com/uber/jaeger/thrift-gen/jaeger"
 )
@@ -74,15 +75,17 @@ var (
 type Builder struct {
 	Processors []ProcessorConfiguration `yaml:"processors"`
 	HTTPServer HTTPServerConfiguration  `yaml:"httpServer"`
+	Metrics    jmetrics.Builder         `yaml:"metrics"`
 
-	// These 3 fields are copied from reporter.Builder because yaml does not parse embedded structs
+	// These 3 fields are copied from tchreporter.Builder because yaml does not parse embedded structs
 	CollectorHostPorts   []string `yaml:"collectorHostPorts"`
 	DiscoveryMinPeers    int      `yaml:"minPeers"`
 	CollectorServiceName string   `yaml:"collectorServiceName"`
 
 	tchreporter.Builder
 
 	otherReporters []reporter.Reporter
+	metricsFactory metrics.Factory
 }
 
 // NewBuilder creates a default builder with three processors.
@@ -152,6 +155,12 @@ func (b *Builder) WithReporter(r reporter.Reporter) *Builder {
 	return b
 }
 
+// WithMetricsFactory sets an externally initialized metrics factory.
+func (b *Builder) WithMetricsFactory(mf metrics.Factory) *Builder {
+	b.metricsFactory = mf
+	return b
+}
+
 func (b *Builder) createMainReporter(mFactory metrics.Factory, logger *zap.Logger) (*tchreporter.Reporter, error) {
 	if len(b.Builder.CollectorHostPorts) == 0 {
 		b.Builder.CollectorHostPorts = b.CollectorHostPorts
@@ -165,8 +174,19 @@ func (b *Builder) createMainReporter(mFactory metrics.Factory, logger *zap.Logge
 	return b.Builder.CreateReporter(mFactory, logger)
 }
 
+func (b *Builder) getMetricsFactory() (metrics.Factory, error) {
+	if b.metricsFactory != nil {
+		return b.metricsFactory, nil
+	}
+	return b.Metrics.CreateMetricsFactory("jaeger_agent")
+}
+
 // CreateAgent creates the Agent
-func (b *Builder) CreateAgent(mFactory metrics.Factory, logger *zap.Logger) (*Agent, error) {
+func (b *Builder) CreateAgent(logger *zap.Logger) (*Agent, error) {
+	mFactory, err := b.getMetricsFactory()
+	if err != nil {
+		return nil, errors.Wrap(err, "cannot create metrics factory")
+	}
 	mainReporter, err := b.createMainReporter(mFactory, logger)
 	if err != nil {
 		return nil, errors.Wrap(err, "cannot create main Reporter")
@@ -181,6 +201,9 @@ func (b *Builder) CreateAgent(mFactory metrics.Factory, logger *zap.Logger) (*Ag
 		return nil, err
 	}
 	httpServer := b.HTTPServer.GetHTTPServer(b.CollectorServiceName, mainReporter.Channel(), mFactory)
+	if b.metricsFactory == nil {
+		b.Metrics.RegisterHandler(httpServer.Handler.(*http.ServeMux))
+	}
 	return NewAgent(processors, httpServer, logger), nil
 }
 

cmd/agent/app/builder_test.go

@@ -119,16 +119,31 @@ func TestBuilderFromConfig(t *testing.T) {
 func TestBuilderWithExtraReporter(t *testing.T) {
 	cfg := &Builder{}
 	cfg.WithReporter(fakeReporter{})
-	agent, err := cfg.CreateAgent(metrics.NullFactory, zap.NewNop())
+	agent, err := cfg.CreateAgent(zap.NewNop())
 	assert.NoError(t, err)
 	assert.NotNil(t, agent)
 }
 
-func TestBuilderWithError(t *testing.T) {
+func TestBuilderMetrics(t *testing.T) {
+	mf := metrics.NullFactory
+	b := new(Builder).WithMetricsFactory(mf)
+	mf2, err := b.getMetricsFactory()
+	assert.NoError(t, err)
+	assert.Equal(t, mf, mf2)
+}
+
+func TestBuilderMetricsError(t *testing.T) {
+	b := &Builder{}
+	b.Metrics.Backend = "invalid"
+	_, err := b.CreateAgent(zap.NewNop())
+	assert.EqualError(t, err, "cannot create metrics factory: unknown metrics backend specified")
+}
+
+func TestBuilderWithDiscoveryError(t *testing.T) {
 	cfg := &Builder{}
 	cfg.WithDiscoverer(fakeDiscoverer{})
-	agent, err := cfg.CreateAgent(metrics.NullFactory, zap.NewNop())
-	assert.Error(t, err)
+	agent, err := cfg.CreateAgent(zap.NewNop())
+	assert.EqualError(t, err, "cannot create main Reporter: cannot enable service discovery: both discovery.Discoverer and discovery.Notifier must be specified")
 	assert.Nil(t, agent)
 }
 
@@ -158,7 +173,7 @@ func TestBuilderWithProcessorErrors(t *testing.T) {
 				},
 			},
 		}
-		_, err := cfg.CreateAgent(metrics.NullFactory, zap.NewNop())
+		_, err := cfg.CreateAgent(zap.NewNop())
 		assert.Error(t, err)
 		if testCase.err != "" {
 			assert.EqualError(t, err, testCase.err)

cmd/agent/app/flags.go

@@ -28,6 +28,7 @@ import (
 
 // Bind binds the agent builder to command line options
 func (b *Builder) Bind(flags *flag.FlagSet) {
+	b.Metrics.Bind(flags)
 	for i := range b.Processors {
 		p := &b.Processors[i]
 		name := "processor." + string(p.Model) + "-" + string(p.Protocol) + "."

cmd/agent/main.go

@@ -24,8 +24,6 @@ import (
 	"flag"
 	"runtime"
 
-	"github.com/uber/jaeger-lib/metrics/go-kit"
-	"github.com/uber/jaeger-lib/metrics/go-kit/expvar"
 	"go.uber.org/zap"
 
 	"github.com/uber/jaeger/cmd/agent/app"
@@ -39,12 +37,11 @@ func main() {
 	runtime.GOMAXPROCS(runtime.NumCPU())
 
 	logger, _ := zap.NewProduction()
-	metricsFactory := xkit.Wrap("jaeger-agent", expvar.NewFactory(10))
 
 	// TODO illustrate discovery service wiring
 	// TODO illustrate additional reporter
 
-	agent, err := builder.CreateAgent(metricsFactory, logger)
+	agent, err := builder.CreateAgent(logger)
 	if err != nil {
 		logger.Fatal("Unable to initialize Jaeger Agent", zap.Error(err))
 	}

cmd/standalone/main.go

@@ -74,7 +74,7 @@ func startAgent(logger *zap.Logger, baseFactory metrics.Factory, builder *agentA
 	if len(builder.CollectorHostPorts) == 0 {
 		builder.CollectorHostPorts = append(builder.CollectorHostPorts, fmt.Sprintf("127.0.0.1:%d", *collector.CollectorPort))
 	}
-	agent, err := builder.CreateAgent(metricsFactory, logger)
+	agent, err := builder.WithMetricsFactory(metricsFactory).CreateAgent(logger)
 	if err != nil {
 		logger.Fatal("Unable to initialize Jaeger Agent", zap.Error(err))
 	}

glide.lock

@@ -18,7 +18,7 @@ import:
   subpackages:
   - transport
 - package: github.com/uber/jaeger-lib
-  version: ffca3143c929e9b97325e6bebcc380e8c6e5fa0d
+  version: 13219516eeab40f3b23e4d50e3f91a0d68064afc
 - package: github.com/uber/tchannel-go
   version: v1.1.0
   subpackages:
@@ -36,7 +36,7 @@ import:
   subpackages:
   - nethttp
 - package: github.com/go-kit/kit
-  version: v0.4.0
+  version: v0.5.0
   subpackages:
   - metrics
 - package: github.com/olivere/elastic