Skip to content
This repository has been archived by the owner on Mar 15, 2024. It is now read-only.

Commit

Permalink
Fix #100 drop pprof and metrics separated servers
Browse files Browse the repository at this point in the history
- use ops server as drop replacement
- fix grpc.WithInsecure deprecation warning

closes #100
  • Loading branch information
im-kulikov committed Jan 27, 2022
1 parent 1bb0c96 commit ad66aa1
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 146 deletions.
79 changes: 4 additions & 75 deletions web/servers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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

Expand All @@ -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")
Expand All @@ -91,68 +69,19 @@ var (
// nolint:gochecknoglobals
DefaultServersModule = module.Combine(
DefaultGRPCModule,
ProfilerModule,
MetricsModule,
OpsModule,
APIModule,
)

// APIModule defines API server module.
// 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{
Expand Down Expand Up @@ -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:
Expand Down
90 changes: 19 additions & 71 deletions web/servers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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()
}))
Expand Down

0 comments on commit ad66aa1

Please sign in to comment.