Skip to content

feat: Add support for pprof in coder agent #1985

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jun 6, 2022
Merged

feat: Add support for pprof in coder agent #1985

merged 8 commits into from
Jun 6, 2022

Conversation

mafredri
Copy link
Member

@mafredri mafredri commented Jun 2, 2022 •
edited
Loading

This PR adds support for starting a pprof server in coder agent.

After #1978 I wanted this to analyze the agent for memory leaks, as mentioned in #1508.

@mafredri mafredri self-assigned this Jun 2, 2022
@mafredri mafredri requested a review from a team June 2, 2022 15:29
kylecarbs
kylecarbs approved these changes Jun 2, 2022
View reviewed changes
Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Should we add a signal to enable this? Seems like that'd be the primary use-case.

Emyrk
Emyrk reviewed Jun 2, 2022
View reviewed changes
@mafredri
Copy link
Member Author

mafredri commented Jun 2, 2022

@kylecarbs what type of signaling were you thinking? Kill signal? That could definitely be useful!

@kylecarbs
Copy link
Member

Not sure what signal... maybe SIGUSR1? I think it'd be helpful to enable pprof on a running agent, that way you could diagnose memory leaks or other runtime issues.

@mafredri
Copy link
Member Author

mafredri commented Jun 3, 2022

@kylecarbs kill -USR1 support has been added! I had to disable it on Windows due to there not being an equivalent signal.

@mafredri mafredri requested a review from kylecarbs June 3, 2022 13:28
@kylecarbs
Copy link
Member

Hmm, how could we trigger it for Windows? I'd love a cross-platform approach first, so that we're not stuck when a Windows user is experiencing bugs.

@mafredri
Copy link
Member Author

mafredri commented Jun 3, 2022

@kylecarbs I was thinking we could trigger it via the server as an alternative. This could allow an admin to turn it on as well as let the user do it via the CLI. This could be further expanded as a admin toggle in the future (whether or not it’s allowed, not sure if there’s a use case though). Another option could be IPC via cli and the running agent.

@kylecarbs
Copy link
Member

Ahh, interesting. This is big progress though, so I think an issue should be made for the Windows support and we merge!

@mafredri mafredri mentioned this pull request Jun 6, 2022
Closed
@mafredri
Copy link
Member Author

mafredri commented Jun 6, 2022

@kylecarbs done. I created #2081 to track support for Windows.

@mafredri mafredri merged commit 59a6826 into main Jun 6, 2022
@mafredri mafredri deleted the mafredri/feat-add-pprof-to-agent branch June 6, 2022 13:38
kylecarbs pushed a commit that referenced this pull request Jun 10, 2022
@mafredri @kylecarbs
feat: Add support for pprof in coder agent (#1985)
cdc95cd 
* feat: Allow USR1 signal to start pprof
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Emyrk Emyrk Emyrk approved these changes

@kylecarbs kylecarbs kylecarbs approved these changes

Assignees

@mafredri mafredri

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mafredri @kylecarbs @Emyrk