RockteMQ-AI commented on code in PR #2191:
URL:
https://github.com/apache/rocketmq-dashboard/pull/2191#discussion_r3781974721
##########
server/src/main/java/org/apache/rocketmq/studio/provider/apache/RocketMQConsumerDiagnosticsProvider.java:
##########
@@ -49,7 +49,7 @@
public class RocketMQConsumerDiagnosticsProvider implements
ConsumerDiagnosticsProvider {
private static final Pattern THREAD_HEADER =
-
Pattern.compile("^(?<name>.+?)\\s+TID:\\s+(?<id>\\d+)\\s+STATE:\\s+(?<state>\\S+)\\s*$");
+
Pattern.compile("^(?<name>.+?)TID:\\s+(?<id>\\d+)\\s+STATE:\\s+(?<state>\\S+)\\s*$");
Review Comment:
Removing the required whitespace before TID: causes the `name` capture group
to include the %-40s padding spaces for short thread names. The previous regex
kept the name trimmed, but now short names can be returned with trailing
spaces, which may break exact-name consumers or display formatting. Consider
using `(?<name>.+?)\s*TID:` and trimming the captured name so both short
(padded) and long (unpadded) names are parsed cleanly.
##########
server/src/test/java/org/apache/rocketmq/studio/provider/apache/RocketMQConsumerDiagnosticsProviderTest.java:
##########
@@ -98,6 +98,25 @@ void
getConsumerStackShouldUseSelectedInstanceAndParseJstack() throws Exception
verify(adminFactory, never()).execute(anyString(), any(), any());
}
+ @Test
Review Comment:
The new test only verifies a thread name longer than the 40-character jstack
column. Add regression cases for short padded names and exactly-40-character
names to ensure the regex change does not alter the parsed thread name for
those cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
