Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix TelnetConsole#process() return status #2397

Merged
merged 1 commit into from
Jan 12, 2023

Conversation

JoeCqupt
Copy link
Contributor

@JoeCqupt JoeCqupt commented Jan 11, 2023

I'm confused that why return STATUS_ERROR at line 218;

maybe it should retrun STATUS_OK?

@JoeCqupt
Copy link
Contributor Author

@hengyunabc PTAL

@hengyunabc
Copy link
Collaborator

这里是由 arthas-boot 调用,如果 telnet 打印出 help 信息,说明没有正常连接到目标端口,说明连接失败了,所以状态是 error。

@hengyunabc hengyunabc closed this Jan 12, 2023
@JoeCqupt
Copy link
Contributor Author

JoeCqupt commented Jan 12, 2023

这里是由 arthas-boot 调用,如果 telnet 打印出 help 信息,说明没有正常连接到目标端口,说明连接失败了,所以状态是 error。

 private static long findProcessByTelnetClient(String arthasHomeDir, int telnetPort) {
        // start java telnet client
        List<String> telnetArgs = new ArrayList<String>();
        telnetArgs.add("-c");
        telnetArgs.add("session");
        telnetArgs.add("--execution-timeout");
        telnetArgs.add("2000");
        // telnet port ,ip
        telnetArgs.add("127.0.0.1");
        telnetArgs.add("" + telnetPort);

        try {
            ByteArrayOutputStream out = new ByteArrayOutputStream(1024);
            String error = null;
            int status = ProcessUtils.startArthasClient(arthasHomeDir, telnetArgs, out);
            if (status == STATUS_EXEC_TIMEOUT) {
                error = "detection timeout";
            } else if (status == STATUS_EXEC_ERROR) {
                error = "detection error";
                AnsiLog.error("process status: {}", status);
                AnsiLog.error("process output: {}", out.toString());
            } else {
                // ignore connect error
            }
            if (error != null) {
                AnsiLog.error("The telnet port {} is used, but process {}, you will connect to an unexpected process.", telnetPort, error);
                AnsiLog.error("Try to use a different telnet port, for example: java -jar arthas-boot.jar --telnet-port 9998 --http-port -1");
                System.exit(1);
            }

我明白arthas-boot会调用,但是它这里没有传递 --help 命令呢。 而且它也没有关注 STATUS_ERROR 状态码。

@hengyunabc
Copy link
Collaborator

目前它可能是无用代码,但改为 STATUS_OK 也是没意义的。

arthas-boot不关注 STATUS_ERROR ,不代表返回 STATUS_ERROR 是不对的。至少显式表明它不会进入到arthas-boot其它的判断逻辑。

@JoeCqupt
Copy link
Contributor Author

JoeCqupt commented Jan 12, 2023

目前它可能是无用代码,但改为 STATUS_OK 也是没意义的。

arthas-boot不关注 STATUS_ERROR ,不代表返回 STATUS_ERROR 是不对的。至少显式表明它不会进入到arthas-boot其它的判断逻辑。

如果我直接使用 arthas-client.jar :

$  java -jar arthas-client.jar --help

Usage: arthas-client [--help] [-c <value>] [-f <value>] [-t <value>] [-w
       <value>] [-h <value>] [target-ip] [port]

Arthas Telnet Client

EXAMPLES:
  java -jar arthas-client.jar 127.0.0.1 3658
  java -jar arthas-client.jar -c 'dashboard -n 1'
  java -jar arthas-client.jar -f batch.as 127.0.0.1

Options and Arguments:
    --help                        Print usage
 -c,--command <value>             Command to execute, multiple commands
                                  separated by ;
 -f,--batch-file <value>          The batch file to execute
 -t,--execution-timeout <value>   The timeout (ms) of execute commands or batch
                                  file
 -w,--width <value>               The terminal width
 -h,--height <value>              The terminal height
 <target-ip>                      Target ip
 <port>                           The remote server port

$ echo $?
1

在这个语意中,我确实只是想看一下help命令并且命令成功执行了。但是返回的状态码其实并不准确。

对比 arthas-boot.jar

$  java -jar arthas-boot.jar --help
[INFO] JAVA_HOME: /Library/Java/JavaVirtualMachines/jdk1.8.0_192.jdk/Contents/Home/jre
[INFO] arthas-boot version: 3.6.7
Usage: arthas-boot [-h] [--target-ip <value>] [--telnet-port <value>]
       [--http-port <value>] [--session-timeout <value>] [--arthas-home <value>]
       [--use-version <value>] [--repo-mirror <value>] [--versions] [--use-http]
       [--attach-only] [-c <value>] [-f <value>] [--height <value>] [--width
       <value>] [-v] [--tunnel-server <value>] [--agent-id <value>] [--app-name
       <value>] [--username <value>] [--password <value>] [--stat-url <value>]
       [--select <value>] [--disabled-commands <value>] [pid]

Bootstrap Arthas

EXAMPLES:
  java -jar arthas-boot.jar <pid>
  java -jar arthas-boot.jar --target-ip 0.0.0.0
  java -jar arthas-boot.jar --telnet-port 9999 --http-port -1
  java -jar arthas-boot.jar --username admin --password <password>
  java -jar arthas-boot.jar --tunnel-server 'ws://192.168.10.11:7777/ws'
--app-name demoapp
  java -jar arthas-boot.jar --tunnel-server 'ws://192.168.10.11:7777/ws'
--agent-id bvDOe8XbTM2pQWjF4cfw
  java -jar arthas-boot.jar --stat-url 'http://192.168.10.11:8080/api/stat'
  java -jar arthas-boot.jar -c 'sysprop; thread' <pid>
  java -jar arthas-boot.jar -f batch.as <pid>
  java -jar arthas-boot.jar --use-version 3.6.7
  java -jar arthas-boot.jar --versions
  java -jar arthas-boot.jar --select math-game
  java -jar arthas-boot.jar --session-timeout 3600
  java -jar arthas-boot.jar --attach-only
  java -jar arthas-boot.jar --disabled-commands stop,dump
  java -jar arthas-boot.jar --repo-mirror aliyun --use-http
WIKI:
  https://arthas.aliyun.com/doc

Options and Arguments:
 -h,--help                        Print usage
    --target-ip <value>           The target jvm listen ip, default 127.0.0.1
    --telnet-port <value>         The target jvm listen telnet port, default
                                  3658
    --http-port <value>           The target jvm listen http port, default 8563
    --session-timeout <value>     The session timeout seconds, default 1800
                                  (30min)
    --arthas-home <value>         The arthas home
    --use-version <value>         Use special version arthas
    --repo-mirror <value>         Use special remote repository mirror, value is
                                  center/aliyun or http repo url.
    --versions                    List local and remote arthas versions
    --use-http                    Enforce use http to download, default use
                                  https
    --attach-only                 Attach target process only, do not connect
 -c,--command <value>             Command to execute, multiple commands
                                  separated by ;
 -f,--batch-file <value>          The batch file to execute
    --height <value>              arthas-client terminal height
    --width <value>               arthas-client terminal width
 -v,--verbose                     Verbose, print debug info.
    --tunnel-server <value>       The tunnel server url
    --agent-id <value>            The agent id register to tunnel server
    --app-name <value>            The app name
    --username <value>            The username
    --password <value>            The password
    --stat-url <value>            The report stat url
    --select <value>              select target process by classname or
                                  JARfilename
    --disabled-commands <value>   disable some commands
 <pid>                            Target pid

$ echo $?
0

@hengyunabc hengyunabc added this to the 3.6.8 milestone Jan 12, 2023
@hengyunabc hengyunabc reopened this Jan 12, 2023
@hengyunabc hengyunabc merged commit 7210985 into alibaba:master Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants