Speed up java agent type matching

[timlueg]

Description

From what I understand it should be possible to narrow the java agent bytebuddy matchers to JDK own classes. This avoids running the expensive isSubTypeOf check for all classes. Additionally using named() instead of the is() type check.
As I understand it any subclass of e.g. FileSystemProvider would now not be instrumented at the class level but since this subclass would normally uses JDK classes to access the file system these would still be instrumented.

Time to Node "started" best of 5:

	Before	After	diff.
./gradlew run	2013 ms	1544 ms	-469 ms (-23.3%)
./gradlew run with plugins	4744 ms	3450 ms	-1294 ms (-27.3%)

./gradlew run -PinstalledPlugins=...

./gradlew run -PinstalledPlugins="['opensearch-job-scheduler', 'opensearch-notifications-core', 'notifications', 'opensearch-ml-plugin', 'opensearch-knn', 'alerting', 'opensearch-anomaly-detection', 'asynchronous-search', 'opensearch-cross-cluster-replication', 'opensearch-custom-codecs', 'opensearch-flow-framework', 'geospatial', 'opensearch-index-management', 'opensearch-skills', 'neural-search', 'opensearch-observability', 'opensearch-reports-scheduler', 'opensearch-sql-plugin', 'query-insights', 'opensearch-system-templates', 'opensearch-ubi', 'opensearch-search-relevance', 'workload-management']"

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit d6891f1)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Incorrect Matcher The pathType matcher uses ElementMatchers.named(Files.class.getName()) which only matches the exact Files class, not its subclasses. The old code used isSubTypeOf(Files.class). Since Files is a final utility class with only static methods, this change is likely correct, but if the intent was to match subclasses (as with the other matchers), this would miss them. Verify that matching only the exact Files class is the intended behavior. final Junction<TypeDescription> pathType = ElementMatchers.named(Files.class.getName()); Incomplete Matcher The socketType matcher combines package name prefixes with isSubTypeOf checks, but the logic may not work as intended. A class can start with Socket.class.getPackageName() (e.g., java.net.SocketImpl) without being a subtype of SocketChannel or Socket, causing the matcher to fail. The .and() requires both conditions to be true, so classes in the specified packages that aren't subtypes won't match, and subtypes outside those packages won't match either. This could miss legitimate socket classes or fail to match any classes if the package filtering is too restrictive. final Junction<TypeDescription> socketType = ElementMatchers.nameStartsWith(Socket.class.getPackageName() + ".") .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) .or(ElementMatchers.nameStartsWith("sun.security.ssl.")) .and(ElementMatchers.isSubTypeOf(SocketChannel.class).or(ElementMatchers.isSubTypeOf(Socket.class)));

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to d6891f1

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Optimize matcher evaluation order The logic order may cause performance issues. The expensive isSubTypeOf checks are performed on all classes matching the package name prefixes, including many unrelated classes. Consider reordering to check isSubTypeOf first, then filter by package names only for matching subtypes. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [77-80] -final Junction<TypeDescription> socketType = ElementMatchers.nameStartsWith(Socket.class.getPackageName() + ".") - .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) - .or(ElementMatchers.nameStartsWith("sun.security.ssl.")) - .and(ElementMatchers.isSubTypeOf(SocketChannel.class).or(ElementMatchers.isSubTypeOf(Socket.class))); +final Junction<TypeDescription> socketType = ElementMatchers.isSubTypeOf(SocketChannel.class) + .or(ElementMatchers.isSubTypeOf(Socket.class)) + .and( + ElementMatchers.nameStartsWith(Socket.class.getPackageName() + ".") + .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) + .or(ElementMatchers.nameStartsWith("sun.security.ssl.")) + ); Suggestion importance[1-10]: 3 __ Why: While the suggestion about reordering matchers for performance is theoretically valid, the actual performance impact is likely minimal in practice. The current approach (package name first, then type check) can actually be more efficient by filtering out unrelated packages before expensive type hierarchy checks. The suggestion reverses this without clear evidence of improvement.	Low

---

Previous suggestions

Suggestions up to commit e9c6fe2

Category	Suggestion	Impact
Possible issue	Fix operator precedence issue Similar to socketType, the and operator has lower precedence than or, causing incorrect grouping. The current logic matches any class starting with "sun.nio.ch." regardless of whether it's a FileChannel subtype. Use parentheses to ensure proper evaluation order. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [81-83] -final Junction<TypeDescription> fileChannelType = ElementMatchers.nameStartsWith(FileChannel.class.getPackageName() + ".") - .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) +final Junction<TypeDescription> fileChannelType = (ElementMatchers.nameStartsWith(FileChannel.class.getPackageName() + ".") + .or(ElementMatchers.nameStartsWith("sun.nio.ch."))) .and(ElementMatchers.isSubTypeOf(FileChannel.class)); Suggestion importance[1-10]: 9 __ Why: This is a critical bug fix. Without parentheses, the and operator binds more tightly than or, causing the logic to match any class starting with "sun.nio.ch." regardless of whether it's a FileChannel subtype. The suggested parentheses ensure correct evaluation order.	High
	Correct boolean operator precedence The operator precedence issue affects this matcher as well. The and condition only applies to the last or clause, not all name patterns. Wrap the or conditions in parentheses to ensure all name-matched classes are validated as FileSystemProvider subtypes. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [84-87] -final Junction<TypeDescription> fileSystemProviderType = ElementMatchers.nameStartsWith("sun.nio.fs.") +final Junction<TypeDescription> fileSystemProviderType = (ElementMatchers.nameStartsWith("sun.nio.fs.") .or(ElementMatchers.nameStartsWith("jdk.nio.zipfs.")) - .or(ElementMatchers.nameStartsWith("jdk.internal.jrtfs.")) + .or(ElementMatchers.nameStartsWith("jdk.internal.jrtfs."))) .and(ElementMatchers.isSubTypeOf(FileSystemProvider.class)); Suggestion importance[1-10]: 9 __ Why: This identifies the same operator precedence bug as suggestion 2. The and condition currently only applies to the last or clause, allowing non-FileSystemProvider classes matching the first two name patterns to be incorrectly matched. Parentheses are needed for correct logic.	High

Suggestions up to commit e664df3

Category	Suggestion	Impact
General	Reorder matcher logic for efficiency The logic order creates a potential issue where the nameStartsWith filters are applied before the type check. If a class name starts with the specified prefixes but is not a subtype of SocketChannel or Socket, it will still be matched initially and then filtered out by the and clause, causing unnecessary type hierarchy checks. Consider reordering to check type hierarchy first for better performance. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [80-82] -final Junction<TypeDescription> socketType = ElementMatchers.nameStartsWith(Socket.class.getPackageName() + ".") - .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) - .and(ElementMatchers.isSubTypeOf(SocketChannel.class).or(ElementMatchers.isSubTypeOf(Socket.class))); +final Junction<TypeDescription> socketType = ElementMatchers.isSubTypeOf(SocketChannel.class) + .or(ElementMatchers.isSubTypeOf(Socket.class)) + .and(ElementMatchers.nameStartsWith(Socket.class.getPackageName() + ".").or(ElementMatchers.nameStartsWith("sun.nio.ch."))); Suggestion importance[1-10]: 4 __ Why: While the suggestion about reordering matchers for performance is theoretically valid, the actual performance impact is likely minimal in practice. The current order (name prefix first) may actually be more efficient as string matching is typically faster than type hierarchy checks. The suggestion is correct but offers marginal improvement.	Low
	Optimize matcher evaluation order Similar to the socket type matcher, the name prefix checks are performed before the type hierarchy check. This can lead to unnecessary processing of classes that match the name pattern but aren't subtypes of FileChannel. Reorder the matchers to check type hierarchy first. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [84-86] -final Junction<TypeDescription> fileChannelType = ElementMatchers.nameStartsWith(FileChannel.class.getPackageName() + ".") - .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) - .and(ElementMatchers.isSubTypeOf(FileChannel.class)); +final Junction<TypeDescription> fileChannelType = ElementMatchers.isSubTypeOf(FileChannel.class) + .and(ElementMatchers.nameStartsWith(FileChannel.class.getPackageName() + ".").or(ElementMatchers.nameStartsWith("sun.nio.ch."))); Suggestion importance[1-10]: 4 __ Why: Similar to the first suggestion, this proposes reordering matchers for performance. However, the performance benefit is debatable since name prefix matching is often faster than type hierarchy traversal. The suggestion is technically correct but the optimization impact is questionable.	Low
	Improve matcher performance ordering The matcher checks multiple name prefixes before verifying the type hierarchy. This approach may match classes with the correct package prefix but wrong type, leading to unnecessary subtype checks. Consider checking the type hierarchy first to avoid processing irrelevant classes. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [87-90] -final Junction<TypeDescription> fileSystemProviderType = ElementMatchers.nameStartsWith("sun.nio.fs.") - .or(ElementMatchers.nameStartsWith("jdk.nio.zipfs.")) - .or(ElementMatchers.nameStartsWith("jdk.internal.jrtfs.")) - .and(ElementMatchers.isSubTypeOf(FileSystemProvider.class)); +final Junction<TypeDescription> fileSystemProviderType = ElementMatchers.isSubTypeOf(FileSystemProvider.class) + .and(ElementMatchers.nameStartsWith("sun.nio.fs.").or(ElementMatchers.nameStartsWith("jdk.nio.zipfs.")).or(ElementMatchers.nameStartsWith("jdk.internal.jrtfs."))); Suggestion importance[1-10]: 4 __ Why: This suggestion follows the same pattern as the previous two, recommending type hierarchy checks before name prefix checks. While logically sound, the actual performance gain is uncertain and may depend on the specific class loading patterns. The suggestion is valid but offers marginal optimization.	Low

Suggestions up to commit d262bfb

Category	Suggestion	Impact
Possible issue	Fix operator precedence in compound matcher Due to operator precedence, the .and(...) only applies to nameStartsWith("sun.nio.ch."), not to the entire nameStartsWith("java.net.").or(nameStartsWith("sun.nio.ch.")) expression. The name-prefix filter should be grouped before applying .and(isSubTypeOf(...)) to ensure both prefixes are required to also match the subtype check. libs/agent-sm/agent/src/main/java/org/opensearch/javaagent/Agent.java [80-82] -final Junction<TypeDescription> socketType = ElementMatchers.nameStartsWith("java.net.") - .or(ElementMatchers.nameStartsWith("sun.nio.ch.")) +final Junction<TypeDescription> socketType = (ElementMatchers.nameStartsWith("java.net.") + .or(ElementMatchers.nameStartsWith("sun.nio.ch."))) .and(ElementMatchers.isSubTypeOf(SocketChannel.class).or(ElementMatchers.isSubTypeOf(Socket.class))); Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a real operator precedence bug where .and(isSubTypeOf(...)) only applies to nameStartsWith("sun.nio.ch.") instead of the entire or expression. This would cause incorrect matching behavior, potentially intercepting classes that don't implement SocketChannel or Socket. The same issue also exists in fileChannelType and fileSystemProviderType matchers.	Medium

[github-actions]

✅ Gradle check result for d262bfb: SUCCESS

[codecov]

Codecov Report

❌ Patch coverage is 0% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.41%. Comparing base (fead3a9) to head (d262bfb).
⚠️ Report is 6 commits behind head on main.

Files with missing lines	Patch %	Lines
.../src/main/java/org/opensearch/javaagent/Agent.java	0.00%	14 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #21473      +/-   ##
============================================
+ Coverage     73.34%   73.41%   +0.07%     
- Complexity    74353    74416      +63     
============================================
  Files          5967     5967              
  Lines        338227   338233       +6     
  Branches      48754    48754              
============================================
+ Hits         248061   248314     +253     
+ Misses        70399    70120     -279     
- Partials      19767    19799      +32     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Persistent review updated to latest commit e664df3

[github-actions]

❌ Gradle check result for e664df3: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit e9c6fe2

[github-actions]

❌ Gradle check result for e9c6fe2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[cwperks]

Thank you @timlueg. The change looks good to me. @reta wdyt?

[reta]

@timlueg @cwperks I am afraid this slippery road: the JDK evolves, the assumptions invalidate fast. Where the decision which package to take into consideration are coming from?

Please, correct me if I am missing something, but clearly we are excluding sun.security.ssl.SSLSocketImpl (JDK) and any user class that implement SocketChannel or Socket by package filters?

[timlueg]

Agree the PR suggestion is not an ideal solution. I selected the JDK packages where I found the relevant Subclasses.
SSLSocketImpl is excluded. But SSLSocketImpl.connect() delegates to java.net.Socket.connect() which is matched. But it is better to still include the package explicitly.

Right, user classes that extend e.g. Socket would no longer be matched unless they delegate like above.

(My understanding is that these Agent matcher(s) cover the most common cases. Because I can imagine a arbitrary user class could for example open a connection without using a Socket subclass.)

[reta]

(My understanding is that these Agent matcher(s) cover the most common cases. Because I can imagine a arbitrary user class could for example open a connection without using a Socket subclass.)

Thanks @timlueg , I think we should be covering any possible scenario (API set is very limited), otherwise the agent is not really serving the purpose

[github-actions]

❌ Gradle check result for e9c6fe2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit d6891f1

[github-actions]

❌ Gradle check result for d6891f1: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?