Merge pull request #1603 from jmunson/jmunson/enable-pprof

Expose net/http/pprof handlers

AUTHORS

@@ -86,3 +86,4 @@ List of contributors, in chronological order:
 * Philip Cramer (https://github.com/PhilipCramer)
 * Kerem Gur (https://github.com/KereMath)
 * Phil Dibowitz (https://github.com/jaymzh)
+* James Munson (https://github.com/jmunson)

api/api_test.go

@@ -42,6 +42,7 @@ func createTestConfig() *os.File {
 	jsonString, err := json.Marshal(gin.H{
 		"architectures":         []string{},
 		"enableMetricsEndpoint": true,
+		"enablePprofEndpoint":   false,
 		"S3PublishEndpoints": map[string]map[string]string{
 			"test-s3": {
 				"region": "us-east-1",

api/pprof_integration_test.go

@@ -0,0 +1,121 @@
+package api
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http/httptest"
+	"os"
+	"os/exec"
+	"strings"
+	"testing"
+
+	"github.com/aptly-dev/aptly/aptly"
+	ctx "github.com/aptly-dev/aptly/context"
+	"github.com/gin-gonic/gin"
+)
+
+// pprofTestServer starts a real httptest.Server with the aptly router configured
+// with enablePprofEndpoint: true. The caller is responsible for calling
+// server.Close() and aptlyCtx.Shutdown().
+func pprofTestServer(t *testing.T) (*httptest.Server, *ctx.AptlyContext) {
+	t.Helper()
+
+	aptly.Version = "testVersion"
+
+	file, err := os.CreateTemp("", "aptly-pprof-integration")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = os.Remove(file.Name()) })
+
+	jsonString, err := json.Marshal(gin.H{
+		"architectures":       []string{},
+		"enablePprofEndpoint": true,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = file.Write(jsonString); err != nil {
+		t.Fatal(err)
+	}
+	_ = file.Close()
+
+	aptlyCtx, router, err := newPprofContext(file.Name())
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return httptest.NewServer(router), aptlyCtx
+}
+
+// runPprof runs "go tool pprof -top <url>" and returns the combined output.
+func runPprof(t *testing.T, url string) string {
+	t.Helper()
+	cmd := exec.Command("go", "tool", "pprof", "-top", url)
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("go tool pprof -top %s failed: %v\noutput:\n%s", url, err, out)
+	}
+	return string(out)
+}
+
+// TestPprofToolGoroutine verifies that go tool pprof can fetch and parse the
+// goroutine profile from the /api/debug/pprof/goroutine endpoint.
+func TestPprofToolGoroutine(t *testing.T) {
+	server, aptlyCtx := pprofTestServer(t)
+	defer server.Close()
+	defer aptlyCtx.Shutdown()
+
+	url := fmt.Sprintf("%s/api/debug/pprof/goroutine", server.URL)
+	out := runPprof(t, url)
+	t.Logf("go tool pprof goroutine output:\n%s", out)
+
+	if !strings.Contains(out, "Type: goroutine") {
+		t.Errorf("expected 'Type: goroutine' in output, got:\n%s", out)
+	}
+	if !strings.Contains(out, "flat") {
+		t.Errorf("expected 'flat' column header in output, got:\n%s", out)
+	}
+}
+
+// TestPprofToolHeap verifies that go tool pprof can fetch and parse the heap
+// profile from the /api/debug/pprof/heap endpoint.
+func TestPprofToolHeap(t *testing.T) {
+	server, aptlyCtx := pprofTestServer(t)
+	defer server.Close()
+	defer aptlyCtx.Shutdown()
+
+	url := fmt.Sprintf("%s/api/debug/pprof/heap", server.URL)
+	out := runPprof(t, url)
+	t.Logf("go tool pprof heap output:\n%s", out)
+
+	// Heap profiles are typed as "inuse_space" by default.
+	if !strings.Contains(out, "Type: inuse_space") {
+		t.Errorf("expected 'Type: inuse_space' in output, got:\n%s", out)
+	}
+	if !strings.Contains(out, "flat") {
+		t.Errorf("expected 'flat' column header in output, got:\n%s", out)
+	}
+}
+
+// TestPprofToolCmdline verifies that the cmdline endpoint is reachable.
+// cmdline returns plain text (the process argv), not a pprof binary profile,
+// so we confirm the HTTP fetch succeeds rather than parsing it as a profile.
+func TestPprofToolCmdline(t *testing.T) {
+	server, aptlyCtx := pprofTestServer(t)
+	defer server.Close()
+	defer aptlyCtx.Shutdown()
+
+	url := fmt.Sprintf("%s/api/debug/pprof/cmdline", server.URL)
+
+	resp, err := server.Client().Get(url)
+	if err != nil {
+		t.Fatalf("GET %s: %v", url, err)
+	}
+	defer func() { _ = resp.Body.Close() }()
+
+	if resp.StatusCode != 200 {
+		t.Errorf("expected 200, got %d", resp.StatusCode)
+	}
+	t.Logf("cmdline endpoint status: %d", resp.StatusCode)
+}

api/pprof_test.go

@@ -0,0 +1,183 @@
+package api
+
+import (
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"os"
+
+	"github.com/aptly-dev/aptly/aptly"
+	ctx "github.com/aptly-dev/aptly/context"
+	"github.com/gin-gonic/gin"
+	"github.com/smira/flag"
+	. "gopkg.in/check.v1"
+)
+
+// newPprofContext builds an aptly context and router from the given config file
+// path. It returns an error if context initialisation fails. The caller is
+// responsible for calling aptlyCtx.Shutdown() when done.
+func newPprofContext(configPath string) (*ctx.AptlyContext, http.Handler, error) {
+	flags := flag.NewFlagSet("fakeFlags", flag.ContinueOnError)
+	flags.Bool("no-lock", false, "dummy")
+	flags.Int("db-open-attempts", 3, "dummy")
+	flags.String("config", configPath, "dummy")
+	flags.String("architectures", "", "dummy")
+
+	aptlyCtx, err := ctx.NewContext(flags)
+	if err != nil {
+		return nil, nil, err
+	}
+	return aptlyCtx, Router(aptlyCtx), nil
+}
+
+// PprofEnabledSuite tests pprof endpoints when EnablePprofEndpoint is true.
+// It uses a dedicated router built from a config that enables only the pprof
+// endpoint, ensuring no interaction with other optional features.
+type PprofEnabledSuite struct {
+	configFile *os.File
+	aptlyCtx   *ctx.AptlyContext
+	router     http.Handler
+}
+
+var _ = Suite(&PprofEnabledSuite{})
+
+func (s *PprofEnabledSuite) SetUpSuite(c *C) {
+	aptly.Version = "testVersion"
+
+	file, err := os.CreateTemp("", "aptly-pprof")
+	c.Assert(err, IsNil)
+	s.configFile = file
+
+	jsonString, err := json.Marshal(gin.H{
+		"architectures":       []string{},
+		"enablePprofEndpoint": true,
+	})
+	c.Assert(err, IsNil)
+	_, err = file.Write(jsonString)
+	c.Assert(err, IsNil)
+	_ = file.Close()
+
+	aptlyCtx, router, err := newPprofContext(file.Name())
+	c.Assert(err, IsNil)
+	s.aptlyCtx = aptlyCtx
+	s.router = router
+}
+
+func (s *PprofEnabledSuite) TearDownSuite(c *C) {
+	if s.configFile != nil {
+		_ = os.Remove(s.configFile.Name())
+	}
+	if s.aptlyCtx != nil {
+		s.aptlyCtx.Shutdown()
+	}
+}
+
+func (s *PprofEnabledSuite) request(method, url string) *httptest.ResponseRecorder {
+	w := httptest.NewRecorder()
+	req, err := http.NewRequest(method, url, nil)
+	if err != nil {
+		panic(err)
+	}
+	s.router.ServeHTTP(w, req)
+	return w
+}
+
+// TestPprofIndexReturns200 verifies the pprof index page is served when the
+// endpoint is enabled.
+func (s *PprofEnabledSuite) TestPprofIndexReturns200(c *C) {
+	w := s.request("GET", "/api/debug/pprof/")
+	c.Check(w.Code, Equals, 200)
+	c.Check(w.Header().Get("Content-Type"), Matches, "text/html.*")
+}
+
+// TestPprofGoroutineReturns200 verifies the goroutine sub-profile is served.
+func (s *PprofEnabledSuite) TestPprofGoroutineReturns200(c *C) {
+	w := s.request("GET", "/api/debug/pprof/goroutine?debug=1")
+	c.Check(w.Code, Equals, 200)
+}
+
+// TestPprofHeapReturns200 verifies the heap sub-profile is served.
+func (s *PprofEnabledSuite) TestPprofHeapReturns200(c *C) {
+	w := s.request("GET", "/api/debug/pprof/heap?debug=1")
+	c.Check(w.Code, Equals, 200)
+}
+
+// TestPprofCmdlineReturns200 verifies the cmdline handler is served.
+func (s *PprofEnabledSuite) TestPprofCmdlineReturns200(c *C) {
+	w := s.request("GET", "/api/debug/pprof/cmdline")
+	c.Check(w.Code, Equals, 200)
+}
+
+// TestPprofSymbolReturns200 verifies the symbol lookup handler responds (GET
+// with no addresses returns 200 with a count of 0 symbols found).
+func (s *PprofEnabledSuite) TestPprofSymbolReturns200(c *C) {
+	w := s.request("GET", "/api/debug/pprof/symbol")
+	c.Check(w.Code, Equals, 200)
+}
+
+// TestPprofTraceReturns200 verifies the execution trace handler is wired
+// correctly. The minimum trace duration is 1 second, so this test blocks
+// briefly but confirms end-to-end handler registration.
+func (s *PprofEnabledSuite) TestPprofTraceReturns200(c *C) {
+	w := s.request("GET", "/api/debug/pprof/trace?seconds=1")
+	c.Check(w.Code, Equals, 200)
+	c.Check(w.Header().Get("Content-Type"), Equals, "application/octet-stream")
+}
+
+// TestNonPprofRoutesUnaffectedWhenEnabled verifies that existing API routes
+// continue to work correctly when the pprof endpoint is enabled.
+func (s *PprofEnabledSuite) TestNonPprofRoutesUnaffectedWhenEnabled(c *C) {
+	w := s.request("GET", "/api/version")
+	c.Check(w.Code, Equals, 200)
+}
+
+// PprofDisabledSuite tests that pprof endpoints are NOT exposed when the config
+// option is off. It embeds APISuite, whose config does not set
+// enablePprofEndpoint, confirming the default state is safe. The inherited
+// APISuite tests also run here, verifying that normal API operation is
+// unaffected when pprof is disabled.
+type PprofDisabledSuite struct {
+	APISuite
+}
+
+var _ = Suite(&PprofDisabledSuite{})
+
+// TestPprofIndexNotExposed verifies the pprof index is not reachable when
+// enablePprofEndpoint is false (the default).
+func (s *PprofDisabledSuite) TestPprofIndexNotExposed(c *C) {
+	response, err := s.HTTPRequest("GET", "/api/debug/pprof/", nil)
+	c.Assert(err, IsNil)
+	c.Check(response.Code, Equals, 404)
+}
+
+// TestPprofGoroutineNotExposed verifies the goroutine profile is not reachable
+// when disabled.
+func (s *PprofDisabledSuite) TestPprofGoroutineNotExposed(c *C) {
+	response, err := s.HTTPRequest("GET", "/api/debug/pprof/goroutine", nil)
+	c.Assert(err, IsNil)
+	c.Check(response.Code, Equals, 404)
+}
+
+// TestPprofHeapNotExposed verifies the heap profile is not reachable when
+// disabled.
+func (s *PprofDisabledSuite) TestPprofHeapNotExposed(c *C) {
+	response, err := s.HTTPRequest("GET", "/api/debug/pprof/heap", nil)
+	c.Assert(err, IsNil)
+	c.Check(response.Code, Equals, 404)
+}
+
+// TestPprofCmdlineNotExposed verifies the cmdline endpoint is not reachable
+// when disabled.
+func (s *PprofDisabledSuite) TestPprofCmdlineNotExposed(c *C) {
+	response, err := s.HTTPRequest("GET", "/api/debug/pprof/cmdline", nil)
+	c.Assert(err, IsNil)
+	c.Check(response.Code, Equals, 404)
+}
+
+// TestPprofTraceNotExposed verifies the trace endpoint is not reachable when
+// disabled.
+func (s *PprofDisabledSuite) TestPprofTraceNotExposed(c *C) {
+	response, err := s.HTTPRequest("GET", "/api/debug/pprof/trace", nil)
+	c.Assert(err, IsNil)
+	c.Check(response.Code, Equals, 404)
+}

api/router.go

@@ -2,6 +2,8 @@ package api
 
 import (
 	"net/http"
+	"net/http/pprof"
+	"strings"
 	"sync/atomic"
 
 	"github.com/aptly-dev/aptly/aptly"
@@ -82,6 +84,27 @@ func Router(c *ctx.AptlyContext) http.Handler {
 		MetricsCollectorRegistrar.Register(router)
 	}
 
+	if c.Config().EnablePprofEndpoint {
+		pprofMux := http.NewServeMux()
+		pprofMux.HandleFunc("/debug/pprof/", pprof.Index)
+		pprofMux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline)
+		pprofMux.HandleFunc("/debug/pprof/profile", pprof.Profile)
+		pprofMux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
+		pprofMux.HandleFunc("/debug/pprof/trace", pprof.Trace)
+
+		router.Any("/api/debug/pprof/*any", func(c *gin.Context) {
+			origPath := c.Request.URL.Path
+			origRawPath := c.Request.URL.RawPath
+			c.Request.URL.Path = strings.TrimPrefix(origPath, "/api")
+			if origRawPath != "" {
+				c.Request.URL.RawPath = strings.TrimPrefix(origRawPath, "/api")
+			}
+			pprofMux.ServeHTTP(c.Writer, c.Request)
+			c.Request.URL.Path = origPath
+			c.Request.URL.RawPath = origRawPath
+		})
+	}
+
 	if c.Config().ServeInAPIMode {
 		router.GET("/repos/", reposListInAPIMode(c.Config().FileSystemPublishRoots))
 		router.GET("/repos/:storage/*pkgPath", reposServeInAPIMode)

debian/aptly.conf

@@ -86,6 +86,9 @@ enable_metrics_endpoint: false
 # Enable API documentation on /docs
 enable_swagger_endpoint: false
 
+# Enable pprof profiling endpoints on /api/debug/pprof/*
+enable_pprof_endpoint: false
+
 # OBSOLETE: use via url param ?_async=true
 async_api: false
 

system/t02_config/ConfigShowTest_gold

@@ -16,6 +16,7 @@
     "serveInAPIMode": true,
     "enableMetricsEndpoint": true,
     "enableSwaggerEndpoint": false,
+    "enablePprofEndpoint": false,
     "AsyncAPI": false,
     "databaseBackend": {
         "type": "",

system/t02_config/ConfigShowYAMLTest_gold

@@ -15,6 +15,7 @@ ppa_baseurl: http://ppa.launchpad.net
 serve_in_api_mode: true
 enable_metrics_endpoint: true
 enable_swagger_endpoint: false
+enable_pprof_endpoint: false
 async_api: false
 database_backend:
     type: ""

system/t02_config/CreateConfigTest_gold

@@ -86,6 +86,9 @@ enable_metrics_endpoint: false
 # Enable API documentation on /docs
 enable_swagger_endpoint: false
 
+# Enable pprof profiling endpoints on /api/debug/pprof/*
+enable_pprof_endpoint: false
+
 # OBSOLETE: use via url param ?_async=true
 async_api: false
 

utils/config.go

@@ -37,6 +37,7 @@ type ConfigStructure struct { // nolint: maligned
 	ServeInAPIMode        bool `json:"serveInAPIMode"                yaml:"serve_in_api_mode"`
 	EnableMetricsEndpoint bool `json:"enableMetricsEndpoint"         yaml:"enable_metrics_endpoint"`
 	EnableSwaggerEndpoint bool `json:"enableSwaggerEndpoint"         yaml:"enable_swagger_endpoint"`
+	EnablePprofEndpoint   bool `json:"enablePprofEndpoint"           yaml:"enable_pprof_endpoint"`
 	AsyncAPI              bool `json:"AsyncAPI"                      yaml:"async_api"` // OBSOLETE
 
 	// Database

utils/config_test.go

@@ -95,6 +95,7 @@ func (s *ConfigSuite) TestSaveConfig(c *C) {
   "serveInAPIMode": false,
   "enableMetricsEndpoint": false,
   "enableSwaggerEndpoint": false,
+  "enablePprofEndpoint": false,
   "AsyncAPI": false,
   "databaseBackend": {
     "type": "",
@@ -291,6 +292,7 @@ func (s *ConfigSuite) TestSaveYAML2Config(c *C) {
     "serve_in_api_mode: false\n"+
     "enable_metrics_endpoint: false\n"+
     "enable_swagger_endpoint: false\n"+
+    "enable_pprof_endpoint: false\n"+
     "async_api: false\n"+
     "database_backend:\n"+
     "    type: \"\"\n"+
@@ -350,6 +352,7 @@ ppa_baseurl: http://ppa.launchpad.net
 serve_in_api_mode: true
 enable_metrics_endpoint: true
 enable_swagger_endpoint: true
+enable_pprof_endpoint: false
 async_api: true
 database_backend:
     type: etcd