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

feat: add debug server with pprof #1564

Merged
merged 7 commits into from
Dec 6, 2022
Merged

feat: add debug server with pprof #1564

merged 7 commits into from
Dec 6, 2022

Conversation

enocom
Copy link
Member

@enocom enocom commented Dec 1, 2022

No description provided.

@enocom enocom requested a review from a team December 1, 2022 19:30
@kurtisvg kurtisvg self-assigned this Dec 6, 2022
README.md Outdated
@@ -434,6 +434,21 @@ To enable Prometheus, use the `--prometheus` flag. This will start an HTTP
server on localhost with a `/metrics` endpoint. The Prometheus namespace may
optionally be set with `--prometheus-namespace`.

## Debug server

The Proxy includes support for a debug server on localhost. By default, the
Copy link
Contributor

Choose a reason for hiding this comment

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

The Proxy includes support for a debug server on localhost

Will this change with the http-addr flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it won't. We purposely make it localhost only to prevent any unwanted access.

README.md Outdated

The Proxy includes support for a debug server on localhost. By default, the
debug server is not enabled. To enable the server, pass the `--debug` flag.
This will start the server on localhost at port 9191. To change the port,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we reuse the http handler for readiness checks and metrics?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the key question. It also applies to the upcoming /quitquitquit endpoint. I've been thinking of pprof support and /quitquitquit as in the same category, i.e., features that could have a negative impact on the Proxy itself if exposed too broadly.

By limiting the handler to localhost only, we have a secure-by-default model, albeit it with the overhead of an extra HTTP server and an additional --debug-port flag.

Copy link
Member Author

Choose a reason for hiding this comment

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

We'll make this 9091.

Copy link
Member Author

@enocom enocom Dec 6, 2022

Choose a reason for hiding this comment

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

We'll change this to --admin-port.

@enocom enocom requested a review from kurtisvg December 6, 2022 18:40
@enocom enocom merged commit d022c56 into main Dec 6, 2022
@enocom enocom deleted the pprof branch December 6, 2022 20:02
enocom added a commit to GoogleCloudPlatform/alloydb-auth-proxy that referenced this pull request Jan 15, 2023
enocom added a commit to GoogleCloudPlatform/alloydb-auth-proxy that referenced this pull request Jan 19, 2023
enocom added a commit to GoogleCloudPlatform/alloydb-auth-proxy that referenced this pull request Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants