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

cmd/syncthing, lib/config: Enable HTTP CPU/heap profile collection for users #3467

Closed
wants to merge 2 commits into from
Closed

Conversation

calmh
Copy link
Member

@calmh calmh commented Aug 2, 2016

Purpose

This adds a config to enable debug functions on the API server, which is by default disabled. When enabled, the /rest/debug things become available and become available without requiring a CSRF token (although authentication is required if configured).

We also add a new endpoint /rest/debug/cpuprof?duration=15s (with the duration being configurable, defaulting to 30s). This runs a CPU profile for the duration and returns it as a file. It sets headers so that a browser will save the file with an informative name.

The same is done for heap profiles, /rest/debug/heapprof, which does not take any parameters. The heap profile won't contain historical allocations, but a snapshot of live data at the time it's taken.

The purpose of this is that any user can enable debugging under advanced, then point their browser to the endpoint above and get a file that contains a CPU/heap profile we can use, with the filename telling us what version and architecture the profile is from.

On the command line, this becomes

curl -O -J http://localhost:8082/rest/debug/cpuprof?duration=5s
curl: Saved to filename
'syncthing-cpu-darwin-amd64-v0.14.3+4-g935bcc0-110307.pprof'

Testing

Works for me

…r users

This adds a config to enable debug functions on the API server, which is
by default disabled. When enabled, the /rest/debug things become
available and become available without requiring a CSRF token (although
authentication is required if configured).

We also add a new endpoint /rest/debug/cpuprof?duration=15s (with the
duration being configurable, defaulting to 30s). This runs a CPU profile
for the duration and returns it as a file. It sets headers so that a
browser will save the file with an informative name.

The same is done for heap profiles, /rest/debug/heapprof, which does not
take any parameters.

The purpose of this is that any user can enable debugging under
advanced, then point their browser to the endpoint above and get a file
that contains a CPU or heap profile we can use, with the filename
telling us what version and architecture the profile is from.

On the command line, this becomes

    curl -O -J http://localhost:8082/rest/debug/cpuprof?duration=5s
    curl: Saved to filename
    'syncthing-cpu-darwin-amd64-v0.14.3+4-g935bcc0-110307.pprof'
@calmh calmh changed the title cmd/syncthing, lib/config: Enable HTTP CPU profile collection for users cmd/syncthing, lib/config: Enable HTTP CPU/heap profile collection for users Aug 2, 2016
@calmh
Copy link
Member Author

calmh commented Aug 2, 2016

@st-jenkins retest this please

@@ -21,6 +21,7 @@ type GUIConfiguration struct {
APIKey string `xml:"apikey,omitempty" json:"apiKey"`
InsecureAdminAccess bool `xml:"insecureAdminAccess,omitempty" json:"insecureAdminAccess"`
Theme string `xml:"theme" json:"theme" default:"default"`
EnableDebugging bool `xml:"debugging,attr" json:"debugging"`
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent naming. Debugging vs enabledebugging

@calmh
Copy link
Member Author

calmh commented Aug 2, 2016

Yeah that looks OK to me as well.

@AudriusButkevicius
Copy link
Member

Do we need to copy the value of the bool somewhere in config saving to prevent restart messages?

@calmh
Copy link
Member Author

calmh commented Aug 2, 2016

No, the restart triggering thing in model doesn't look at the GUI config at all.

@AudriusButkevicius
Copy link
Member

@st-review merge

@st-review
Copy link

👌 Merged as ffe7a2f. Thanks, @calmh!

st-review pushed a commit that referenced this pull request Aug 2, 2016
…r users

This adds a config to enable debug functions on the API server, which is
by default disabled. When enabled, the /rest/debug things become
available and become available without requiring a CSRF token (although
authentication is required if configured).

We also add a new endpoint /rest/debug/cpuprof?duration=15s (with the
duration being configurable, defaulting to 30s). This runs a CPU profile
for the duration and returns it as a file. It sets headers so that a
browser will save the file with an informative name.

The same is done for heap profiles, /rest/debug/heapprof, which does not
take any parameters.

The purpose of this is that any user can enable debugging under
advanced, then point their browser to the endpoint above and get a file
that contains a CPU or heap profile we can use, with the filename
telling us what version and architecture the profile is from.

On the command line, this becomes

    curl -O -J http://localhost:8082/rest/debug/cpuprof?duration=5s
    curl: Saved to filename
    'syncthing-cpu-darwin-amd64-v0.14.3+4-g935bcc0-110307.pprof'

GitHub-Pull-Request: #3467
@st-review st-review closed this Aug 2, 2016
@calmh calmh deleted the profiling branch August 8, 2016 18:11
@st-review st-review added the frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion label Aug 2, 2017
@syncthing syncthing locked and limited conversation to collaborators Aug 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
frozen-due-to-age Issues closed and untouched for a long time, together with being locked for discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants