Skip to content

Add bootstrap check for disabling agent when required#2951

Merged
eyalkoren merged 21 commits intoelastic:mainfrom
eyalkoren:bootstrap-check-disable-agent
Jan 26, 2023
Merged

Add bootstrap check for disabling agent when required#2951
eyalkoren merged 21 commits intoelastic:mainfrom
eyalkoren:bootstrap-check-disable-agent

Conversation

@eyalkoren
Copy link
Contributor

@eyalkoren eyalkoren commented Jan 5, 2023

What does this PR do?

  • allow to disable JVM bootstrap checks with ELASTIC_APM_DISABLE_BOOTSTRAP_CHECKS environment variable, this is currently only possible with -Delastic.apm.disable_bootstrap_checks JVM argument (using JVM system properties), hence allowing to disable bootstrap checks when JVM arguments modification is not possible.
  • ensure that JVM tools detection issues an error and prevents the agent to start.
  • provide an additional way to exclude some processes from instrumentation by relying on JVM system properties.

Checklist

  • This is an enhancement of existing feature - bootstrap checks
    • I have updated CHANGELOG.asciidoc
    • I have added tests that prove my fix is effective or that my feature works
    • I tested the added functionalities manually
    • I added documentation for bootstrap checks
@ghost
Copy link

ghost commented Jan 5, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview previewSnapshots

Expand to view the summary

Build stats

  • Start Time: 2023-01-26T09:50:56.008+0000

  • Duration: 56 min 49 sec

Test stats 🧪

Test Results
Failed 0
Passed 3437
Skipped 42
Total 3479

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark tests.

  • run jdk compatibility tests : Run the JDK Compatibility tests.

  • run integration tests : Run the Agent Integration tests.

  • run end-to-end tests : Run the APM-ITs.

  • run windows tests : Build & tests on windows.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@eyalkoren eyalkoren marked this pull request as ready for review January 5, 2023 18:01
@github-actions
Copy link

github-actions bot commented Jan 5, 2023

/test

Comment on lines 103 to 106
1. Allow JVM attachment only on Tomcat and a proprietary Java app: +
`-Delastic.apm.bootstrap_allowlist=\*org.apache.catalina.startup.Bootstrap*,my.cool.app.*`
2. Disable when some custom System properties are set: +
`-Delastic.apm.bootstrap_exclude_list=custom.property.1,custom.property.2`
Copy link
Member

Choose a reason for hiding this comment

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

[question] Does the patterns are matched against the whole JVM command line or only the main class and the system properties ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an explanation above for the allow list and in the table.
Is it not clear from those what is used in each?
As for the allowlist - it is only the main class or jar and JVM parameters, basically the contents of the sun.java.command system property. I will add this info. Let me know if anything else is required

@eyalkoren eyalkoren requested a review from SylvainJuge January 25, 2023 11:11
@eyalkoren
Copy link
Contributor Author

@SylvainJuge I removed the default exclude list, as it doesn't seem required at this point.
Please only review that and the added documentation. Thanks!

@eyalkoren eyalkoren enabled auto-merge (squash) January 26, 2023 07:17
@eyalkoren eyalkoren merged commit 73b6262 into elastic:main Jan 26, 2023
@eyalkoren eyalkoren deleted the bootstrap-check-disable-agent branch January 26, 2023 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants