changeset 2013:189fa2a6b1f5

Remove hard-coded Boot-Class-Path in profiler jar. Backport of: http://icedtea.classpath.org/hg/thermostat/rev/5cc6faab7e19 PR3153 Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-August/020729.html
author Jie Kang <jkang@redhat.com>
date Mon, 29 Aug 2016 14:50:28 -0400
parents 4851a6db01a9
children 95d4e43f484f
files vm-profiler/agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/internal/Activator.java vm-profiler/agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/internal/VmProfiler.java vm-profiler/agent/src/main/resources/com/redhat/thermostat/vm/profiler/agent/internal/settings.properties vm-profiler/agent/src/test/java/com/redhat/thermostat/vm/profiler/agent/internal/ActivatorTest.java vm-profiler/agent/src/test/java/com/redhat/thermostat/vm/profiler/agent/internal/VmProfilerTest.java vm-profiler/jvm-agent/pom.xml vm-profiler/jvm-agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/jvm/ProfilerAgent.java vm-profiler/jvm-agent/src/main/resources/META-INF/MANIFEST.MF vm-profiler/pom.xml
diffstat 9 files changed, 54 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/vm-profiler/agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/internal/Activator.java	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/internal/Activator.java	Mon Aug 29 14:50:28 2016 -0400
@@ -36,8 +36,11 @@
 
 package com.redhat.thermostat.vm.profiler.agent.internal;
 
+import java.io.File;
+import java.io.IOException;
 import java.util.Map;
 import java.util.Properties;
+import java.util.logging.Logger;
 
 import org.osgi.framework.BundleActivator;
 import org.osgi.framework.BundleContext;
@@ -46,6 +49,8 @@
 import com.redhat.thermostat.agent.command.ReceiverRegistry;
 import com.redhat.thermostat.agent.utils.management.MXBeanConnectionPool;
 import com.redhat.thermostat.common.MultipleServiceTracker;
+import com.redhat.thermostat.common.utils.LoggingUtils;
+import com.redhat.thermostat.shared.config.CommonPaths;
 import com.redhat.thermostat.storage.core.WriterID;
 import com.redhat.thermostat.vm.profiler.common.ProfileDAO;
 
@@ -55,10 +60,8 @@
 
     @Override
     public void start(final BundleContext context) throws Exception {
-        final Properties configuration = new Properties();
-        configuration.load(this.getClass().getResourceAsStream("settings.properties"));
-
         Class<?>[] deps = new Class<?>[] {
+                CommonPaths.class,
                 MXBeanConnectionPool.class,
                 ProfileDAO.class,
                 WriterID.class,
@@ -82,11 +85,20 @@
 
             @Override
             public void dependenciesAvailable(Map<String, Object> services) {
+                CommonPaths path = get(CommonPaths.class, services);
                 MXBeanConnectionPool pool = get(MXBeanConnectionPool.class, services);
                 WriterID writerIdProvider = get(WriterID.class, services);
                 ProfileDAO dao = get(ProfileDAO.class, services);
                 String writerId = writerIdProvider.getWriterID();
 
+                final Properties configuration = new Properties();
+                try {
+                    configuration.load(this.getClass().getResourceAsStream("settings.properties"));
+                    prefixJarsWithThermostatHome(path, configuration);
+                } catch (IOException e) {
+                    throw new AssertionError("Missing resource:", e);
+                }
+
                 VmProfiler profiler = new VmProfiler(writerId, configuration, dao, pool);
                 profileRequestHandler = new ProfilerRequestReceiver(profiler);
 
@@ -97,6 +109,17 @@
                 vmStatusRegistrar = new VmStatusListenerRegistrar(context);
                 vmStatusRegistrar.register(livenessListener);
             }
+
+            private void prefixJarsWithThermostatHome(CommonPaths path, final Properties configuration) {
+                for (String name : configuration.stringPropertyNames()) {
+                    String currentValue = configuration.getProperty(name);
+                    if (currentValue.endsWith(".jar")) {
+                        configuration.setProperty(name,
+                                path.getSystemThermostatHome().getAbsolutePath() + File.separator + currentValue);
+                    }
+                }
+            }
+
             private <T> T get(Class<T> klass, Map<String, Object> services) {
                 return (T) services.get(klass.getName());
             }
--- a/vm-profiler/agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/internal/VmProfiler.java	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/internal/VmProfiler.java	Mon Aug 29 14:50:28 2016 -0400
@@ -142,9 +142,7 @@
         }
 
         if (!vmsWithAgentLoaded.contains((Integer)pid)) {
-            // TODO make this adjustable at run-time
-            // eg: asmJarPath + ":" + agentJarPath;
-            String jarsToLoad = "";
+            String jarsToLoad = asmJarPath + ":" + agentJarPath;
             logger.info("Asking " + pid + " to load agent '" + agentJarPath + "' with arguments '" + jarsToLoad + "'");
 
             remote.loadAgentIntoPid(pid, agentJarPath, jarsToLoad);
--- a/vm-profiler/agent/src/main/resources/com/redhat/thermostat/vm/profiler/agent/internal/settings.properties	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/agent/src/main/resources/com/redhat/thermostat/vm/profiler/agent/internal/settings.properties	Mon Aug 29 14:50:28 2016 -0400
@@ -1,2 +1,3 @@
-AGENT_JAR = ${thermostat.vm.profiler.dir}/thermostat-vm-profiler-jvm-agent-${project.version}.jar
-ASM_JAR = ${thermostat.vm.profiler.dir}/asm-all-${asm.version}.jar
\ No newline at end of file
+# These values are prefixed with thermostat home at run-time
+AGENT_JAR = plugins/vm-profiler/thermostat-vm-profiler-jvm-agent-${project.version}.jar
+ASM_JAR = plugins/vm-profiler/asm-all-${asm.version}.jar
--- a/vm-profiler/agent/src/test/java/com/redhat/thermostat/vm/profiler/agent/internal/ActivatorTest.java	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/agent/src/test/java/com/redhat/thermostat/vm/profiler/agent/internal/ActivatorTest.java	Mon Aug 29 14:50:28 2016 -0400
@@ -39,12 +39,16 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.File;
 
 import org.junit.Test;
 
 import com.redhat.thermostat.agent.VmStatusListener;
 import com.redhat.thermostat.agent.command.RequestReceiver;
 import com.redhat.thermostat.agent.utils.management.MXBeanConnectionPool;
+import com.redhat.thermostat.shared.config.CommonPaths;
 import com.redhat.thermostat.storage.core.WriterID;
 import com.redhat.thermostat.testutils.StubBundleContext;
 import com.redhat.thermostat.vm.profiler.common.ProfileDAO;
@@ -53,11 +57,16 @@
 
     @Test
     public void requestHandlerIsRegistered() throws Exception {
+        File homeFile = mock(File.class);
+        when(homeFile.getAbsolutePath()).thenReturn("${thermostat.home}");
+        CommonPaths paths = mock(CommonPaths.class);
+        when(paths.getSystemThermostatHome()).thenReturn(homeFile);
         MXBeanConnectionPool pool = mock(MXBeanConnectionPool.class);
         WriterID writerService = mock(WriterID.class);
         ProfileDAO dao = mock(ProfileDAO.class);
 
         StubBundleContext context = new StubBundleContext();
+        context.registerService(CommonPaths.class, paths, null);
         context.registerService(MXBeanConnectionPool.class, pool, null);
         context.registerService(ProfileDAO.class, dao, null);
         context.registerService(WriterID.class, writerService, null);
--- a/vm-profiler/agent/src/test/java/com/redhat/thermostat/vm/profiler/agent/internal/VmProfilerTest.java	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/agent/src/test/java/com/redhat/thermostat/vm/profiler/agent/internal/VmProfilerTest.java	Mon Aug 29 14:50:28 2016 -0400
@@ -66,6 +66,7 @@
 
     private static final String AGENT_JAR = "foo";
     private static final String ASM_JAR = "bar";
+    private static final String AGENT_OPTIONS = ASM_JAR + ":" + AGENT_JAR;
     private static final long TIMESTAMP = 1_000_000_000;
 
     private VmProfiler profiler;
@@ -121,7 +122,7 @@
         profiler.vmStarted(VM_ID, PID);
         profiler.startProfiling(VM_ID);
 
-        verify(remote).loadAgentIntoPid(PID, AGENT_JAR, "");
+        verify(remote).loadAgentIntoPid(PID, AGENT_JAR, AGENT_OPTIONS);
         verify(remote).startProfiling(PID);
         verify(dao).addStatus(new ProfileStatusChange(AGENT_ID, VM_ID, TIMESTAMP, true));
         verifyNoMoreInteractions(remote);
@@ -137,7 +138,7 @@
         profiler.stopProfiling(VM_ID);
         profiler.startProfiling(VM_ID);
 
-        verify(remote, times(1)).loadAgentIntoPid(PID, AGENT_JAR, "");
+        verify(remote, times(1)).loadAgentIntoPid(PID, AGENT_JAR, AGENT_OPTIONS);
         verify(remote, times(2)).startProfiling(PID);
     }
 
@@ -154,7 +155,7 @@
         profiler.vmStarted(VM_ID, PID);
         profiler.startProfiling(VM_ID);
 
-        verify(remote, times(2)).loadAgentIntoPid(PID, AGENT_JAR, "");
+        verify(remote, times(2)).loadAgentIntoPid(PID, AGENT_JAR, AGENT_OPTIONS);
         verify(remote, times(2)).startProfiling(PID);
     }
 
--- a/vm-profiler/jvm-agent/pom.xml	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/jvm-agent/pom.xml	Mon Aug 29 14:50:28 2016 -0400
@@ -63,9 +63,6 @@
         <configuration>
           <archive>
             <manifestFile>src/main/resources/META-INF/MANIFEST.MF</manifestFile>
-            <manifestEntries>
-              <Boot-Class-Path>${thermostat.vm.profiler.dir}asm-all-${asm.version}.jar ${thermostat.vm.profiler.dir}thermostat-vm-profiler-jvm-agent-${project.version}.jar</Boot-Class-Path>
-            </manifestEntries>
           </archive>
         </configuration>
       </plugin>
--- a/vm-profiler/jvm-agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/jvm/ProfilerAgent.java	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/jvm-agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/jvm/ProfilerAgent.java	Mon Aug 29 14:50:28 2016 -0400
@@ -46,6 +46,13 @@
 /**
  * This is an {@link Instrumentation} agent that sets up the classpath with jars
  * supplied as agent parameters and then invokes {@link Main}.
+ * <p>
+ * Agent jars (including this one) are loaded into the system classpath by
+ * default. The class that records profiling events has to have a single
+ * instance JVM-wide, otherwise we will miss events being recorded or won't be
+ * able to query them from a different instance of the "singleton". To ensure a
+ * single instance across all classloaders, we add the jars to bootstrap
+ * classpath and then use that instance explicitly (via reflection).
  */
 public class ProfilerAgent {
 
@@ -61,10 +68,8 @@
 
         Debug.println("AGENT: loaded");
 
-        // This is the saner approach, but for now we are brute-forcing a
-        // hardcoded path using a manifest entry
-        // String jars = args;
-        // addJarsToClassPath(jars, instrumentation);
+        String jars = args;
+        addJarsToClassPath(jars, instrumentation);
 
         invokeMain(instrumentation);
     }
@@ -77,7 +82,8 @@
             JarFile jarFile = null;
             try {
                 jarFile = new JarFile(jarPath);
-                // FIXME needs to be bootclassloader if it is to be visible everywhere
+                // This needs to be bootclassloader if it is to be loaded and visible everywhere
+                // without hitting recursive classloading.
                 if (addToBoot) {
                     instrumentation.appendToBootstrapClassLoaderSearch(jarFile);
                 } else {
--- a/vm-profiler/jvm-agent/src/main/resources/META-INF/MANIFEST.MF	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/jvm-agent/src/main/resources/META-INF/MANIFEST.MF	Mon Aug 29 14:50:28 2016 -0400
@@ -1,4 +1,3 @@
 Premain-Class: com.redhat.thermostat.vm.profiler.agent.jvm.ProfilerAgent
 Agent-Class: com.redhat.thermostat.vm.profiler.agent.jvm.ProfilerAgent
 Can-Retransform-Classes: true
-Comment: Boot-Class-Path added by build system
\ No newline at end of file
--- a/vm-profiler/pom.xml	Fri Aug 26 13:53:09 2016 -0400
+++ b/vm-profiler/pom.xml	Mon Aug 29 14:50:28 2016 -0400
@@ -50,11 +50,6 @@
 
   <name>Thermostat VM CPU plugin</name>
 
-  <properties>
-    <thermostat.vm.profiler.dir>${thermostat.home}/plugins/vm-profiler/</thermostat.vm.profiler.dir>
-    <thermostat.vm.profiler.agent.asm.path>${thermostat.home}/plugins/vm-profiler/asm-all-${asm.version}.jar</thermostat.vm.profiler.agent.asm.path>
-  </properties>
-
   <modules>
     <module>agent</module>
     <module>jvm-agent</module>