Skip to content

Commit

Permalink
Fix profiler port (#90)
Browse files Browse the repository at this point in the history
Two endpoints should be on the profiler port rather than the main
port: `gc_stats` and `live_profiler`. This is done by installing them
on the default mux used by the profiler, rather than a custom mux
passed on to the main server.

Once `live_profiler` is on the internal profiler port, we can remove
the `127.0.0.1` remote-ip restriction to make it easier to use, given
that the default base image is a no-login image.
  • Loading branch information
vmallet authored Jan 22, 2024
1 parent 98aad24 commit 4184af1
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 19 deletions.
6 changes: 1 addition & 5 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package main

import (
"fmt"
"net/http"
_ "net/http/pprof"
"os"

"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -40,10 +38,8 @@ func main() {

func startServer(config *config.Config, logger *logrus.Logger) (err error) {
logger.Infoln("starting")
mux := http.NewServeMux()
registry := streaming.NewSocketRegistry()

monitoring.StartProfilerServer(config, mux, logger)
if config.StatusPort > 0 {
monitoring.StartStatusServer(config, logger)
}
Expand All @@ -56,7 +52,7 @@ func startServer(config *config.Config, logger *logrus.Logger) (err error) {
return err
}

server, _, err := streaming.InitServer(config, mux, producerRules, logger, registry)
server, _, err := streaming.InitServer(config, producerRules, logger, registry)
if err != nil {
return err
}
Expand Down
5 changes: 5 additions & 0 deletions server/monitoring/metrics_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import (
"sync"
"time"

// This registers the profiler on the default mux which we will use for monitoring port.
_ "net/http/pprof"

"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/sirupsen/logrus"

Expand Down Expand Up @@ -42,6 +45,8 @@ func StartServerMetrics(config *config.Config, logger *logrus.Logger, registry *

if config.Monitoring.ProfilerPort > 0 {
go func() {
StartProfilerServer(config, http.DefaultServeMux, logger)

if err := http.ListenAndServe(fmt.Sprintf(":%d", config.Monitoring.ProfilerPort), nil); err != nil {
logger.Errorf("profiler_listen_error: %v", err)
}
Expand Down
6 changes: 0 additions & 6 deletions server/monitoring/profile_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"fmt"
"net/http"
"runtime/debug"
"strings"

"github.com/sirupsen/logrus"

Expand All @@ -19,11 +18,6 @@ type profileServer struct {
// liveProfiler profiles https requests
func (p *profileServer) liveProfiler(config *config.Config) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
if !strings.Contains(r.RemoteAddr, "127.0.0.1") { // enable this only locally
w.WriteHeader(404)
return
}

if config.Monitoring == nil || config.Monitoring.ProfilingPath == "" { // disabled by default
w.WriteHeader(405)
_, _ = w.Write([]byte(`{"error":"profiler not configured"}`))
Expand Down
3 changes: 2 additions & 1 deletion server/streaming/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type Server struct {
}

// InitServer initializes the main server
func InitServer(c *config.Config, mux *http.ServeMux, producerRules map[string][]telemetry.Producer, logger *logrus.Logger, registry *SocketRegistry) (*http.Server, *Server, error) {
func InitServer(c *config.Config, producerRules map[string][]telemetry.Producer, logger *logrus.Logger, registry *SocketRegistry) (*http.Server, *Server, error) {
reliableAck := false
if c.Kafka != nil {
reliableAck = c.ReliableAck
Expand All @@ -52,6 +52,7 @@ func InitServer(c *config.Config, mux *http.ServeMux, producerRules map[string][
logger: logger,
}

mux := http.NewServeMux()
mux.HandleFunc("/", socketServer.ServeBinaryWs(c, registry))
mux.HandleFunc("/status", socketServer.Status())

Expand Down
3 changes: 1 addition & 2 deletions server/streaming/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ var _ = Describe("Socket handler test", func() {
req := httptest.NewRequest("GET", "http://tel.vn.tesla.com", nil)

producerRules := make(map[string][]telemetry.Producer)
mux := http.NewServeMux()
_, s, err := streaming.InitServer(conf, mux, producerRules, logger, registry)
_, s, err := streaming.InitServer(conf, producerRules, logger, registry)
Expect(err).NotTo(HaveOccurred())

srv := httptest.NewServer(http.HandlerFunc(s.ServeBinaryWs(conf, registry)))
Expand Down
5 changes: 0 additions & 5 deletions test/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,6 @@ var _ = Describe("Test messages", Ordered, func() {
Expect(string(body)).To(Equal("ok"))
})

It("returns 200 for gc stats", func() {
_, err := VerifyHTTPSRequest(serviceURL, "gc_stats", tlsConfig)
Expect(err).NotTo(HaveOccurred())
})

It("returns 200 for prom metrics", func() {
_, err := VerifyHTTPRequest(prometheusURL, "metrics")
Expect(err).NotTo(HaveOccurred())
Expand Down
59 changes: 59 additions & 0 deletions test/integration/profiler_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package integration_test

import (
"fmt"
"net/http"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

var (
gcStatsPath = "/gc_stats"
liveProfilerPath = "/live_profiler?mode=off"
pprofPath = "/debug/pprof/"
)

var _ = Describe("Profiler Endpoints", func() {
var (
client *http.Client
)

BeforeEach(func() {
cfg, err := GetTLSConfig()
Expect(err).NotTo(HaveOccurred())

client = &http.Client{
Transport: &http.Transport{
TLSClientConfig: cfg,
},
}
})

Context("Service port", func() {
DescribeTable("returns 400",
func(path string) {
r, err := client.Get(fmt.Sprintf("https://%s%s", serviceURL, path))
Expect(err).NotTo(HaveOccurred())
Expect(r.StatusCode).To(Equal(400))
},

Entry("for "+gcStatsPath, gcStatsPath),
Entry("for "+liveProfilerPath, liveProfilerPath),
Entry("for "+pprofPath, pprofPath))
})

Context("Profiler port", func() {
DescribeTable("succeeds",
func(path string, expectedCode int) {
r, err := client.Get(fmt.Sprintf("http://%s%s", profilerURL, path))
Expect(err).NotTo(HaveOccurred())
Expect(r.StatusCode).To(Equal(expectedCode))
},

Entry("for "+gcStatsPath, gcStatsPath, 200),
Entry("for "+liveProfilerPath, liveProfilerPath, 304), // We get 304 when the profiler is enabled, and we ask it to switch to the mode it is already in
Entry("for "+pprofPath, pprofPath, 200),
)
})
})
1 change: 1 addition & 0 deletions test/integration/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (

serviceURL = "app:4443"
statusURL = "app:8080"
profilerURL = "app:4269"
prometheusURL = "app:9090"

clientCert = "./test-certs/vehicle_device.device-1.cert"
Expand Down

0 comments on commit 4184af1

Please sign in to comment.