(moved from #13334) feat: fix #13327 #13358
Conversation
-support custom print stack trace depth -write error message directly into OutputStream
Linchen-Xu
changed the title
| System.setProperty(DUBBO_JSTACK_MAXLINE, depth.toString()); | ||
| try (FileOutputStream jStackStream = | ||
| new FileOutputStream(new File("/tmp", "Dubbo_JStack.log" + "." + dateStr))) { | ||
| jstack(jStackStream); |
There was a problem hiding this comment.
Check the result which it print
There was a problem hiding this comment.
Added the comparison between the depth printed by the old logic and the new logic. PTAL.
When it traces back to JVMUtil, the line number will be different, making purely comparing two outputs impractical, unless we ignore some lines in the output.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 3.2 #13358 +/- ##
============================================
+ Coverage 69.56% 69.59% +0.02%
Complexity 2 2
============================================
Files 1652 1652
Lines 71608 71609 +1
Branches 10264 10263 -1
============================================
+ Hits 49814 49835 +21
+ Misses 17098 17086 -12
+ Partials 4696 4688 -8 ☔ View full report in Codecov by Sentry. |
| try { | ||
| byte[] encoded = Files.readAllBytes(Paths.get(String.format("/tmp/%s", filename))); | ||
| String newOutput = new String(encoded, StandardCharsets.UTF_8); | ||
| int newStackDepth = calculateStackDepth(newOutput); |
There was a problem hiding this comment.
Maybe we can decode the stack file with some tools to verify if it match the standard output style.
There was a problem hiding this comment.
I replaced it with ByteArrayOutputStream to get the result directly, there's no need to decode anymore.
By the way, I tried dumping threads and using the same thread info to generate the stack trace info in both the old way and the new way. Therefore, we can compare the stack trace directly, which turns out to be working correctly.
Optimize logic in JVMUtil to test without writing files while being able to compare the two string outputs directly.
| ThreadInfo[] threadInfos = JVMUtil.dumpAllThreads(); | ||
| // generate new stack trace info | ||
| JVMUtil.jstack(stream, threadInfos); | ||
| String newStackTrace = stream.toString(); | ||
| // generate old stack trace info | ||
| String oldStackTrace = OldJVMUtil.jstack(threadInfos); | ||
| // calculate stack trace depth | ||
| int newStackDepth = calculateStackDepth(newStackTrace); | ||
| int oldStackDepth = calculateStackDepth(oldStackTrace); | ||
|
|
There was a problem hiding this comment.
I think we need to verify if JVMUtil work as expected, not work as previous version/
There was a problem hiding this comment.
Added stack dump test code in AbortPolicyWithReportTest.jStackDumpTest to ensure that the new code is working as expected, but have to check the output file under /tmp.
The output file after running AbortPolicyWithReportTest.jStackDumpTest when setting dubbo.jstack-dump.max-line=-1:
|
Kudos, SonarCloud Quality Gate passed! |
|
@CrazyHZM PTAL |
Moved from #13334 due to branch operation error.
What is the purpose of the change
fix #13327
dubbo.jstack-dump.max-linecan be used to indicate that all stack trace lines should be printed now by setting it to a negative number, -1 for example.JVMUtilTest.Brief changelog
org.apache.dubbo.common.utils.JVMUtil.org.apache.dubbo.common.utils.JVMUtilTest.AbortPolicyWithReportTest.Verifying this change
Checklist