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
6656: Allow capturing field values with path syntax #20
Conversation
# Conflicts: # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Parameter.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/TransformDescriptor.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/impl/DefaultTransformRegistry.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/jfr/JFRTransformDescriptor.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/jfr/impl/JFRMethodAdvisor.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/jfrnext/impl/JFRNextEventClassGenerator.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/jfrnext/impl/JFRNextMethodAdvisor.java # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/TypeUtils.java
|
/help |
@tabjy Available commands:
|
Please ignore this comment, it is just for the bots to refresh their caches and re-evaluate this PR |
First quick review. Will take a closer look as soon as I can find some more time.
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Attribute.java
Outdated
Show resolved
Hide resolved
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Field.java
Outdated
Show resolved
Hide resolved
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Transformer.java
Outdated
Show resolved
Hide resolved
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Transformer.java
Outdated
Show resolved
Hide resolved
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/AccessUtils.java
Outdated
Show resolved
Hide resolved
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/TypeUtils.java
Show resolved
Hide resolved
...dk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/expression/IllegalSyntaxException.java
Outdated
Show resolved
Hide resolved
...rg.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/expression/ReferenceChain.java
Outdated
Show resolved
Hide resolved
...rg.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/expression/ReferenceChain.java
Outdated
Show resolved
Hide resolved
...jdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/util/expression/ReferenceChainElement.java
Outdated
Show resolved
Hide resolved
…sion # Conflicts: # core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Transformer.java
and to re-trigger GitHub actions
All copyrights need to have 2020 added now, like so:
Happy New Year! :) |
core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Transformer.java
Outdated
Show resolved
Hide resolved
Please ensure that the code is formatted with the Mission Control formatting settings. The agent isn't taking part in the automated formatting checks yet. |
It's fine. I'll accept this, and open a new issue for getting the agent built, and the formatting applied, as part of the check in tests. Thanks for the contribution! |
@tabjy This change now passes all automated pre-integration checks. When the change also fulfills all project specific requirements, type
Since the source branch of this PR was last updated there have been 14 commits pushed to the As you do not have Committer status in this project, an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@thegreystone) but any other Committer may sponsor as well.
|
/integrate |
/sponsor |
@thegreystone @tabjy The following commits have been pushed to master since your change was applied:
Your commit was automatically rebased without conflicts. Pushed as commit 689812b. |
Mailing list message from Marcus Hirt on jmc-dev: Changeset: 689812b 6656: Allow capturing field values with path syntax Reviewed-by: hirt + core/org.openjdk.jmc.agent/src/main/java/org/openjdk/jmc/agent/Attribute.java |
This patch implements JMC-6656: Allow capturing field values with path syntax.
In the xml configuration, a field capture looks like:
See
org/openjdk/jmc/agent/test/jfrprobes_template.xml
for more examples.There are currently two limitations to pay attention to:
Instrumentation point cannot be in synthesized classes:
Instrumented classes are first loaded by obtaining the bytecode and inspected reflectively to resolve typing information. This fails if the class load is unable to provide the bytecode when
ClassLoader.getResouce()
is called. The cases where it might fail are unlikely to be for classes like proxies or auxiliaries for Java frameworks that have no visible equivalent.Instrumentation is unable to access nestmates' private fields:
Before nest-base access control was introduced in Java 11, accessing nestmates' private fields is, while lexically correct, forbidden by the JVM. The Java compiler works around by inserting synthetic getters/setters. However, it's not for a BCI agent since changing the class structure is not allowed during class transformation.
Please let me know your thoughts. Thank you very much!
Progress
Issue
JMC-6656: Allow capturing field values with path syntax
Approvers