Skip to content

Include grpc plugin in agent jar#1162

Merged
SylvainJuge merged 3 commits intoelastic:masterfrom
felixbarny:include-grpc-in-agent
Apr 28, 2020
Merged

Include grpc plugin in agent jar#1162
SylvainJuge merged 3 commits intoelastic:masterfrom
felixbarny:include-grpc-in-agent

Conversation

@felixbarny
Copy link
Member

What does this PR do?

Currently, we don't ship the gRPC plugin, this PR fixes that.

Checklist

  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated CHANGELOG.asciidoc
  • I have updated supported-technologies.asciidoc
  • Added an API method or config option? Document in which version this will be introduced
  • Added an instrumentation plugin? How did you make sure that old, non-supported versions are not instrumented by accident?

Author's Checklist

Related issues

Use cases

Screenshots

@felixbarny felixbarny requested a review from SylvainJuge April 28, 2020 11:13
@ghost
Copy link

ghost commented Apr 28, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview stats

Expand to view the summary

Build stats

Test stats 🧪

Test Results
Failed 0
Passed 1232
Skipped 12
Total 1244

@felixbarny
Copy link
Member Author

There are more issues with the plugin

Failed to start agent
java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at co.elastic.apm.agent.bci.AgentMain.init(AgentMain.java:99)
	at co.elastic.apm.agent.bci.AgentMain.agentmain(AgentMain.java:65)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sun.instrument.InstrumentationImpl.loadClassAndStartAgent(InstrumentationImpl.java:386)
	at sun.instrument.InstrumentationImpl.loadClassAndCallAgentmain(InstrumentationImpl.java:411)
Caused by: java.lang.NoClassDefFoundError: io/grpc/MethodDescriptor
	at co.elastic.apm.agent.grpc.ClientCallImplInstrumentation$Constructor.getMethodMatcher(ClientCallImplInstrumentation.java:108)
	at co.elastic.apm.agent.bci.ElasticApmAgent.applyAdvice(ElasticApmAgent.java:250)
	at co.elastic.apm.agent.bci.ElasticApmAgent.initAgentBuilder(ElasticApmAgent.java:212)
	at co.elastic.apm.agent.bci.ElasticApmAgent.initInstrumentation(ElasticApmAgent.java:160)
	at co.elastic.apm.agent.bci.ElasticApmAgent.initialize(ElasticApmAgent.java:121)
	... 12 more

The problem is that grpc specific classes are referenced in the method matcher that are executed in the bootstrap classloader where these types are not available:

public ElementMatcher<? super MethodDescription> getMethodMatcher() {
return isConstructor().and(takesArgument(0, MethodDescriptor.class));
}

@codecov-io
Copy link

Codecov Report

Merging #1162 into master will increase coverage by 3.34%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1162      +/-   ##
============================================
+ Coverage     60.48%   63.83%   +3.34%     
- Complexity       85     2782    +2697     
============================================
  Files           330      284      -46     
  Lines         16284    13589    -2695     
  Branches       2411     1865     -546     
============================================
- Hits           9849     8674    -1175     
+ Misses         5801     4326    -1475     
+ Partials        634      589      -45     
Impacted Files Coverage Δ Complexity Δ
...pm/agent/profiler/ProfilingActivationListener.java 57.14% <0.00%> (-7.86%) 3.00% <0.00%> (+3.00%) ⬇️
.../agent/plugin/api/AbstractSpanInstrumentation.java 48.31% <0.00%> (-7.40%) 3.00% <0.00%> (+3.00%) ⬇️
...o/elastic/apm/agent/profiler/SamplingProfiler.java 77.93% <0.00%> (-7.16%) 46.00% <0.00%> (+46.00%) ⬇️
...va/co/elastic/apm/agent/impl/transaction/Span.java 78.72% <0.00%> (-6.09%) 33.00% <0.00%> (+33.00%) ⬇️
...va/co/elastic/apm/agent/impl/ElasticApmTracer.java 69.89% <0.00%> (-5.74%) 84.00% <0.00%> (+84.00%) ⬇️
...ic/apm/agent/plugin/api/TracedInstrumentation.java 26.47% <0.00%> (-5.45%) 6.00% <0.00%> (+6.00%) ⬇️
...lastic/apm/agent/objectpool/ObjectPoolFactory.java 87.50% <0.00%> (-5.36%) 9.00% <0.00%> (+9.00%) ⬇️
...bci/methodmatching/TraceMethodInstrumentation.java 57.14% <0.00%> (-4.77%) 9.00% <0.00%> (+9.00%) ⬇️
...astic/apm/agent/impl/transaction/AbstractSpan.java 74.24% <0.00%> (-4.56%) 39.00% <0.00%> (+39.00%) ⬇️
...astic/apm/agent/impl/transaction/TraceContext.java 77.53% <0.00%> (-4.52%) 77.00% <0.00%> (+77.00%) ⬇️
... and 63 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 b45bf3e...7eae3da. Read the comment docs.

@SylvainJuge SylvainJuge merged commit b460698 into elastic:master Apr 28, 2020
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.

3 participants