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

users/strelaysrv: Document new access control, update options #799

Merged
merged 1 commit into from
Sep 3, 2023

Conversation

GermanCoding
Copy link
Contributor

Documentation for PR syncthing/syncthing#8561

Also took the opportunity to document missing flags -pprof and -version

Comment on lines +92 to +95
.. cmdoption:: -pprof

Enable the built in profiling on the status server

Copy link

Choose a reason for hiding this comment

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

Does this mean the Go profiler? Would this dump profile data? Or is this the /status endpoint - wasn't that enabled by default?

Maybe it's just me but I don't understand this sentence as written :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was copy-pasted from the strelaysrv -help output. It's in the relaysrv source code here: https://github.com/syncthing/syncthing/blob/81a4b22d4342b187ca910a1e6f4ce0ff61b89569/cmd/strelaysrv/main.go#L102

What it does is it enables a new /debug/pprof endpoint you can request to get a Go profile (see the code here: https://github.com/syncthing/syncthing/blob/81a4b22d4342b187ca910a1e6f4ce0ff61b89569/cmd/strelaysrv/status.go#L24). I've no idea if this is documented anywhere (probably not), but it's similar to how it's handles in the main syncthing service (that also has a rest/debug/pprof endpoint you can enable).

Copy link

Choose a reason for hiding this comment

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

Mildly feel like this wording is weird, I might say "enable go's builtin profiler" or something, but if it's copy pasted from elsewhere I feel like it's fine to keep it.. probably.

@st-review
Copy link

🤖 beep boop

I'm going to close this pull request as it has been idle for more than 90 days.

This is not a rejection, merely a removal from the list of active pull requests that is periodically reviewed by humans. The pull request can be reopened when there is new activity, and merged once any remaining issues are resolved.

To permanently exempt this PR from bot nagging, add the slow-pr label.

@st-review st-review closed this Sep 3, 2023
@calmh calmh reopened this Sep 3, 2023
@calmh calmh merged commit 40f8418 into syncthing:main Sep 3, 2023
1 check passed
@syncthing syncthing locked and limited conversation to collaborators Sep 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants