agentlog: add pprof debug http handler #3237

christoph-zededa · 2023-05-23T17:56:59Z

pprof show various insights into the process and can be accessed via http://localhost:6543/

to start it, send USR2 to zedbox or run eve http-debug; to stop it, do curl localhost:6543/stop -X POST
or eve http-debug stop

pkg/pillar/agentlog/agentlog.go

christoph-zededa · 2023-05-23T18:00:37Z

TODO @christoph-zededa : yetus warnings

done ...

christoph-zededa · 2023-05-23T18:02:19Z

Previous comment from @milan-zededa : Maybe we should take this opportunity to document all the features of agentlog that one may use for debugging purposes? (e.g. under this folder) - #3229 (comment)

If it is not too much, I can add your suggestions. Also I added https://github.com/lf-edge/eve/pull/3237/files#diff-e76641ce9bfa02962b0687e68a963a44dfa9d1a4f427cfafd9fbfa36d55986f4R225

rouming · 2023-05-24T13:29:04Z

conf/grub.cfg

@@ -3,3 +3,4 @@
 #   set_global eve_flavor kvm
 # to force booting in Xen mode, uncomment:
 #   set_global eve_flavor xen
+set_getty


Added by mistake. And yes, we need to change how we prepare the debug grub config. Otherwise it is really annoying.

removed it
(I let this comment open in case I accidentally re-add it ...)

eriknordmark

One heads up. Some stack trace code is invoke from the logrus log.Panic callback, and that can in turn not call back into logrus. Should be comments around that code, but I don't recall how it relates to dumping all of the goroutine stacks. Can you double-check that that code path is not affected by this restructuring?

eriknordmark · 2023-05-25T07:56:06Z

pkg/dom0-ztools/rootfs/bin/eve

+        pkill -USR2 /opt/zededa/bin/zedbox
+    fi
+
+    wget -O - "$URL" --post-data ''


I didn't think we have wget included in EVE-OS.
We have curl e.g., in the debug container, but not in pillar etc.Will this be invoked from inside the debug container?
Note that a console (keyboard and screen) lands in a different place than a ssh to EVE-OS.,

I switched to wget as we don't have curl in the root container (terminal / serial console).

Now I also checked if we have wget in the debug container and when using ssh - we do.
So it should all work.

I didn't think we had curl outside of the debug container either. I wonder where that and wget gets pulled in (we want to minimize the set of packages we use to minimize the CVEs we need to look for).

Okay, I changed it to nc: 83f3bec

christoph-zededa · 2023-05-25T15:59:14Z

One heads up. Some stack trace code is invoke from the logrus log.Panic callback, and that can in turn not call back into logrus. Should be comments around that code, but I don't recall how it relates to dumping all of the goroutine stacks. Can you double-check that that code path is not affected by this restructuring?

so, initImpl registers an "exit handler" on logrus which calls https://github.com/lf-edge/eve/blob/master/pkg/pillar/agentlog/agentlog.go#L242 which uses getStacks (https://github.com/lf-edge/eve/blob/master/pkg/pillar/agentlog/agentlog.go#L467) to retrieve the stacktrace which is the same method used when getting the stacktrace via USR1 (this one does not log).

So, as far as I understand, the stacktrace is only printed via log as an ExitHandler which is called by logrus.Exit or when a "Fatal log entry is made" (https://github.com/sirupsen/logrus/blob/master/alt_exit.go#L56) and log.Fatal is only used before registering the ExitHandler (in spoofStdFDs); so there is no re-entrancy.
Our FatalHandler is calling RebootReason which has the comment "NOTE: can not use log here since we are called from a log hook!" I guess you meant. Nothing has been changed here.

pkg/dom0-ztools/rootfs/bin/eve

pprof show various insights into the process and can be accessed via http://localhost:6543/ to start it, send USR2 to zedbox or run `eve http-debug`; to stop it, do curl localhost:6543/stop -X POST or `eve http-debug stop` Signed-off-by: Christoph Ostarek <christoph@zededa.com>

this first starts the pprof interface on zedbox and then forwards the port to the edgeview client once the user is done the pprof interface is closed again Signed-off-by: Christoph Ostarek <christoph@zededa.com>

in order to keep the image small and reduce the attack surface wget may be removed in the future, so instead use nc Signed-off-by: Christoph Ostarek <christoph@zededa.com>

naiming-zededa

LGTM

naiming-zededa · 2023-05-23T23:16:59Z

Makefile

@@ -46,6 +46,8 @@ INSTALLER_IMG_FORMAT=raw
 VERIFICATION_IMG_FORMAT=raw
 # SSH port to use for running images live
 SSH_PORT=2222
+# Debug port to access debugging information (e.g. pprof)
+DEBUG_PORT=6543


Isn't the normal pprof port '6060' is used? https://pkg.go.dev/net/http/pprof

Code does not exist anymore - is outdated.

naiming-zededa · 2023-05-23T23:18:09Z

pkg/dom0-ztools/rootfs/bin/eve

+              iptables -t mangle -F
+              echo "Listening on :6543 -- use 'eve http-debug stop' to stop"
+          elif [ "$2" = "stop" ]; then
+              wget -O - 127.1:6543/stop --post-data '' > /dev/null 2>&1


do we need to remove the iptable setup after stop?

Code does not exist anymore - is outdated.

pkg/pillar/agentlog/agentlog.go

naiming-zededa · 2023-05-23T23:29:06Z

pkg/pillar/agentlog/agentlog.go

+	    (both scripts can be found <a href="https://github.com/brendangregg/FlameGraph">here</a>)
+		`
+
+	mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {


zedcloud side has plan to move away from mux router due to: https://github.com/gorilla#gorilla-toolkit, not sure about the EVE status on this.

I am not sure about the status in EVE; I guess we need to discuss this soon.

@naiming-zededa @christoph-zededa this is not gorillamux; just the simple http.NewServeMux() which we also use in zedrouter/server.go

naiming-zededa · 2023-06-05T17:15:12Z

pkg/edgeview/src/system.go

@@ -982,6 +983,22 @@ func dispAFile(f os.FileInfo) {
 	fmt.Printf("%s, %v, %d, %s\n", f.Mode().String(), f.ModTime(), f.Size(), f.Name())
 }

+func runPprof() {
+	cmd := exec.Command("/usr/bin/pkill", "-USR2", "/opt/zededa/bin/zedbox")
+


do you expect the client side to query the pprof stats quite soon? otherwise the tcp will timeout if no activity. Or in rare case the cmd run has error, we need to exit here. Would be more flexible to offer more options. For example, with 'pprof/on' to trigger it to start and send back status of success or not and print out the port being used, 'pprof/off' to shut it down. either use general tcp/localhost:port, or do a 'pprof/run' to do all 3.
also, add the 'help' line in the basics.go for the new command.

"do a 'pprof/run' to do all 3" <- that was my initial plan, we can just drop
5ea17eb
and it works that way.

naiming-zededa · 2023-06-07T17:13:33Z

pkg/edgeview/src/system.go

@@ -1215,7 +1233,7 @@ func runTechSupport(cmds cmdOpt, isLocal bool) {
 	closePipe(true)

 	printTitle("\n       - system info -\n\n", colorRED, false)
-	runSystem(cmds, "hw,model,pci,usb,lastreboot,newlog,volume,app,datastore,cipher,dmesg,configitem")
+	runSystem(cmds, "hw,model,pci,usb,lastreboot,newlog,volume,app,datastore,cipher,dmesg,configitem,pprof")


This should not be in techsupport, since without option input it will just return, and it will not generate any output into the techsupport file.

naiming-zededa · 2023-06-08T00:16:22Z

also, if you can update this document: https://wiki.lfedge.org/display/EVE/EdgeView+Commands to add the new command there. thanks.

let the user of edgeview do it manually Signed-off-by: Christoph Ostarek <christoph@zededa.com>

eriknordmark · 2023-06-12T19:07:22Z

pkg/pillar/agentlog/agentlog.go

 	"syscall"
 	"time"

 	"github.com/lf-edge/eve/pkg/pillar/base"
 	"github.com/lf-edge/eve/pkg/pillar/types"
 	fileutils "github.com/lf-edge/eve/pkg/pillar/utils/file"
-	"github.com/satori/go.uuid"


I don't understand the need for this change; the package is referenced by uuid in any case.

It was just done by gopls/goimports.
I've read a bit on golang/go#28428 and it seems it speeds up checking for imports.

eriknordmark

LGTM.
@naiming-zededa have your issues been resolved?

christoph-zededa commented May 23, 2023

View reviewed changes

pkg/pillar/agentlog/agentlog.go Outdated Show resolved Hide resolved

eriknordmark requested review from naiming-zededa and milan-zededa May 23, 2023 21:42

christoph-zededa force-pushed the enable_pprof branch 2 times, most recently from 4d7812c to 4a89c72 Compare May 24, 2023 12:20

rouming reviewed May 24, 2023

View reviewed changes

christoph-zededa force-pushed the enable_pprof branch from 4a89c72 to 7740c64 Compare May 24, 2023 15:58

eriknordmark reviewed May 25, 2023

View reviewed changes

christoph-zededa force-pushed the enable_pprof branch from 7740c64 to b92677c Compare May 31, 2023 16:38

christoph-zededa marked this pull request as ready for review May 31, 2023 22:01

christoph-zededa requested a review from rvs as a code owner May 31, 2023 22:01

milan-zededa approved these changes Jun 5, 2023

View reviewed changes

christoph-zededa force-pushed the enable_pprof branch from b92677c to 83f3bec Compare June 7, 2023 12:17

eriknordmark reviewed Jun 7, 2023

View reviewed changes

pkg/dom0-ztools/rootfs/bin/eve Outdated Show resolved Hide resolved

eriknordmark reviewed Jun 7, 2023

View reviewed changes

pkg/dom0-ztools/rootfs/bin/eve Outdated Show resolved Hide resolved

christoph-zededa added 2 commits June 7, 2023 18:01

edgeview: allow pprof access

a71accf

this first starts the pprof interface on zedbox and then forwards the port to the edgeview client once the user is done the pprof interface is closed again Signed-off-by: Christoph Ostarek <christoph@zededa.com>

christoph-zededa force-pushed the enable_pprof branch 2 times, most recently from 7dee404 to 8c67fea Compare June 7, 2023 16:02

eve tool: replace wget with nc

bc2eb80

in order to keep the image small and reduce the attack surface wget may be removed in the future, so instead use nc Signed-off-by: Christoph Ostarek <christoph@zededa.com>

christoph-zededa force-pushed the enable_pprof branch from 8c67fea to 5ea17eb Compare June 7, 2023 16:44

naiming-zededa approved these changes Jun 7, 2023

View reviewed changes

edgeview: do not forward tcp for pprof

235d4f6

let the user of edgeview do it manually Signed-off-by: Christoph Ostarek <christoph@zededa.com>

christoph-zededa force-pushed the enable_pprof branch from 5ea17eb to 235d4f6 Compare June 12, 2023 11:27

eriknordmark reviewed Jun 12, 2023

View reviewed changes

eriknordmark approved these changes Jun 12, 2023

View reviewed changes

eriknordmark merged commit 2f6675b into lf-edge:master Jun 14, 2023

christoph-zededa mentioned this pull request Apr 22, 2024

Collect info include memtrace #3858

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

agentlog: add pprof debug http handler #3237

agentlog: add pprof debug http handler #3237

christoph-zededa commented May 23, 2023

christoph-zededa commented May 23, 2023 •

edited

Loading

christoph-zededa commented May 23, 2023

rouming May 24, 2023

christoph-zededa May 24, 2023

eriknordmark left a comment

eriknordmark May 25, 2023

christoph-zededa May 25, 2023

eriknordmark Jun 6, 2023

christoph-zededa Jun 7, 2023

christoph-zededa commented May 25, 2023

naiming-zededa left a comment

naiming-zededa May 23, 2023

christoph-zededa Jun 12, 2023

naiming-zededa May 23, 2023

christoph-zededa Jun 12, 2023

naiming-zededa May 23, 2023

christoph-zededa Jun 12, 2023

eriknordmark Jun 12, 2023

naiming-zededa Jun 5, 2023

christoph-zededa Jun 12, 2023

naiming-zededa Jun 7, 2023

christoph-zededa Jun 12, 2023

naiming-zededa commented Jun 8, 2023

eriknordmark Jun 12, 2023

christoph-zededa Jun 13, 2023

eriknordmark left a comment

agentlog: add pprof debug http handler #3237

agentlog: add pprof debug http handler #3237

Conversation

christoph-zededa commented May 23, 2023

christoph-zededa commented May 23, 2023 • edited Loading

christoph-zededa commented May 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eriknordmark left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christoph-zededa commented May 25, 2023

naiming-zededa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naiming-zededa commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eriknordmark left a comment

Choose a reason for hiding this comment

christoph-zededa commented May 23, 2023 •

edited

Loading