Fix metrics handler registration in agent (#1178)

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

Author: pavolloffay

cmd/agent/app/agent.go

@@ -49,6 +49,11 @@ func NewAgent(
 	return a
 }
 
+// GetServer returns HTTP server used by the agent.
+func (a *Agent) GetServer() *http.Server {
+	return a.httpServer
+}
+
 // Run runs all of agent UDP and HTTP servers in separate go-routines.
 // It returns an error when it's immediately apparent on startup, but
 // any errors happening after starting the servers are only logged.
@@ -60,6 +65,7 @@ func (a *Agent) Run() error {
 	a.httpAddr.Store(listener.Addr().String())
 	a.closer = listener
 	go func() {
+		a.logger.Info("Starting jaeger-agent HTTP server", zap.Int("http-port", listener.Addr().(*net.TCPAddr).Port))
 		if err := a.httpServer.Serve(listener); err != nil {
 			a.logger.Error("http server failure", zap.Error(err))
 		}

cmd/agent/app/agent_test.go

@@ -95,21 +95,24 @@ func withRunningAgent(t *testing.T, testcase func(string, chan error)) {
 		HTTPServer: HTTPServerConfiguration{
 			HostPort: ":0",
 		},
-		Metrics: jmetrics.Builder{
-			Backend:   "prometheus",
-			HTTPRoute: "/metrics",
-		},
 	}
 	logger, logBuf := testutils.NewLogger()
-	f, _ := cfg.Metrics.CreateMetricsFactory("jaeger")
-	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, f)
+	//f, _ := cfg.Metrics.CreateMetricsFactory("jaeger")
+	mBldr := &jmetrics.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
+	mFactory, err := mBldr.CreateMetricsFactory("jaeger")
+	require.NoError(t, err)
+	agent, err := cfg.CreateAgent(fakeCollectorProxy{}, logger, mFactory)
 	require.NoError(t, err)
 	ch := make(chan error, 2)
 	go func() {
 		if err := agent.Run(); err != nil {
 			t.Errorf("error from agent.Run(): %s", err)
 			ch <- err
 		}
+		if h := mBldr.Handler(); mFactory != nil && h != nil {
+			logger.Info("Registering metrics handler with HTTP server", zap.String("route", mBldr.HTTPRoute))
+			agent.GetServer().Handler.(*http.ServeMux).Handle(mBldr.HTTPRoute, h)
+		}
 		close(ch)
 	}()
 

cmd/agent/app/builder.go

@@ -28,7 +28,6 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/agent/app/reporter"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/servers"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/servers/thriftudp"
-	jmetrics "github.com/jaegertracing/jaeger/pkg/metrics"
 	zipkinThrift "github.com/jaegertracing/jaeger/thrift-gen/agent"
 	jaegerThrift "github.com/jaegertracing/jaeger/thrift-gen/jaeger"
 )
@@ -72,7 +71,6 @@ type CollectorProxy interface {
 type Builder struct {
 	Processors []ProcessorConfiguration `yaml:"processors"`
 	HTTPServer HTTPServerConfiguration  `yaml:"httpServer"`
-	Metrics    jmetrics.Builder         `yaml:"metrics"`
 
 	reporters []reporter.Reporter
 }
@@ -110,7 +108,7 @@ func (b *Builder) CreateAgent(primaryProxy CollectorProxy, logger *zap.Logger, m
 	if err != nil {
 		return nil, err
 	}
-	server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory, &b.Metrics)
+	server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory)
 	return NewAgent(processors, server, logger), nil
 }
 
@@ -156,15 +154,11 @@ func (b *Builder) getProcessors(rep reporter.Reporter, mFactory metrics.Factory,
 }
 
 // GetHTTPServer creates an HTTP server that provides sampling strategies and baggage restrictions to client libraries.
-func (c HTTPServerConfiguration) getHTTPServer(manager httpserver.ClientConfigManager, mFactory metrics.Factory, mBuilder *jmetrics.Builder) *http.Server {
+func (c HTTPServerConfiguration) getHTTPServer(manager httpserver.ClientConfigManager, mFactory metrics.Factory) *http.Server {
 	if c.HostPort == "" {
 		c.HostPort = defaultHTTPServerHostPort
 	}
-	server := httpserver.NewHTTPServer(c.HostPort, manager, mFactory)
-	if h := mBuilder.Handler(); mFactory != nil && h != nil {
-		server.Handler.(*http.ServeMux).Handle(mBuilder.HTTPRoute, h)
-	}
-	return server
+	return httpserver.NewHTTPServer(c.HostPort, manager, mFactory)
 }
 
 // GetThriftProcessor gets a TBufferedServer backed Processor using the collector configuration

cmd/agent/app/builder_test.go

@@ -107,15 +107,6 @@ func TestBuilderWithExtraReporter(t *testing.T) {
 	assert.NotNil(t, agent)
 }
 
-func TestBuilderMetricsHandler(t *testing.T) {
-	b := &Builder{}
-	b.Metrics.Backend = "expvar"
-	b.Metrics.HTTPRoute = "/expvar"
-	agent, err := b.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), metrics.NullFactory)
-	assert.NoError(t, err)
-	assert.NotNil(t, agent)
-}
-
 func TestBuilderWithProcessorErrors(t *testing.T) {
 	testCases := []struct {
 		model       Model

cmd/agent/app/flags.go

@@ -56,8 +56,6 @@ func AddFlags(flags *flag.FlagSet) {
 
 // InitFromViper initializes Builder with properties retrieved from Viper.
 func (b *Builder) InitFromViper(v *viper.Viper) *Builder {
-	b.Metrics.InitFromViper(v)
-
 	for _, processor := range defaultProcessors {
 		prefix := fmt.Sprintf("processor.%s-%s.", processor.model, processor.protocol)
 		p := &ProcessorConfiguration{Model: processor.model, Protocol: processor.protocol}

cmd/agent/main.go

@@ -17,6 +17,7 @@ package main
 import (
 	"fmt"
 	"io/ioutil"
+	"net/http"
 	"os"
 
 	"github.com/pkg/errors"
@@ -54,8 +55,7 @@ func main() {
 				return err
 			}
 
-			builder := &app.Builder{}
-			builder.InitFromViper(v)
+			builder := new(app.Builder).InitFromViper(v)
 			mBldr := new(metrics.Builder).InitFromViper(v)
 
 			mFactory, err := mBldr.CreateMetricsFactory("jaeger")
@@ -79,6 +79,11 @@ func main() {
 				return errors.Wrap(err, "Unable to initialize Jaeger Agent")
 			}
 
+			if h := mBldr.Handler(); mFactory != nil && h != nil {
+				logger.Info("Registering metrics handler with HTTP server", zap.String("route", mBldr.HTTPRoute))
+				agent.GetServer().Handler.(*http.ServeMux).Handle(mBldr.HTTPRoute, h)
+			}
+
 			logger.Info("Starting agent")
 			if err := agent.Run(); err != nil {
 				return errors.Wrap(err, "Failed to run the agent")