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

Exclude glassfish Executor which does not permit wrapped runnables #596

Merged
merged 5 commits into from
Apr 23, 2019

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented Apr 17, 2019

fixes #589

@felixbarny felixbarny self-assigned this Apr 17, 2019
@felixbarny felixbarny requested a review from eyalkoren April 17, 2019 14:39
@felixbarny
Copy link
Member Author

Contains no tests because we would have to instrument java.util.concurrent.ThreadPoolExecutor which is loaded from the bootstrap class loader and thus can't be instrumented in unit tests.

@felixbarny
Copy link
Member Author

Snapshot for this PR: elastic-apm-agent-1.6.1-SNAPSHOT.jar.zip

@felixbarny
Copy link
Member Author

@mdindoffer could you give this snapshot a try?

elastic-apm-agent-1.6.1-SNAPSHOT.jar.zip

@mdindoffer
Copy link
Contributor

@felixbarny Perfect, that seems to do the trick.

@codecov-io
Copy link

Codecov Report

Merging #596 into master will increase coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #596      +/-   ##
============================================
+ Coverage      62.4%   62.42%   +0.02%     
  Complexity       68       68              
============================================
  Files           188      188              
  Lines          7062     7066       +4     
  Branches        814      814              
============================================
+ Hits           4407     4411       +4     
  Misses         2387     2387              
  Partials        268      268
Impacted Files Coverage Δ Complexity Δ
.../apm/agent/concurrent/ExecutorInstrumentation.java 52.63% <66.66%> (+5.57%) 0 <0> (ø) ⬇️

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 1ca0be8...aa7f50a. Read the comment docs.

@felixbarny felixbarny merged commit cc8108f into elastic:master Apr 23, 2019
@felixbarny felixbarny deleted the glassfish-executor-fix branch April 23, 2019 09:14
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.

The agent is unable to instrument ManagedExecutorService and crashes the thread
4 participants