From 2b23a07a1a177b10add45bbd0b45751817830bdb Mon Sep 17 00:00:00 2001 From: Jerry Duffy Date: Thu, 24 Aug 2023 10:23:19 -0400 Subject: [PATCH 1/5] Decouple jar scanning function from class match vistor collection --- .../internal/IgnoringJarCollectorService.java | 9 +-- .../InstrumentationClassTransformer.java | 7 +++ .../InstrumentationContextManager.java | 1 - .../agent/service/ServiceManagerImpl.java | 5 +- .../module/ClassToJarPathSubmitter.java | 14 +++++ ....java => ClassToJarPathSubmitterImpl.java} | 62 +++++++++---------- .../service/module/JarCollectorInputs.java | 18 +++--- .../service/module/JarCollectorService.java | 5 +- .../module/JarCollectorServiceImpl.java | 16 +++-- .../newrelic/agent/MockServiceManager.java | 1 - .../ClassTransformerConfigImplTest.java | 1 - .../instrumentation/IncludeExcludeTest.java | 1 - ....java => ClassToJarPathSubmitterTest.java} | 40 +++++------- .../module/JarCollectorInputsTest.java | 8 +-- .../module/JarCollectorServiceImplTest.java | 4 +- 15 files changed, 97 insertions(+), 95 deletions(-) create mode 100644 newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitter.java rename newrelic-agent/src/main/java/com/newrelic/agent/service/module/{ClassNoticingFactory.java => ClassToJarPathSubmitterImpl.java} (55%) rename newrelic-agent/src/test/java/com/newrelic/agent/service/module/{ClassNoticingFactoryTest.java => ClassToJarPathSubmitterTest.java} (73%) diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java index 6539e833db..35fd5e9619 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java @@ -10,17 +10,18 @@ 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.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 null; } @Override diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java index 886ff980c6..4074402b2a 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java @@ -63,6 +63,13 @@ 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 + if((protectionDomain != null) && (protectionDomain.getCodeSource() != null)) { + Agent.LOG.finest(MessageFormat.format("DUF- submitting {0} to jar collector", className)); + ServiceFactory.getJarCollectorService().getClassToJarPathSubmitter().processUrl(protectionDomain.getCodeSource().getLocation()); + } + try { if (className == null) { return null; diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContextManager.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContextManager.java index 6835fffb71..5b6a2f03bc 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationContextManager.java +++ b/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 { diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/ServiceManagerImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/ServiceManagerImpl.java index e5fb9ebe5c..e708b1b460 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/ServiceManagerImpl.java +++ b/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()); diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitter.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitter.java new file mode 100644 index 0000000000..a8c082b95d --- /dev/null +++ b/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); +} diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassNoticingFactory.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitterImpl.java similarity index 55% rename from newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassNoticingFactory.java rename to newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassToJarPathSubmitterImpl.java index 4fa57486f3..c7ffb62403 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/ClassNoticingFactory.java +++ b/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 seenPaths = Collections.newSetFromMap(new ConcurrentHashMap()); 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 } } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorInputs.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorInputs.java index 5df734c3a3..b50ef1abd0 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorInputs.java +++ b/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; } } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java index d3e497a386..f2f28111fa 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java @@ -14,8 +14,7 @@ * Interface for collecting and sending jars to RPM. */ public interface JarCollectorService extends Service { - - ClassMatchVisitorFactory getSourceVisitor(); - void harvest(String appName); + + ClassToJarPathSubmitter getClassToJarPathSubmitter(); } diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceImpl.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceImpl.java index df2e52df8f..b7f12243a8 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceImpl.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorServiceImpl.java @@ -31,8 +31,7 @@ public class JarCollectorServiceImpl extends AbstractService implements JarColle private final boolean enabled; private final AtomicBoolean shouldSendAllJars; private final TrackedAddSet analyzedJars; - private final ClassMatchVisitorFactory classMatchVisitorFactory; - + private final ClassToJarPathSubmitter classToJarPathSubmitter; private volatile List jarsNotSentLastHarvest = Collections.emptyList(); public JarCollectorServiceImpl( @@ -40,13 +39,13 @@ public JarCollectorServiceImpl( boolean enabled, AtomicBoolean shouldSendAllJars, TrackedAddSet 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 getJars() { if (shouldSendAllJars.getAndSet(false)) { diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java b/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java index 21d9bb7972..5062fcb071 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java @@ -135,7 +135,6 @@ 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)); sourceLanguageService = new SourceLanguageService(); expirationService = new ExpirationService(); distributedTraceService = Mockito.mock(DistributedTraceService.class); diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/config/ClassTransformerConfigImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/config/ClassTransformerConfigImplTest.java index 29837ffd5e..6d7450f6e5 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/config/ClassTransformerConfigImplTest.java +++ b/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())); diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/IncludeExcludeTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/IncludeExcludeTest.java index 9e917edf5c..36c9867f10 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/IncludeExcludeTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/instrumentation/IncludeExcludeTest.java @@ -36,7 +36,6 @@ private void createServiceManager(Map 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 createConfigMap() { diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/ClassNoticingFactoryTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/ClassToJarPathSubmitterTest.java similarity index 73% rename from newrelic-agent/src/test/java/com/newrelic/agent/service/module/ClassNoticingFactoryTest.java rename to newrelic-agent/src/test/java/com/newrelic/agent/service/module/ClassToJarPathSubmitterTest.java index 3ae28a9df8..b32b7574a1 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/ClassNoticingFactoryTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/ClassToJarPathSubmitterTest.java @@ -21,23 +21,17 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verifyNoInteractions; -public class ClassNoticingFactoryTest { +public class ClassToJarPathSubmitterTest { @Test - public void noticingJunit() throws IOException { + public void detectJunitLocation() throws IOException { ExecutorService executorService = mock(ExecutorService.class); JarAnalystFactory factory = mock(JarAnalystFactory.class); when(factory.createURLAnalyzer(any(URL.class))).thenReturn(mock(Runnable.class)); - ClassNoticingFactory target = new ClassNoticingFactory(factory, executorService, new TestLogger()); - - target.newClassMatchVisitor( - Test.class.getClassLoader(), - Test.class, - new ClassReader("org.junit.Test"), - new ClassWriter(0), - new InstrumentationContext(null, Test.class, Test.class.getProtectionDomain()) - ); + ClassToJarPathSubmitter classToJarPathSubmitter = new ClassToJarPathSubmitterImpl(factory, executorService, new TestLogger()); + InstrumentationContext ic = new InstrumentationContext(null, Test.class, Test.class.getProtectionDomain()); + classToJarPathSubmitter.processUrl(ic.getCodeSourceLocation()); ArgumentCaptor captor = ArgumentCaptor.forClass(URL.class); verify(factory, times(1)).createURLAnalyzer(captor.capture()); @@ -47,21 +41,15 @@ public void noticingJunit() throws IOException { } @Test - public void noticingTestClass() throws IOException { + public void detectTestClassLocation() throws IOException { ExecutorService executorService = mock(ExecutorService.class); JarAnalystFactory factory = mock(JarAnalystFactory.class); when(factory.createURLAnalyzer(any(URL.class))).thenReturn(mock(Runnable.class)); - ClassNoticingFactory target = new ClassNoticingFactory(factory, executorService, new TestLogger()); - - target.newClassMatchVisitor( - this.getClass().getClassLoader(), - this.getClass(), - new ClassReader(this.getClass().getName()), - new ClassWriter(0), - new InstrumentationContext(null, this.getClass(), this.getClass().getProtectionDomain()) - ); + ClassToJarPathSubmitter classToJarPathSubmitter = new ClassToJarPathSubmitterImpl(factory, executorService, new TestLogger()); + InstrumentationContext ic = new InstrumentationContext(null, this.getClass(), this.getClass().getProtectionDomain()); + classToJarPathSubmitter.processUrl(ic.getCodeSourceLocation()); String codeSourceLocation = this.getClass().getProtectionDomain().getCodeSource().getLocation().toString(); if (codeSourceLocation.endsWith("/")) { @@ -80,11 +68,11 @@ public void doesNotAddDuplicates() throws MalformedURLException { JarAnalystFactory factory = mock(JarAnalystFactory.class); when(factory.createURLAnalyzer(any(URL.class))).thenReturn(mock(Runnable.class)); - ClassNoticingFactory target = new ClassNoticingFactory(factory, executorService, new TestLogger()); + ClassToJarPathSubmitter classToJarPathSubmitter = new ClassToJarPathSubmitterImpl(factory, executorService, new TestLogger()); - target.addURL(new URL( + classToJarPathSubmitter.processUrl(new URL( "file:/Users/roger/webapps/java_test_webapp/WEB-INF/lib/commons-httpclient-3.1.jar!/org/apache/commons/httpclient/HttpVersion.class")); - target.addURL(new URL( + classToJarPathSubmitter.processUrl(new URL( "file:/Users/roger/webapps/java_test_webapp/WEB-INF/lib/commons-httpclient-3.1.jar!/org/apache/commons/httpclient/Dude.class")); verify(factory, times(1)).createURLAnalyzer(new URL("file:/Users/roger/webapps/java_test_webapp/WEB-INF/lib/commons-httpclient-3.1.jar")); @@ -98,10 +86,10 @@ public void jarProtocol() throws MalformedURLException { JarAnalystFactory factory = mock(JarAnalystFactory.class); when(factory.createURLAnalyzer(any(URL.class))).thenReturn(mock(Runnable.class)); - ClassNoticingFactory target = new ClassNoticingFactory(factory, executorService, new TestLogger()); + ClassToJarPathSubmitter classToJarPathSubmitter = new ClassToJarPathSubmitterImpl(factory, executorService, new TestLogger()); // jboss sends us complex urls like this - target.addURL(new URL( + classToJarPathSubmitter.processUrl(new URL( "jar:file:/Users/sdaubin/servers/jboss-as-7.1.1.Final/modules/org/apache/xerces/main/xercesImpl-2.9.1-jbossas-1.jar!/")); verify(factory, times(1)).createURLAnalyzer(new URL("file:/Users/sdaubin/servers/jboss-as-7.1.1.Final/modules/org/apache/xerces/main/xercesImpl-2.9.1-jbossas-1.jar")); diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java index 50becf45ae..ac2a9c0fce 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java @@ -17,18 +17,18 @@ public class JarCollectorInputsTest { @Test public void correctInputsWhenEnabled() { JarCollectorInputs target = JarCollectorInputs.build(true, mock(JarAnalystFactory.class), mock(ExecutorService.class), mock(Logger.class)); - assertTrue(target.getClassNoticingFactory() instanceof ClassNoticingFactory); + assertTrue(target.getClassToJarPathSubmitter() instanceof ClassToJarPathSubmitterImpl); assertTrue(target.getExtensionAnalysisProducer() instanceof ExtensionAnalysisProducer); - assertNotSame(ClassMatchVisitorFactory.NO_OP_FACTORY, target.getClassNoticingFactory()); + assertNotSame(ClassMatchVisitorFactory.NO_OP_FACTORY, target.getClassToJarPathSubmitter()); assertNotSame(ExtensionsLoadedListener.NOOP, target.getExtensionAnalysisProducer()); } @Test public void correctInputsWhenNotEnabled() { JarCollectorInputs target = JarCollectorInputs.build(false, mock(JarAnalystFactory.class), mock(ExecutorService.class), mock(Logger.class)); - assertFalse(target.getClassNoticingFactory() instanceof ClassNoticingFactory); + assertFalse(target.getClassToJarPathSubmitter() instanceof ClassToJarPathSubmitterImpl); assertFalse(target.getExtensionAnalysisProducer() instanceof ExtensionAnalysisProducer); - assertSame(ClassMatchVisitorFactory.NO_OP_FACTORY, target.getClassNoticingFactory()); + assertSame(ClassMatchVisitorFactory.NO_OP_FACTORY, target.getClassToJarPathSubmitter()); assertSame(ExtensionsLoadedListener.NOOP, target.getExtensionAnalysisProducer()); } diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java index 8689dda4f3..091c9648aa 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java @@ -33,7 +33,7 @@ public class JarCollectorServiceImplTest { @Mock - public ClassNoticingFactory classNoticingFactory; + public ClassToJarPathSubmitterImpl classNoticingFactory; @Mock public IRPMService rpmService; @@ -179,7 +179,7 @@ private JarCollectorServiceImpl getTarget( true, shouldSendAllJars, set, - classNoticingFactory + mock(ClassToJarPathSubmitter.class) ); } From b705de7eec710395b037e1ca8a21c09ff6da91e9 Mon Sep 17 00:00:00 2001 From: Jerry Duffy Date: Thu, 24 Aug 2023 15:03:44 -0400 Subject: [PATCH 2/5] test corrections --- .../src/test/java/com/newrelic/agent/MockServiceManager.java | 2 ++ .../newrelic/agent/service/module/JarCollectorInputsTest.java | 4 ++-- .../agent/service/module/JarCollectorServiceImplTest.java | 3 --- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java b/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java index 5062fcb071..552ce674eb 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/MockServiceManager.java +++ b/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,6 +136,7 @@ public MockServiceManager(ConfigService configService) { dbService = new DatabaseService(); extensionService = new ExtensionService(configService, ExtensionsLoadedListener.NOOP); jarCollectorService = Mockito.mock(JarCollectorService.class); + Mockito.when(jarCollectorService.getClassToJarPathSubmitter()).thenReturn(ClassToJarPathSubmitterImpl.NO_OP_INSTANCE); sourceLanguageService = new SourceLanguageService(); expirationService = new ExpirationService(); distributedTraceService = Mockito.mock(DistributedTraceService.class); diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java index ac2a9c0fce..f8ed92abdd 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorInputsTest.java @@ -19,7 +19,7 @@ public void correctInputsWhenEnabled() { JarCollectorInputs target = JarCollectorInputs.build(true, mock(JarAnalystFactory.class), mock(ExecutorService.class), mock(Logger.class)); assertTrue(target.getClassToJarPathSubmitter() instanceof ClassToJarPathSubmitterImpl); assertTrue(target.getExtensionAnalysisProducer() instanceof ExtensionAnalysisProducer); - assertNotSame(ClassMatchVisitorFactory.NO_OP_FACTORY, target.getClassToJarPathSubmitter()); + assertNotSame(ClassToJarPathSubmitterImpl.NO_OP_INSTANCE, target.getClassToJarPathSubmitter()); assertNotSame(ExtensionsLoadedListener.NOOP, target.getExtensionAnalysisProducer()); } @@ -28,7 +28,7 @@ public void correctInputsWhenNotEnabled() { JarCollectorInputs target = JarCollectorInputs.build(false, mock(JarAnalystFactory.class), mock(ExecutorService.class), mock(Logger.class)); assertFalse(target.getClassToJarPathSubmitter() instanceof ClassToJarPathSubmitterImpl); assertFalse(target.getExtensionAnalysisProducer() instanceof ExtensionAnalysisProducer); - assertSame(ClassMatchVisitorFactory.NO_OP_FACTORY, target.getClassToJarPathSubmitter()); + assertSame(ClassToJarPathSubmitterImpl.NO_OP_INSTANCE, target.getClassToJarPathSubmitter()); assertSame(ExtensionsLoadedListener.NOOP, target.getExtensionAnalysisProducer()); } diff --git a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java index 091c9648aa..d0c08e75d7 100644 --- a/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java +++ b/newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceImplTest.java @@ -32,9 +32,6 @@ public class JarCollectorServiceImplTest { - @Mock - public ClassToJarPathSubmitterImpl classNoticingFactory; - @Mock public IRPMService rpmService; From 044a2e887a6d4abfcbeb6313c6cacb2c1cc16296 Mon Sep 17 00:00:00 2001 From: Jerry Duffy Date: Thu, 24 Aug 2023 16:00:28 -0400 Subject: [PATCH 3/5] remove debug line --- .../instrumentation/context/InstrumentationClassTransformer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java index 4074402b2a..d6b3508dd0 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java @@ -66,7 +66,6 @@ public byte[] transform(ClassLoader loader, String className, Class classBein //Submit the class for possible analysis from the jar collector if((protectionDomain != null) && (protectionDomain.getCodeSource() != null)) { - Agent.LOG.finest(MessageFormat.format("DUF- submitting {0} to jar collector", className)); ServiceFactory.getJarCollectorService().getClassToJarPathSubmitter().processUrl(protectionDomain.getCodeSource().getLocation()); } From 0f95eab4055fa14d44a30a0d8c070852617c8ae8 Mon Sep 17 00:00:00 2001 From: Jerry Duffy Date: Mon, 28 Aug 2023 12:45:30 -0400 Subject: [PATCH 4/5] test corrections --- .../agent/introspec/internal/IgnoringJarCollectorService.java | 3 ++- .../context/InstrumentationClassTransformer.java | 1 + .../com/newrelic/agent/service/module/JarCollectorService.java | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java index 35fd5e9619..5a312b8b00 100644 --- a/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java +++ b/instrumentation-test/src/main/java/com/newrelic/agent/introspec/internal/IgnoringJarCollectorService.java @@ -11,6 +11,7 @@ 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 { @@ -21,7 +22,7 @@ public void harvest(String appName) { @Override public ClassToJarPathSubmitter getClassToJarPathSubmitter() { - return null; + return ClassToJarPathSubmitterImpl.NO_OP_INSTANCE; } @Override diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java index d6b3508dd0..34a25fe750 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java +++ b/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; diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java index f2f28111fa..1b379fd3db 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/service/module/JarCollectorService.java @@ -7,7 +7,6 @@ package com.newrelic.agent.service.module; -import com.newrelic.agent.instrumentation.context.ClassMatchVisitorFactory; import com.newrelic.agent.service.Service; /** From 2ea78126c043cd4be270947c4f0a4bf0ba51bcef Mon Sep 17 00:00:00 2001 From: Jerry Duffy Date: Thu, 31 Aug 2023 13:24:52 -0400 Subject: [PATCH 5/5] refactor collector submission into method --- .../context/InstrumentationClassTransformer.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java index 34a25fe750..0d4135172f 100644 --- a/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java +++ b/newrelic-agent/src/main/java/com/newrelic/agent/instrumentation/context/InstrumentationClassTransformer.java @@ -66,9 +66,7 @@ public byte[] transform(ClassLoader loader, String className, Class classBein long transformStartTimeInNs = System.nanoTime(); //Submit the class for possible analysis from the jar collector - if((protectionDomain != null) && (protectionDomain.getCodeSource() != null)) { - ServiceFactory.getJarCollectorService().getClassToJarPathSubmitter().processUrl(protectionDomain.getCodeSource().getLocation()); - } + submitTransformCandidateToJarCollector(protectionDomain); try { if (className == null) { @@ -182,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()); + } + } }