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

add new flag scan external jar package #416

Closed
wants to merge 10 commits into from

Conversation

cyemars
Copy link

@cyemars cyemars commented Sep 29, 2020

Describe what this PR does / why we need it

解决Java工程无法调用外部jar 包

Does this pull request fix one issue?

添加flag 指定外部jar 包扫描路径

Describe how you did it

在agent挂载的时候传入jar包路径 扫描二方包

@cyemars cyemars changed the title chaosblade扫描外部jar包 flag添加 add new flag scan external jar package Sep 29, 2020
@cyemars cyemars closed this Sep 29, 2020
@cyemars cyemars reopened this Sep 29, 2020
@xcaspar xcaspar self-requested a review September 29, 2020 07:09
pc.command.Flags().StringVarP(&pc.javaHome, "javaHome", "j", "", "the java jdk home path")
pc.command.Flags().StringVarP(&pc.chaosbaldeJarPath, "chaosbaldeJarPath", "jarPath", "", "the java jdk home path")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag name is too long.

@@ -102,14 +105,14 @@ func (pc *PrepareJvmCommand) prepareJvm() error {
"please append or modify the --port %s argument in prepare command for retry", record.Port))
}
}
response, username := jvm.Attach(record.Port, pc.javaHome, pc.processId)
response, username := jvm.Attach(record.Port, pc.javaHome, pc.processId,pc.chaosbaldeJarPath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please format your codes.

jvmOpts = fmt.Sprintf("-Xms128M -Xmx128M -Xnoclassgc -ea -Xbootclasspath/a:%s:%s", toolsJar, chaosbaldeJarPath)
logrus.Printf("chaosblade jvmOpts:%s", jvmOpts)
} else {
jvmOpts = fmt.Sprintf("-Xms128M -Xmx128M -Xnoclassgc -ea -Xbootclasspath/a:%s:", toolsJar)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An extra colon is written.
You can write in the following way:

jvmOpts = xxxx
if condition {
  jvmOpts = fmt.Sprintf("%s:%s", jvmOpts, xxx)
}

@tiny-x
Copy link
Member

tiny-x commented Oct 9, 2020

Describe what this PR does / why we need it

解决Java工程无法调用外部jar 包

Does this pull request fix one issue?

添加flag 指定外部jar 包扫描路径

Describe how you did it

在agent挂载的时候传入jar包路径 扫描二方包

./blade -h

panic: "jp" shorthand is more than one ASCII character

goroutine 1 [running]:
github.com/spf13/pflag.(*FlagSet).AddFlag(0xc000279400, 0xc000583360)
/Users/yefei/go/pkg/mod/github.com/spf13/pflag@v1.0.5/flag.go:864 +0x749
github.com/spf13/pflag.(*FlagSet).VarPF(0xc000279400, 0x539acc0, 0xc000193cd0, 0x51a1a92, 0x7, 0x519f61a, 0x2, 0x51b0af2, 0x16, 0xc0005832c0)
/Users/yefei/go/pkg/mod/github.com/spf13/pflag@v1.0.5/flag.go:831 +0x10b
github.com/spf13/pflag.(*FlagSet).VarP(...)
/Users/yefei/go/pkg/mod/github.com/spf13/pflag@v1.0.5/flag.go:837
github.com/spf13/pflag.(*FlagSet).StringVarP(0xc000279400, 0xc000193cd0, 0x51a1a92, 0x7, 0x519f61a, 0x2, 0x0, 0x0, 0x51b0af2, 0x16)
/Users/yefei/go/pkg/mod/github.com/spf13/pflag@v1.0.5/string.go:42 +0xad
github.com/chaosblade-io/chaosblade/cli/cmd.(*PrepareJvmCommand).Init(0xc000193c80)
/temp/chaosblade/cli/cmd/prepare_jvm.go:56 +0x1d2
github.com/chaosblade-io/chaosblade/cli/cmd.(*baseCommand).AddCommand(0xc00019e4c8, 0x5398a00, 0xc000193c80)
/temp/chaosblade/cli/cmd/command.go:136 +0x35
github.com/chaosblade-io/chaosblade/cli/cmd.CmdInit(0xc00077bf48)
/temp/chaosblade/cli/cmd/cmd.go:33 +0x102
main.main()
/temp/chaosblade/cli/main.go:27 +0x26

@xcaspar xcaspar requested a review from tiny-x October 10, 2020 03:32
pc.command.Flags().StringVarP(&pc.javaHome, "javaHome", "j", "", "the java jdk home path")
pc.command.Flags().StringVarP(&pc.chaosbladeJarPath, "jarPath", "jp", "", "the java jdk home path")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"jp" shorthand is more than one ASCII character

@tiny-x
Copy link
Member

tiny-x commented Oct 10, 2020

@cyemars This scheme seems not to work. -Xbootclasspath is only started for the JVM that loads the agent, not the JVM that affects the application

@tiny-x
Copy link
Member

tiny-x commented Mar 4, 2021

@tiny-x tiny-x closed this Mar 4, 2021
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.

None yet

3 participants