From ad66aa111baae1e119d7f0e4bc03641e38ff0384 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Thu, 27 Jan 2022 21:05:03 +0300 Subject: [PATCH] Fix #100 drop pprof and metrics separated servers - use ops server as drop replacement - fix grpc.WithInsecure deprecation warning closes #100 --- web/servers.go | 79 ++------------------------------------- web/servers_test.go | 90 ++++++++++----------------------------------- 2 files changed, 23 insertions(+), 146 deletions(-) diff --git a/web/servers.go b/web/servers.go index b526a7c..b58ca8f 100644 --- a/web/servers.go +++ b/web/servers.go @@ -3,9 +3,7 @@ package web import ( "net" "net/http" - "net/http/pprof" - "github.com/prometheus/client_golang/prometheus/promhttp" "github.com/spf13/viper" "go.uber.org/dig" "go.uber.org/zap" @@ -46,24 +44,6 @@ type ( Server service.Service `group:"services"` } - profileParams struct { - dig.In - - Logger *zap.Logger - Viper *viper.Viper - Handler http.Handler `name:"pprof_handler" optional:"true"` - Listener net.Listener `name:"pprof_listener" optional:"true"` - } - - metricParams struct { - dig.In - - Logger *zap.Logger - Viper *viper.Viper - Handler http.Handler `name:"metric_handler" optional:"true"` - Listener net.Listener `name:"metric_listener" optional:"true"` - } - grpcParams struct { dig.In @@ -77,10 +57,8 @@ type ( ) const ( - apiServer = "api" - gRPCServer = "grpc" - profileServer = "pprof" - metricsServer = "metric" + apiServer = "api" + gRPCServer = "grpc" // ErrEmptyLogger is raised when empty logger passed into New function. ErrEmptyLogger = internal.Error("empty logger") @@ -91,8 +69,7 @@ var ( // nolint:gochecknoglobals DefaultServersModule = module.Combine( DefaultGRPCModule, - ProfilerModule, - MetricsModule, + OpsModule, APIModule, ) @@ -100,59 +77,11 @@ var ( // nolint:gochecknoglobals APIModule = module.New(NewAPIServer) - // ProfilerModule defines pprof server module. - // nolint:gochecknoglobals - ProfilerModule = module.New(newProfileServer) - - // MetricsModule defines prometheus server module. - // nolint:gochecknoglobals - MetricsModule = module.New(newMetricServer) - // DefaultGRPCModule defines default gRPC server module. // nolint:gochecknoglobals DefaultGRPCModule = module.New(newDefaultGRPCServer) ) -func newProfileServer(p profileParams) (ServerResult, error) { - mux := http.NewServeMux() - mux.HandleFunc("/debug/pprof/", pprof.Index) - mux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - mux.HandleFunc("/debug/pprof/profile", pprof.Profile) - mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - mux.HandleFunc("/debug/pprof/trace", pprof.Trace) - - if p.Handler != nil { - mux.Handle("/", p.Handler) - } - - return NewHTTPServer(HTTPParams{ - Config: p.Viper, - Logger: p.Logger, - Name: profileServer, - Key: profileServer, - Handler: mux, - Listener: p.Listener, - }) -} - -func newMetricServer(p metricParams) (ServerResult, error) { - mux := http.NewServeMux() - mux.Handle("/metrics", promhttp.Handler()) - - if p.Handler != nil { - mux.Handle("/", p.Handler) - } - - return NewHTTPServer(HTTPParams{ - Config: p.Viper, - Logger: p.Logger, - Name: metricsServer, - Key: metricsServer, - Handler: mux, - Listener: p.Listener, - }) -} - // NewAPIServer creates api server by http.Handler from DI container. func NewAPIServer(p APIParams) (ServerResult, error) { return NewHTTPServer(HTTPParams{ @@ -225,7 +154,7 @@ func newDefaultGRPCServer(p grpcParams) (ServerResult, error) { return ServerResult{Server: serve}, err } -// NewHTTPServer creates http-server that will be embedded into multi-server. +// NewHTTPServer creates http-server that will be embedded into multiple server. func NewHTTPServer(p HTTPParams) (ServerResult, error) { switch { case p.Logger == nil: diff --git a/web/servers_test.go b/web/servers_test.go index 4d52299..3759620 100644 --- a/web/servers_test.go +++ b/web/servers_test.go @@ -18,6 +18,7 @@ import ( "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "google.golang.org/grpc/test/bufconn" gt "google.golang.org/grpc/test/grpc_testing" @@ -153,88 +154,35 @@ func TestServers(t *testing.T) { }) }) - t.Run("check pprof server", func(t *testing.T) { - t.Run("without logger", func(t *testing.T) { - params := profileParams{Viper: v} - serve, err := newProfileServer(params) - require.EqualError(t, err, ErrEmptyLogger.Error()) - require.Nil(t, serve.Server) - }) - - t.Run("without config", func(t *testing.T) { - params := profileParams{Logger: zaptest.NewLogger(t)} - serve, err := newProfileServer(params) - require.NoError(t, err) - require.Nil(t, serve.Server) - }) - - t.Run("with config", func(t *testing.T) { - lis := bufconn.Listen(listenSize) - defer require.NoError(t, lis.Close()) - - params := profileParams{Viper: v, Listener: lis, Logger: zaptest.NewLogger(t)} - serve, err := newProfileServer(params) - require.NoError(t, err) - require.NotNil(t, serve.Server) - require.IsType(t, &httpService{}, serve.Server) - }) - }) - - t.Run("check metrics server", func(t *testing.T) { - t.Run("without config", func(t *testing.T) { - params := metricParams{Logger: zaptest.NewLogger(t)} - serve, err := newMetricServer(params) - require.NoError(t, err) - require.Nil(t, serve.Server) - }) - - t.Run("should fail on NewHTTPService", func(t *testing.T) { - cfg := viper.New() - cfg.SetDefault(metricsServer+".address", "test") - cfg.SetDefault(metricsServer+".network", "test") - - params := metricParams{ - Viper: cfg, - Logger: zaptest.NewLogger(t), - } - - serve, err := newMetricServer(params) - require.EqualError(t, err, "listen test: unknown network test") - require.Nil(t, serve.Server) - }) - - t.Run("with config", func(t *testing.T) { - lis := bufconn.Listen(listenSize) - defer require.NoError(t, lis.Close()) - - params := metricParams{Viper: v, Listener: lis, Logger: zaptest.NewLogger(t)} - serve, err := newMetricServer(params) - require.NoError(t, err) - require.NotNil(t, serve.Server) - require.IsType(t, &httpService{}, serve.Server) - }) - }) - t.Run("empty viper or config key for http-server", func(t *testing.T) { is := require.New(t) v.SetDefault("test-api.disabled", true) - z, err := zap.NewDevelopment() - is.NoError(err) testHTTPHandler(is) t.Run("empty key", func(t *testing.T) { - serve, err := NewHTTPServer(HTTPParams{Logger: z}) + serve, err := NewHTTPServer(HTTPParams{Logger: zaptest.NewLogger(t)}) require.NoError(t, err) require.Nil(t, serve.Server) }) t.Run("empty viper", func(t *testing.T) { - serve, err := NewHTTPServer(HTTPParams{Logger: z, Key: testHTTPServe}) + serve, err := NewHTTPServer(HTTPParams{Logger: zaptest.NewLogger(t), Key: testHTTPServe}) require.NoError(t, err) require.Nil(t, serve.Server) }) + + t.Run("empty http-address", func(t *testing.T) { + serve, err := NewHTTPServer(HTTPParams{ + Key: testHTTPServe, + Config: viper.New(), + Handler: http.NewServeMux(), + Logger: zaptest.NewLogger(t), + }) + require.Nil(t, serve.Server) + require.EqualError(t, err, ErrEmptyHTTPAddress.Error()) + }) }) t.Run("disabled http-server", func(t *testing.T) { @@ -335,11 +283,11 @@ func TestServers(t *testing.T) { cfg.SetDefault(apiServer+".read_header_timeout", time.Second) cfg.SetDefault(apiServer+".max_header_bytes", math.MaxInt32) + OpsDefaults(cfg) + listeners := map[string]*bufconn.Listener{ - apiServer: bufconn.Listen(listenSize), - gRPCServer: bufconn.Listen(listenSize), - profileServer: bufconn.Listen(listenSize), - metricsServer: bufconn.Listen(listenSize), + apiServer: bufconn.Listen(listenSize), + gRPCServer: bufconn.Listen(listenSize), } mod := module.Module{ @@ -428,7 +376,7 @@ func TestServers(t *testing.T) { case gRPCServer: conn, err := grpc.DialContext(ctx, lis.Addr().String(), grpc.WithBlock(), - grpc.WithInsecure(), + grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { return lis.Dial() }))