Ignore appdynamics/cisco multi tenant agent module

ryandens commented 3 years ago

Description

When running the Hypertrace Javaagent with the Appdynamics javaagent on 9+ JVM, the following error occurs after ~3 minutes of leaving an app server running with no HTTP traffic

[Cisco-Multi-Tenant-Agent-Bootstrapping] ERROR io.opentelemetry.javaagent.tooling.HelperInjector - Error preparing helpers while processing class okhttp3.OkHttpClient for okhttp. Failed to inject helper classes into instance MultiTenantAgentClassLoader
java.lang.IllegalStateException: Error invoking (accessor)::defineClass
    at net.bytebuddy.dynamic.loading.ClassInjector$UsingReflection$Dispatcher$UsingUnsafeInjection.defineClass(ClassInjector.java:999)
    at net.bytebuddy.dynamic.loading.ClassInjector$UsingReflection.injectRaw(ClassInjector.java:251)
    at io.opentelemetry.javaagent.tooling.HelperInjector.injectClassLoader(HelperInjector.java:205)
    at io.opentelemetry.javaagent.tooling.HelperInjector.transform(HelperInjector.java:137)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.doTransform(AgentBuilder.java:10364)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:10302)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.access$1600(AgentBuilder.java:10068)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer$Java9CapableVmDispatcher.run(AgentBuilder.java:10761)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer$Java9CapableVmDispatcher.run(AgentBuilder.java:10699)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer.transform(AgentBuilder.java:10258)
    at net.bytebuddy.agent.builder.AgentBuilder$Default$ExecutingTransformer$ByteBuddy$ModuleSupport.transform(Unknown Source)
    at java.instrument/sun.instrument.TransformerManager.transform(TransformerManager.java:188)
    at java.instrument/sun.instrument.InstrumentationImpl.transform(InstrumentationImpl.java:563)
    at java.base/java.lang.ClassLoader.defineClass1(Native Method)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
    at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174)
    at java.base/java.net.URLClassLoader.defineClass(URLClassLoader.java:550)
    at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:458)
    at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:452)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:451)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:589)
    at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
    at java.base/java.lang.Class.getDeclaredMethods0(Native Method)
    at java.base/java.lang.Class.privateGetDeclaredMethods(Class.java:3166)
    at java.base/java.lang.Class.getDeclaredMethods(Class.java:2309)
    at MultiTenantAgentClassLoader/org.picocontainer.injectors.AdaptingInjection$1.run(AdaptingInjection.java:203)
    at java.base/java.security.AccessController.doPrivileged(Native Method)
    at MultiTenantAgentClassLoader/org.picocontainer.injectors.AdaptingInjection.injectionMethodAnnotated(AdaptingInjection.java:200)
    at MultiTenantAgentClassLoader/org.picocontainer.injectors.AdaptingInjection.methodAnnotatedInjectionAdapter(AdaptingInjection.java:171)
    at MultiTenantAgentClassLoader/org.picocontainer.injectors.AdaptingInjection.createComponentAdapter(AdaptingInjection.java:70)
    at MultiTenantAgentClassLoader/org.picocontainer.behaviors.AbstractBehaviorFactory.createComponentAdapter(AbstractBehaviorFactory.java:44)
    at MultiTenantAgentClassLoader/org.picocontainer.behaviors.Caching.createComponentAdapter(Caching.java:46)
    at MultiTenantAgentClassLoader/org.picocontainer.behaviors.AdaptingBehavior.createComponentAdapter(AdaptingBehavior.java:58)
    at MultiTenantAgentClassLoader/org.picocontainer.DefaultPicoContainer.addComponent(DefaultPicoContainer.java:536)
    at MultiTenantAgentClassLoader/org.picocontainer.DefaultPicoContainer.addComponent(DefaultPicoContainer.java:501)
    at MultiTenantAgentClassLoader/org.picocontainer.DefaultPicoContainer.access$400(DefaultPicoContainer.java:84)
    at MultiTenantAgentClassLoader/org.picocontainer.DefaultPicoContainer$AsPropertiesPicoContainer.addComponent(DefaultPicoContainer.java:1157)
    at MultiTenantAgentClassLoader/com.cisco.mtagent.core.AgentPicoContainer.createSingleton(AgentPicoContainer.java:135)
    at MultiTenantAgentClassLoader/com.cisco.mtagent.core.AgentPicoContainer.initializeMultiTenantAgent(AgentPicoContainer.java:108)
    at MultiTenantAgentClassLoader/com.cisco.mtagent.core.AgentPicoContainer.bootstrapMultiAgentInstances(AgentPicoContainer.java:71)
    at MultiTenantAgentClassLoader/com.cisco.mtagent.core.AgentPicoContainer.bootstrapMultiTenantAgent(AgentPicoContainer.java:58)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at com.cisco.mtagent.entry.MTAgent$2.run(MTAgent.java:324)
Caused by: java.lang.IllegalAccessError: superinterface check failed: class io.opentelemetry.javaagent.instrumentation.okhttp.v3_0.TracingInterceptor (in unnamed module @0x7aeb0422) cannot access class okhttp3.Interceptor (in module MultiTenantAgentClassLoader) because module MultiTenantAgentClassLoader does not export okhttp3 to unnamed module @0x7aeb0422
    at java.base/java.lang.ClassLoader.defineClass1(Native Method)
    at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1017)
    at java.base/java.lang.ClassLoader$ByteBuddyAccessor$GCVZL2mm.defineClass(Unknown Source)
    at java.base/jdk.internal.reflect.GeneratedMethodAccessor44.invoke(Unknown Source)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at net.bytebuddy.dynamic.loading.ClassInjector$UsingReflection$Dispatcher$UsingUnsafeInjection.defineClass(ClassInjector.java:995)
    ... 47 more

Testing

I verified this locally using Tomcat 9 on Correto 11 with Hypertrace and Appdynamics installed.

Checklist:

[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works (N/A, we don't have a good way to test other agenets)
[x] Any dependent changes have been merged and published in downstream modules

Documentation

Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.

pavolloffay commented 3 years ago

Circling back to using the final keyword for all local vars and parameters https://github.com/hypertrace/javaagent/pull/321#discussion_r651296470. Have you seen this style anywhere in high-quality code bases e.g. Spring, Rxjava, Kafka, JBoss, Google projects? I personally haven't, final is useful in places where it makes sense: constants, not allowing inheritance, very large methods. Otherwise it's just noise and pollutes the code.

ryandens commented 3 years ago

Re: the final keyword, I think I explained my take on why I think it's useful and not noise/pollution when I read code in the other PR (last comment here). I'm not actively invovled in other open source projects, so I can't speak to those, but I've seen it used in a couple closed source projects I've worked on and enforced with the FinalLocalVariableCheck checkstyle rule. Ultimately, IMO, code style is a compromise between everyone that maintains/contributes to a project, so as a I stated on the other PR I'm happy to table the discussion for another time and remove the final keyword from local variables until such a time that we can add automation to enforce whatever decision we come to. As the Hypertrace user base grows, so will our contributor base and having automation to enforce simple code style will help others more easily contribute to the project

pavolloffay commented 3 years ago

I'm not actively invovled in other open source projects, so I can't speak to those,

I encourage anybody to have a look, it's a great learning experience on how to write quality code.

pavolloffay commented 3 years ago

As the Hypertrace user base grows, so will our contributor base and having automation to enforce simple code style will help others more easily contribute to the project

I haven't seen a code style check that would do anything with final. This keyword should be used by the developer's careful judgement when needed.

pavolloffay commented 3 years ago

cisco is going to be the top company in classloader exclusions (appd, singularity, now cisco).

davexroth commented 3 years ago

cisco is going to be the top company in classloader exclusions (appd, singularity, now cisco).

This is a good point - @ryandens can you scope the package down?

ryandens commented 3 years ago

cisco is going to be the top company in classloader exclusions (appd, singularity, now cisco).

This is a good point - @ryandens can you scope the package down?

Yep, this is already done! I scoped it down to com.cisco.mtagent which Pavol and I agree on as the appropriate scope

hypertrace / javaagent