-
Notifications
You must be signed in to change notification settings - Fork 350
Remove check for JAVA_TOOLS_OPTIONS in verifyAgentJarIntegrations #12103
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -622,6 +622,10 @@ tasks.register('verifyAgentJarIntegrations', JavaExec) { | |
| classpath = objects.fileCollection().from(jarProvider) | ||
| args = ['--list-integrations'] | ||
|
|
||
| // Listing integrations only inspects classes and metadata in the assembled agent jar, so it does | ||
| // not need inherited JAVA_TOOL_OPTIONS. | ||
| environment.remove('JAVA_TOOL_OPTIONS') | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Does this remove the ENV from this Gradle task only? Meaning this would not affect other tasks running as part of the same build which may require
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes -
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: There are another special env var That said, I'm not sure this is worth a follow-up at this time. |
||
|
|
||
| // Capture both stdout and stderr: InstrumenterIndex.buildModule() logs ERROR and returns null when a module | ||
| // fails to load, while the process exits with status 0. | ||
| def capturedOutput = new ByteArrayOutputStream() | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can follow this way as a fix (and maybe even keep it).
As alternative solution we may probably need to make check logic more robust.
like add some sort of
begin markerandend markerand analyze output between markers.I just executed locally:
java -jar dd-java-agent/build/libs/dd-java-agent-1.65.0-SNAPSHOT.jar --list-integrationsand got the output:
So as possible improvements we can generate something like:
java -jar dd-java-agent/build/libs/dd-java-agent-1.65.0-SNAPSHOT.jar --list-integrations --with-markerThis is just my idea and this solution will require more work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AlexeyKuznetsov-DD I'm not sure it's worth introducing this option in the tracer. I.e. I don't see how it would benefit other usages.