From dd7f12f7c08dcf654145a86d3b2649192dec54ba Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 7 Jul 2023 01:35:19 +0200 Subject: [PATCH 1/6] Add http pprof profiling endpoint Signed-off-by: Tim Vaillancourt --- control.go | 16 ++++++++-------- examples/config.yml | 4 +++- http.go | 34 ++++++++++++++++++++++++++++++++++ main.go | 17 ++++++++++++++--- stats.go | 27 ++++++++++++--------------- 5 files changed, 71 insertions(+), 27 deletions(-) create mode 100644 http.go diff --git a/control.go b/control.go index 203278dff..7391f0c76 100644 --- a/control.go +++ b/control.go @@ -18,12 +18,12 @@ import ( // core. This means copying IP objects, slices, de-referencing pointers and taking the actual value, etc type Control struct { - f *Interface - l *logrus.Logger - cancel context.CancelFunc - sshStart func() - statsStart func() - dnsStart func() + f *Interface + l *logrus.Logger + cancel context.CancelFunc + sshStart func() + httpStart func() + dnsStart func() } type ControlHostInfo struct { @@ -48,8 +48,8 @@ func (c *Control) Start() { if c.sshStart != nil { go c.sshStart() } - if c.statsStart != nil { - go c.statsStart() + if c.httpStart != nil { + go c.httpStart() } if c.dnsStart != nil { go c.dnsStart() diff --git a/examples/config.yml b/examples/config.yml index a7acb737c..42efa5f04 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -244,6 +244,9 @@ logging: # As an example, to log as RFC3339 with millisecond precision, set to: #timestamp_format: "2006-01-02T15:04:05.000Z07:00" +#http: + #listen: 127.0.0.1:8080 + #stats: #type: graphite #prefix: nebula @@ -252,7 +255,6 @@ logging: #interval: 10s #type: prometheus - #listen: 127.0.0.1:8080 #path: /metrics #namespace: prometheusns #subsystem: nebula diff --git a/http.go b/http.go new file mode 100644 index 000000000..873ddca67 --- /dev/null +++ b/http.go @@ -0,0 +1,34 @@ +package nebula + +import ( + "fmt" + "net/http" + _ "net/http/pprof" + + "github.com/sirupsen/logrus" + "github.com/slackhq/nebula/config" +) + +func startHttp(l *logrus.Logger, c *config.C, statsHandler statsHandlerFunc, listen string) (f func(), err error) { + if listen == "" { + return nil, nil + } + + var statsPath string + if statsHandler != nil { + statsPath = c.GetString("stats.path", "") + if statsPath == "" { + return nil, fmt.Errorf("stats.path should not be empty") + } + } + + f = func() { + l.Infof("Go pprof handler listening on %s at /debug/pprof", listen) + if statsHandler != nil { + http.Handle(statsPath, statsHandler(listen, statsPath)) + } + l.Fatal(http.ListenAndServe(listen, nil)) + } + + return f, err +} diff --git a/main.go b/main.go index e4c262413..ee2b8eda5 100644 --- a/main.go +++ b/main.go @@ -324,14 +324,25 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg go lightHouse.LhUpdateWorker(ctx, ifce) } + httpListen := c.GetString("http.listen", "") + if httpListen == "" { + if httpListen = c.GetString("stats.listen", ""); httpListen != "" { + l.Warn("http.listen is undef, falling back to stats.listen. stats.listen will be deprecated in a future release.") + } + } + // TODO - stats third-party modules start uncancellable goroutines. Update those libs to accept // a context so that they can exit when the context is Done. - statsStart, err := startStats(l, c, buildVersion, configTest) - + statsHTTPHandler, err := startStats(l, c, httpListen, buildVersion, configTest) if err != nil { return nil, util.NewContextualError("Failed to start stats emitter", nil, err) } + httpStart, err := startHttp(l, c, statsHTTPHandler, httpListen) + if err != nil { + return nil, util.NewContextualError("Failed to start http server", nil, err) + } + if configTest { return nil, nil } @@ -348,5 +359,5 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg dnsStart = dnsMain(l, hostMap, c) } - return &Control{ifce, l, cancel, sshStart, statsStart, dnsStart}, nil + return &Control{ifce, l, cancel, sshStart, httpStart, dnsStart}, nil } diff --git a/stats.go b/stats.go index c88c45cc9..e44446999 100644 --- a/stats.go +++ b/stats.go @@ -3,7 +3,6 @@ package nebula import ( "errors" "fmt" - "log" "net" "net/http" "runtime" @@ -19,10 +18,12 @@ import ( "github.com/slackhq/nebula/config" ) +type statsHandlerFunc func(listen, path string) http.Handler + // startStats initializes stats from config. On success, if any further work -// is needed to serve stats, it returns a func to handle that work. If no +// is needed to serve stats, it returns an http.Handler for that work. If no // work is needed, it'll return nil. On failure, it returns nil, error. -func startStats(l *logrus.Logger, c *config.C, buildVersion string, configTest bool) (func(), error) { +func startStats(l *logrus.Logger, c *config.C, listen, buildVersion string, configTest bool) (f statsHandlerFunc, err error) { mType := c.GetString("stats.type", "") if mType == "" || mType == "none" { return nil, nil @@ -33,7 +34,6 @@ func startStats(l *logrus.Logger, c *config.C, buildVersion string, configTest b return nil, fmt.Errorf("stats.interval was an invalid duration: %s", c.GetString("stats.interval", "")) } - var startFn func() switch mType { case "graphite": err := startGraphiteStats(l, interval, c, configTest) @@ -41,8 +41,7 @@ func startStats(l *logrus.Logger, c *config.C, buildVersion string, configTest b return nil, err } case "prometheus": - var err error - startFn, err = startPrometheusStats(l, interval, c, buildVersion, configTest) + f, err = startPrometheusStats(l, interval, c, listen, buildVersion, configTest) if err != nil { return nil, err } @@ -56,7 +55,7 @@ func startStats(l *logrus.Logger, c *config.C, buildVersion string, configTest b go metrics.CaptureDebugGCStats(metrics.DefaultRegistry, interval) go metrics.CaptureRuntimeMemStats(metrics.DefaultRegistry, interval) - return startFn, nil + return f, nil } func startGraphiteStats(l *logrus.Logger, i time.Duration, c *config.C, configTest bool) error { @@ -79,13 +78,12 @@ func startGraphiteStats(l *logrus.Logger, i time.Duration, c *config.C, configTe return nil } -func startPrometheusStats(l *logrus.Logger, i time.Duration, c *config.C, buildVersion string, configTest bool) (func(), error) { +func startPrometheusStats(l *logrus.Logger, i time.Duration, c *config.C, listen, buildVersion string, configTest bool) (statsHandlerFunc, error) { namespace := c.GetString("stats.namespace", "") subsystem := c.GetString("stats.subsystem", "") - listen := c.GetString("stats.listen", "") if listen == "" { - return nil, fmt.Errorf("stats.listen should not be empty") + return nil, fmt.Errorf("http.listen or stats.listen must be defined to use promtheus stats") } path := c.GetString("stats.path", "") @@ -114,14 +112,13 @@ func startPrometheusStats(l *logrus.Logger, i time.Duration, c *config.C, buildV pr.MustRegister(g) g.Set(1) - var startFn func() + var f statsHandlerFunc if !configTest { - startFn = func() { + f = func(listen, path string) http.Handler { l.Infof("Prometheus stats listening on %s at %s", listen, path) - http.Handle(path, promhttp.HandlerFor(pr, promhttp.HandlerOpts{ErrorLog: l})) - log.Fatal(http.ListenAndServe(listen, nil)) + return promhttp.HandlerFor(pr, promhttp.HandlerOpts{ErrorLog: l}) } } - return startFn, nil + return f, nil } From 90aa8efb2976af18bcdcbe383d68cae9d35f75e4 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 7 Jul 2023 01:37:36 +0200 Subject: [PATCH 2/6] remove dupe stats.path check Signed-off-by: Tim Vaillancourt --- stats.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/stats.go b/stats.go index e44446999..56d293893 100644 --- a/stats.go +++ b/stats.go @@ -86,11 +86,6 @@ func startPrometheusStats(l *logrus.Logger, i time.Duration, c *config.C, listen return nil, fmt.Errorf("http.listen or stats.listen must be defined to use promtheus stats") } - path := c.GetString("stats.path", "") - if path == "" { - return nil, fmt.Errorf("stats.path should not be empty") - } - pr := prometheus.NewRegistry() pClient := mp.NewPrometheusProvider(metrics.DefaultRegistry, namespace, subsystem, pr, i) if !configTest { From 70779b26ad438e88db172f096efeef52af2c39bc Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 7 Jul 2023 01:43:42 +0200 Subject: [PATCH 3/6] Fix typo in log Signed-off-by: Tim Vaillancourt --- stats.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stats.go b/stats.go index 56d293893..5ec5c5c64 100644 --- a/stats.go +++ b/stats.go @@ -83,7 +83,7 @@ func startPrometheusStats(l *logrus.Logger, i time.Duration, c *config.C, listen subsystem := c.GetString("stats.subsystem", "") if listen == "" { - return nil, fmt.Errorf("http.listen or stats.listen must be defined to use promtheus stats") + return nil, fmt.Errorf("http.listen or stats.listen must be defined to use Prometheus stats") } pr := prometheus.NewRegistry() From af958676b81a40f91be9cea2c70062dd82b77af5 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 7 Jul 2023 02:00:30 +0200 Subject: [PATCH 4/6] Cleanup code Signed-off-by: Tim Vaillancourt --- http.go | 10 +++++----- main.go | 2 +- stats.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/http.go b/http.go index 873ddca67..427974ff8 100644 --- a/http.go +++ b/http.go @@ -9,7 +9,9 @@ import ( "github.com/slackhq/nebula/config" ) -func startHttp(l *logrus.Logger, c *config.C, statsHandler statsHandlerFunc, listen string) (f func(), err error) { +// startHttp returns a function to start an http server with pprof support and optionally, a provided stats +// http handler. +func startHttp(l *logrus.Logger, c *config.C, listen string, statsHandler statsHandlerFunc) (func(), error) { if listen == "" { return nil, nil } @@ -22,13 +24,11 @@ func startHttp(l *logrus.Logger, c *config.C, statsHandler statsHandlerFunc, lis } } - f = func() { + return func() { l.Infof("Go pprof handler listening on %s at /debug/pprof", listen) if statsHandler != nil { http.Handle(statsPath, statsHandler(listen, statsPath)) } l.Fatal(http.ListenAndServe(listen, nil)) - } - - return f, err + }, nil } diff --git a/main.go b/main.go index ee2b8eda5..e9a774d12 100644 --- a/main.go +++ b/main.go @@ -338,7 +338,7 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg return nil, util.NewContextualError("Failed to start stats emitter", nil, err) } - httpStart, err := startHttp(l, c, statsHTTPHandler, httpListen) + httpStart, err := startHttp(l, c, httpListen, statsHTTPHandler) if err != nil { return nil, util.NewContextualError("Failed to start http server", nil, err) } diff --git a/stats.go b/stats.go index 5ec5c5c64..a66a7fa47 100644 --- a/stats.go +++ b/stats.go @@ -107,13 +107,13 @@ func startPrometheusStats(l *logrus.Logger, i time.Duration, c *config.C, listen pr.MustRegister(g) g.Set(1) - var f statsHandlerFunc + var startHandler statsHandlerFunc if !configTest { - f = func(listen, path string) http.Handler { + startHandler = func(listen, path string) http.Handler { l.Infof("Prometheus stats listening on %s at %s", listen, path) return promhttp.HandlerFor(pr, promhttp.HandlerOpts{ErrorLog: l}) } } - return f, nil + return startHandler, nil } From c14ad0f27b1f7343765ff28b8a09c048cbb749f4 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 7 Jul 2023 02:07:33 +0200 Subject: [PATCH 5/6] Cleanup code comment Signed-off-by: Tim Vaillancourt --- stats.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stats.go b/stats.go index a66a7fa47..570372bf0 100644 --- a/stats.go +++ b/stats.go @@ -21,8 +21,8 @@ import ( type statsHandlerFunc func(listen, path string) http.Handler // startStats initializes stats from config. On success, if any further work -// is needed to serve stats, it returns an http.Handler for that work. If no -// work is needed, it'll return nil. On failure, it returns nil, error. +// is needed to serve stats, it returns an statsHandlerFunc for that work. If +// no work is needed, it'll return nil. On failure, it returns nil, error. func startStats(l *logrus.Logger, c *config.C, listen, buildVersion string, configTest bool) (f statsHandlerFunc, err error) { mType := c.GetString("stats.type", "") if mType == "" || mType == "none" { From 0af7e6a1ddadd96dfe967ba49e456f2c67969b1e Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 7 Jul 2023 17:06:19 +0200 Subject: [PATCH 6/6] improve comments Signed-off-by: Tim Vaillancourt --- http.go | 2 +- stats.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/http.go b/http.go index 427974ff8..60044b12e 100644 --- a/http.go +++ b/http.go @@ -9,7 +9,7 @@ import ( "github.com/slackhq/nebula/config" ) -// startHttp returns a function to start an http server with pprof support and optionally, a provided stats +// startHttp returns a function to start an http server with pprof support and optionally a provided stats // http handler. func startHttp(l *logrus.Logger, c *config.C, listen string, statsHandler statsHandlerFunc) (func(), error) { if listen == "" { diff --git a/stats.go b/stats.go index 570372bf0..683285744 100644 --- a/stats.go +++ b/stats.go @@ -21,7 +21,7 @@ import ( type statsHandlerFunc func(listen, path string) http.Handler // startStats initializes stats from config. On success, if any further work -// is needed to serve stats, it returns an statsHandlerFunc for that work. If +// is needed to serve stats, it returns a statsHandlerFunc for that work. If // no work is needed, it'll return nil. On failure, it returns nil, error. func startStats(l *logrus.Logger, c *config.C, listen, buildVersion string, configTest bool) (f statsHandlerFunc, err error) { mType := c.GetString("stats.type", "")