Decouple jar scanning function from class match vistor collection

instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java

@@ -10,17 +10,19 @@
 import com.newrelic.agent.Agent;
 import com.newrelic.agent.instrumentation.context.ClassMatchVisitorFactory;
 import com.newrelic.agent.logging.IAgentLogger;
+import com.newrelic.agent.service.module.ClassToJarPathSubmitter;
+import com.newrelic.agent.service.module.ClassToJarPathSubmitterImpl;
 import com.newrelic.agent.service.module.JarCollectorService;
 
 public class IgnoringJarCollectorService implements JarCollectorService {
     @Override
-    public ClassMatchVisitorFactory getSourceVisitor() {
-        return null;
+    public void harvest(String appName) {
+
     }
 
     @Override
-    public void harvest(String appName) {
-
+    public ClassToJarPathSubmitter getClassToJarPathSubmitter() {
+        return ClassToJarPathSubmitterImpl.NO_OP_INSTANCE;
     }
 
     @Override

newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java

@@ -15,6 +15,7 @@
 import com.newrelic.agent.instrumentation.custom.ScalaTraitFinalFieldTransformer;
 import com.newrelic.agent.instrumentation.tracing.TraceClassTransformer;
 import com.newrelic.agent.service.ServiceFactory;
+import com.newrelic.agent.service.module.ClassToJarPathSubmitterImpl;
 import com.newrelic.agent.stats.StatsWorks;
 import com.newrelic.agent.util.asm.Utils;
 import org.objectweb.asm.ClassReader;
@@ -63,6 +64,10 @@ public void setInitialized(boolean isInitialized) {
     public byte[] transform(ClassLoader loader, String className, Class<?> classBeingRedefined,
             ProtectionDomain protectionDomain, byte[] classfileBuffer) throws IllegalClassFormatException {
         long transformStartTimeInNs = System.nanoTime();
+
+        //Submit the class for possible analysis from the jar collector
+        submitTransformCandidateToJarCollector(protectionDomain);
+
         try {
             if (className == null) {
                 return null;
@@ -175,4 +180,9 @@ private static boolean skipInterfaceMarkers(ClassReader reader) {
         return false;
     }
 
+    private void submitTransformCandidateToJarCollector(ProtectionDomain protectionDomain) {
+        if ((protectionDomain != null) && (protectionDomain.getCodeSource() != null)) {
+            ServiceFactory.getJarCollectorService().getClassToJarPathSubmitter().processUrl(protectionDomain.getCodeSource().getLocation());
+        }
+    }
 }

newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContextManager.java

@@ -147,7 +147,6 @@ public InstrumentationContextManager(Instrumentation instrumentation) {
         }
 
         classloaderExclusions = agentConfig.getClassTransformerConfig().getClassloaderExclusions();
-        matchVisitors.put(ServiceFactory.getJarCollectorService().getSourceVisitor(), NO_OP_TRANSFORMER);
         matchVisitors.put(ServiceFactory.getSourceLanguageService().getSourceVisitor(), NO_OP_TRANSFORMER);
 
         try {

newrelic-agent/src/main/java/com/newrelic/agent/service/ServiceManagerImpl.java

@@ -184,9 +184,8 @@ protected synchronized void doStart() throws Exception {
 
         JarCollectorInputs jarCollectorInputs = JarCollectorInputs.build(jarCollectorEnabled, jarAnalystFactory, executorService, jarCollectorLogger);
 
-        jarCollectorService = new JarCollectorServiceImpl(
-                jarCollectorLogger, jarCollectorEnabled, shouldSendAllJars, analyzedJars, jarCollectorInputs.getClassNoticingFactory()
-        );
+        jarCollectorService = new JarCollectorServiceImpl(jarCollectorLogger, jarCollectorEnabled, shouldSendAllJars, analyzedJars,
+                jarCollectorInputs.getClassToJarPathSubmitter());
 
         extensionService = new ExtensionService(configService, jarCollectorInputs.getExtensionAnalysisProducer());
 

newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitter.java

@@ -0,0 +1,14 @@
+package com.newrelic.agent.service.module;
+
+import java.net.URL;
+
+public interface ClassToJarPathSubmitter {
+
+    /**
+     * Determines if the supplied URL maps to a jar-type code location and if so, submits
+     * the jar to the jar collector analysis queue.
+     *
+     * @param url the URL to analyse
+     */
+    void processUrl(URL url);
+}

newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassNoticingFactory.java → newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitterImpl.java

@@ -6,11 +6,7 @@
  */
 package com.newrelic.agent.service.module;
 
-import com.newrelic.agent.instrumentation.context.ClassMatchVisitorFactory;
-import com.newrelic.agent.instrumentation.context.InstrumentationContext;
 import com.newrelic.api.agent.Logger;
-import org.objectweb.asm.ClassReader;
-import org.objectweb.asm.ClassVisitor;
 
 import java.net.MalformedURLException;
 import java.net.URL;
@@ -21,38 +17,34 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 
-public class ClassNoticingFactory implements ClassMatchVisitorFactory {
+/**
+ * This class takes URLs that represent the location of a specific class. If the location is determined
+ * to be a jar file, that jar is submitted to the jar collector analysis queue, only if this jar hasn't
+ * been submitted before.
+ */
+public class ClassToJarPathSubmitterImpl implements ClassToJarPathSubmitter{
+    public static ClassToJarPathSubmitter NO_OP_INSTANCE = new NoOpClassToJarPathSubmitter();
     private final JarAnalystFactory jarAnalystFactory;
     private final ExecutorService executorService;
     private final Logger logger;
     private final Set<String> seenPaths = Collections.newSetFromMap(new ConcurrentHashMap<String, Boolean>());
     private final AtomicInteger classSeenCount = new AtomicInteger(0);
 
-    public ClassNoticingFactory(JarAnalystFactory jarAnalystFactory, ExecutorService executorService, Logger logger) {
+    public ClassToJarPathSubmitterImpl(JarAnalystFactory jarAnalystFactory, ExecutorService executorService, Logger logger) {
         this.jarAnalystFactory = jarAnalystFactory;
         this.executorService = executorService;
         this.logger = logger;
     }
 
     @Override
-    public ClassVisitor newClassMatchVisitor(ClassLoader loader, Class<?> classBeingRedefined,
-            ClassReader reader, ClassVisitor cv, InstrumentationContext context) {
-        URL codeSourceLocation = context.getCodeSourceLocation();
-        if (codeSourceLocation != null){
-            addURL(codeSourceLocation);
-        }
-        return null;
-    }
-
-    /**
-     * Adds a url which represents a jar or a directory.
-     */
-    public void addURL(URL url) {
-        classSeenCount.incrementAndGet();
-        try {
-            addSingleURL(url);
-        } catch (MalformedURLException exception) {
-            logger.log(Level.FINEST, exception, "{0} unable to process url", url);
+    public void processUrl(URL url) {
+        if (url != null) {
+            classSeenCount.incrementAndGet();
+            try {
+                addSingleURL(url);
+            } catch (MalformedURLException exception) {
+                logger.log(Level.FINEST, exception, "{0} unable to process url", url);
+            }
         }
     }
 
@@ -73,9 +65,7 @@ private void addOtherURL(URL url) throws MalformedURLException {
 
             if (seenPaths.add(path)) {
                 URL finalUrl = new URL(url.getProtocol(), url.getHost(), path);
-                executorService.submit(jarAnalystFactory.createURLAnalyzer(finalUrl));
-                logger.log(Level.FINEST, "{0} offered to analysis queue; {1} paths seen and {2} classes seen.", finalUrl, seenPaths.size(),
-                        classSeenCount.get());
+                submitJarUrlForAnalysis(finalUrl);
             }
         }
     }
@@ -88,15 +78,25 @@ private void addJarProtocolURL(URL url) throws MalformedURLException {
         }
 
         if (seenPaths.add(path)) {
-            executorService.submit(jarAnalystFactory.createURLAnalyzer(new URL(path)));
-            logger.log(Level.FINEST, "{0} offered to analysis queue; {1} paths seen and {2} classes seen.", url, seenPaths.size(), classSeenCount.get());
+            submitJarUrlForAnalysis(new URL(path));
         }
     }
 
     private void addURLEndingWithJar(URL url) {
         if (seenPaths.add(url.getFile())) {
-            executorService.submit(jarAnalystFactory.createURLAnalyzer(url));
-            logger.log(Level.FINEST, "{0} offered to analysis queue; {1} paths seen and {2} classes seen.", url, seenPaths.size(), classSeenCount.get());
+            submitJarUrlForAnalysis(url);
+        }
+    }
+
+    private void submitJarUrlForAnalysis(URL finalUrl) {
+        executorService.submit(jarAnalystFactory.createURLAnalyzer(finalUrl));
+        logger.log(Level.FINEST, "{0} offered to analysis queue; {1} paths seen and {2} classes seen.", finalUrl, seenPaths.size(), classSeenCount.get());
+    }
+
+    private static class NoOpClassToJarPathSubmitter implements ClassToJarPathSubmitter {
+        @Override
+        public void processUrl(URL url) {
+            //No-op
         }
     }
 }

newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorInputs.java

@@ -8,30 +8,30 @@
 
 public class JarCollectorInputs {
     private final ExtensionsLoadedListener extensionAnalysisProducer;
-    private final ClassMatchVisitorFactory classNoticingFactory;
+    private final ClassToJarPathSubmitter classToJarPathSubmitter;
 
-    JarCollectorInputs(ExtensionsLoadedListener extensionAnalysisProducer, ClassMatchVisitorFactory classNoticingFactory) {
+    JarCollectorInputs(ExtensionsLoadedListener extensionAnalysisProducer, ClassToJarPathSubmitter classToJarPathSubmitter) {
         this.extensionAnalysisProducer = extensionAnalysisProducer;
-        this.classNoticingFactory = classNoticingFactory;
+        this.classToJarPathSubmitter = classToJarPathSubmitter;
     }
 
     public static JarCollectorInputs build(boolean jarCollectorEnabled, JarAnalystFactory jarAnalystFactory, ExecutorService executorService,
                               Logger jarCollectorLogger) {
-        ClassMatchVisitorFactory classNoticingFactory = jarCollectorEnabled
-                ? new ClassNoticingFactory(jarAnalystFactory, executorService, jarCollectorLogger)
-                : ClassMatchVisitorFactory.NO_OP_FACTORY;
+        ClassToJarPathSubmitter classToJarPathSubmitter = jarCollectorEnabled
+                ? new ClassToJarPathSubmitterImpl(jarAnalystFactory, executorService, jarCollectorLogger)
+                : ClassToJarPathSubmitterImpl.NO_OP_INSTANCE;
 
         ExtensionsLoadedListener extensionAnalysisProducer = jarCollectorEnabled
                 ? new ExtensionAnalysisProducer(jarAnalystFactory, executorService, jarCollectorLogger)
                 : ExtensionsLoadedListener.NOOP;
-        return new JarCollectorInputs(extensionAnalysisProducer, classNoticingFactory);
+        return new JarCollectorInputs(extensionAnalysisProducer, classToJarPathSubmitter);
     }
 
     public ExtensionsLoadedListener getExtensionAnalysisProducer() {
         return extensionAnalysisProducer;
     }
 
-    public ClassMatchVisitorFactory getClassNoticingFactory() {
-        return classNoticingFactory;
+    public ClassToJarPathSubmitter getClassToJarPathSubmitter() {
+        return classToJarPathSubmitter;
     }
 }

newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java

@@ -7,15 +7,13 @@
 
 package com.newrelic.agent.service.module;
 
-import com.newrelic.agent.instrumentation.context.ClassMatchVisitorFactory;
 import com.newrelic.agent.service.Service;
 
 /**
  * Interface for collecting and sending jars to RPM.
  */
 public interface JarCollectorService extends Service {
-
-    ClassMatchVisitorFactory getSourceVisitor();
-
     void harvest(String appName);
+
+    ClassToJarPathSubmitter getClassToJarPathSubmitter();
 }

newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceImpl.java

@@ -31,22 +31,21 @@ public class JarCollectorServiceImpl extends AbstractService implements JarColle
     private final boolean enabled;
     private final AtomicBoolean shouldSendAllJars;
     private final TrackedAddSet<JarData> analyzedJars;
-    private final ClassMatchVisitorFactory classMatchVisitorFactory;
-
+    private final ClassToJarPathSubmitter classToJarPathSubmitter;
     private volatile List<JarData> jarsNotSentLastHarvest = Collections.emptyList();
 
     public JarCollectorServiceImpl(
             Logger logger,
             boolean enabled,
             AtomicBoolean shouldSendAllJars,
             TrackedAddSet<JarData> analyzedJars,
-            ClassMatchVisitorFactory classNoticingFactory) {
+            ClassToJarPathSubmitter classToJarPathSubmitter) {
         super(JarCollectorService.class.getSimpleName());
 
         this.shouldSendAllJars = shouldSendAllJars;
         this.analyzedJars = analyzedJars;
         this.logger = logger;
-        this.classMatchVisitorFactory = classNoticingFactory;
+        this.classToJarPathSubmitter = classToJarPathSubmitter;
         this.enabled = enabled;
 
         if (JarCollectorConfigImpl.isUsingDeprecatedConfigSettings()) {
@@ -63,11 +62,6 @@ public final boolean isEnabled() {
         return enabled;
     }
 
-    @Override
-    public ClassMatchVisitorFactory getSourceVisitor() {
-        return classMatchVisitorFactory;
-    }
-
     @Override
     protected void doStart() throws Exception {
     }
@@ -100,6 +94,10 @@ public void harvest(String appName) {
         }
     }
 
+    @Override
+    public ClassToJarPathSubmitter getClassToJarPathSubmitter() {
+        return classToJarPathSubmitter;
+    }
 
     private List<JarData> getJars() {
         if (shouldSendAllJars.getAndSet(false)) {

newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java

@@ -45,6 +45,7 @@
 import com.newrelic.agent.service.async.AsyncTransactionService;
 import com.newrelic.agent.service.logging.LogSenderService;
 import com.newrelic.agent.service.logging.LogSenderServiceImpl;
+import com.newrelic.agent.service.module.ClassToJarPathSubmitterImpl;
 import com.newrelic.agent.service.module.JarCollectorService;
 import com.newrelic.agent.sql.SqlTraceService;
 import com.newrelic.agent.stats.StatsService;
@@ -135,7 +136,7 @@ public MockServiceManager(ConfigService configService) {
         dbService = new DatabaseService();
         extensionService = new ExtensionService(configService, ExtensionsLoadedListener.NOOP);
         jarCollectorService = Mockito.mock(JarCollectorService.class);
-        Mockito.when(jarCollectorService.getSourceVisitor()).thenReturn(Mockito.mock(ClassMatchVisitorFactory.class));
+        Mockito.when(jarCollectorService.getClassToJarPathSubmitter()).thenReturn(ClassToJarPathSubmitterImpl.NO_OP_INSTANCE);
         sourceLanguageService = new SourceLanguageService();
         expirationService = new ExpirationService();
         distributedTraceService = Mockito.mock(DistributedTraceService.class);

newrelic-agent/src/test/java/com/newrelic/agent/config/ClassTransformerConfigImplTest.java

@@ -162,7 +162,6 @@ public void classloaderExcludes() throws Exception {
         MockServiceManager serviceManager = new MockServiceManager(configService);
         ServiceFactory.setServiceManager(serviceManager);
         JarCollectorService mockJarCollector = serviceManager.getJarCollectorService();
-        when(mockJarCollector.getSourceVisitor()).thenReturn(ClassMatchVisitorFactory.NO_OP_FACTORY);
 
         InstrumentationContextManager icm = new InstrumentationContextManager(Mockito.mock(InstrumentationProxy.class));
         Assert.assertFalse(icm.isClassloaderExcluded(new GoodClassLoader()));

newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/IncludeExcludeTest.java

@@ -36,7 +36,6 @@ private void createServiceManager(Map<String, Object> configMap) throws Exceptio
         MockServiceManager serviceManager = new MockServiceManager(configService);
         ServiceFactory.setServiceManager(serviceManager);
         JarCollectorService mockJarCollector = serviceManager.getJarCollectorService();
-        when(mockJarCollector.getSourceVisitor()).thenReturn(ClassMatchVisitorFactory.NO_OP_FACTORY);
     }
 
     private Map<String, Object> createConfigMap() {