Skip to content

Commit

Permalink
Fix profiler port
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 committed Jan 22, 2024
1 parent 98aad24 commit 00587e4
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 00587e4

Please sign in to comment.