Skip to content
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

Decouple jar scanning function from class match vistor collection #1462

Merged
merged 5 commits into from Sep 29, 2023

Conversation

jtduffy
Copy link
Contributor

@jtduffy jtduffy commented Aug 24, 2023

Resolves #1342

Currently, the jar collector functionality is implemented via the ClassMatchVisitor pattern. The problem with this approach is that the registered visitors aren't executed when a class is excluded from transformation. In the case of certain jars (Spring Security for example), every single class in the jar is excluded, so the visitors are never executed, so the jar collector is never aware of the enclosing jar.

This PR keeps the exact same jar discovery logic in place, however the flow is no longer invoked after a class is transformed. Instead, all classes, prior to being checked for transformation eligibility are submitted to the jar collector. This means that no jars will be missed, even if every class within a jar is excluded, skipped, etc.

@codecov-commenter
Copy link

codecov-commenter commented Aug 24, 2023

Codecov Report

Merging #1462 (0f95eab) into main (258edd7) will increase coverage by 0.05%.
Report is 15 commits behind head on main.
The diff coverage is 76.92%.

@@             Coverage Diff              @@
##               main    #1462      +/-   ##
============================================
+ Coverage     70.49%   70.54%   +0.05%     
- Complexity     9726     9734       +8     
============================================
  Files           814      813       -1     
  Lines         39258    39256       -2     
  Branches       5956     5961       +5     
============================================
+ Hits          27673    27695      +22     
+ Misses         8906     8878      -28     
- Partials       2679     2683       +4     
Files Changed Coverage Δ
...ation/context/InstrumentationClassTransformer.java 64.10% <0.00%> (-1.69%) ⬇️
...ntation/context/InstrumentationContextManager.java 56.19% <ø> (-0.36%) ⬇️
.../agent/service/module/JarCollectorServiceImpl.java 80.55% <50.00%> (-2.78%) ⬇️
...nt/service/module/ClassToJarPathSubmitterImpl.java 85.36% <80.00%> (ø)
...com/newrelic/agent/service/ServiceManagerImpl.java 77.61% <100.00%> (-0.37%) ⬇️
...relic/agent/service/module/JarCollectorInputs.java 100.00% <100.00%> (ø)

... and 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@jtduffy jtduffy merged commit 2c57f56 into main Sep 29, 2023
103 checks passed
@jtduffy jtduffy deleted the decouple-jar-collector branch September 29, 2023 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Java agent fails to detect dependency - spring-security-oauth2-client.jar
4 participants