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

add option to hide /debug/ pages #8596

Closed
wants to merge 1 commit into from
Closed

Conversation

csko
Copy link

@csko csko commented Jul 15, 2017

closes #5305

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated
  • Sign CLA (if not already signed)
Required only if applicable
  • Config changes: update sample config (etc/config.sample.toml), server NewDemoConfig method, and Diagnostics methods reporting config settings, if necessary
  • InfluxData Documentation: issue filed or pull request submitted

@@ -219,6 +219,10 @@
# troubleshooting and monitoring.
# pprof-enabled = true

# Determines whether the debug endpoint is enabled. This endpoint is used for
# gathering runtime statistics.
# pprof-enabled = true
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be updated to debug-enabled rather than pprof-enabled.

@@ -256,9 +256,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {

if strings.HasPrefix(r.URL.Path, "/debug/pprof") && h.Config.PprofEnabled {
h.handleProfiles(w, r)
} else if strings.HasPrefix(r.URL.Path, "/debug/vars") {
} else if strings.HasPrefix(r.URL.Path, "/debug/vars") && h.Config.DebugEnabled {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should setting debug-enabled to false also disable /debug/pprof?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that is what pprof-enabled does.

@@ -24,6 +24,7 @@ type Config struct {
LogEnabled bool `toml:"log-enabled"`
WriteTracing bool `toml:"write-tracing"`
PprofEnabled bool `toml:"pprof-enabled"`
DebugEnabled bool `toml:"debug-enabled"`
Copy link
Contributor

Choose a reason for hiding this comment

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

The default for this should be set to true.

Copy link
Contributor

@jwilder jwilder left a comment

Choose a reason for hiding this comment

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

This features is a little confusing as implemented. There is a /debug/... endpoint that has pprof, requests, vars, etc.. handlers.

There is also already a pprof-enabled setting that disables the /debug/pprof endpoint only.

I would expect a debug-enabled flag to disable the whole /debug handler and everything under it since all of these endpoints are for debugging.

Following the same pattern as pprof-enabled, there could be a vars-enabled, requests-enabled to disable those endpoints, but I'm not sure there is much value in doing that is it just adds a bunch of config options and you might as well disable all of /debug.

@jwilder
Copy link
Contributor

jwilder commented Oct 2, 2017

Closing due to inactivity.

@jwilder jwilder closed this Oct 2, 2017
@ghost ghost removed the proposed label Oct 2, 2017
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.

[feature request] Add configuration option to disable the /debug/vars HTTP API endpoint
4 participants