From 1608ad15075c91cafa26c98625d6ff565d48590a Mon Sep 17 00:00:00 2001 From: Sergey Chubaryan Date: Fri, 7 Feb 2025 13:26:22 +0300 Subject: [PATCH] move server functions to internal/httpserver --- cmd/backend/app.go | 2 +- cmd/backend/server/middleware/request_log.go | 56 ------- cmd/backend/server/server.go | 48 ++---- cmd/shortlinks/main.go | 7 +- internal/http_server/middleware/recovery.go | 155 ------------------ internal/http_server/middleware/tracing.go | 33 ---- .../http_server}/recovery.go | 2 +- .../{middleware => }/request_log.go | 2 +- .../http_server}/tracing.go | 2 +- 9 files changed, 18 insertions(+), 289 deletions(-) delete mode 100644 cmd/backend/server/middleware/request_log.go delete mode 100644 internal/http_server/middleware/recovery.go delete mode 100644 internal/http_server/middleware/tracing.go rename {cmd/backend/server/middleware => internal/http_server}/recovery.go (99%) rename internal/http_server/{middleware => }/request_log.go (98%) rename {cmd/backend/server/middleware => internal/http_server}/tracing.go (97%) diff --git a/cmd/backend/app.go b/cmd/backend/app.go index aa520d2..59c1b26 100644 --- a/cmd/backend/app.go +++ b/cmd/backend/app.go @@ -186,7 +186,7 @@ func (a *App) Run(p RunParams) { }() } - srv := server.New( + srv := server.NewServer( server.NewServerOpts{ DebugMode: debugMode, Logger: logger, diff --git a/cmd/backend/server/middleware/request_log.go b/cmd/backend/server/middleware/request_log.go deleted file mode 100644 index 587abc6..0000000 --- a/cmd/backend/server/middleware/request_log.go +++ /dev/null @@ -1,56 +0,0 @@ -package middleware - -import ( - "backend/internal/integrations" - log "backend/pkg/logger" - "time" - - "github.com/gin-gonic/gin" - "github.com/google/uuid" - "go.opentelemetry.io/otel/trace" -) - -func NewRequestLogMiddleware(logger log.Logger, tracer trace.Tracer, prometheus *integrations.Prometheus) gin.HandlerFunc { - return func(c *gin.Context) { - prometheus.RequestInc() - defer prometheus.RequestDec() - - requestId := c.GetHeader("X-Request-Id") - if requestId == "" { - requestId = uuid.New().String() - } - c.Header("X-Request-Id", requestId) - c.Header("Access-Control-Allow-Origin", "*") - - log.SetCtxRequestId(c, requestId) - - path := c.Request.URL.Path - if c.Request.URL.RawQuery != "" { - path = path + "?" + c.Request.URL.RawQuery - } - - start := time.Now() - c.Next() - latency := time.Since(start) - - prometheus.AddRequestTime(float64(latency.Microseconds())) - - method := c.Request.Method - statusCode := c.Writer.Status() - - if statusCode >= 200 && statusCode < 400 { - return - } - - ctxLogger := logger.WithContext(c) - - if statusCode >= 400 && statusCode < 500 { - prometheus.Add4xxError() - ctxLogger.Warning().Msgf("Request %s %s %d %v", method, path, statusCode, latency) - return - } - - prometheus.Add5xxError() - ctxLogger.Error().Msgf("Request %s %s %d %v", method, path, statusCode, latency) - } -} diff --git a/cmd/backend/server/server.go b/cmd/backend/server/server.go index e968048..9c70bc4 100644 --- a/cmd/backend/server/server.go +++ b/cmd/backend/server/server.go @@ -5,21 +5,14 @@ import ( "backend/cmd/backend/server/middleware" "backend/cmd/backend/server/utils" "backend/internal/core/services" + httpserver "backend/internal/http_server" "backend/internal/integrations" "backend/pkg/logger" - "context" - "fmt" - "net" "github.com/gin-gonic/gin" "go.opentelemetry.io/otel/trace" ) -type Server struct { - logger logger.Logger - ginEngine *gin.Engine -} - type NewServerOpts struct { DebugMode bool Logger logger.Logger @@ -28,7 +21,7 @@ type NewServerOpts struct { Tracer trace.Tracer } -func New(opts NewServerOpts) *Server { +func NewServer(opts NewServerOpts) *httpserver.Server { if !opts.DebugMode { gin.SetMode(gin.ReleaseMode) } @@ -42,9 +35,9 @@ func New(opts NewServerOpts) *Server { prometheus := integrations.NewPrometheus() r.Any("/metrics", gin.WrapH(prometheus.GetRequestHandler())) - r.Use(middleware.NewRecoveryMiddleware(opts.Logger, prometheus, opts.DebugMode)) - r.Use(middleware.NewRequestLogMiddleware(opts.Logger, opts.Tracer, prometheus)) - r.Use(middleware.NewTracingMiddleware(opts.Tracer)) + r.Use(httpserver.NewRecoveryMiddleware(opts.Logger, prometheus, opts.DebugMode)) + r.Use(httpserver.NewRequestLogMiddleware(opts.Logger, opts.Tracer, prometheus)) + r.Use(httpserver.NewTracingMiddleware(opts.Tracer)) userGroup := r.Group("/user") userGroup.POST("/create", handlers.NewUserCreateHandler(opts.Logger, opts.UserService)) @@ -60,29 +53,10 @@ func New(opts NewServerOpts) *Server { }) } - return &Server{ - logger: opts.Logger, - ginEngine: r, - } -} - -func (s *Server) Run(ctx context.Context, port uint16) { - listenAddr := fmt.Sprintf("0.0.0.0:%d", port) - s.logger.Log().Msgf("server listening on %s", listenAddr) - - listener, err := (&net.ListenConfig{}).Listen(ctx, "tcp", listenAddr) - if err != nil { - s.logger.Fatal().Err(err).Msg("can not create network listener") - } - - go func() { - <-ctx.Done() - s.logger.Log().Msg("stopping tcp listener...") - listener.Close() - }() - - err = s.ginEngine.RunListener(listener) - if err != nil && err == net.ErrClosed { - s.logger.Fatal().Err(err).Msg("server stopped with error") - } + return httpserver.New( + httpserver.NewServerOpts{ + Logger: opts.Logger, + HttpServer: r, + }, + ) } diff --git a/cmd/shortlinks/main.go b/cmd/shortlinks/main.go index 4fb9324..2adbf3d 100644 --- a/cmd/shortlinks/main.go +++ b/cmd/shortlinks/main.go @@ -6,7 +6,6 @@ import ( grpcserver "backend/internal/grpc_server" "backend/internal/grpc_server/shortlinks" httpserver "backend/internal/http_server" - "backend/internal/http_server/middleware" "backend/internal/integrations" "backend/pkg/cache" "backend/pkg/logger" @@ -94,9 +93,9 @@ func RunServer(ctx context.Context, log logger.Logger, tracer trace.Tracer, conf ctx.Status(200) }) - r.Use(middleware.NewRecoveryMiddleware(log, prometheus, debugMode)) - r.Use(middleware.NewRequestLogMiddleware(log, tracer, prometheus)) - r.Use(middleware.NewTracingMiddleware(tracer)) + r.Use(httpserver.NewRecoveryMiddleware(log, prometheus, debugMode)) + r.Use(httpserver.NewRequestLogMiddleware(log, tracer, prometheus)) + r.Use(httpserver.NewTracingMiddleware(tracer)) linkGroup := r.Group("/s") linkGroup.POST("/new", NewShortlinkCreateHandler(log, shortlinkService, host)) diff --git a/internal/http_server/middleware/recovery.go b/internal/http_server/middleware/recovery.go deleted file mode 100644 index 472e126..0000000 --- a/internal/http_server/middleware/recovery.go +++ /dev/null @@ -1,155 +0,0 @@ -package middleware - -// Modified recovery from gin, use own logger - -import ( - "backend/internal/integrations" - "backend/pkg/logger" - "bytes" - "errors" - "fmt" - "net" - "net/http" - "net/http/httputil" - "os" - "runtime" - "strings" - "time" - - "github.com/gin-gonic/gin" -) - -const ( - reset = "\033[0m" -) - -var ( - dunno = []byte("???") - centerDot = []byte("·") - dot = []byte(".") - slash = []byte("/") -) - -func NewRecoveryMiddleware(logger logger.Logger, prometheus *integrations.Prometheus, debugMode bool) gin.HandlerFunc { - handle := defaultHandleRecovery - return func(c *gin.Context) { - defer func() { - if err := recover(); err != nil { - prometheus.AddPanic() - - // Check for a broken connection, as it is not really a - // condition that warrants a panic stack trace. - var brokenPipe bool - if ne, ok := err.(*net.OpError); ok { - var se *os.SyscallError - if errors.As(ne, &se) { - seStr := strings.ToLower(se.Error()) - if strings.Contains(seStr, "broken pipe") || - strings.Contains(seStr, "connection reset by peer") { - brokenPipe = true - } - } - } - if logger != nil { - stack := stack(3) - httpRequest, _ := httputil.DumpRequest(c.Request, false) - headers := strings.Split(string(httpRequest), "\r\n") - for idx, header := range headers { - current := strings.Split(header, ":") - if current[0] == "Authorization" { - headers[idx] = current[0] + ": *" - } - } - headersToStr := strings.Join(headers, "\r\n") - if brokenPipe { - logger.Printf("%s\n%s%s", err, headersToStr, reset) - } else if debugMode { - logger.Printf("[Recovery] %s panic recovered:\n%s\n%s\n%s%s", - timeFormat(time.Now()), headersToStr, err, stack, reset) - } else { - logger.Printf("[Recovery] %s panic recovered:\n%s\n%s%s", - timeFormat(time.Now()), err, stack, reset) - } - } - if brokenPipe { - // If the connection is dead, we can't write a status to it. - c.Error(err.(error)) //nolint: errcheck - c.Abort() - } else { - handle(c, err) - } - } - }() - c.Next() - } -} - -func defaultHandleRecovery(c *gin.Context, _ any) { - c.AbortWithStatus(http.StatusInternalServerError) -} - -// stack returns a nicely formatted stack frame, skipping skip frames. -func stack(skip int) []byte { - buf := new(bytes.Buffer) // the returned data - // As we loop, we open files and read them. These variables record the currently - // loaded file. - var lines [][]byte - var lastFile string - for i := skip; ; i++ { // Skip the expected number of frames - pc, file, line, ok := runtime.Caller(i) - if !ok { - break - } - // Print this much at least. If we can't find the source, it won't show. - fmt.Fprintf(buf, "%s:%d (0x%x)\n", file, line, pc) - if file != lastFile { - data, err := os.ReadFile(file) - if err != nil { - continue - } - lines = bytes.Split(data, []byte{'\n'}) - lastFile = file - } - fmt.Fprintf(buf, "\t%s: %s\n", function(pc), source(lines, line)) - } - return buf.Bytes() -} - -// source returns a space-trimmed slice of the n'th line. -func source(lines [][]byte, n int) []byte { - n-- // in stack trace, lines are 1-indexed but our array is 0-indexed - if n < 0 || n >= len(lines) { - return dunno - } - return bytes.TrimSpace(lines[n]) -} - -// function returns, if possible, the name of the function containing the PC. -func function(pc uintptr) []byte { - fn := runtime.FuncForPC(pc) - if fn == nil { - return dunno - } - name := []byte(fn.Name()) - // The name includes the path name to the package, which is unnecessary - // since the file name is already included. Plus, it has center dots. - // That is, we see - // runtime/debug.*T·ptrmethod - // and want - // *T.ptrmethod - // Also the package path might contain dot (e.g. code.google.com/...), - // so first eliminate the path prefix - if lastSlash := bytes.LastIndex(name, slash); lastSlash >= 0 { - name = name[lastSlash+1:] - } - if period := bytes.Index(name, dot); period >= 0 { - name = name[period+1:] - } - name = bytes.ReplaceAll(name, centerDot, dot) - return name -} - -// timeFormat returns a customized time string for logger. -func timeFormat(t time.Time) string { - return t.Format("2006/01/02 - 15:04:05") -} diff --git a/internal/http_server/middleware/tracing.go b/internal/http_server/middleware/tracing.go deleted file mode 100644 index a739c3c..0000000 --- a/internal/http_server/middleware/tracing.go +++ /dev/null @@ -1,33 +0,0 @@ -package middleware - -import ( - "fmt" - - "github.com/gin-gonic/gin" - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/propagation" - "go.opentelemetry.io/otel/trace" -) - -func NewTracingMiddleware(tracer trace.Tracer) gin.HandlerFunc { - prop := otel.GetTextMapPropagator() - - return func(c *gin.Context) { - savedCtx := c.Request.Context() - defer func() { - c.Request = c.Request.WithContext(savedCtx) - }() - - ctx := prop.Extract(savedCtx, propagation.HeaderCarrier(c.Request.Header)) - - ctx, span := tracer.Start(ctx, fmt.Sprintf("%s %s", c.Request.Method, c.Request.URL.Path)) - defer span.End() - - traceId := span.SpanContext().TraceID() - c.Header("X-Trace-Id", traceId.String()) - - c.Request = c.Request.WithContext(ctx) - - c.Next() - } -} diff --git a/cmd/backend/server/middleware/recovery.go b/internal/http_server/recovery.go similarity index 99% rename from cmd/backend/server/middleware/recovery.go rename to internal/http_server/recovery.go index 472e126..4611dab 100644 --- a/cmd/backend/server/middleware/recovery.go +++ b/internal/http_server/recovery.go @@ -1,4 +1,4 @@ -package middleware +package httpserver // Modified recovery from gin, use own logger diff --git a/internal/http_server/middleware/request_log.go b/internal/http_server/request_log.go similarity index 98% rename from internal/http_server/middleware/request_log.go rename to internal/http_server/request_log.go index 814d86e..45123ca 100644 --- a/internal/http_server/middleware/request_log.go +++ b/internal/http_server/request_log.go @@ -1,4 +1,4 @@ -package middleware +package httpserver import ( "backend/internal/integrations" diff --git a/cmd/backend/server/middleware/tracing.go b/internal/http_server/tracing.go similarity index 97% rename from cmd/backend/server/middleware/tracing.go rename to internal/http_server/tracing.go index a739c3c..9781378 100644 --- a/cmd/backend/server/middleware/tracing.go +++ b/internal/http_server/tracing.go @@ -1,4 +1,4 @@ -package middleware +package httpserver import ( "fmt"