Write stack dump to os.TempDir() as well #3190
Conversation
lowenna
changed the title
os.TempDir() as well
Codecov Report
@@ Coverage Diff @@
## master #3190 +/- ##
======================================
Coverage 45.2% 45.2%
======================================
Files 111 111
Lines 11982 11982
======================================
Hits 5416 5416
Misses 5732 5732
Partials 834 834
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #3190 +/- ##
==========================================
- Coverage 44.63% 44.62% -0.01%
==========================================
Files 113 113
Lines 12161 12161
==========================================
- Hits 5428 5427 -1
- Misses 5898 5899 +1
Partials 835 835
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Can we log the location where the dump is written to?
There was a problem hiding this comment.
Also wondering if we should add something random (or a time stamp) to the name, as Pid might be reused
There was a problem hiding this comment.
Any reason not to just use https://golang.org/pkg/io/ioutil/#TempFile . That adds the random component and does the create.
There was a problem hiding this comment.
@thaJeztah @dmcgowan Randomising it is actually the complete opposite of what I was deliberately trying to achieve....
There was a problem hiding this comment.
I should probably explain the rationale here - getting stacks out is really a pretty advanced thing to do. You really only care about the stacks at this point in time, usually to investigate a deadlock. For that reason, in many ways even using the PID here is overkill.
estesp
left a comment
estesp
left a comment
There was a problem hiding this comment.
LGTM
This makes sense to me as-is; it is writing files in the temp dir of the OS, so will not cause long-term storage issues. My expectation is this is a debug "helper" given the only way to get stackdump now is to have info level logging (or higher) enabled before containerd start. This allows you to quickly get a stack dump into a separate file without a restart of containerd if your loglevel isn't high enough. I don't see the need for any more "randomness" in the filename as it is a manual step to force a stackdump (usually because of a hang), and so you know the PID, and the filename having the PID makes it easy to find.
|
Would #3190 (comment) be useful? Something along the lines of logrus.Infof("goroutine stack dump written to %s", path) |
@thaJeztah Updated |
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM (not a maintainer)
thanks!
|
@jhowardmsft your last push lost the initial |
@estesp Oooops. Fixed. |
|
Please rebase on |
Signed-off-by: John Howard <[email protected]>
@estesp Done |
|
@kevpar - Can we verify that we dont write this file on a etw capture state? |
7 participants
Signed-off-by: John Howard [email protected]
As it says in the title. @jterry75 PTAL.