From f4c3b5528a2e63b5ac637c8e1583abd75b3da026 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Fri, 13 May 2022 22:46:26 +0200 Subject: [PATCH 1/7] PMM-5492 Adding pprof query parameter to /logs.zip --- .../json/client/server/logs_parameters.go | 38 +++++++++++++++++++ api/serverpb/json/header.json | 9 +++++ api/serverpb/json/serverpb.json | 9 +++++ api/swagger/swagger-dev.json | 9 +++++ api/swagger/swagger.json | 9 +++++ 5 files changed, 74 insertions(+) diff --git a/api/serverpb/json/client/server/logs_parameters.go b/api/serverpb/json/client/server/logs_parameters.go index 10cd0b4893..f06efc16ba 100644 --- a/api/serverpb/json/client/server/logs_parameters.go +++ b/api/serverpb/json/client/server/logs_parameters.go @@ -14,6 +14,7 @@ import ( "github.com/go-openapi/runtime" cr "github.com/go-openapi/runtime/client" "github.com/go-openapi/strfmt" + "github.com/go-openapi/swag" ) // NewLogsParams creates a new LogsParams object, @@ -58,6 +59,15 @@ func NewLogsParamsWithHTTPClient(client *http.Client) *LogsParams { Typically these are written to a http.Request. */ type LogsParams struct { + + /* Pprof. + + Include performance profiling data, + + Format: int32 + */ + Pprof *int32 + timeout time.Duration Context context.Context HTTPClient *http.Client @@ -111,6 +121,17 @@ func (o *LogsParams) SetHTTPClient(client *http.Client) { o.HTTPClient = client } +// WithPprof adds the pprof to the logs params +func (o *LogsParams) WithPprof(pprof *int32) *LogsParams { + o.SetPprof(pprof) + return o +} + +// SetPprof adds the pprof to the logs params +func (o *LogsParams) SetPprof(pprof *int32) { + o.Pprof = pprof +} + // WriteToRequest writes these params to a swagger request func (o *LogsParams) WriteToRequest(r runtime.ClientRequest, reg strfmt.Registry) error { @@ -119,6 +140,23 @@ func (o *LogsParams) WriteToRequest(r runtime.ClientRequest, reg strfmt.Registry } var res []error + if o.Pprof != nil { + + // query param pprof + var qrPprof int32 + + if o.Pprof != nil { + qrPprof = *o.Pprof + } + qPprof := swag.FormatInt32(qrPprof) + if qPprof != "" { + + if err := r.SetQueryParam("pprof", qPprof); err != nil { + return err + } + } + } + if len(res) > 0 { return errors.CompositeValidationError(res...) } diff --git a/api/serverpb/json/header.json b/api/serverpb/json/header.json index ad56799b64..333f40efd9 100644 --- a/api/serverpb/json/header.json +++ b/api/serverpb/json/header.json @@ -17,6 +17,15 @@ "description": "Returns the PMM Server logs.", "summary": "Logs", "operationId": "Logs", + "parameters": [ + { + "type": "integer", + "format": "int32", + "description": "Include performance profiling data,", + "name": "pprof", + "in": "query" + } + ], "produces": ["application/zip"], "responses": { "200": { diff --git a/api/serverpb/json/serverpb.json b/api/serverpb/json/serverpb.json index f2b1e1a2d9..cee9ca0bc1 100644 --- a/api/serverpb/json/serverpb.json +++ b/api/serverpb/json/serverpb.json @@ -26,6 +26,15 @@ ], "summary": "Logs", "operationId": "Logs", + "parameters": [ + { + "type": "integer", + "format": "int32", + "description": "Include performance profiling data,", + "name": "pprof", + "in": "query" + } + ], "responses": { "200": { "description": "A successful response.", diff --git a/api/swagger/swagger-dev.json b/api/swagger/swagger-dev.json index 50f63f3665..07e5cbbde7 100644 --- a/api/swagger/swagger-dev.json +++ b/api/swagger/swagger-dev.json @@ -27,6 +27,15 @@ ], "summary": "Logs", "operationId": "Logs", + "parameters": [ + { + "type": "integer", + "format": "int32", + "description": "Include performance profiling data,", + "name": "pprof", + "in": "query" + } + ], "responses": { "200": { "description": "A successful response.", diff --git a/api/swagger/swagger.json b/api/swagger/swagger.json index c0a0822df5..90ac6d2343 100644 --- a/api/swagger/swagger.json +++ b/api/swagger/swagger.json @@ -26,6 +26,15 @@ ], "summary": "Logs", "operationId": "Logs", + "parameters": [ + { + "type": "integer", + "format": "int32", + "description": "Include performance profiling data,", + "name": "pprof", + "in": "query" + } + ], "responses": { "200": { "description": "A successful response.", From 09f121d3db4b6420b374a12b290581406457c435 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Tue, 7 Jun 2022 22:56:48 +0200 Subject: [PATCH 2/7] PMM-5492 Adding pprof query parameter to /logs.zip --- admin/commands/summary.go | 12 ++++++++---- api/serverpb/json/client/server/logs_parameters.go | 2 -- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/admin/commands/summary.go b/admin/commands/summary.go index e5b3639977..022e1b826f 100644 --- a/admin/commands/summary.go +++ b/admin/commands/summary.go @@ -153,9 +153,14 @@ func addClientData(ctx context.Context, zipW *zip.Writer) { } // addServerData adds logs.zip from PMM Server to zip file. -func addServerData(ctx context.Context, zipW *zip.Writer) { +func addServerData(ctx context.Context, zipW *zip.Writer, usePprof bool) { + var pprof int32 + if usePprof { + pprof = 1 + } + var buf bytes.Buffer - _, err := client.Default.Server.Logs(&server.LogsParams{Context: ctx}, &buf) + _, err := client.Default.Server.Logs(&server.LogsParams{Context: ctx, Pprof: &pprof, HTTPClient: nil}, &buf) if err != nil { logrus.Errorf("%s", err) return @@ -264,7 +269,6 @@ func addPprofData(ctx context.Context, zipW *zip.Writer, skipServer bool) { "client/pprof/pmm-agent": fmt.Sprintf("http://%s:%d/debug/pprof", agentlocal.Localhost, GlobalFlags.PMMAgentListenPort), } if !skipServer { - sources["server/pprof/pmm-managed"] = fmt.Sprintf("http://%s:7773/debug/pprof", agentlocal.Localhost) sources["server/pprof/qan-api2"] = fmt.Sprintf("http://%s:9933/debug/pprof", agentlocal.Localhost) } @@ -332,7 +336,7 @@ func (cmd *summaryCommand) makeArchive(ctx context.Context) (err error) { } if !cmd.SkipServer { - addServerData(ctx, zipW) + addServerData(ctx, zipW, cmd.Pprof) } return //nolint:nakedret diff --git a/api/serverpb/json/client/server/logs_parameters.go b/api/serverpb/json/client/server/logs_parameters.go index 618d06476f..39cf28a110 100644 --- a/api/serverpb/json/client/server/logs_parameters.go +++ b/api/serverpb/json/client/server/logs_parameters.go @@ -59,7 +59,6 @@ func NewLogsParamsWithHTTPClient(client *http.Client) *LogsParams { Typically these are written to a http.Request. */ type LogsParams struct { - /* Pprof. Include performance profiling data, @@ -149,7 +148,6 @@ func (o *LogsParams) WriteToRequest(r runtime.ClientRequest, reg strfmt.Registry } qPprof := swag.FormatInt32(qrPprof) if qPprof != "" { - if err := r.SetQueryParam("pprof", qPprof); err != nil { return err } From e9f17b023c2d5ee1ace5bcaa5d5b3372c26ba6a7 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Mon, 13 Jun 2022 21:16:22 +0200 Subject: [PATCH 3/7] PMM-5492 Code review adjustments. --- admin/commands/summary.go | 7 +---- .../client/mock_defaults_file_parser_test.go | 30 +++++++++++++++++++ .../json/client/server/logs_parameters.go | 12 ++++---- api/serverpb/json/header.json | 3 +- api/serverpb/json/serverpb.json | 3 +- api/swagger/swagger-dev.json | 3 +- api/swagger/swagger.json | 3 +- 7 files changed, 40 insertions(+), 21 deletions(-) create mode 100644 agent/client/mock_defaults_file_parser_test.go diff --git a/admin/commands/summary.go b/admin/commands/summary.go index dbd93c81b5..501e9e77c0 100644 --- a/admin/commands/summary.go +++ b/admin/commands/summary.go @@ -154,13 +154,8 @@ func addClientData(ctx context.Context, zipW *zip.Writer) { // addServerData adds logs.zip from PMM Server to zip file. func addServerData(ctx context.Context, zipW *zip.Writer, usePprof bool) { - var pprof int32 - if usePprof { - pprof = 1 - } - var buf bytes.Buffer - _, err := client.Default.Server.Logs(&server.LogsParams{Context: ctx, Pprof: &pprof, HTTPClient: nil}, &buf) + _, err := client.Default.Server.Logs(&server.LogsParams{Context: ctx, Pprof: &usePprof, HTTPClient: nil}, &buf) if err != nil { logrus.Errorf("%s", err) return diff --git a/agent/client/mock_defaults_file_parser_test.go b/agent/client/mock_defaults_file_parser_test.go new file mode 100644 index 0000000000..cd04e795e8 --- /dev/null +++ b/agent/client/mock_defaults_file_parser_test.go @@ -0,0 +1,30 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +package client + +import ( + mock "github.com/stretchr/testify/mock" + + agentpb "github.com/percona/pmm/api/agentpb" +) + +// mockDefaultsFileParser is an autogenerated mock type for the defaultsFileParser type +type mockDefaultsFileParser struct { + mock.Mock +} + +// ParseDefaultsFile provides a mock function with given fields: req +func (_m *mockDefaultsFileParser) ParseDefaultsFile(req *agentpb.ParseDefaultsFileRequest) *agentpb.ParseDefaultsFileResponse { + ret := _m.Called(req) + + var r0 *agentpb.ParseDefaultsFileResponse + if rf, ok := ret.Get(0).(func(*agentpb.ParseDefaultsFileRequest) *agentpb.ParseDefaultsFileResponse); ok { + r0 = rf(req) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*agentpb.ParseDefaultsFileResponse) + } + } + + return r0 +} diff --git a/api/serverpb/json/client/server/logs_parameters.go b/api/serverpb/json/client/server/logs_parameters.go index 39cf28a110..96d552d3d6 100644 --- a/api/serverpb/json/client/server/logs_parameters.go +++ b/api/serverpb/json/client/server/logs_parameters.go @@ -62,10 +62,8 @@ type LogsParams struct { /* Pprof. Include performance profiling data, - - Format: int32 */ - Pprof *int32 + Pprof *bool timeout time.Duration Context context.Context @@ -121,13 +119,13 @@ func (o *LogsParams) SetHTTPClient(client *http.Client) { } // WithPprof adds the pprof to the logs params -func (o *LogsParams) WithPprof(pprof *int32) *LogsParams { +func (o *LogsParams) WithPprof(pprof *bool) *LogsParams { o.SetPprof(pprof) return o } // SetPprof adds the pprof to the logs params -func (o *LogsParams) SetPprof(pprof *int32) { +func (o *LogsParams) SetPprof(pprof *bool) { o.Pprof = pprof } @@ -141,12 +139,12 @@ func (o *LogsParams) WriteToRequest(r runtime.ClientRequest, reg strfmt.Registry if o.Pprof != nil { // query param pprof - var qrPprof int32 + var qrPprof bool if o.Pprof != nil { qrPprof = *o.Pprof } - qPprof := swag.FormatInt32(qrPprof) + qPprof := swag.FormatBool(qrPprof) if qPprof != "" { if err := r.SetQueryParam("pprof", qPprof); err != nil { return err diff --git a/api/serverpb/json/header.json b/api/serverpb/json/header.json index 333f40efd9..3dbc7c393a 100644 --- a/api/serverpb/json/header.json +++ b/api/serverpb/json/header.json @@ -19,8 +19,7 @@ "operationId": "Logs", "parameters": [ { - "type": "integer", - "format": "int32", + "type": "boolean", "description": "Include performance profiling data,", "name": "pprof", "in": "query" diff --git a/api/serverpb/json/serverpb.json b/api/serverpb/json/serverpb.json index 5dc00bcf16..28c3b92a85 100644 --- a/api/serverpb/json/serverpb.json +++ b/api/serverpb/json/serverpb.json @@ -28,8 +28,7 @@ "operationId": "Logs", "parameters": [ { - "type": "integer", - "format": "int32", + "type": "boolean", "description": "Include performance profiling data,", "name": "pprof", "in": "query" diff --git a/api/swagger/swagger-dev.json b/api/swagger/swagger-dev.json index e590519122..d4f9c3aab9 100644 --- a/api/swagger/swagger-dev.json +++ b/api/swagger/swagger-dev.json @@ -29,8 +29,7 @@ "operationId": "Logs", "parameters": [ { - "type": "integer", - "format": "int32", + "type": "boolean", "description": "Include performance profiling data,", "name": "pprof", "in": "query" diff --git a/api/swagger/swagger.json b/api/swagger/swagger.json index b6834275d6..296c85d69e 100644 --- a/api/swagger/swagger.json +++ b/api/swagger/swagger.json @@ -28,8 +28,7 @@ "operationId": "Logs", "parameters": [ { - "type": "integer", - "format": "int32", + "type": "boolean", "description": "Include performance profiling data,", "name": "pprof", "in": "query" From 4af4928e8a001cfa5f895c515eeb0c43a5a4f443 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Sun, 3 Jul 2022 17:48:04 +0200 Subject: [PATCH 4/7] PMM-5492 Pprof tool implementation. --- managed/main.go | 23 ++++++- managed/services/supervisord/logs.go | 49 ++++++++++++-- managed/services/supervisord/logs_test.go | 4 +- managed/utils/pprof/pprof.go | 79 +++++++++++++++++++++++ managed/utils/pprof/pprof_config.go | 31 +++++++++ managed/utils/pprof/pprof_test.go | 73 +++++++++++++++++++++ 6 files changed, 251 insertions(+), 8 deletions(-) create mode 100644 managed/utils/pprof/pprof.go create mode 100644 managed/utils/pprof/pprof_config.go create mode 100644 managed/utils/pprof/pprof_test.go diff --git a/managed/main.go b/managed/main.go index f46f6b9607..b2c0118bac 100644 --- a/managed/main.go +++ b/managed/main.go @@ -92,6 +92,7 @@ import ( "github.com/percona/pmm/managed/utils/clean" "github.com/percona/pmm/managed/utils/interceptors" "github.com/percona/pmm/managed/utils/logger" + "github.com/percona/pmm/managed/utils/pprof" pmmerrors "github.com/percona/pmm/utils/errors" "github.com/percona/pmm/utils/sqlmetrics" "github.com/percona/pmm/version" @@ -107,14 +108,32 @@ const ( cleanInterval = 10 * time.Minute cleanOlderThan = 30 * time.Minute + + defaultContextTimeout = 10 * time.Second + pProfProfileDuration = 30 * time.Second + pProfTraceDuration = 10 * time.Second ) func addLogsHandler(mux *http.ServeMux, logs *supervisord.Logs) { l := logrus.WithField("component", "logs.zip") mux.HandleFunc("/logs.zip", func(rw http.ResponseWriter, req *http.Request) { + contextTimeout := defaultContextTimeout + // increase context timeout if pprof query parameter exist in request + pprofQueryParameter, err := strconv.ParseBool(req.FormValue("pprof")) + if err != nil { + l.Debug("Unable to read 'pprof' query param. Using default: pprof=false") + } + var pprofConfig *pprof.Config + if pprofQueryParameter { + contextTimeout += pProfProfileDuration + pProfTraceDuration + pprofConfig = &pprof.Config{ + ProfileDuration: pProfProfileDuration, + TraceDuration: pProfTraceDuration, + } + } // fail-safe - ctx, cancel := context.WithTimeout(req.Context(), 10*time.Second) + ctx, cancel := context.WithTimeout(req.Context(), contextTimeout) defer cancel() filename := fmt.Sprintf("pmm-server_%s.zip", time.Now().UTC().Format("2006-01-02_15-04")) @@ -123,7 +142,7 @@ func addLogsHandler(mux *http.ServeMux, logs *supervisord.Logs) { rw.Header().Set(`Content-Disposition`, `attachment; filename="`+filename+`"`) ctx = logger.Set(ctx, "logs") - if err := logs.Zip(ctx, rw); err != nil { + if err := logs.Zip(ctx, rw, pprofConfig); err != nil { l.Errorf("%+v", err) } }) diff --git a/managed/services/supervisord/logs.go b/managed/services/supervisord/logs.go index 5513a1bf4c..3344b5e82f 100644 --- a/managed/services/supervisord/logs.go +++ b/managed/services/supervisord/logs.go @@ -31,12 +31,14 @@ import ( "os/exec" "path/filepath" "sort" + "sync" "time" "github.com/pkg/errors" "golang.org/x/sys/unix" "github.com/percona/pmm/managed/utils/logger" + pprofUtils "github.com/percona/pmm/managed/utils/pprof" "github.com/percona/pmm/utils/pdeathsig" ) @@ -69,7 +71,7 @@ func NewLogs(pmmVersion string, pmmUpdateChecker *PMMUpdateChecker) *Logs { } // Zip creates .zip archive with all logs. -func (l *Logs) Zip(ctx context.Context, w io.Writer) error { +func (l *Logs) Zip(ctx context.Context, w io.Writer, pprofConfig *pprofUtils.Config) error { start := time.Now() log := logger.Get(ctx).WithField("component", "logs") log.WithField("d", time.Since(start).Seconds()).Info("Starting...") @@ -80,7 +82,7 @@ func (l *Logs) Zip(ctx context.Context, w io.Writer) error { zw := zip.NewWriter(w) now := time.Now().UTC() - files := l.files(ctx) + files := l.files(ctx, pprofConfig) log.WithField("d", time.Since(start).Seconds()).Infof("Collected %d files.", len(files)) for _, file := range files { @@ -127,8 +129,8 @@ func (l *Logs) Zip(ctx context.Context, w io.Writer) error { return nil } -// files reads log/config files and returns content. -func (l *Logs) files(ctx context.Context) []fileContent { +// files reads log/config/pprof files and returns content. +func (l *Logs) files(ctx context.Context, pprofConfig *pprofUtils.Config) []fileContent { files := make([]fileContent, 0, 20) // add logs @@ -214,6 +216,45 @@ func (l *Logs) files(ctx context.Context) []fileContent { Err: err, }) + // add pprof + if pprofConfig != nil { + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + traceBytes, err := pprofUtils.Trace(pprofConfig.TraceDuration) + files = append(files, fileContent{ + Name: "pprof/trace.out", + Data: traceBytes, + Err: err, + }) + }() + + wg.Add(1) + go func() { + defer wg.Done() + profileBytes, err := pprofUtils.Profile(pprofConfig.ProfileDuration) + files = append(files, fileContent{ + Name: "pprof/profile.pb.gz", + Data: profileBytes, + Err: err, + }) + }() + + wg.Add(1) + go func() { + defer wg.Done() + heapBytes, err := pprofUtils.Heap(true) + files = append(files, fileContent{ + Name: "pprof/heap.pb.gz", + Data: heapBytes, + Err: err, + }) + }() + + wg.Wait() + } + sort.Slice(files, func(i, j int) bool { return files[i].Name < files[j].Name }) return files } diff --git a/managed/services/supervisord/logs_test.go b/managed/services/supervisord/logs_test.go index 1c75bffb67..c6615bd1c0 100644 --- a/managed/services/supervisord/logs_test.go +++ b/managed/services/supervisord/logs_test.go @@ -124,7 +124,7 @@ func TestFiles(t *testing.T) { l := NewLogs("2.4.5", checker) ctx := logger.Set(context.Background(), t.Name()) - files := l.files(ctx) + files := l.files(ctx, nil) actual := make([]string, 0, len(files)) for _, f := range files { // present only after update @@ -157,7 +157,7 @@ func TestZip(t *testing.T) { ctx := logger.Set(context.Background(), t.Name()) var buf bytes.Buffer - require.NoError(t, l.Zip(ctx, &buf)) + require.NoError(t, l.Zip(ctx, &buf, nil)) reader := bytes.NewReader(buf.Bytes()) r, err := zip.NewReader(reader, reader.Size()) require.NoError(t, err) diff --git a/managed/utils/pprof/pprof.go b/managed/utils/pprof/pprof.go new file mode 100644 index 0000000000..de1486daaf --- /dev/null +++ b/managed/utils/pprof/pprof.go @@ -0,0 +1,79 @@ +// pmm-managed +// Copyright (C) 2017 Percona LLC +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package pprof + +import ( + "bytes" + "fmt" + "runtime" + "runtime/pprof" + "runtime/trace" + "time" +) + +// Profile responds with the pprof-formatted cpu profile. +// Profiling lasts for duration specified in seconds. +func Profile(duration time.Duration) ([]byte, error) { + var profileBuf bytes.Buffer + if err := pprof.StartCPUProfile(&profileBuf); err != nil { + return nil, err + } + + time.Sleep(duration) + pprof.StopCPUProfile() + + return profileBuf.Bytes(), nil +} + +// Trace responds with the execution trace in binary form. +// Tracing lasts for duration specified in seconds. +func Trace(duration time.Duration) ([]byte, error) { + var traceBuf bytes.Buffer + if err := trace.Start(&traceBuf); err != nil { + return nil, err + } + + time.Sleep(duration) + trace.Stop() + + return traceBuf.Bytes(), nil +} + +// Heap responds with the pprof-formatted profile named "heap". +// listing the available profiles. +// You can specify the gc parameter to run gc before taking the heap sample. +func Heap(gc bool) ([]byte, error) { + var heapBuf bytes.Buffer + debug := 0 + profile := "heap" + + p := pprof.Lookup(profile) + if p == nil { + return nil, fmt.Errorf("profile cannot be found: %s", profile) + } + + if gc { + runtime.GC() + } + + err := p.WriteTo(&heapBuf, debug) + if err != nil { + return nil, err + } + + return heapBuf.Bytes(), nil +} diff --git a/managed/utils/pprof/pprof_config.go b/managed/utils/pprof/pprof_config.go new file mode 100644 index 0000000000..03709795ed --- /dev/null +++ b/managed/utils/pprof/pprof_config.go @@ -0,0 +1,31 @@ +// pmm-managed +// Copyright (C) 2017 Percona LLC +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package pprof + +import ( + "time" +) + +// Config pprof settings. +type Config struct { + ProfileDuration time.Duration `yaml:"profile_duration"` //nolint:tagliatelle + TraceDuration time.Duration `yaml:"trace_duration"` //nolint:tagliatelle +} + +// Init pprof config init. +func (c *Config) Init() { +} diff --git a/managed/utils/pprof/pprof_test.go b/managed/utils/pprof/pprof_test.go new file mode 100644 index 0000000000..ac8cf193c6 --- /dev/null +++ b/managed/utils/pprof/pprof_test.go @@ -0,0 +1,73 @@ +// pmm-managed +// Copyright (C) 2017 Percona LLC +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package pprof + +import ( + "bytes" + "compress/gzip" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestHeap(t *testing.T) { + t.Parallel() + t.Run("Heap test", func(t *testing.T) { + heapBytes, err := Heap(true) + assert.NoError(t, err) + + // read gzip + reader, err := gzip.NewReader(bytes.NewBuffer(heapBytes)) + assert.NoError(t, err) + + var resB bytes.Buffer + _, err = resB.ReadFrom(reader) + assert.NoError(t, err) + assert.NotEmpty(t, resB.Bytes()) + }) +} + +func TestProfile(t *testing.T) { + t.Parallel() + t.Run("Profile test", func(t *testing.T) { + profileBytes, err := Profile(1 * time.Second) + + assert.NoError(t, err) + assert.NotEmpty(t, profileBytes) + + // read gzip + reader, err := gzip.NewReader(bytes.NewBuffer(profileBytes)) + assert.NoError(t, err) + + var resB bytes.Buffer + _, err = resB.ReadFrom(reader) + assert.NoError(t, err) + + assert.NotEmpty(t, resB.Bytes()) + }) +} + +func TestTrace(t *testing.T) { + t.Parallel() + t.Run("Trace test", func(t *testing.T) { + traceBytes, err := Trace(1 * time.Second) + + assert.NoError(t, err) + assert.NotEmpty(t, traceBytes) + }) +} From b4619904ad2f6a6f69bf84cc55fbb933a6014ed3 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Sun, 3 Jul 2022 18:02:05 +0200 Subject: [PATCH 5/7] PMM-5492 Linter fixes. --- managed/utils/pprof/pprof_config.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/managed/utils/pprof/pprof_config.go b/managed/utils/pprof/pprof_config.go index 03709795ed..a68250b93e 100644 --- a/managed/utils/pprof/pprof_config.go +++ b/managed/utils/pprof/pprof_config.go @@ -16,9 +16,7 @@ package pprof -import ( - "time" -) +import "time" // Config pprof settings. type Config struct { From 1231760c6010077435d490ac5d2f4bb4e43929d7 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Mon, 11 Jul 2022 20:05:10 +0200 Subject: [PATCH 6/7] PMM-5492 Code review adjustments. --- admin/commands/summary.go | 2 +- managed/main.go | 5 +-- managed/services/supervisord/logs.go | 8 ++-- .../supervisord}/pprof_config.go | 14 +++---- managed/utils/pprof/pprof.go | 29 +++++++++----- managed/utils/pprof/pprof_test.go | 38 ++++++++++++++++++- 6 files changed, 67 insertions(+), 29 deletions(-) rename managed/{utils/pprof => services/supervisord}/pprof_config.go (72%) diff --git a/admin/commands/summary.go b/admin/commands/summary.go index 501e9e77c0..c3918cdb02 100644 --- a/admin/commands/summary.go +++ b/admin/commands/summary.go @@ -155,7 +155,7 @@ func addClientData(ctx context.Context, zipW *zip.Writer) { // addServerData adds logs.zip from PMM Server to zip file. func addServerData(ctx context.Context, zipW *zip.Writer, usePprof bool) { var buf bytes.Buffer - _, err := client.Default.Server.Logs(&server.LogsParams{Context: ctx, Pprof: &usePprof, HTTPClient: nil}, &buf) + _, err := client.Default.Server.Logs(&server.LogsParams{Context: ctx, Pprof: &usePprof}, &buf) if err != nil { logrus.Errorf("%s", err) return diff --git a/managed/main.go b/managed/main.go index 8bc3398fe7..60625a3c2d 100644 --- a/managed/main.go +++ b/managed/main.go @@ -94,7 +94,6 @@ import ( "github.com/percona/pmm/managed/utils/interceptors" "github.com/percona/pmm/managed/utils/logger" platformClient "github.com/percona/pmm/managed/utils/platform" - "github.com/percona/pmm/managed/utils/pprof" pmmerrors "github.com/percona/pmm/utils/errors" "github.com/percona/pmm/utils/sqlmetrics" "github.com/percona/pmm/version" @@ -126,10 +125,10 @@ func addLogsHandler(mux *http.ServeMux, logs *supervisord.Logs) { if err != nil { l.Debug("Unable to read 'pprof' query param. Using default: pprof=false") } - var pprofConfig *pprof.Config + var pprofConfig *supervisord.PprofConfig if pprofQueryParameter { contextTimeout += pProfProfileDuration + pProfTraceDuration - pprofConfig = &pprof.Config{ + pprofConfig = &supervisord.PprofConfig{ ProfileDuration: pProfProfileDuration, TraceDuration: pProfTraceDuration, } diff --git a/managed/services/supervisord/logs.go b/managed/services/supervisord/logs.go index 3344b5e82f..f6f54a635e 100644 --- a/managed/services/supervisord/logs.go +++ b/managed/services/supervisord/logs.go @@ -71,7 +71,7 @@ func NewLogs(pmmVersion string, pmmUpdateChecker *PMMUpdateChecker) *Logs { } // Zip creates .zip archive with all logs. -func (l *Logs) Zip(ctx context.Context, w io.Writer, pprofConfig *pprofUtils.Config) error { +func (l *Logs) Zip(ctx context.Context, w io.Writer, pprofConfig *PprofConfig) error { start := time.Now() log := logger.Get(ctx).WithField("component", "logs") log.WithField("d", time.Since(start).Seconds()).Info("Starting...") @@ -130,7 +130,7 @@ func (l *Logs) Zip(ctx context.Context, w io.Writer, pprofConfig *pprofUtils.Con } // files reads log/config/pprof files and returns content. -func (l *Logs) files(ctx context.Context, pprofConfig *pprofUtils.Config) []fileContent { +func (l *Logs) files(ctx context.Context, pprofConfig *PprofConfig) []fileContent { files := make([]fileContent, 0, 20) // add logs @@ -222,7 +222,7 @@ func (l *Logs) files(ctx context.Context, pprofConfig *pprofUtils.Config) []file wg.Add(1) go func() { defer wg.Done() - traceBytes, err := pprofUtils.Trace(pprofConfig.TraceDuration) + traceBytes, err := pprofUtils.Trace(pprofConfig.TraceDuration, ctx) files = append(files, fileContent{ Name: "pprof/trace.out", Data: traceBytes, @@ -233,7 +233,7 @@ func (l *Logs) files(ctx context.Context, pprofConfig *pprofUtils.Config) []file wg.Add(1) go func() { defer wg.Done() - profileBytes, err := pprofUtils.Profile(pprofConfig.ProfileDuration) + profileBytes, err := pprofUtils.Profile(pprofConfig.ProfileDuration, ctx) files = append(files, fileContent{ Name: "pprof/profile.pb.gz", Data: profileBytes, diff --git a/managed/utils/pprof/pprof_config.go b/managed/services/supervisord/pprof_config.go similarity index 72% rename from managed/utils/pprof/pprof_config.go rename to managed/services/supervisord/pprof_config.go index a68250b93e..ce41cfc64f 100644 --- a/managed/utils/pprof/pprof_config.go +++ b/managed/services/supervisord/pprof_config.go @@ -14,16 +14,12 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -package pprof +package supervisord import "time" -// Config pprof settings. -type Config struct { - ProfileDuration time.Duration `yaml:"profile_duration"` //nolint:tagliatelle - TraceDuration time.Duration `yaml:"trace_duration"` //nolint:tagliatelle -} - -// Init pprof config init. -func (c *Config) Init() { +// PprofConfig pprof settings. +type PprofConfig struct { + ProfileDuration time.Duration + TraceDuration time.Duration } diff --git a/managed/utils/pprof/pprof.go b/managed/utils/pprof/pprof.go index de1486daaf..557c423da7 100644 --- a/managed/utils/pprof/pprof.go +++ b/managed/utils/pprof/pprof.go @@ -18,6 +18,7 @@ package pprof import ( "bytes" + "context" "fmt" "runtime" "runtime/pprof" @@ -27,30 +28,38 @@ import ( // Profile responds with the pprof-formatted cpu profile. // Profiling lasts for duration specified in seconds. -func Profile(duration time.Duration) ([]byte, error) { +func Profile(duration time.Duration, ctx context.Context) ([]byte, error) { var profileBuf bytes.Buffer if err := pprof.StartCPUProfile(&profileBuf); err != nil { return nil, err } - time.Sleep(duration) - pprof.StopCPUProfile() - - return profileBuf.Bytes(), nil + select { + case <-time.After(duration): + pprof.StopCPUProfile() + return profileBuf.Bytes(), nil + case <-ctx.Done(): + pprof.StopCPUProfile() + return nil, fmt.Errorf("pprof.Profile was canceled") + } } // Trace responds with the execution trace in binary form. // Tracing lasts for duration specified in seconds. -func Trace(duration time.Duration) ([]byte, error) { +func Trace(duration time.Duration, ctx context.Context) ([]byte, error) { var traceBuf bytes.Buffer if err := trace.Start(&traceBuf); err != nil { return nil, err } - time.Sleep(duration) - trace.Stop() - - return traceBuf.Bytes(), nil + select { + case <-time.After(duration): + trace.Stop() + return traceBuf.Bytes(), nil + case <-ctx.Done(): + trace.Stop() + return nil, fmt.Errorf("pprof.Trace was canceled") + } } // Heap responds with the pprof-formatted profile named "heap". diff --git a/managed/utils/pprof/pprof_test.go b/managed/utils/pprof/pprof_test.go index ac8cf193c6..cca8c6b4e1 100644 --- a/managed/utils/pprof/pprof_test.go +++ b/managed/utils/pprof/pprof_test.go @@ -19,6 +19,7 @@ package pprof import ( "bytes" "compress/gzip" + "context" "testing" "time" @@ -45,7 +46,9 @@ func TestHeap(t *testing.T) { func TestProfile(t *testing.T) { t.Parallel() t.Run("Profile test", func(t *testing.T) { - profileBytes, err := Profile(1 * time.Second) + // Create a new context + ctx := context.Background() + profileBytes, err := Profile(1*time.Second, ctx) assert.NoError(t, err) assert.NotEmpty(t, profileBytes) @@ -60,14 +63,45 @@ func TestProfile(t *testing.T) { assert.NotEmpty(t, resB.Bytes()) }) + + t.Run("Profile break test", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + go func() { + profileBytes, err := Trace(30*time.Second, ctx) + assert.Empty(t, profileBytes) + assert.Error(t, err) + }() + + go func() { + time.Sleep(1 * time.Second) + cancel() + }() + }) } func TestTrace(t *testing.T) { t.Parallel() t.Run("Trace test", func(t *testing.T) { - traceBytes, err := Trace(1 * time.Second) + // Create a new context + ctx := context.Background() + traceBytes, err := Trace(1*time.Second, ctx) assert.NoError(t, err) assert.NotEmpty(t, traceBytes) }) + + t.Run("Trace break test", func(t *testing.T) { + // Create a new context + ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) + go func() { + traceBytes, err := Trace(30*time.Second, ctx) + assert.Empty(t, traceBytes) + assert.Error(t, err) + }() + + go func() { + time.Sleep(1 * time.Second) + cancel() + }() + }) } From dade1007e0355191b2ea1909602b888eb16a9c57 Mon Sep 17 00:00:00 2001 From: Przemyslaw Kadej Date: Mon, 11 Jul 2022 20:31:13 +0200 Subject: [PATCH 7/7] PMM-5492 Code review adjustments. --- managed/services/supervisord/logs.go | 4 ++-- managed/utils/pprof/pprof.go | 16 ++++++++-------- managed/utils/pprof/pprof_test.go | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/managed/services/supervisord/logs.go b/managed/services/supervisord/logs.go index f6f54a635e..18e7e96751 100644 --- a/managed/services/supervisord/logs.go +++ b/managed/services/supervisord/logs.go @@ -222,7 +222,7 @@ func (l *Logs) files(ctx context.Context, pprofConfig *PprofConfig) []fileConten wg.Add(1) go func() { defer wg.Done() - traceBytes, err := pprofUtils.Trace(pprofConfig.TraceDuration, ctx) + traceBytes, err := pprofUtils.Trace(ctx, pprofConfig.TraceDuration) files = append(files, fileContent{ Name: "pprof/trace.out", Data: traceBytes, @@ -233,7 +233,7 @@ func (l *Logs) files(ctx context.Context, pprofConfig *PprofConfig) []fileConten wg.Add(1) go func() { defer wg.Done() - profileBytes, err := pprofUtils.Profile(pprofConfig.ProfileDuration, ctx) + profileBytes, err := pprofUtils.Profile(ctx, pprofConfig.ProfileDuration) files = append(files, fileContent{ Name: "pprof/profile.pb.gz", Data: profileBytes, diff --git a/managed/utils/pprof/pprof.go b/managed/utils/pprof/pprof.go index 557c423da7..c4b9f58775 100644 --- a/managed/utils/pprof/pprof.go +++ b/managed/utils/pprof/pprof.go @@ -19,16 +19,17 @@ package pprof import ( "bytes" "context" - "fmt" "runtime" "runtime/pprof" "runtime/trace" "time" + + "github.com/pkg/errors" ) // Profile responds with the pprof-formatted cpu profile. // Profiling lasts for duration specified in seconds. -func Profile(duration time.Duration, ctx context.Context) ([]byte, error) { +func Profile(ctx context.Context, duration time.Duration) ([]byte, error) { var profileBuf bytes.Buffer if err := pprof.StartCPUProfile(&profileBuf); err != nil { return nil, err @@ -40,13 +41,13 @@ func Profile(duration time.Duration, ctx context.Context) ([]byte, error) { return profileBuf.Bytes(), nil case <-ctx.Done(): pprof.StopCPUProfile() - return nil, fmt.Errorf("pprof.Profile was canceled") + return nil, errors.New("pprof.Profile was canceled") } } // Trace responds with the execution trace in binary form. // Tracing lasts for duration specified in seconds. -func Trace(duration time.Duration, ctx context.Context) ([]byte, error) { +func Trace(ctx context.Context, duration time.Duration) ([]byte, error) { var traceBuf bytes.Buffer if err := trace.Start(&traceBuf); err != nil { return nil, err @@ -58,12 +59,11 @@ func Trace(duration time.Duration, ctx context.Context) ([]byte, error) { return traceBuf.Bytes(), nil case <-ctx.Done(): trace.Stop() - return nil, fmt.Errorf("pprof.Trace was canceled") + return nil, errors.New("pprof.Trace was canceled") } } -// Heap responds with the pprof-formatted profile named "heap". -// listing the available profiles. +// Heap responds with the pprof-formatted profile named "heap". Listing the available profiles. // You can specify the gc parameter to run gc before taking the heap sample. func Heap(gc bool) ([]byte, error) { var heapBuf bytes.Buffer @@ -72,7 +72,7 @@ func Heap(gc bool) ([]byte, error) { p := pprof.Lookup(profile) if p == nil { - return nil, fmt.Errorf("profile cannot be found: %s", profile) + return nil, errors.Errorf("profile cannot be found: %s", profile) } if gc { diff --git a/managed/utils/pprof/pprof_test.go b/managed/utils/pprof/pprof_test.go index cca8c6b4e1..627780d8fc 100644 --- a/managed/utils/pprof/pprof_test.go +++ b/managed/utils/pprof/pprof_test.go @@ -48,7 +48,7 @@ func TestProfile(t *testing.T) { t.Run("Profile test", func(t *testing.T) { // Create a new context ctx := context.Background() - profileBytes, err := Profile(1*time.Second, ctx) + profileBytes, err := Profile(ctx, 1*time.Second) assert.NoError(t, err) assert.NotEmpty(t, profileBytes) @@ -67,7 +67,7 @@ func TestProfile(t *testing.T) { t.Run("Profile break test", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) go func() { - profileBytes, err := Trace(30*time.Second, ctx) + profileBytes, err := Profile(ctx, 30*time.Second) assert.Empty(t, profileBytes) assert.Error(t, err) }() @@ -84,7 +84,7 @@ func TestTrace(t *testing.T) { t.Run("Trace test", func(t *testing.T) { // Create a new context ctx := context.Background() - traceBytes, err := Trace(1*time.Second, ctx) + traceBytes, err := Trace(ctx, 1*time.Second) assert.NoError(t, err) assert.NotEmpty(t, traceBytes) @@ -94,7 +94,7 @@ func TestTrace(t *testing.T) { // Create a new context ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) go func() { - traceBytes, err := Trace(30*time.Second, ctx) + traceBytes, err := Trace(ctx, 30*time.Second) assert.Empty(t, traceBytes) assert.Error(t, err) }()