From ebb724da262474ad2fea48a4207f1fbed308abcd Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Thu, 25 Jul 2024 22:57:42 -0700 Subject: [PATCH 01/11] Set up a pprof server with optional mutex profiling in cmdutil --- cmdutil/server.go | 79 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/cmdutil/server.go b/cmdutil/server.go index dbf6325b..a5001003 100644 --- a/cmdutil/server.go +++ b/cmdutil/server.go @@ -4,6 +4,9 @@ package cmdutil import ( "context" + "net/http" + "net/http/pprof" + "runtime" "github.com/oklog/run" ) @@ -93,3 +96,79 @@ func MultiServer(servers ...Server) Server { StopFunc: s.Stop, } } + +// NewPprofServer sets up a pprof server with optional mutex profiling. +// func NewPprofServer(addr string, enableMutexProfiling bool) Server { +// if addr == "" { +// addr = "127.0.0.1:9998" // Default port +// } + +// return NewContextServer(func(ctx context.Context) error { +// if enableMutexProfiling { +// runtime.SetMutexProfileFraction(2) +// } + +// pprofSrv := &http.Server{ +// Addr: addr, +// Handler: http.HandlerFunc(pprof.Index), +// } + +// go func() { +// <-ctx.Done() +// pprofSrv.Close() +// }() + +// return pprofSrv.ListenAndServe() +// }) +// } + +// ProfileConfig holds the configuration for the pprof server. +type ProfileConfig struct { + Addr string + Profiles []string +} + +// NewPprofServer sets up a pprof server with optional mutex profiling and configurable profiling types. +func NewPprofServer(config ProfileConfig) Server { + if config.Addr == "" { + config.Addr = "127.0.0.1:9998" // Default port + } + + return NewContextServer(func(ctx context.Context) error { + mux := http.NewServeMux() + + profileHandlers := map[string]http.HandlerFunc{ + "index": pprof.Index, + "cmdline": pprof.Cmdline, + "profile": pprof.Profile, + "symbol": pprof.Symbol, + "heap": pprof.Handler("heap").ServeHTTP, + "goroutine": pprof.Handler("goroutine").ServeHTTP, + "threadcreate": pprof.Handler("threadcreate").ServeHTTP, + "block": pprof.Handler("block").ServeHTTP, + "trace": pprof.Trace, + "mutex": pprof.Handler("mutex").ServeHTTP, + } + + for _, profile := range config.Profiles { + if handler, exists := profileHandlers[profile]; exists { + if profile == "mutex" { + runtime.SetMutexProfileFraction(2) + } + mux.HandleFunc("/debug/pprof/"+profile, handler) + } + } + + pprofSrv := &http.Server{ + Addr: config.Addr, + Handler: mux, + } + + go func() { + <-ctx.Done() + pprofSrv.Close() + }() + + return pprofSrv.ListenAndServe() + }) +} From bd669c73177f194e38ddc809adc2f592f80ed60b Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Thu, 25 Jul 2024 23:11:09 -0700 Subject: [PATCH 02/11] Set up a ReadHeaderTimeout --- cmdutil/server.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmdutil/server.go b/cmdutil/server.go index a5001003..4d8c6e7c 100644 --- a/cmdutil/server.go +++ b/cmdutil/server.go @@ -7,6 +7,7 @@ import ( "net/http" "net/http/pprof" "runtime" + "time" "github.com/oklog/run" ) @@ -160,8 +161,9 @@ func NewPprofServer(config ProfileConfig) Server { } pprofSrv := &http.Server{ - Addr: config.Addr, - Handler: mux, + Addr: config.Addr, + Handler: mux, + ReadHeaderTimeout: 5 * time.Second, } go func() { From 850c21a0ec73c3f0dfce6bec6dafcc86d3b91633 Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Fri, 2 Aug 2024 00:19:57 -0700 Subject: [PATCH 03/11] Put the method in debug package and improve it --- cmdutil/debug/debug.go | 84 ++++++++++++++++++++++++++++++++++++++++-- cmdutil/server.go | 81 ---------------------------------------- 2 files changed, 81 insertions(+), 84 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index f8a72306..cdd514d8 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -15,7 +15,11 @@ package debug import ( + "context" "fmt" + "net/http" + "runtime" + "time" "github.com/google/gops/agent" "github.com/sirupsen/logrus" @@ -36,9 +40,10 @@ func New(l logrus.FieldLogger, port int) *Server { // Server wraps a gops server for easy use with oklog/group. type Server struct { - logger logrus.FieldLogger - addr string - done chan struct{} + logger logrus.FieldLogger + addr string + done chan struct{} + pprofServer *http.Server } // Run starts the debug server. @@ -59,6 +64,14 @@ func (s *Server) Run() error { return err } + if s.pprofServer != nil { + go func() { + if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + s.logger.WithError(err).Error("pprof server error") + } + }() + } + <-s.done return nil } @@ -70,4 +83,69 @@ func (s *Server) Stop(_ error) { agent.Close() close(s.done) + + if s.pprofServer != nil { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := s.pprofServer.Shutdown(ctx); err != nil { + s.logger.WithError(err).Error("Error shutting down pprof server") + } + } +} + +// ProfileConfig holds the configuration for the pprof server. +type ProfileConfig struct { + Addr string + ProfileHandlers map[string]http.HandlerFunc +} + +// NewPprofServer sets up a pprof server with configurable profiling types and returns a Server instance. +func NewPprofServer(config ProfileConfig, l logrus.FieldLogger) *Server { + if config.Addr == "" { + config.Addr = "127.0.0.1:9998" // Default port + } + + // Create a new HTTP mux for handling pprof routes. + mux := http.NewServeMux() + + // Iterate over the profile handlers and add them to the mux. + for profile, handler := range config.ProfileHandlers { + if handler != nil { + if profile == "mutex" { + runtime.SetMutexProfileFraction(2) + } + mux.HandleFunc("/debug/pprof/"+profile, handler) + l.WithFields(logrus.Fields{ + "at": "adding", + "service": "pprof", + "profile": profile, + }).Info("Added pprof profile handler") + } else { + l.WithFields(logrus.Fields{ + "at": "ignoring", + "service": "pprof", + "profile": profile, + }).Warn("Unknown pprof profile type") + } + } + + l.WithFields(logrus.Fields{ + "at": "binding", + "service": "pprof", + "addr": config.Addr, + }).Info() + + // Create a new HTTP server for serving pprof endpoints. + httpServer := &http.Server{ + Addr: config.Addr, + Handler: mux, + ReadHeaderTimeout: 5 * time.Second, + } + + return &Server{ + logger: l, + addr: config.Addr, + done: make(chan struct{}), + pprofServer: httpServer, + } } diff --git a/cmdutil/server.go b/cmdutil/server.go index 4d8c6e7c..dbf6325b 100644 --- a/cmdutil/server.go +++ b/cmdutil/server.go @@ -4,10 +4,6 @@ package cmdutil import ( "context" - "net/http" - "net/http/pprof" - "runtime" - "time" "github.com/oklog/run" ) @@ -97,80 +93,3 @@ func MultiServer(servers ...Server) Server { StopFunc: s.Stop, } } - -// NewPprofServer sets up a pprof server with optional mutex profiling. -// func NewPprofServer(addr string, enableMutexProfiling bool) Server { -// if addr == "" { -// addr = "127.0.0.1:9998" // Default port -// } - -// return NewContextServer(func(ctx context.Context) error { -// if enableMutexProfiling { -// runtime.SetMutexProfileFraction(2) -// } - -// pprofSrv := &http.Server{ -// Addr: addr, -// Handler: http.HandlerFunc(pprof.Index), -// } - -// go func() { -// <-ctx.Done() -// pprofSrv.Close() -// }() - -// return pprofSrv.ListenAndServe() -// }) -// } - -// ProfileConfig holds the configuration for the pprof server. -type ProfileConfig struct { - Addr string - Profiles []string -} - -// NewPprofServer sets up a pprof server with optional mutex profiling and configurable profiling types. -func NewPprofServer(config ProfileConfig) Server { - if config.Addr == "" { - config.Addr = "127.0.0.1:9998" // Default port - } - - return NewContextServer(func(ctx context.Context) error { - mux := http.NewServeMux() - - profileHandlers := map[string]http.HandlerFunc{ - "index": pprof.Index, - "cmdline": pprof.Cmdline, - "profile": pprof.Profile, - "symbol": pprof.Symbol, - "heap": pprof.Handler("heap").ServeHTTP, - "goroutine": pprof.Handler("goroutine").ServeHTTP, - "threadcreate": pprof.Handler("threadcreate").ServeHTTP, - "block": pprof.Handler("block").ServeHTTP, - "trace": pprof.Trace, - "mutex": pprof.Handler("mutex").ServeHTTP, - } - - for _, profile := range config.Profiles { - if handler, exists := profileHandlers[profile]; exists { - if profile == "mutex" { - runtime.SetMutexProfileFraction(2) - } - mux.HandleFunc("/debug/pprof/"+profile, handler) - } - } - - pprofSrv := &http.Server{ - Addr: config.Addr, - Handler: mux, - ReadHeaderTimeout: 5 * time.Second, - } - - go func() { - <-ctx.Done() - pprofSrv.Close() - }() - - return pprofSrv.ListenAndServe() - }) -} From 3f6f3f61e778d6fadd68cf408db4379676c0969f Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Fri, 2 Aug 2024 11:57:36 -0700 Subject: [PATCH 04/11] Make PProfServer a seperate server and update the logging info --- cmdutil/debug/debug.go | 91 ++++++++++++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 26 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index cdd514d8..9d6ce532 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -19,6 +19,7 @@ import ( "fmt" "net/http" "runtime" + "strings" "time" "github.com/google/gops/agent" @@ -93,14 +94,23 @@ func (s *Server) Stop(_ error) { } } +// PProfServer wraps a pprof server. +type PProfServer struct { + logger logrus.FieldLogger + addr string + done chan struct{} + pprofServer *http.Server + profileConfig ProfileConfig +} + // ProfileConfig holds the configuration for the pprof server. type ProfileConfig struct { Addr string ProfileHandlers map[string]http.HandlerFunc } -// NewPprofServer sets up a pprof server with configurable profiling types and returns a Server instance. -func NewPprofServer(config ProfileConfig, l logrus.FieldLogger) *Server { +// NewPProfServer sets up a pprof server with configurable profiling types and returns a PProfServer instance. +func NewPProfServer(config ProfileConfig, l logrus.FieldLogger) *PProfServer { if config.Addr == "" { config.Addr = "127.0.0.1:9998" // Default port } @@ -108,44 +118,73 @@ func NewPprofServer(config ProfileConfig, l logrus.FieldLogger) *Server { // Create a new HTTP mux for handling pprof routes. mux := http.NewServeMux() - // Iterate over the profile handlers and add them to the mux. + // Iterate over the handlers and add them to the mux. for profile, handler := range config.ProfileHandlers { if handler != nil { if profile == "mutex" { runtime.SetMutexProfileFraction(2) } mux.HandleFunc("/debug/pprof/"+profile, handler) - l.WithFields(logrus.Fields{ - "at": "adding", - "service": "pprof", - "profile": profile, - }).Info("Added pprof profile handler") - } else { - l.WithFields(logrus.Fields{ - "at": "ignoring", - "service": "pprof", - "profile": profile, - }).Warn("Unknown pprof profile type") } } - l.WithFields(logrus.Fields{ - "at": "binding", - "service": "pprof", - "addr": config.Addr, - }).Info() - - // Create a new HTTP server for serving pprof endpoints. httpServer := &http.Server{ Addr: config.Addr, Handler: mux, ReadHeaderTimeout: 5 * time.Second, } - return &Server{ - logger: l, - addr: config.Addr, - done: make(chan struct{}), - pprofServer: httpServer, + return &PProfServer{ + logger: l, + addr: config.Addr, + done: make(chan struct{}), + pprofServer: httpServer, + profileConfig: config, + } +} + +// Run starts the pprof server. +// +// It implements oklog group's runFn. +func (s *PProfServer) Run() error { + s.logger.WithFields(logrus.Fields{ + "at": "binding", + "service": "pprof", + "addr": s.addr, + "profiles": strings.Join(s.getProfileNames(), ","), + }).Info() + + if s.pprofServer != nil { + go func() { + if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + s.logger.WithError(err).Error("pprof server error") + } + }() + } + + <-s.done + return nil +} + +// Stop shuts down the pprof server. +// +// It implements oklog group's interruptFn. +func (s *PProfServer) Stop(_ error) { + if s.pprofServer != nil { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := s.pprofServer.Shutdown(ctx); err != nil { + s.logger.WithError(err).Error("Error shutting down pprof server") + } + } + close(s.done) +} + +// getProfileNames returns the list of profile names configured. +func (s *PProfServer) getProfileNames() []string { + var profiles []string + for profile := range s.profileConfig.ProfileHandlers { + profiles = append(profiles, profile) } + return profiles } From 9b39c0df556a793b4a6f5a758d0c5b19296aa8a7 Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Fri, 2 Aug 2024 12:02:56 -0700 Subject: [PATCH 05/11] Remove unnecessary changes --- cmdutil/debug/debug.go | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index 9d6ce532..48a4b2be 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -41,10 +41,9 @@ func New(l logrus.FieldLogger, port int) *Server { // Server wraps a gops server for easy use with oklog/group. type Server struct { - logger logrus.FieldLogger - addr string - done chan struct{} - pprofServer *http.Server + logger logrus.FieldLogger + addr string + done chan struct{} } // Run starts the debug server. @@ -65,14 +64,6 @@ func (s *Server) Run() error { return err } - if s.pprofServer != nil { - go func() { - if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { - s.logger.WithError(err).Error("pprof server error") - } - }() - } - <-s.done return nil } @@ -84,14 +75,6 @@ func (s *Server) Stop(_ error) { agent.Close() close(s.done) - - if s.pprofServer != nil { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - if err := s.pprofServer.Shutdown(ctx); err != nil { - s.logger.WithError(err).Error("Error shutting down pprof server") - } - } } // PProfServer wraps a pprof server. From 51a1892c38e31128a228072e027faecbb00165d3 Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Fri, 2 Aug 2024 12:11:44 -0700 Subject: [PATCH 06/11] Run golangci-lint --- cmdutil/debug/debug.go | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index 48a4b2be..a44003a2 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -19,7 +19,6 @@ import ( "fmt" "net/http" "runtime" - "strings" "time" "github.com/google/gops/agent" @@ -131,10 +130,9 @@ func NewPProfServer(config ProfileConfig, l logrus.FieldLogger) *PProfServer { // It implements oklog group's runFn. func (s *PProfServer) Run() error { s.logger.WithFields(logrus.Fields{ - "at": "binding", - "service": "pprof", - "addr": s.addr, - "profiles": strings.Join(s.getProfileNames(), ","), + "at": "binding", + "service": "pprof", + "addr": s.addr, }).Info() if s.pprofServer != nil { @@ -162,12 +160,3 @@ func (s *PProfServer) Stop(_ error) { } close(s.done) } - -// getProfileNames returns the list of profile names configured. -func (s *PProfServer) getProfileNames() []string { - var profiles []string - for profile := range s.profileConfig.ProfileHandlers { - profiles = append(profiles, profile) - } - return profiles -} From 1a67670dffdee5caf080717926264170b4d5d3bd Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Mon, 5 Aug 2024 12:06:59 -0700 Subject: [PATCH 07/11] Updated PProfServerConfig, used pprof.Handler and included Run() err return --- cmdutil/debug/debug.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index a44003a2..816b06f4 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -18,6 +18,7 @@ import ( "context" "fmt" "net/http" + "net/http/pprof" "runtime" "time" @@ -82,17 +83,18 @@ type PProfServer struct { addr string done chan struct{} pprofServer *http.Server - profileConfig ProfileConfig + profileConfig PProfServerConfig } // ProfileConfig holds the configuration for the pprof server. -type ProfileConfig struct { - Addr string - ProfileHandlers map[string]http.HandlerFunc +type PProfServerConfig struct { + Addr string + ProfileNames []string + MutexProfileFraction int } // NewPProfServer sets up a pprof server with configurable profiling types and returns a PProfServer instance. -func NewPProfServer(config ProfileConfig, l logrus.FieldLogger) *PProfServer { +func NewPProfServer(config PProfServerConfig, l logrus.FieldLogger) *PProfServer { if config.Addr == "" { config.Addr = "127.0.0.1:9998" // Default port } @@ -101,13 +103,14 @@ func NewPProfServer(config ProfileConfig, l logrus.FieldLogger) *PProfServer { mux := http.NewServeMux() // Iterate over the handlers and add them to the mux. - for profile, handler := range config.ProfileHandlers { - if handler != nil { - if profile == "mutex" { - runtime.SetMutexProfileFraction(2) + for _, profile := range config.ProfileNames { + if profile == "mutex" { + if config.MutexProfileFraction == 0 { + config.MutexProfileFraction = 2 // Use default value of 2 if not set } - mux.HandleFunc("/debug/pprof/"+profile, handler) + runtime.SetMutexProfileFraction(config.MutexProfileFraction) } + mux.Handle("/debug/pprof/"+profile, pprof.Handler(profile)) } httpServer := &http.Server{ @@ -136,11 +139,9 @@ func (s *PProfServer) Run() error { }).Info() if s.pprofServer != nil { - go func() { - if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { - s.logger.WithError(err).Error("pprof server error") - } - }() + if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + return err + } } <-s.done From a7b309c2866f9f39cc87b79c88475afbf6d4ef9d Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Mon, 5 Aug 2024 16:03:26 -0700 Subject: [PATCH 08/11] Updated mutex and used pprof.Index --- cmdutil/debug/debug.go | 41 +++++++++++++++++------------------------ 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index 816b06f4..7c88ddd4 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -79,52 +79,45 @@ func (s *Server) Stop(_ error) { // PProfServer wraps a pprof server. type PProfServer struct { - logger logrus.FieldLogger - addr string - done chan struct{} - pprofServer *http.Server - profileConfig PProfServerConfig + logger logrus.FieldLogger + addr string + done chan struct{} + pprofServer *http.Server } // ProfileConfig holds the configuration for the pprof server. type PProfServerConfig struct { Addr string - ProfileNames []string MutexProfileFraction int } +// defaultMutexProfileFraction is the default value for MutexProfileFraction +const defaultMutexProfileFraction = 2 + // NewPProfServer sets up a pprof server with configurable profiling types and returns a PProfServer instance. func NewPProfServer(config PProfServerConfig, l logrus.FieldLogger) *PProfServer { if config.Addr == "" { config.Addr = "127.0.0.1:9998" // Default port } - // Create a new HTTP mux for handling pprof routes. - mux := http.NewServeMux() - - // Iterate over the handlers and add them to the mux. - for _, profile := range config.ProfileNames { - if profile == "mutex" { - if config.MutexProfileFraction == 0 { - config.MutexProfileFraction = 2 // Use default value of 2 if not set - } - runtime.SetMutexProfileFraction(config.MutexProfileFraction) - } - mux.Handle("/debug/pprof/"+profile, pprof.Handler(profile)) + // Use a local variable for the mutex profile fraction + mpf := defaultMutexProfileFraction + if config.MutexProfileFraction != 0 { + mpf = config.MutexProfileFraction } + runtime.SetMutexProfileFraction(mpf) httpServer := &http.Server{ Addr: config.Addr, - Handler: mux, + Handler: http.HandlerFunc(pprof.Index), ReadHeaderTimeout: 5 * time.Second, } return &PProfServer{ - logger: l, - addr: config.Addr, - done: make(chan struct{}), - pprofServer: httpServer, - profileConfig: config, + logger: l, + addr: config.Addr, + done: make(chan struct{}), + pprofServer: httpServer, } } From 37fed8c755de0628b46ab5bc142255cb71e4efa8 Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Tue, 6 Aug 2024 09:24:07 -0700 Subject: [PATCH 09/11] Run unit test --- cmdutil/debug/debug_test.go | 122 ++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 cmdutil/debug/debug_test.go diff --git a/cmdutil/debug/debug_test.go b/cmdutil/debug/debug_test.go new file mode 100644 index 00000000..1d31f988 --- /dev/null +++ b/cmdutil/debug/debug_test.go @@ -0,0 +1,122 @@ +package debug + +import ( + "net/http" + "runtime" + "testing" + "time" + + "github.com/sirupsen/logrus" +) + +func TestNewPProfServer(t *testing.T) { + logger := logrus.New() + + tests := []struct { + name string + config PProfServerConfig + expectedAddr string + expectedMutexFraction int + }{ + { + name: "DefaultAddr", + config: PProfServerConfig{}, + expectedAddr: "127.0.0.1:9998", + expectedMutexFraction: defaultMutexProfileFraction, + }, + { + name: "CustomAddr", + config: PProfServerConfig{Addr: "127.0.0.1:9090"}, + expectedAddr: "127.0.0.1:9090", + expectedMutexFraction: defaultMutexProfileFraction, + }, + { + name: "CustomMutexProfileFraction", + config: PProfServerConfig{MutexProfileFraction: 5}, + expectedAddr: "127.0.0.1:9998", + expectedMutexFraction: 5, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := NewPProfServer(tt.config, logger) + + // Check server address + if server.addr != tt.expectedAddr { + t.Errorf("NewPProfServer() addr = %v, want %v", server.addr, tt.expectedAddr) + } + + // Start the server + go func() { + if err := server.Run(); err != nil { + t.Errorf("NewPProfServer() run error = %v", err) + } + }() + + // Give the server a moment to start + time.Sleep(100 * time.Millisecond) + + // Check mutex profile fraction + if got := runtime.SetMutexProfileFraction(0); got != tt.expectedMutexFraction { + t.Errorf("runtime.SetMutexProfileFraction() = %v, want %v", got, tt.expectedMutexFraction) + } + runtime.SetMutexProfileFraction(tt.expectedMutexFraction) // Reset to the expected value + + // Perform HTTP GET requests to ensure the server is running and all handlers respond correctly + profiles := []string{"", "heap", "goroutine", "threadcreate", "block", "mutex"} + for _, profile := range profiles { + url := "http://" + server.addr + "/debug/pprof/" + profile + resp, err := http.Get(url) + if err != nil { + t.Errorf("http.Get(%s) error = %v", url, err) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("http.Get(%s) status = %v, want %v", url, resp.StatusCode, http.StatusOK) + } + } + + // urls := []string{ + // "http://" + server.addr + "/debug/pprof/", + // "http://" + server.addr + "/debug/pprof/heap", + // "http://" + server.addr + "/debug/pprof/goroutine", + // "http://" + server.addr + "/debug/pprof/threadcreate", + // "http://" + server.addr + "/debug/pprof/block", + // "http://" + server.addr + "/debug/pprof/mutex", + // } + + // // Perform HTTP GET requests to ensure the server is running and all handlers respond correctly + // client := &http.Client{} + // for _, url := range urls { + // t.Run("GET "+url, func(t *testing.T) { + // req, err := http.NewRequest("GET", url, nil) + // if err != nil { + // t.Errorf("http.NewRequest(%s) error = %v", url, err) + // } + + // resp, err := client.Do(req) + // if err != nil { + // t.Errorf("http.Client.Do() error = %v", err) + // } + + // if resp.StatusCode != http.StatusOK { + // t.Errorf("http.Client.Do() status = %v, want %v", resp.StatusCode, http.StatusOK) + // } + + // resp.Body.Close() + // }) + // } + + // Stop the server + server.Stop(nil) + + // Ensure the server is stopped + select { + case <-server.done: + // success + case <-time.After(1 * time.Second): + t.Fatal("server did not stop in time") + } + }) + } +} From ad16243d2ea9071ca10d6ceb5b9738948c136f0b Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Tue, 6 Aug 2024 09:26:29 -0700 Subject: [PATCH 10/11] Update unit test --- cmdutil/debug/debug_test.go | 69 +++++++++++++++---------------------- 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/cmdutil/debug/debug_test.go b/cmdutil/debug/debug_test.go index 1d31f988..1d169f2a 100644 --- a/cmdutil/debug/debug_test.go +++ b/cmdutil/debug/debug_test.go @@ -63,49 +63,36 @@ func TestNewPProfServer(t *testing.T) { } runtime.SetMutexProfileFraction(tt.expectedMutexFraction) // Reset to the expected value - // Perform HTTP GET requests to ensure the server is running and all handlers respond correctly - profiles := []string{"", "heap", "goroutine", "threadcreate", "block", "mutex"} - for _, profile := range profiles { - url := "http://" + server.addr + "/debug/pprof/" + profile - resp, err := http.Get(url) - if err != nil { - t.Errorf("http.Get(%s) error = %v", url, err) - } - if resp.StatusCode != http.StatusOK { - t.Errorf("http.Get(%s) status = %v, want %v", url, resp.StatusCode, http.StatusOK) - } + urls := []string{ + "http://" + server.addr + "/debug/pprof/", + "http://" + server.addr + "/debug/pprof/heap", + "http://" + server.addr + "/debug/pprof/goroutine", + "http://" + server.addr + "/debug/pprof/threadcreate", + "http://" + server.addr + "/debug/pprof/block", + "http://" + server.addr + "/debug/pprof/mutex", } - // urls := []string{ - // "http://" + server.addr + "/debug/pprof/", - // "http://" + server.addr + "/debug/pprof/heap", - // "http://" + server.addr + "/debug/pprof/goroutine", - // "http://" + server.addr + "/debug/pprof/threadcreate", - // "http://" + server.addr + "/debug/pprof/block", - // "http://" + server.addr + "/debug/pprof/mutex", - // } - - // // Perform HTTP GET requests to ensure the server is running and all handlers respond correctly - // client := &http.Client{} - // for _, url := range urls { - // t.Run("GET "+url, func(t *testing.T) { - // req, err := http.NewRequest("GET", url, nil) - // if err != nil { - // t.Errorf("http.NewRequest(%s) error = %v", url, err) - // } - - // resp, err := client.Do(req) - // if err != nil { - // t.Errorf("http.Client.Do() error = %v", err) - // } - - // if resp.StatusCode != http.StatusOK { - // t.Errorf("http.Client.Do() status = %v, want %v", resp.StatusCode, http.StatusOK) - // } - - // resp.Body.Close() - // }) - // } + // Perform HTTP GET requests to ensure the server is running and all handlers respond correctly + client := &http.Client{} + for _, url := range urls { + t.Run("GET "+url, func(t *testing.T) { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + t.Errorf("http.NewRequest(%s) error = %v", url, err) + } + + resp, err := client.Do(req) + if err != nil { + t.Errorf("http.Client.Do() error = %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("http.Client.Do() status = %v, want %v", resp.StatusCode, http.StatusOK) + } + + resp.Body.Close() + }) + } // Stop the server server.Stop(nil) From 60faef34f092bb3016f2734552a20c1c192e65c9 Mon Sep 17 00:00:00 2001 From: Sherry Yao Date: Tue, 6 Aug 2024 14:18:57 -0700 Subject: [PATCH 11/11] Considered pprofServer as nil and simplified test path for HTTP requests --- cmdutil/debug/debug.go | 10 +++++--- cmdutil/debug/debug_test.go | 49 +++++++++++++++---------------------- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/cmdutil/debug/debug.go b/cmdutil/debug/debug.go index 7c88ddd4..d43cc642 100644 --- a/cmdutil/debug/debug.go +++ b/cmdutil/debug/debug.go @@ -125,16 +125,18 @@ func NewPProfServer(config PProfServerConfig, l logrus.FieldLogger) *PProfServer // // It implements oklog group's runFn. func (s *PProfServer) Run() error { + if s.pprofServer == nil { + return fmt.Errorf("pprofServer is nil") + } + s.logger.WithFields(logrus.Fields{ "at": "binding", "service": "pprof", "addr": s.addr, }).Info() - if s.pprofServer != nil { - if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { - return err - } + if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + return err } <-s.done diff --git a/cmdutil/debug/debug_test.go b/cmdutil/debug/debug_test.go index 1d169f2a..350a08b0 100644 --- a/cmdutil/debug/debug_test.go +++ b/cmdutil/debug/debug_test.go @@ -63,36 +63,27 @@ func TestNewPProfServer(t *testing.T) { } runtime.SetMutexProfileFraction(tt.expectedMutexFraction) // Reset to the expected value - urls := []string{ - "http://" + server.addr + "/debug/pprof/", - "http://" + server.addr + "/debug/pprof/heap", - "http://" + server.addr + "/debug/pprof/goroutine", - "http://" + server.addr + "/debug/pprof/threadcreate", - "http://" + server.addr + "/debug/pprof/block", - "http://" + server.addr + "/debug/pprof/mutex", - } - - // Perform HTTP GET requests to ensure the server is running and all handlers respond correctly + // Perform HTTP GET request to the root path + url := "http://" + server.addr + "/debug/pprof/" client := &http.Client{} - for _, url := range urls { - t.Run("GET "+url, func(t *testing.T) { - req, err := http.NewRequest("GET", url, nil) - if err != nil { - t.Errorf("http.NewRequest(%s) error = %v", url, err) - } - - resp, err := client.Do(req) - if err != nil { - t.Errorf("http.Client.Do() error = %v", err) - } - - if resp.StatusCode != http.StatusOK { - t.Errorf("http.Client.Do() status = %v, want %v", resp.StatusCode, http.StatusOK) - } - - resp.Body.Close() - }) - } + + t.Run("GET "+url, func(t *testing.T) { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + t.Errorf("http.NewRequest(%s) error = %v", url, err) + } + + resp, err := client.Do(req) + if err != nil { + t.Errorf("http.Client.Do() error = %v", err) + } + + if resp.StatusCode != http.StatusOK { + t.Errorf("http.Client.Do() status = %v, want %v", resp.StatusCode, http.StatusOK) + } + + resp.Body.Close() + }) // Stop the server server.Stop(nil)