diff --git a/cmd/agent/app/builder.go b/cmd/agent/app/builder.go index b09137f39d1..cb4c884e8fd 100644 --- a/cmd/agent/app/builder.go +++ b/cmd/agent/app/builder.go @@ -111,7 +111,7 @@ func (b *Builder) CreateAgent(primaryProxy CollectorProxy, logger *zap.Logger, m if err != nil { return nil, fmt.Errorf("cannot create processors: %w", err) } - server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory) + server := b.HTTPServer.getHTTPServer(primaryProxy.GetManager(), mFactory, logger) b.publishOpts(mFactory) return NewAgent(processors, server, logger), nil @@ -170,11 +170,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 configmanager.ClientConfigManager, mFactory metrics.Factory) *http.Server { +func (c HTTPServerConfiguration) getHTTPServer(manager configmanager.ClientConfigManager, mFactory metrics.Factory, logger *zap.Logger) *http.Server { if c.HostPort == "" { c.HostPort = defaultHTTPServerHostPort } - return httpserver.NewHTTPServer(c.HostPort, manager, mFactory) + return httpserver.NewHTTPServer(c.HostPort, manager, mFactory, logger) } // GetThriftProcessor gets a TBufferedServer backed Processor using the collector configuration diff --git a/cmd/agent/app/httpserver/srv.go b/cmd/agent/app/httpserver/srv.go index 37a4dce1534..227be0bb93b 100644 --- a/cmd/agent/app/httpserver/srv.go +++ b/cmd/agent/app/httpserver/srv.go @@ -20,6 +20,8 @@ import ( "github.com/gorilla/mux" "github.com/uber/jaeger-lib/metrics" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" "github.com/jaegertracing/jaeger/cmd/agent/app/configmanager" "github.com/jaegertracing/jaeger/pkg/clientcfg/clientcfghttp" @@ -27,7 +29,7 @@ import ( // NewHTTPServer creates a new server that hosts an HTTP/JSON endpoint for clients // to query for sampling strategies and baggage restrictions. -func NewHTTPServer(hostPort string, manager configmanager.ClientConfigManager, mFactory metrics.Factory) *http.Server { +func NewHTTPServer(hostPort string, manager configmanager.ClientConfigManager, mFactory metrics.Factory, logger *zap.Logger) *http.Server { handler := clientcfghttp.NewHTTPHandler(clientcfghttp.HTTPHandlerParams{ ConfigManager: manager, MetricsFactory: mFactory, @@ -35,5 +37,10 @@ func NewHTTPServer(hostPort string, manager configmanager.ClientConfigManager, m }) r := mux.NewRouter() handler.RegisterRoutes(r) - return &http.Server{Addr: hostPort, Handler: r} + errorLog, _ := zap.NewStdLogAt(logger, zapcore.ErrorLevel) + return &http.Server{ + Addr: hostPort, + Handler: r, + ErrorLog: errorLog, + } } diff --git a/cmd/agent/app/httpserver/srv_test.go b/cmd/agent/app/httpserver/srv_test.go index e14427ff785..4edda60ab0f 100644 --- a/cmd/agent/app/httpserver/srv_test.go +++ b/cmd/agent/app/httpserver/srv_test.go @@ -19,9 +19,10 @@ import ( "testing" "github.com/stretchr/testify/assert" + "go.uber.org/zap" ) func TestHTTPServer(t *testing.T) { - s := NewHTTPServer(":1", nil, nil) + s := NewHTTPServer(":1", nil, nil, zap.NewNop()) assert.NotNil(t, s) } diff --git a/cmd/collector/app/server/http.go b/cmd/collector/app/server/http.go index 7860caf0de8..09edb742b7e 100644 --- a/cmd/collector/app/server/http.go +++ b/cmd/collector/app/server/http.go @@ -21,6 +21,7 @@ import ( "github.com/gorilla/mux" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/sampling/strategystore" @@ -46,7 +47,11 @@ type HTTPServerParams struct { func StartHTTPServer(params *HTTPServerParams) (*http.Server, error) { params.Logger.Info("Starting jaeger-collector HTTP server", zap.String("http host-port", params.HostPort)) - server := &http.Server{Addr: params.HostPort} + errorLog, _ := zap.NewStdLogAt(params.Logger, zapcore.ErrorLevel) + server := &http.Server{ + Addr: params.HostPort, + ErrorLog: errorLog, + } if params.TLSConfig.Enabled { tlsCfg, err := params.TLSConfig.Config(params.Logger) // This checks if the certificates are correctly provided if err != nil { diff --git a/cmd/collector/app/server/zipkin.go b/cmd/collector/app/server/zipkin.go index a537b5feef3..64ebc3c338b 100644 --- a/cmd/collector/app/server/zipkin.go +++ b/cmd/collector/app/server/zipkin.go @@ -23,6 +23,7 @@ import ( "github.com/rs/cors" "github.com/uber/jaeger-lib/metrics" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "github.com/jaegertracing/jaeger/cmd/collector/app/handler" "github.com/jaegertracing/jaeger/cmd/collector/app/zipkin" @@ -56,7 +57,11 @@ func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) { return nil, err } - server := &http.Server{Addr: params.HostPort} + errorLog, _ := zap.NewStdLogAt(params.Logger, zapcore.ErrorLevel) + server := &http.Server{ + Addr: params.HostPort, + ErrorLog: errorLog, + } serveZipkin(server, listener, params) return server, nil diff --git a/cmd/flags/admin.go b/cmd/flags/admin.go index 43f54e8295f..734b9978ec0 100644 --- a/cmd/flags/admin.go +++ b/cmd/flags/admin.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/viper" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "github.com/jaegertracing/jaeger/pkg/healthcheck" "github.com/jaegertracing/jaeger/pkg/recoveryhandler" @@ -107,7 +108,12 @@ func (s *AdminServer) serveWithListener(l net.Listener) { version.RegisterHandler(s.mux, s.logger) s.registerPprofHandlers() recoveryHandler := recoveryhandler.NewRecoveryHandler(s.logger, true) - s.server = &http.Server{Handler: recoveryHandler(s.mux)} + errorLog, _ := zap.NewStdLogAt(s.logger, zapcore.ErrorLevel) + s.server = &http.Server{ + Handler: recoveryHandler(s.mux), + ErrorLog: errorLog, + } + s.logger.Info("Starting admin HTTP server", zap.String("http-addr", s.adminHostPort)) go func() { switch err := s.server.Serve(l); err { diff --git a/cmd/query/app/server.go b/cmd/query/app/server.go index 4a460caec86..3c410294fba 100644 --- a/cmd/query/app/server.go +++ b/cmd/query/app/server.go @@ -26,6 +26,7 @@ import ( "github.com/opentracing/opentracing-go" "github.com/soheilhy/cmux" "go.uber.org/zap" + "go.uber.org/zap/zapcore" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -50,6 +51,7 @@ type Server struct { conn net.Listener grpcConn net.Listener httpConn net.Listener + cmuxServer cmux.CMux grpcServer *grpc.Server httpServer *http.Server separatePorts bool @@ -78,7 +80,7 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, metricsQuery return nil, err } - httpServer, cancelFunc, err := createHTTPServer(querySvc, metricsQuerySvc, options, tracer, logger) + httpServer, closeGRPCGateway, err := createHTTPServer(querySvc, metricsQuerySvc, options, tracer, logger) if err != nil { return nil, err } @@ -92,7 +94,7 @@ func NewServer(logger *zap.Logger, querySvc *querysvc.QueryService, metricsQuery httpServer: httpServer, separatePorts: grpcPort != httpPort, unavailableChannel: make(chan healthcheck.Status), - grpcGatewayCancel: cancelFunc, + grpcGatewayCancel: closeGRPCGateway, }, nil } @@ -145,9 +147,9 @@ func createHTTPServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc. r = r.PathPrefix(queryOpts.BasePath).Subrouter() } - ctx, cancelFunc := context.WithCancel(context.Background()) + ctx, closeGRPCGateway := context.WithCancel(context.Background()) if err := apiv3.RegisterGRPCGateway(ctx, logger, r, queryOpts.BasePath, queryOpts.GRPCHostPort, queryOpts.TLSGRPC); err != nil { - cancelFunc() // make go vet happy + closeGRPCGateway() return nil, nil, err } @@ -161,20 +163,22 @@ func createHTTPServer(querySvc *querysvc.QueryService, metricsQuerySvc querysvc. handler = handlers.CompressHandler(handler) recoveryHandler := recoveryhandler.NewRecoveryHandler(logger, true) + errorLog, _ := zap.NewStdLogAt(logger, zapcore.ErrorLevel) server := &http.Server{ - Handler: recoveryHandler(handler), + Handler: recoveryHandler(handler), + ErrorLog: errorLog, } if queryOpts.TLSHTTP.Enabled { tlsCfg, err := queryOpts.TLSHTTP.Config(logger) // This checks if the certificates are correctly provided if err != nil { - cancelFunc() + closeGRPCGateway() return nil, nil, err } server.TLSConfig = tlsCfg } - return server, cancelFunc, nil + return server, closeGRPCGateway, nil } // initListener initialises listeners of the server @@ -222,7 +226,6 @@ func (s *Server) initListener() (cmux.CMux, error) { s.httpConn = cmuxServer.Match(cmux.Any()) return cmuxServer, nil - } // Start http, GRPC and cmux servers concurrently @@ -231,6 +234,7 @@ func (s *Server) Start() error { if err != nil { return err } + s.cmuxServer = cmuxServer var tcpPort int if !s.separatePorts { @@ -283,7 +287,7 @@ func (s *Server) Start() error { s.logger.Info("Starting CMUX server", zap.Int("port", tcpPort), zap.String("addr", s.queryOptions.HTTPHostPort)) err := cmuxServer.Serve() - // TODO: Remove string comparison when https://github.com/soheilhy/cmux/pull/69 is merged + // TODO: find a way to avoid string comparison. Even though cmux has ErrServerClosed, it's not returned here. if err != nil && !strings.Contains(err.Error(), "use of closed network connection") { s.logger.Error("Could not start multiplexed server", zap.Error(err)) } @@ -305,6 +309,7 @@ func (s *Server) Close() error { s.httpConn.Close() s.grpcConn.Close() } else { + s.cmuxServer.Close() s.conn.Close() } return nil