Enable agent-based instrumentation by default #23297
Assignees
Labels
Milestone
Comments
mlopatkin
added
a:feature
This was referenced Dec 30, 2022
bot-gradle
added a commit
that referenced
this issue
Jan 27, 2023
…ntation around TransformedClassPath Part of #23297 Co-authored-by: Mikhail Lopatkin <mlopatkin@gradle.com>
This was
linked to
pull requests
Feb 8, 2023
bot-gradle
added a commit
that referenced
this issue
Feb 24, 2023
## Review notes This PR adds the ability to use the instrumenting agent in all Gradle integration tests, but using the new instrumentation mode is still gated behind a feature flag, so there are no observable behavior changes (these are to come). As of this PR, you have to specify `-Porg.gradle.integtest.agent.allowed=true` flag to run tests with the new instrumentation mode. The follow-up PR will change the default value of the feature flag. I strived to make commits self-contained and to avoid back-and-forth, so the PR commits can be reviewed one-by-one. Some TODOs are addressed in the follow-up commits, but several left for coming PRs. ## A summary of changes A quick recap: there are two essential pieces to make the new instrumentation work. First, the agent has to be applied to the process running the build, and the `DefaultClassFileTransformer` has to be registered within the agent. Second, the `DefaultCachedClasspathTransformer` has to output `TransformedClassPath` instead of `DefaultClassPath`. Prior to this PR, applying the agent would toggle the DefaultCachedClasspathTransformer. However, to make feature flag working in all modes, we have to break this link. When the build runs in the "--no-daemon" mode (i.e. everything happens in the client process), the agent is being applied with a flag in the start script. It is not possible to figure out the state of the feature flag in the start script. The same applies to the "--foreground" mode of starting the daemon. This PR introduces the AgentStatus interface that is being controlled by both the presence of the agent and the value of the feature flag. With this, it becomes possible to apply the agent in the start scripts. For foreground daemons, there are changes to the compatibility checking: the `-javaagent:..gradle-instrumentation-agent` switch doesn't affect the compatibility of daemon contexts, as the agent status is present in the context explicitly anyway. It is not possible for the daemon/process to change its instrumentation mode. The daemon flags (maybe coming from the `gradle.properties` of the first build) at startup determine the mode. The rest of the changes are mostly dealing with the test infrastructure to allow enabling the new instrumentation in different modes: 1. Embedded mode: agent is applied to test processes, and to the daemons spawned in the tests. 2. No-daemon mode: start script tweaks and process compatibility tweaks (process with agent applied can run tests with legacy instrumentation) 3. Forking mode: it was working already. In the end, when `-Porg.gradle.integtest.agent.allowed=true` is used, all executors enable the feature flag in their command line. ## Follow-up work - TAPI tests are not executed with the agent instrumentation enabled. This is going to be revisited when the feature flag becomes on by default. It is just too hard to plumb the feature flag all the way for TAPI. - A few tests are ignored for now, but the breakages are not really affecting correctness. Part of #23297. Co-authored-by: Mikhail Lopatkin <mlopatkin@gradle.com>
github-project-automation
bot
moved this from In Progress
to Todo
in Stable Configuration Cache
This was referenced Apr 17, 2023
This was referenced Apr 17, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Currently, the agent-based instrumentation is under a feature flag. There are some rough edges that prevent us from enabling it:
Running Testkit tests in embedded mode ((Tracked in a separate issue Support agent-based instrumentation with the embedded TestKit mode #23817)withDebug(true)) is not supportedThe text was updated successfully, but these errors were encountered: