diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java index da8018c74f..48a059258e 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/context/FieldBackedProvider.java @@ -70,14 +70,15 @@ import net.bytebuddy.utility.JavaModule; @Slf4j public class FieldBackedProvider implements InstrumentationContextProvider { - /* - Note: the value here has to be inside on of the prefixes in - datadog.trace.agent.tooling.Utils#BOOTSTRAP_PACKAGE_PREFIXES. This ensures that 'isolating' (or 'module') - classloaders like jboss and osgi see injected classes. This works because we instrument those classloaders - to load everything inside bootstrap packages. + /** + * Note: the value here has to be inside on of the prefixes in + * datadog.trace.agent.tooling.Utils#BOOTSTRAP_PACKAGE_PREFIXES. This ensures that 'isolating' (or + * 'module') classloaders like jboss and osgi see injected classes. This works because we + * instrument those classloaders to load everything inside bootstrap packages. */ private static final String DYNAMIC_CLASSES_PACKAGE = "datadog.trace.bootstrap.instrumentation.context."; + private static final String INJECTED_FIELDS_MARKER_CLASS_NAME = Utils.getInternalName(FieldBackedContextStoreAppliedMarker.class.getName()); @@ -118,24 +119,24 @@ public class FieldBackedProvider implements InstrumentationContextProvider { public AgentBuilder.Identified.Extendable instrumentationTransformer( AgentBuilder.Identified.Extendable builder) { if (instrumenter.contextStore().size() > 0) { - /* - Install transformer that rewrites accesses to context store with specialized bytecode that invokes appropriate - storage implementation. + /** + * Install transformer that rewrites accesses to context store with specialized bytecode that + * invokes appropriate storage implementation. */ builder = builder.transform(getTransformerForASMVisitor(getContextStoreReadsRewritingVisitor())); - /* - We inject into bootstrap classloader because field accessor interfaces are needed by - context store implementations. Unfortunately this forces us to remove stored type checking - because actual classes may not be available at this point. + /** + * We inject into bootstrap classloader because field accessor interfaces are needed by + * context store implementations. Unfortunately this forces us to remove stored type checking + * because actual classes may not be available at this point. */ builder = builder.transform(bootstrapHelperInjector(fieldAccessorInterfaces.values())); - /* - * We inject context store implementation into bootstrap classloader because same implementation - * may be used by different instrumentations and it has to use same static map in case of - * fallback to map-backed storage. + /** + * We inject context store implementation into bootstrap classloader because same + * implementation may be used by different instrumentations and it has to use same static map + * in case of fallback to map-backed storage. */ builder = builder.transform(bootstrapHelperInjector(contextStoreImplementations.values())); } @@ -329,13 +330,13 @@ public class FieldBackedProvider implements InstrumentationContextProvider { @Override public AgentBuilder.Identified.Extendable additionalInstrumentation( - AgentBuilder.Identified.Extendable builder, final AgentBuilder.RawMatcher muzzleMatcher) { + AgentBuilder.Identified.Extendable builder, final AgentBuilder.RawMatcher muzzleMatcher) { if (fieldInjectionEnabled) { for (final Map.Entry entry : instrumenter.contextStore().entrySet()) { - /* - For each context store defined in a current instrumentation we create an agent builder - that injects necessary fields. + /** + * For each context store defined in a current instrumentation we create an agent builder + * that injects necessary fields. */ builder = builder @@ -343,6 +344,12 @@ public class FieldBackedProvider implements InstrumentationContextProvider { safeHasSuperType(named(entry.getKey())).and(not(isInterface())), instrumenter.classLoaderMatcher()) .and(safeToInjectFieldsMatcher()) + /** + * By adding the muzzleMatcher here, we are adding risk that the rules for injecting + * the classes into the classloader and the rules for adding the field to the class + * might be different. However the consequences are much greater if the class is not + * injected but the field is added, since that results in a NoClassDef error. + */ .and(muzzleMatcher) .transform( getTransformerForASMVisitor( @@ -361,13 +368,14 @@ public class FieldBackedProvider implements InstrumentationContextProvider { final JavaModule module, final Class classBeingRedefined, final ProtectionDomain protectionDomain) { - /* - The idea here is that we can add fields if class is just being loaded (classBeingRedefined == null) - and we have to add same fields again if class we added fields before is being transformed again. - Note: here we assume that Class#getInterfaces() returns list of interfaces defined immediately on a - given class, not inherited from its parents. It looks like current JVM implementation does exactly - this but javadoc is not explicit about that. - */ + /** + * The idea here is that we can add fields if class is just being loaded + * (classBeingRedefined == null) and we have to add same fields again if class we added + * fields before is being transformed again. Note: here we assume that Class#getInterfaces() + * returns list of interfaces defined immediately on a given class, not inherited from its + * parents. It looks like current JVM implementation does exactly this but javadoc is not + * explicit about that. + */ return classBeingRedefined == null || Arrays.asList(classBeingRedefined.getInterfaces()) .contains(FieldBackedContextStoreAppliedMarker.class); diff --git a/dd-java-agent/instrumentation/hibernate/core-3.3/core-3.3.gradle b/dd-java-agent/instrumentation/hibernate/core-3.3/core-3.3.gradle index 07d13ceeb9..805585960d 100644 --- a/dd-java-agent/instrumentation/hibernate/core-3.3/core-3.3.gradle +++ b/dd-java-agent/instrumentation/hibernate/core-3.3/core-3.3.gradle @@ -44,6 +44,7 @@ dependencies { testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:jdbc') + // Added to ensure cross compatibility: testCompile project(':dd-java-agent:instrumentation:hibernate:core-4.0') testCompile project(':dd-java-agent:instrumentation:hibernate:core-4.3') diff --git a/dd-java-agent/instrumentation/hibernate/core-4.0/core-4.0.gradle b/dd-java-agent/instrumentation/hibernate/core-4.0/core-4.0.gradle index 5017696ece..a1f7f3e6cf 100644 --- a/dd-java-agent/instrumentation/hibernate/core-4.0/core-4.0.gradle +++ b/dd-java-agent/instrumentation/hibernate/core-4.0/core-4.0.gradle @@ -37,6 +37,7 @@ dependencies { testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:jdbc') + // Added to ensure cross compatibility: testCompile project(':dd-java-agent:instrumentation:hibernate:core-3.3') testCompile project(':dd-java-agent:instrumentation:hibernate:core-4.3') diff --git a/dd-java-agent/instrumentation/hibernate/core-4.3/core-4.3.gradle b/dd-java-agent/instrumentation/hibernate/core-4.3/core-4.3.gradle index f1f7fad96f..cc97eeddff 100644 --- a/dd-java-agent/instrumentation/hibernate/core-4.3/core-4.3.gradle +++ b/dd-java-agent/instrumentation/hibernate/core-4.3/core-4.3.gradle @@ -37,12 +37,17 @@ dependencies { testCompile project(':dd-java-agent:testing') testCompile project(':dd-java-agent:instrumentation:jdbc') + // Added to ensure cross compatibility: testCompile project(':dd-java-agent:instrumentation:hibernate:core-3.3') testCompile project(':dd-java-agent:instrumentation:hibernate:core-4.0') testCompile group: 'org.hibernate', name: 'hibernate-core', version: '4.3.0.Final' + testCompile group: 'org.hibernate', name: 'hibernate-entitymanager', version: '4.3.0.Final' testCompile group: 'org.hsqldb', name: 'hsqldb', version: '2.0.0' + testCompile group: 'org.springframework.data', name: 'spring-data-jpa', version: '1.5.1.RELEASE' latestDepTestCompile group: 'org.hibernate', name: 'hibernate-core', version: '+' + latestDepTestCompile group: 'org.hibernate', name: 'hibernate-entitymanager', version: '+' latestDepTestCompile group: 'org.hsqldb', name: 'hsqldb', version: '2.0.0' + latestDepTestCompile group: 'org.springframework.data', name: 'spring-data-jpa', version: '+' }