Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

debug: rename cluster.json -> members.json and fix handling of Interrupt Signal #10804

Merged
merged 6 commits into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/10804.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
debug: rename cluster capture target to members, to be more consistent with the terms used by the API.
```
22 changes: 22 additions & 0 deletions api/debug.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package api

import (
"context"
"fmt"
"io"
"io/ioutil"
"strconv"
)
Expand Down Expand Up @@ -70,6 +72,26 @@ func (d *Debug) Profile(seconds int) ([]byte, error) {
return body, nil
}

// PProf returns a pprof profile for the specified number of seconds. The caller
// is responsible for closing the returned io.ReadCloser once all bytes are read.
func (d *Debug) PProf(ctx context.Context, name string, seconds int) (io.ReadCloser, error) {
r := d.c.newRequest("GET", "/debug/pprof/"+name)
r.ctx = ctx

// Capture a profile for the specified number of seconds
r.params.Set("seconds", strconv.Itoa(seconds))

_, resp, err := d.c.doRequest(r)
if err != nil {
return nil, fmt.Errorf("error making request: %s", err)
}

if resp.StatusCode != 200 {
return nil, generateUnexpectedResponseCodeError(resp)
}
return resp.Body, nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case for returning the resp.Body raw vs ioutil.ReadAll-ing and returning []byte?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question! That is indeed a change from the previous methods.

The nice thing about returning an io.ReadCloser is that we don't have to buffer the entire response in memory. We can write the response to a file as a stream, so the memory of the CLI process should stay relatively constant.

With ioutil.ReadAll we have to read the entire profile into the CLI process memory, which could be quite large. I don't know if that's actually a probably in this case, but in general using an io.Reader or io.ReadCloser will give the caller the option to either stream or buffer, where as returning a []byte forces them to use the fully buffered response.

}

// Trace returns an execution trace
func (d *Debug) Trace(seconds int) ([]byte, error) {
r := d.c.newRequest("GET", "/debug/pprof/trace")
Expand Down
2 changes: 1 addition & 1 deletion command/commands_oss.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ func init() {
Register("connect envoy pipe-bootstrap", func(ui cli.Ui) (cli.Command, error) { return pipebootstrap.New(ui), nil })
Register("connect expose", func(ui cli.Ui) (cli.Command, error) { return expose.New(ui), nil })
Register("connect redirect-traffic", func(ui cli.Ui) (cli.Command, error) { return redirecttraffic.New(ui), nil })
Register("debug", func(ui cli.Ui) (cli.Command, error) { return debug.New(ui, MakeShutdownCh()), nil })
Register("debug", func(ui cli.Ui) (cli.Command, error) { return debug.New(ui), nil })
Register("event", func(ui cli.Ui) (cli.Command, error) { return event.New(ui), nil })
Register("exec", func(ui cli.Ui) (cli.Command, error) { return exec.New(ui, MakeShutdownCh()), nil })
Register("force-leave", func(ui cli.Ui) (cli.Command, error) { return forceleave.New(ui), nil })
Expand Down
Loading