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

avoid loading application log4j plugins #1214

Merged
merged 2 commits into from Jun 5, 2020

Conversation

SylvainJuge
Copy link
Member

Found while testing with an application that is using log4j2.

Shaded log4j2 from agent tries to load application plugins, this is due to having hard-coded references to Log4j2Plugins.dat within META-INF folder (which is not covered by relocation patterns)

Classes that trigger this issue can be found using the following one-liner command within an expanded copy of the agent jar.

find . -name '*.class' | xargs grep 'META-INF/org/' -ali 

We only have two of them

./co/elastic/apm/agent/shaded/apache/logging/log4j/core/config/plugins/processor/PluginProcessor.class
./co/elastic/apm/agent/shaded/apache/logging/log4j/core/config/plugins/util/PluginRegistry.class

@apmmachine
Copy link
Collaborator

apmmachine commented Jun 3, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #1214 updated]

  • Start Time: 2020-06-05T07:34:38.241+0000

  • Duration: 37 min 29 sec

Test stats 🧪

Test Results
Failed 0
Passed 1332
Skipped 12
Total 1344

@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2020

Codecov Report

Merging #1214 into master will increase coverage by 0.65%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1214      +/-   ##
============================================
+ Coverage     59.24%   59.90%   +0.65%     
- Complexity       87       94       +7     
============================================
  Files           339      344       +5     
  Lines         16026    16680     +654     
  Branches       2248     2384     +136     
============================================
+ Hits           9495     9992     +497     
- Misses         5881     6006     +125     
- Partials        650      682      +32     
Impacted Files Coverage Δ Complexity Δ
...c/main/java/co/elastic/apm/agent/util/IOUtils.java 56.88% <50.00%> (-1.22%) 0.00 <0.00> (ø)
...lastic/apm/agent/servlet/AsyncInstrumentation.java 63.04% <0.00%> (-1.41%) 0.00% <0.00%> (ø%)
...astic/apm/agent/impl/transaction/AbstractSpan.java 75.18% <0.00%> (-0.26%) 0.00% <0.00%> (ø%)
...lastic/apm/agent/objectpool/ObjectPoolFactory.java 100.00% <0.00%> (ø) 0.00% <0.00%> (ø%)
...lconnection/HttpsUrlConnectionInstrumentation.java 80.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...t/grails/GrailsTransactionNameInstrumentation.java 40.47% <0.00%> (ø) 7.00% <0.00%> (?%)
...main/java/co/elastic/apm/agent/util/CallDepth.java 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...t/RunnableCallableForkJoinTaskInstrumentation.java 69.23% <0.00%> (ø) 0.00% <0.00%> (?%)
...o/elastic/apm/agent/concurrent/JavaConcurrent.java 71.87% <0.00%> (ø) 0.00% <0.00%> (?%)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 09e3bc7...ae8096c. Read the comment docs.

@SylvainJuge SylvainJuge merged commit 595a1b6 into elastic:master Jun 5, 2020
@SylvainJuge SylvainJuge deleted the fix-log4j-relocation branch June 5, 2020 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants