From b3c8dc1ef7d36c11be8346b6ddf6a65c1988a8e0 Mon Sep 17 00:00:00 2001 From: Matty Evans Date: Tue, 14 Jan 2025 11:57:11 +1000 Subject: [PATCH] feat(metrics): metrics srv disabled by default --- cmd/sentry/main.go | 37 +++++++-- cmd/sentry/main_test.go | 155 +++++++++++++++++++++++++++++++++++ pkg/config/v1/config.go | 4 + pkg/config/v1/config_test.go | 6 +- 4 files changed, 190 insertions(+), 12 deletions(-) create mode 100644 cmd/sentry/main_test.go diff --git a/cmd/sentry/main.go b/cmd/sentry/main.go index 389db9c..afcfd76 100644 --- a/cmd/sentry/main.go +++ b/cmd/sentry/main.go @@ -3,6 +3,7 @@ package main import ( "context" "fmt" + "net" "net/http" _ "net/http/pprof" //nolint:gosec // pprof only enabled if pprofAddr config is set. "os" @@ -211,24 +212,34 @@ func (s *contributoor) stop(ctx context.Context) error { } func (s *contributoor) startMetricsServer() error { + metricsHost, metricsPort := s.config.GetMetricsHostPort() + if metricsHost == "" { + return nil + } + + var addr = fmt.Sprintf(":%s", metricsPort) + + // Start listening before creating the server to catch invalid addresses. + listener, err := net.Listen("tcp", addr) + if err != nil { + return fmt.Errorf("failed to start metrics server: %w", err) + } + sm := http.NewServeMux() sm.Handle("/metrics", promhttp.Handler()) - var ( - _, metricsPort = s.config.GetMetricsHostPort() - addr = fmt.Sprintf(":%s", metricsPort) - ) - - s.metricsServer = &http.Server{ + server := &http.Server{ Addr: addr, ReadHeaderTimeout: 15 * time.Second, Handler: sm, } + s.metricsServer = server + s.log.Infof("Serving metrics at %s", addr) go func() { - if err := s.metricsServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { s.log.Fatal(err) } }() @@ -244,15 +255,23 @@ func (s *contributoor) startPProfServer() error { var addr = fmt.Sprintf(":%s", pprofPort) - s.pprofServer = &http.Server{ + // Start listening before creating the server to catch invalid addresses. + listener, err := net.Listen("tcp", addr) + if err != nil { + return fmt.Errorf("failed to start pprof server: %w", err) + } + + server := &http.Server{ Addr: addr, ReadHeaderTimeout: 120 * time.Second, } + s.pprofServer = server + s.log.Infof("Serving pprof at %s", addr) go func() { - if err := s.pprofServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { + if err := server.Serve(listener); err != nil && err != http.ErrServerClosed { s.log.Fatal(err) } }() diff --git a/cmd/sentry/main_test.go b/cmd/sentry/main_test.go new file mode 100644 index 0000000..8db30bc --- /dev/null +++ b/cmd/sentry/main_test.go @@ -0,0 +1,155 @@ +package main + +import ( + "fmt" + "net" + "net/http" + "testing" + "time" + + "github.com/ethpandaops/contributoor/pkg/config/v1" + "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStartMetricsServer(t *testing.T) { + tests := []struct { + name string + metricsAddr string + expectServer bool + expectError bool + }{ + { + name: "empty address skips server", + metricsAddr: "", + expectServer: false, + }, + { + name: "valid address starts server", + metricsAddr: "localhost:9090", + expectServer: true, + }, + { + name: "invalid address errors", + metricsAddr: "256.256.256.256:99999", + expectServer: false, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &contributoor{ + log: logrus.New(), + config: &config.Config{ + MetricsAddress: tt.metricsAddr, + }, + } + + err := s.startMetricsServer() + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + if tt.expectServer { + require.NotNil(t, s.metricsServer) + + waitForServer(t, s.metricsServer.Addr) + + client := http.Client{Timeout: time.Second} + + resp, err := client.Get(fmt.Sprintf("http://%s/metrics", s.metricsServer.Addr)) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + resp.Body.Close() + _ = s.metricsServer.Close() + } else { + assert.Nil(t, s.metricsServer) + } + }) + } +} + +func TestStartPProfServer(t *testing.T) { + tests := []struct { + name string + pprofAddr string + expectServer bool + expectError bool + }{ + { + name: "empty address skips server", + pprofAddr: "", + expectServer: false, + }, + { + name: "valid address starts server", + pprofAddr: "localhost:6060", + expectServer: true, + }, + { + name: "invalid address errors", + pprofAddr: "256.256.256.256:99999", + expectServer: false, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &contributoor{ + log: logrus.New(), + config: &config.Config{ + PprofAddress: tt.pprofAddr, + }, + } + + err := s.startPProfServer() + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + if tt.expectServer { + require.NotNil(t, s.pprofServer) + + waitForServer(t, s.pprofServer.Addr) + + client := http.Client{Timeout: time.Second} + + resp, err := client.Get(fmt.Sprintf("http://%s/debug/pprof/", s.pprofServer.Addr)) + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + + resp.Body.Close() + _ = s.pprofServer.Close() + } else { + assert.Nil(t, s.pprofServer) + } + }) + } +} + +func waitForServer(t *testing.T, addr string) { + t.Helper() + + deadline := time.Now().Add(2 * time.Second) + + for time.Now().Before(deadline) { + conn, err := net.DialTimeout("tcp", addr, 100*time.Millisecond) + if err == nil { + conn.Close() + + return + } + + time.Sleep(100 * time.Millisecond) + } + + t.Fatalf("Server at %s did not start within deadline", addr) +} diff --git a/pkg/config/v1/config.go b/pkg/config/v1/config.go index f95394e..b01cd3d 100644 --- a/pkg/config/v1/config.go +++ b/pkg/config/v1/config.go @@ -137,6 +137,10 @@ func ParseAddress(address, defaultHost, defaultPort string) (host, port string) // GetMetricsHostPort returns the metrics host and port. // If MetricsAddress is not set, returns default values. func (c *Config) GetMetricsHostPort() (host, port string) { + if c.MetricsAddress == "" { + return "", "" + } + return ParseAddress(c.MetricsAddress, defaultMetricsHost, defaultMetricsPort) } diff --git a/pkg/config/v1/config_test.go b/pkg/config/v1/config_test.go index f7b8a4a..76b9593 100644 --- a/pkg/config/v1/config_test.go +++ b/pkg/config/v1/config_test.go @@ -169,10 +169,10 @@ func TestConfig_GetMetricsHostPort(t *testing.T) { expectedPort string }{ { - name: "empty address returns defaults", + name: "empty address returns empty strings", config: &Config{MetricsAddress: ""}, - expectedHost: defaultMetricsHost, - expectedPort: defaultMetricsPort, + expectedHost: "", + expectedPort: "", }, { name: "port only",