From bb8341a3fc1ee173d2b9b7fc916222193531a24c Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Tue, 13 Nov 2018 16:19:47 +0100 Subject: [PATCH] Fix metrics handler registration in agent Signed-off-by: Pavol Loffay --- cmd/agent/app/agent.go | 6 ++++++ cmd/agent/app/agent_test.go | 15 +++++++++------ cmd/agent/app/builder.go | 12 +++--------- cmd/agent/app/builder_test.go | 9 --------- cmd/agent/app/flags.go | 2 -- cmd/agent/main.go | 9 +++++++-- 6 files changed, 25 insertions(+), 28 deletions(-) diff --git a/cmd/agent/app/agent.go b/cmd/agent/app/agent.go index 1d04e2b47f6..f7ba833726a 100644 --- a/cmd/agent/app/agent.go +++ b/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)) } diff --git a/cmd/agent/app/agent_test.go b/cmd/agent/app/agent_test.go index 804d41425eb..6eb9b5d9ec1 100644 --- a/cmd/agent/app/agent_test.go +++ b/cmd/agent/app/agent_test.go @@ -95,14 +95,13 @@ 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() { @@ -110,6 +109,10 @@ func withRunningAgent(t *testing.T, testcase func(string, chan error)) { 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) }() diff --git a/cmd/agent/app/builder.go b/cmd/agent/app/builder.go index e779313ac18..55557007bd6 100644 --- a/cmd/agent/app/builder.go +++ b/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 diff --git a/cmd/agent/app/builder_test.go b/cmd/agent/app/builder_test.go index 89f0de528c0..665278c2254 100644 --- a/cmd/agent/app/builder_test.go +++ b/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 diff --git a/cmd/agent/app/flags.go b/cmd/agent/app/flags.go index a9341f80c20..45b752d0a67 100644 --- a/cmd/agent/app/flags.go +++ b/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} diff --git a/cmd/agent/main.go b/cmd/agent/main.go index a3c2d5147c1..064711ea404 100644 --- a/cmd/agent/main.go +++ b/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")