From fa7fd5f07657880a0a2a56cfa155ec9d789029f9 Mon Sep 17 00:00:00 2001 From: Quentin McGaw Date: Wed, 12 Apr 2023 10:14:05 +0000 Subject: [PATCH] fix(pprof): settings rates can be `nil` --- internal/configuration/sources/env/pprof.go | 4 +- internal/pprof/helpers_test.go | 2 + internal/pprof/server.go | 4 +- internal/pprof/server_test.go | 5 +- internal/pprof/settings.go | 24 +++---- internal/pprof/settings_test.go | 76 +++++++++++---------- 6 files changed, 63 insertions(+), 52 deletions(-) diff --git a/internal/configuration/sources/env/pprof.go b/internal/configuration/sources/env/pprof.go index 43fddeac..eef0f93f 100644 --- a/internal/configuration/sources/env/pprof.go +++ b/internal/configuration/sources/env/pprof.go @@ -12,12 +12,12 @@ func readPprof() (settings pprof.Settings, err error) { return settings, fmt.Errorf("environment variable PPROF_ENABLED: %w", err) } - settings.BlockProfileRate, err = envToInt("PPROF_BLOCK_PROFILE_RATE") + settings.BlockProfileRate, err = envToIntPtr("PPROF_BLOCK_PROFILE_RATE") if err != nil { return settings, fmt.Errorf("environment variable PPROF_BLOCK_PROFILE_RATE: %w", err) } - settings.MutexProfileRate, err = envToInt("PPROF_MUTEX_PROFILE_RATE") + settings.MutexProfileRate, err = envToIntPtr("PPROF_MUTEX_PROFILE_RATE") if err != nil { return settings, fmt.Errorf("environment variable PPROF_MUTEX_PROFILE_RATE: %w", err) } diff --git a/internal/pprof/helpers_test.go b/internal/pprof/helpers_test.go index 2e34f7c6..740d0a0c 100644 --- a/internal/pprof/helpers_test.go +++ b/internal/pprof/helpers_test.go @@ -8,6 +8,8 @@ import ( func boolPtr(b bool) *bool { return &b } +func intPtr(n int) *int { return &n } + var _ gomock.Matcher = (*regexMatcher)(nil) type regexMatcher struct { diff --git a/internal/pprof/server.go b/internal/pprof/server.go index 10cb7420..0ac4b793 100644 --- a/internal/pprof/server.go +++ b/internal/pprof/server.go @@ -13,8 +13,8 @@ import ( // with the settings given. It returns an error // if one of the settings is not valid. func New(settings Settings) (server *httpserver.Server, err error) { - runtime.SetBlockProfileRate(settings.BlockProfileRate) - runtime.SetMutexProfileFraction(settings.MutexProfileRate) + runtime.SetBlockProfileRate(*settings.BlockProfileRate) + runtime.SetMutexProfileFraction(*settings.MutexProfileRate) handler := http.NewServeMux() handler.HandleFunc("/debug/pprof/", pprof.Index) diff --git a/internal/pprof/server_test.go b/internal/pprof/server_test.go index a421da31..0a2d43bc 100644 --- a/internal/pprof/server_test.go +++ b/internal/pprof/server_test.go @@ -26,6 +26,8 @@ func Test_Server(t *testing.T) { const httpServerShutdownTimeout = 10 * time.Second // 10s in case test worker is slow settings := Settings{ + BlockProfileRate: intPtr(0), + MutexProfileRate: intPtr(0), HTTPServer: httpserver.Settings{ Address: address, Logger: logger, @@ -112,7 +114,8 @@ func Test_Server_BadSettings(t *testing.T) { t.Parallel() settings := Settings{ - BlockProfileRate: -1, + BlockProfileRate: intPtr(-1), + MutexProfileRate: intPtr(0), } server, err := New(settings) diff --git a/internal/pprof/settings.go b/internal/pprof/settings.go index 40f9faea..9e3031c1 100644 --- a/internal/pprof/settings.go +++ b/internal/pprof/settings.go @@ -16,10 +16,10 @@ type Settings struct { Enabled *bool // See runtime.SetBlockProfileRate // Set to 0 to disable profiling. - BlockProfileRate int + BlockProfileRate *int // See runtime.SetMutexProfileFraction // Set to 0 to disable profiling. - MutexProfileRate int + MutexProfileRate *int // HTTPServer contains settings to configure // the HTTP server serving pprof data. HTTPServer httpserver.Settings @@ -44,15 +44,15 @@ func (s Settings) Copy() (copied Settings) { func (s *Settings) MergeWith(other Settings) { s.Enabled = helpers.MergeWithBool(s.Enabled, other.Enabled) - s.BlockProfileRate = helpers.MergeWithInt(s.BlockProfileRate, other.BlockProfileRate) - s.MutexProfileRate = helpers.MergeWithInt(s.MutexProfileRate, other.MutexProfileRate) + s.BlockProfileRate = helpers.MergeWithIntPtr(s.BlockProfileRate, other.BlockProfileRate) + s.MutexProfileRate = helpers.MergeWithIntPtr(s.MutexProfileRate, other.MutexProfileRate) s.HTTPServer.MergeWith(other.HTTPServer) } func (s *Settings) OverrideWith(other Settings) { s.Enabled = helpers.OverrideWithBool(s.Enabled, other.Enabled) - s.BlockProfileRate = helpers.OverrideWithInt(s.BlockProfileRate, other.BlockProfileRate) - s.MutexProfileRate = helpers.OverrideWithInt(s.MutexProfileRate, other.MutexProfileRate) + s.BlockProfileRate = helpers.OverrideWithIntPtr(s.BlockProfileRate, other.BlockProfileRate) + s.MutexProfileRate = helpers.OverrideWithIntPtr(s.MutexProfileRate, other.MutexProfileRate) s.HTTPServer.OverrideWith(other.HTTPServer) } @@ -62,11 +62,11 @@ var ( ) func (s Settings) Validate() (err error) { - if s.BlockProfileRate < 0 { + if *s.BlockProfileRate < 0 { return ErrBlockProfileRateNegative } - if s.MutexProfileRate < 0 { + if *s.MutexProfileRate < 0 { return ErrMutexProfileRateNegative } @@ -80,12 +80,12 @@ func (s Settings) ToLinesNode() (node *gotree.Node) { node = gotree.New("Pprof settings:") - if s.BlockProfileRate > 0 { - node.Appendf("Block profile rate: %d", s.BlockProfileRate) + if *s.BlockProfileRate > 0 { + node.Appendf("Block profile rate: %d", *s.BlockProfileRate) } - if s.MutexProfileRate > 0 { - node.Appendf("Mutex profile rate: %d", s.MutexProfileRate) + if *s.MutexProfileRate > 0 { + node.Appendf("Mutex profile rate: %d", *s.MutexProfileRate) } node.AppendNode(s.HTTPServer.ToLinesNode()) diff --git a/internal/pprof/settings_test.go b/internal/pprof/settings_test.go index b229e8ab..9ede0dcc 100644 --- a/internal/pprof/settings_test.go +++ b/internal/pprof/settings_test.go @@ -31,8 +31,8 @@ func Test_Settings_SetDefaults(t *testing.T) { "non empty settings": { initial: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":6061", ReadHeaderTimeout: time.Second, @@ -42,8 +42,8 @@ func Test_Settings_SetDefaults(t *testing.T) { }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":6061", ReadHeaderTimeout: time.Second, @@ -77,8 +77,8 @@ func Test_Settings_Copy(t *testing.T) { "non empty settings": { initial: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":6061", ShutdownTimeout: time.Second, @@ -86,8 +86,8 @@ func Test_Settings_Copy(t *testing.T) { }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":6061", ShutdownTimeout: time.Second, @@ -120,16 +120,16 @@ func Test_Settings_MergeWith(t *testing.T) { "merge empty with filled": { other: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, @@ -138,16 +138,16 @@ func Test_Settings_MergeWith(t *testing.T) { "merge filled with empty": { settings: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, @@ -179,16 +179,16 @@ func Test_Settings_OverrideWith(t *testing.T) { "override empty with filled": { other: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, @@ -197,16 +197,16 @@ func Test_Settings_OverrideWith(t *testing.T) { "override filled with empty": { settings: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, @@ -215,24 +215,24 @@ func Test_Settings_OverrideWith(t *testing.T) { "override filled with filled": { settings: Settings{ Enabled: boolPtr(false), - BlockProfileRate: 1, - MutexProfileRate: 1, + BlockProfileRate: intPtr(1), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8001", }, }, other: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 2, - MutexProfileRate: 3, + BlockProfileRate: intPtr(2), + MutexProfileRate: intPtr(3), HTTPServer: httpserver.Settings{ Address: ":8002", }, }, expected: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 2, - MutexProfileRate: 3, + BlockProfileRate: intPtr(2), + MutexProfileRate: intPtr(3), HTTPServer: httpserver.Settings{ Address: ":8002", }, @@ -262,27 +262,33 @@ func Test_Settings_Validate(t *testing.T) { }{ "negative block profile rate": { settings: Settings{ - BlockProfileRate: -1, + BlockProfileRate: intPtr(-1), + MutexProfileRate: intPtr(0), }, errWrapped: ErrBlockProfileRateNegative, errMessage: ErrBlockProfileRateNegative.Error(), }, "negative mutex profile rate": { settings: Settings{ - MutexProfileRate: -1, + BlockProfileRate: intPtr(0), + MutexProfileRate: intPtr(-1), }, errWrapped: ErrMutexProfileRateNegative, errMessage: ErrMutexProfileRateNegative.Error(), }, "http server validation error": { settings: Settings{ - HTTPServer: httpserver.Settings{}, + BlockProfileRate: intPtr(0), + MutexProfileRate: intPtr(0), + HTTPServer: httpserver.Settings{}, }, errWrapped: address.ErrValueNotValid, errMessage: "value is not valid: missing port in address", }, "valid settings": { settings: Settings{ + BlockProfileRate: intPtr(0), + MutexProfileRate: intPtr(0), HTTPServer: httpserver.Settings{ Address: ":8000", Handler: http.NewServeMux(), @@ -325,8 +331,8 @@ func Test_Settings_String(t *testing.T) { "all values": { settings: Settings{ Enabled: boolPtr(true), - BlockProfileRate: 2, - MutexProfileRate: 1, + BlockProfileRate: intPtr(2), + MutexProfileRate: intPtr(1), HTTPServer: httpserver.Settings{ Address: ":8000", ShutdownTimeout: time.Second,