# HG changeset patch # User Omair Majid # Date 1358267938 18000 # Node ID 421d8a954893c30089ad0db1df25f6aeaa2c1024 # Parent 2930e0c260cca05d353185263a1edda8eb81a1ab Generic agent-side API for VM start/stop Introduce an API that all backends can use to find out when a VM starts up or shuts down. Currently, all backends re-implement this functionality using jvmstat explicitly. With an explicit API for this, we will have less duplicated code for each plugin and we can change/improve the implementation that detects when VMs start or stop without having to change each plugin. All classes implementing VmStatusChangeListener registered as OSGi services now get notified about the VM's pid when a vm starts or stops. The GC backend now obtains the pid from this new API but still uses the MonitoredVm class form jvmstat connect to the VM and gather data. Reviewed-by: ebaron, vanaltj Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-January/005122.html PR1258 diff -r 2930e0c260cc -r 421d8a954893 agent/core/src/main/java/com/redhat/thermostat/agent/JvmStatusListener.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/JvmStatusListener.java Mon Jan 14 14:04:50 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,47 +0,0 @@ -/* - * Copyright 2012 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * . - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - -package com.redhat.thermostat.agent; - -/** - * A listener that is notified when a JVM starts or is stopped. - */ -public interface JvmStatusListener { - - public void jvmStarted(int pid); - - public void jvmStopped(int pid); -} diff -r 2930e0c260cc -r 421d8a954893 agent/core/src/main/java/com/redhat/thermostat/agent/JvmStatusNotifier.java --- a/agent/core/src/main/java/com/redhat/thermostat/agent/JvmStatusNotifier.java Mon Jan 14 14:04:50 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,55 +0,0 @@ -/* - * Copyright 2012 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * . - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - -package com.redhat.thermostat.agent; - -public interface JvmStatusNotifier { - - /** - * Request to be informed when JVM processes are started or stopped. - * - * @param listener the receiver of future {@link JvmStatusListener.jvmStarted()} - * and {@link JvmStatusListener.jvmStopped()} calls - */ - public void addJvmStatusListener(JvmStatusListener listener); - - /** - * Request to no longer be informed when JVM processes are started or stopped. - * @param listener the {@link JvmStatusListener} to be unregistered. - */ - public void removeJvmStatusListener(JvmStatusListener listener); - -} diff -r 2930e0c260cc -r 421d8a954893 agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListener.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListener.java Tue Jan 15 11:38:58 2013 -0500 @@ -0,0 +1,69 @@ +/* + * Copyright 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.agent; + +/** + * A listener that is notified when a JVM starts or is stopped. + *

+ * Register an instance of this class as an OSGi service, and it will be + * notified of all VM {@link Status} events. + */ +//@ExtensionPoint +public interface VmStatusListener { + + enum Status { + + /** A VM has just started now */ + VM_STARTED, + + /** + * A delayed version of {@link #VM_STARTED}. A VM was started at some + * point in the past. The listener was not informed about it then + * (probably because the listener was not registered at the time), but + * is being informed about it now. + */ + VM_ACTIVE, + + /** A VM was stopped just now */ + VM_STOPPED, + } + + // TODO what other information other than pid may be useful for plugins? + + public void vmStatusChanged(Status newStatus, int pid); + +} diff -r 2930e0c260cc -r 421d8a954893 agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListenerRegistrar.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/agent/core/src/main/java/com/redhat/thermostat/agent/VmStatusListenerRegistrar.java Tue Jan 15 11:38:58 2013 -0500 @@ -0,0 +1,75 @@ +/* + * Copyright 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.agent; + +import java.util.HashMap; +import java.util.Map; + +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceRegistration; + +/** + * Registers a {@link VmStatusListener} as an OSGi Service. + *

+ * This is just a convenience wrapper over + * {@link BundleContext#registerService(String, Object, java.util.Dictionary)}. + * It does absolutely nothing special. + * + * @see VmStatusListener + */ +public class VmStatusListenerRegistrar { + + private final BundleContext context; + private final Map registrations = new HashMap<>(); + + public VmStatusListenerRegistrar(BundleContext context) { + this.context = context; + } + + public void register(VmStatusListener listener) { + registrations.put(listener, context.registerService(VmStatusListener.class.getName(), listener, null)); + } + + public void unregister(VmStatusListener listener) { + ServiceRegistration registration = registrations.remove(listener); + if (registration == null) { + throw new IllegalArgumentException("no listener found"); + } + + registration.unregister(); + } +} diff -r 2930e0c260cc -r 421d8a954893 agent/core/src/test/java/com/redhat/thermostat/agent/VmStatusListenerRegistrarTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/agent/core/src/test/java/com/redhat/thermostat/agent/VmStatusListenerRegistrarTest.java Tue Jan 15 11:38:58 2013 -0500 @@ -0,0 +1,76 @@ +/* + * Copyright 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.agent; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; + +import org.junit.Test; + +import com.redhat.thermostat.agent.VmStatusListener; +import com.redhat.thermostat.agent.VmStatusListenerRegistrar; +import com.redhat.thermostat.test.StubBundleContext; + +public class VmStatusListenerRegistrarTest { + + @Test + public void testRegister() { + StubBundleContext context = new StubBundleContext(); + VmStatusListener listener = mock(VmStatusListener.class); + + VmStatusListenerRegistrar registerer = new VmStatusListenerRegistrar(context); + registerer.register(listener); + + assertTrue(context.isServiceRegistered(VmStatusListener.class.getName(), listener.getClass())); + assertEquals(1, context.getAllServices().size()); + } + + @Test + public void testUnregister() { + StubBundleContext context = new StubBundleContext(); + VmStatusListener listener = mock(VmStatusListener.class); + + VmStatusListenerRegistrar registerer = new VmStatusListenerRegistrar(context); + registerer.register(listener); + registerer.unregister(listener); + + assertFalse(context.isServiceRegistered(VmStatusListener.class.getName(), listener.getClass())); + assertEquals(0, context.getAllServices().size()); + } +} diff -r 2930e0c260cc -r 421d8a954893 client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java --- a/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java Mon Jan 14 14:04:50 2013 -0500 +++ b/client/cli/src/test/java/com/redhat/thermostat/client/cli/internal/ActivatorTest.java Tue Jan 15 11:38:58 2013 -0500 @@ -38,6 +38,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.Matchers.anyString; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -64,6 +65,9 @@ Bundle mockBundle = mock(Bundle.class); when(FrameworkUtil.getBundle(ShellCommand.class)).thenReturn(mockBundle); when(FrameworkUtil.getBundle(VMStatCommand.class)).thenReturn(mockBundle); + // When we call createFilter, we need a real return value + when(FrameworkUtil.createFilter(anyString())).thenCallRealMethod(); + StubBundleContext ctx = new StubBundleContext(); when(mockBundle.getBundleContext()).thenReturn(ctx); diff -r 2930e0c260cc -r 421d8a954893 common/core/src/main/java/com/redhat/thermostat/test/StubBundleContext.java --- a/common/core/src/main/java/com/redhat/thermostat/test/StubBundleContext.java Mon Jan 14 14:04:50 2013 -0500 +++ b/common/core/src/main/java/com/redhat/thermostat/test/StubBundleContext.java Tue Jan 15 11:38:58 2013 -0500 @@ -50,14 +50,21 @@ import org.osgi.framework.BundleListener; import org.osgi.framework.Filter; import org.osgi.framework.FrameworkListener; +import org.osgi.framework.FrameworkUtil; import org.osgi.framework.InvalidSyntaxException; +import org.osgi.framework.ServiceEvent; import org.osgi.framework.ServiceListener; import org.osgi.framework.ServiceReference; import org.osgi.framework.ServiceRegistration; import com.redhat.thermostat.common.NotImplementedException; -import com.redhat.thermostat.test.StubBundleContext.ServiceInformation; +/** + * An implementation of BundleContext that's useful for writing unit tests. + *

+ * WARNING: if you static mock {@link FrameworkUtil#createFilter(String)}, you + * are going to have a bad time. + */ public class StubBundleContext implements BundleContext { static class ServiceInformation { @@ -180,17 +187,28 @@ public ServiceRegistration registerService(String className, Object service, Dictionary properties) { ServiceInformation info = new ServiceInformation(className, service, properties); registeredServices.add(info); + + notifyServiceChange(new StubServiceReference(info), true); + return new StubServiceRegistration(this, info); } @Override public ServiceReference[] getServiceReferences(String clazz, String filter) throws InvalidSyntaxException { - Filter toMatch = createFilter(filter); List toReturn = new ArrayList<>(); - for (ServiceInformation info : registeredServices) { - // how does filter matching in OSGI work again? - if (info.serviceInterface.equals(clazz) && toMatch.match(info.properties)) { - toReturn.add(new StubServiceReference(info)); + + if (filter == null) { + for (ServiceInformation info : registeredServices) { + if (info.serviceInterface.equals(clazz)) { + toReturn.add(new StubServiceReference(info)); + } + } + } else { + Filter toMatch = createFilter(filter); + for (ServiceInformation info : registeredServices) { + if (info.serviceInterface.equals(clazz) && toMatch.match(info.properties)) { + toReturn.add(new StubServiceReference(info)); + } } } return toReturn.toArray(new ServiceReference[0]); @@ -253,7 +271,11 @@ @Override public Filter createFilter(String filter) throws InvalidSyntaxException { - return new StubFilter(filter); + // FIXME this will break service trackers if FrameworkUtil is mocked + // the following call will return null if FrameworkUtil is mocked. + // that's a problem because this is meant to be used (mostly) in test + // environments and that's where FrameworkUtil is likely to be mocked. + return FrameworkUtil.createFilter(filter); } @Override @@ -291,7 +313,17 @@ throw new IllegalStateException("service not registered"); } registeredServices.remove(info); + notifyServiceChange(new StubServiceReference(info), false); + } + private void notifyServiceChange(ServiceReference serviceReference, boolean registered) { + int eventType = registered ? ServiceEvent.REGISTERED : ServiceEvent.UNREGISTERING; + ServiceEvent event = new ServiceEvent(eventType, serviceReference); + for (ListenerSpec l : registeredListeners) { + if (l.filter.match(serviceReference)) { + l.listener.serviceChanged(event); + } + } } public int getExportedServiceCount(ServiceRegistration registration) { diff -r 2930e0c260cc -r 421d8a954893 common/core/src/main/java/com/redhat/thermostat/test/StubFilter.java --- a/common/core/src/main/java/com/redhat/thermostat/test/StubFilter.java Mon Jan 14 14:04:50 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,87 +0,0 @@ -/* - * Copyright 2012 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * . - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - -package com.redhat.thermostat.test; - -import java.util.Dictionary; -import java.util.Map; - -import org.osgi.framework.Filter; -import org.osgi.framework.ServiceReference; - -import com.redhat.thermostat.common.NotImplementedException; - -public class StubFilter implements Filter { - - private final String filter; - - public StubFilter(String filter) { - this.filter = filter; - } - - @Override - public boolean match(ServiceReference reference) { - if (filter == null) { - return true; - } - throw new NotImplementedException(); - } - - @Override - public boolean match(Dictionary dictionary) { - if (filter == null) { - return true; - } - throw new NotImplementedException(); - } - - @Override - public boolean matchCase(Dictionary dictionary) { - if (filter == null) { - return true; - } - throw new NotImplementedException(); - } - - @Override - public boolean matches(Map map) { - if (filter == null) { - return true; - } - throw new NotImplementedException(); - } - -} diff -r 2930e0c260cc -r 421d8a954893 common/core/src/main/java/com/redhat/thermostat/test/StubServiceReference.java --- a/common/core/src/main/java/com/redhat/thermostat/test/StubServiceReference.java Mon Jan 14 14:04:50 2013 -0500 +++ b/common/core/src/main/java/com/redhat/thermostat/test/StubServiceReference.java Tue Jan 15 11:38:58 2013 -0500 @@ -1,5 +1,5 @@ /* - * Copyright 2012 Red Hat, Inc. + * Copyright 2013 Red Hat, Inc. * * This file is part of Thermostat. * @@ -36,6 +36,11 @@ package com.redhat.thermostat.test; +import java.util.ArrayList; +import java.util.Dictionary; +import java.util.Enumeration; +import java.util.List; + import org.osgi.framework.Bundle; import org.osgi.framework.Constants; import org.osgi.framework.ServiceReference; @@ -57,12 +62,22 @@ return new String[] { information.serviceInterface }; } - throw new NotImplementedException(); + return information.properties.get(key); } @Override public String[] getPropertyKeys() { - throw new NotImplementedException(); + if (information.properties == null) { + return new String[] { Constants.OBJECTCLASS }; + } else { + Dictionary props = information.properties; + List toReturn = new ArrayList<>(props.size()); + Enumeration keyEnumeration = props.keys(); + while (keyEnumeration.hasMoreElements()) { + toReturn.add((String) keyEnumeration.nextElement()); + } + return toReturn.toArray(new String[0]); + } } @Override diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/main/java/com/redhat/thermostat/backend/system/JvmStatHostListener.java --- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/JvmStatHostListener.java Mon Jan 14 14:04:50 2013 -0500 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/JvmStatHostListener.java Tue Jan 15 11:38:58 2013 -0500 @@ -52,14 +52,14 @@ import sun.jvmstat.monitor.event.HostListener; import sun.jvmstat.monitor.event.VmStatusChangeEvent; -import com.redhat.thermostat.agent.JvmStatusListener; -import com.redhat.thermostat.agent.JvmStatusNotifier; +import com.redhat.thermostat.agent.VmStatusListener; +import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.storage.model.VmInfo; import com.redhat.thermostat.utils.ProcDataSource; -public class JvmStatHostListener implements HostListener, JvmStatusNotifier { +public class JvmStatHostListener implements HostListener { private static final Logger logger = LoggingUtils.getLogger(JvmStatHostListener.class); @@ -67,10 +67,11 @@ private Map monitoredVms = new HashMap<>(); - private Set statusListeners = new CopyOnWriteArraySet(); + private VmStatusChangeNotifier notifier; - JvmStatHostListener(VmInfoDAO vmInfoDAO) { + JvmStatHostListener(VmInfoDAO vmInfoDAO, VmStatusChangeNotifier notifier) { this.vmInfoDAO = vmInfoDAO; + this.notifier = notifier; } @Override @@ -121,9 +122,7 @@ logger.log(Level.WARNING, "error getting vm info for " + vmId, me); } - for (JvmStatusListener statusListener : statusListeners) { - statusListener.jvmStarted(vmId); - } + notifier.notifyVmStatusChange(Status.VM_STARTED, vmId); monitoredVms.put(vmId, vm); } @@ -149,9 +148,9 @@ VmIdentifier resolvedVmID = host.getHostIdentifier().resolve(new VmIdentifier(vmId.toString())); if (resolvedVmID != null) { long stopTime = System.currentTimeMillis(); - for (JvmStatusListener statusListener : statusListeners) { - statusListener.jvmStopped(vmId); - } + + notifier.notifyVmStatusChange(Status.VM_STOPPED, vmId); + vmInfoDAO.putVmStoppedTime(vmId, stopTime); MonitoredVm vm = monitoredVms.remove(vmId); @@ -159,16 +158,6 @@ } } - @Override - public void addJvmStatusListener(JvmStatusListener listener) { - statusListeners.add(listener); - } - - @Override - public void removeJvmStatusListener(JvmStatusListener listener) { - statusListeners.remove(listener); - } - /* * For testing purposes only. */ diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/main/java/com/redhat/thermostat/backend/system/SystemBackend.java --- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/SystemBackend.java Mon Jan 14 14:04:50 2013 -0500 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/SystemBackend.java Tue Jan 15 11:38:58 2013 -0500 @@ -48,8 +48,7 @@ import sun.jvmstat.monitor.MonitorException; import sun.jvmstat.monitor.MonitoredHost; -import com.redhat.thermostat.agent.JvmStatusListener; -import com.redhat.thermostat.agent.JvmStatusNotifier; +import com.redhat.thermostat.agent.VmStatusListener; import com.redhat.thermostat.backend.Backend; import com.redhat.thermostat.backend.BackendID; import com.redhat.thermostat.backend.BackendsProperties; @@ -59,14 +58,16 @@ import com.redhat.thermostat.storage.model.NetworkInterfaceInfo; import com.redhat.thermostat.utils.ProcDataSource; -public class SystemBackend extends Backend implements JvmStatusNotifier, JvmStatusListener { +public class SystemBackend extends Backend { private static final Logger logger = LoggingUtils.getLogger(SystemBackend.class); private HostInfoDAO hostInfos; private NetworkInterfaceInfoDAO networkInterfaces; - private final Set pidsToMonitor = new CopyOnWriteArraySet(); + private final VmStatusChangeNotifier notifier; + + private final Set pidsToMonitor = new CopyOnWriteArraySet<>(); private long procCheckInterval = 1000; // TODO make this configurable. @@ -78,8 +79,10 @@ private final HostInfoBuilder hostInfoBuilder; - public SystemBackend() { + + public SystemBackend(VmStatusChangeNotifier notifier) { super(new BackendID("System Backend", SystemBackend.class.getName())); + this.notifier = notifier; setConfigurationValue(BackendsProperties.VENDOR.name(), "Red Hat, Inc."); setConfigurationValue(BackendsProperties.DESCRIPTION.name(), "Gathers basic information from the system"); @@ -93,7 +96,7 @@ protected void setDAOFactoryAction() { hostInfos = df.getHostInfoDAO(); networkInterfaces = df.getNetworkInterfaceInfoDAO(); - hostListener = new JvmStatHostListener(df.getVmInfoDAO()); + hostListener = new JvmStatHostListener(df.getVmInfoDAO(), notifier); } @Override @@ -105,8 +108,6 @@ throw new IllegalStateException("Cannot activate backend without DAOFactory."); } - addJvmStatusListener(this); - if (!getObserveNewJvm()) { logger.fine("not monitoring new vms"); } @@ -144,8 +145,6 @@ timer.cancel(); timer = null; - removeJvmStatusListener(this); - try { host.removeHostListener(hostListener); } catch (MonitorException me) { @@ -173,28 +172,6 @@ } @Override - public void addJvmStatusListener(JvmStatusListener listener) { - hostListener.addJvmStatusListener(listener); - } - - @Override - public void removeJvmStatusListener(JvmStatusListener listener) { - hostListener.removeJvmStatusListener(listener); - } - - @Override - public void jvmStarted(int vmId) { - if (getObserveNewJvm()) { - pidsToMonitor.add(vmId); - } - } - - @Override - public void jvmStopped(int vmId) { - pidsToMonitor.remove(vmId); - } - - @Override public int getOrderValue() { return ORDER_DEFAULT_GROUP; } diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifier.java Tue Jan 15 11:38:58 2013 -0500 @@ -0,0 +1,135 @@ +/* + * Copyright 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.backend.system; + +import java.util.HashMap; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.TreeSet; + +import org.osgi.framework.BundleContext; +import org.osgi.framework.ServiceReference; +import org.osgi.util.tracker.ServiceTracker; + +import com.redhat.thermostat.agent.VmStatusListener; +import com.redhat.thermostat.agent.VmStatusListener.Status; + +/** + * Notifies any and all {@link VmStatusListener} registered as OSGi Services + * about VM status changes: {@link VmStatusListener.Status#VM_STARTED} and + * {@link VmStatusListener.Status#VM_STOPPED}. + *

+ * Any listeners registered after a {@link VmStatusListener.Status#VM_STARTED} + * was delivered receive a {@link VmStatusListener.Status#VM_ACTIVE} event + * instead as an indication that a VM was started at some unknown point + * previously. + */ +public class VmStatusChangeNotifier { + + private final Object listenerLock = new Object(); + private final Set activePids; + private final Map> listeners = new HashMap<>(); + + private final ServiceTracker tracker; + + public VmStatusChangeNotifier(BundleContext bundleContext) { + this.activePids = new TreeSet<>(); + + tracker = new ServiceTracker(bundleContext, VmStatusListener.class, null) { + @Override + public VmStatusListener addingService(ServiceReference reference) { + VmStatusListener listener = (VmStatusListener) super.addingService(reference); + + synchronized (listenerLock) { + Set notifiedAbout = new TreeSet<>(); + for (Integer pid : activePids) { + listener.vmStatusChanged(Status.VM_ACTIVE, pid); + notifiedAbout.add(pid); + } + + listeners.put(listener, notifiedAbout); + } + + return listener; + } + + @Override + public void removedService(ServiceReference reference, + Object service) { + VmStatusListener listener = (VmStatusListener) service; + listeners.remove(listener); + super.removedService(reference, service); + } + }; + } + + public void start() { + tracker.open(); + } + + public void stop() { + tracker.close(); + } + + /** + * Notify all registered listeners about a Vm status change. + * + * @param newStatus either {@link VmStatusListener.Status#VM_STARTED} or + * {@link VmStatusListener.Status#VM_STOPPED} + * @param pid + */ + public void notifyVmStatusChange(VmStatusListener.Status newStatus, int pid) { + if (newStatus == Status.VM_ACTIVE) { + throw new IllegalArgumentException("Dont pass in " + Status.VM_ACTIVE + ", that will be handled automatically"); + } + + synchronized (listenerLock) { + for (Entry> entry : listeners.entrySet()) { + entry.getKey().vmStatusChanged(newStatus, pid); + entry.getValue().add(pid); + } + + if (newStatus == Status.VM_STARTED) { + activePids.add(pid); + } else { + activePids.remove(pid); + } + } + } + +} diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/main/java/com/redhat/thermostat/backend/system/osgi/SystemBackendActivator.java --- a/system-backend/src/main/java/com/redhat/thermostat/backend/system/osgi/SystemBackendActivator.java Mon Jan 14 14:04:50 2013 -0500 +++ b/system-backend/src/main/java/com/redhat/thermostat/backend/system/osgi/SystemBackendActivator.java Tue Jan 15 11:38:58 2013 -0500 @@ -44,18 +44,24 @@ import com.redhat.thermostat.backend.Backend; import com.redhat.thermostat.backend.BackendService; import com.redhat.thermostat.backend.system.SystemBackend; +import com.redhat.thermostat.backend.system.VmStatusChangeNotifier; @SuppressWarnings("rawtypes") public class SystemBackendActivator implements BundleActivator { private ServiceTracker tracker; private SystemBackend backend; + + private VmStatusChangeNotifier notifier; @SuppressWarnings("unchecked") @Override public void start(BundleContext context) throws Exception { - backend = new SystemBackend(); + notifier = new VmStatusChangeNotifier(context); + notifier.start(); + + backend = new SystemBackend(notifier); tracker = new ServiceTracker(context, BackendService.class, null) { @Override @@ -84,5 +90,6 @@ backend.deactivate(); } tracker.close(); + notifier.stop(); } } diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/test/java/com/redhat/thermostat/backend/system/JvmStatHostListenerTest.java --- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/JvmStatHostListenerTest.java Mon Jan 14 14:04:50 2013 -0500 +++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/JvmStatHostListenerTest.java Tue Jan 15 11:38:58 2013 -0500 @@ -41,7 +41,9 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -62,6 +64,7 @@ import sun.jvmstat.monitor.VmIdentifier; import sun.jvmstat.monitor.event.VmStatusChangeEvent; +import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.common.dao.VmInfoDAO; import com.redhat.thermostat.storage.model.VmInfo; @@ -82,11 +85,14 @@ private MonitoredVm monitoredVm2; private JvmStatDataExtractor extractor; private VmInfoDAO vmInfoDAO; + private VmStatusChangeNotifier notifier; @Before public void setup() throws MonitorException, URISyntaxException { vmInfoDAO = mock(VmInfoDAO.class); - hostListener = new JvmStatHostListener(vmInfoDAO); + notifier = mock(VmStatusChangeNotifier.class); + + hostListener = new JvmStatHostListener(vmInfoDAO, notifier); host = mock(MonitoredHost.class); HostIdentifier hostId = mock(HostIdentifier.class); @@ -126,6 +132,8 @@ assertTrue(hostListener.getMonitoredVms().containsKey(2)); assertEquals(monitoredVm1, hostListener.getMonitoredVms().get(1)); assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2)); + + verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), (isA(Integer.class))); } @Test @@ -146,6 +154,9 @@ assertFalse(hostListener.getMonitoredVms().containsKey(1)); assertTrue(hostListener.getMonitoredVms().containsKey(2)); assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2)); + + verify(notifier).notifyVmStatusChange(eq(Status.VM_STOPPED), (isA(Integer.class))); + } private void startVMs() throws InterruptedException, MonitorException { @@ -159,6 +170,8 @@ when(event.getStarted()).thenReturn(started); when(event.getTerminated()).thenReturn(Collections.emptySet()); hostListener.vmStatusChanged(event); + + verify(notifier, times(2)).notifyVmStatusChange(eq(Status.VM_STARTED), isA(Integer.class)); } @Test diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/test/java/com/redhat/thermostat/backend/system/SystemBackendTest.java --- a/system-backend/src/test/java/com/redhat/thermostat/backend/system/SystemBackendTest.java Mon Jan 14 14:04:50 2013 -0500 +++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/SystemBackendTest.java Tue Jan 15 11:38:58 2013 -0500 @@ -62,7 +62,10 @@ when(df.getStorage()).thenReturn(s); when(df.getHostInfoDAO()).thenReturn(hDAO); when(df.getNetworkInterfaceInfoDAO()).thenReturn(nDAO); - b = new SystemBackend(); + + VmStatusChangeNotifier notifier = mock(VmStatusChangeNotifier.class); + + b = new SystemBackend(notifier); b.setDAOFactory(df); } diff -r 2930e0c260cc -r 421d8a954893 system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/system-backend/src/test/java/com/redhat/thermostat/backend/system/VmStatusChangeNotifierTest.java Tue Jan 15 11:38:58 2013 -0500 @@ -0,0 +1,96 @@ +/* + * Copyright 2013 Red Hat, Inc. + * + * This file is part of Thermostat. + * + * Thermostat is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published + * by the Free Software Foundation; either version 2, or (at your + * option) any later version. + * + * Thermostat is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Thermostat; see the file COPYING. If not see + * . + * + * Linking this code with other modules is making a combined work + * based on this code. Thus, the terms and conditions of the GNU + * General Public License cover the whole combination. + * + * As a special exception, the copyright holders of this code give + * you permission to link this code with independent modules to + * produce an executable, regardless of the license terms of these + * independent modules, and to copy and distribute the resulting + * executable under terms of your choice, provided that you also + * meet, for each linked independent module, the terms and conditions + * of the license of that module. An independent module is a module + * which is not derived from or based on this code. If you modify + * this code, you may extend this exception to your version of the + * library, but you are not obligated to do so. If you do not wish + * to do so, delete this exception statement from your version. + */ + +package com.redhat.thermostat.backend.system; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import org.junit.Test; + +import com.redhat.thermostat.agent.VmStatusListener; +import com.redhat.thermostat.agent.VmStatusListener.Status; +import com.redhat.thermostat.test.StubBundleContext; + +public class VmStatusChangeNotifierTest { + + @Test + public void verifyWorksWithoutAnyListeners() { + final int VM_ID = 2; + StubBundleContext bundleContext = new StubBundleContext(); + + VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); + notifier.start(); + notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID); + + notifier.notifyVmStatusChange(Status.VM_STOPPED, VM_ID); + } + + @Test + public void verifyAllListenersAreNotified() { + final int VM_ID = 2; + StubBundleContext bundleContext = new StubBundleContext(); + + VmStatusListener listener = mock(VmStatusListener.class); + bundleContext.registerService(VmStatusListener.class, listener, null); + + VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); + notifier.start(); + notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID); + + verify(listener).vmStatusChanged(Status.VM_STARTED, VM_ID); + + notifier.notifyVmStatusChange(Status.VM_STOPPED, VM_ID); + + verify(listener).vmStatusChanged(Status.VM_STOPPED, VM_ID); + } + + @Test + public void verifyListenersAddedAfterVmStartRecieveVmActiveEvent() { + final int VM_ID = 2; + StubBundleContext bundleContext = new StubBundleContext(); + + VmStatusChangeNotifier notifier = new VmStatusChangeNotifier(bundleContext); + notifier.start(); + notifier.notifyVmStatusChange(Status.VM_STARTED, VM_ID); + + VmStatusListener listener = mock(VmStatusListener.class); + bundleContext.registerService(VmStatusListener.class, listener, null); + + verify(listener).vmStatusChanged(Status.VM_ACTIVE, VM_ID); + + } +} diff -r 2930e0c260cc -r 421d8a954893 vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/Activator.java --- a/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/Activator.java Mon Jan 14 14:04:50 2013 -0500 +++ b/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/Activator.java Tue Jan 15 11:38:58 2013 -0500 @@ -42,6 +42,7 @@ import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceRegistration; +import com.redhat.thermostat.agent.VmStatusListenerRegistrar; import com.redhat.thermostat.backend.Backend; import com.redhat.thermostat.backend.BackendService; import com.redhat.thermostat.common.MultipleServiceTracker; @@ -61,13 +62,16 @@ BackendService.class, VmGcStatDAO.class }; + + final VmStatusListenerRegistrar registerer = new VmStatusListenerRegistrar(context); + tracker = new MultipleServiceTracker(context, deps, new Action() { @Override public void dependenciesAvailable(Map services) { VmGcStatDAO vmGcStatDao = (VmGcStatDAO) services.get(VmGcStatDAO.class.getName()); Version version = new Version(context.getBundle()); - backend = new VmGcBackend(vmGcStatDao, version); + backend = new VmGcBackend(vmGcStatDao, version, registerer); reg = context.registerService(Backend.class.getName(), backend, null); } diff -r 2930e0c260cc -r 421d8a954893 vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackend.java --- a/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackend.java Mon Jan 14 14:04:50 2013 -0500 +++ b/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackend.java Tue Jan 15 11:38:58 2013 -0500 @@ -37,13 +37,20 @@ package com.redhat.thermostat.vm.gc.agent.internal; import java.net.URISyntaxException; +import java.util.HashMap; +import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; import sun.jvmstat.monitor.HostIdentifier; import sun.jvmstat.monitor.MonitorException; import sun.jvmstat.monitor.MonitoredHost; +import sun.jvmstat.monitor.MonitoredVm; +import sun.jvmstat.monitor.VmIdentifier; +import sun.jvmstat.monitor.event.VmListener; +import com.redhat.thermostat.agent.VmStatusListener; +import com.redhat.thermostat.agent.VmStatusListenerRegistrar; import com.redhat.thermostat.backend.Backend; import com.redhat.thermostat.backend.BackendID; import com.redhat.thermostat.backend.BackendsProperties; @@ -51,28 +58,29 @@ import com.redhat.thermostat.common.utils.LoggingUtils; import com.redhat.thermostat.vm.gc.common.VmGcStatDAO; -public class VmGcBackend extends Backend { +public class VmGcBackend extends Backend implements VmStatusListener { private static final Logger LOGGER = LoggingUtils.getLogger(VmGcBackend.class); - private VmGcStatDAO vmGcStats; - private HostIdentifier hostId; + private final VmGcStatDAO vmGcStats; + private final VmStatusListenerRegistrar registerer; + + private final Map registeredListeners = new HashMap<>(); private MonitoredHost host; - private VmGcHostListener hostListener; private boolean started; - public VmGcBackend(VmGcStatDAO vmGcStatDAO, Version version) { + public VmGcBackend(VmGcStatDAO vmGcStatDAO, Version version, VmStatusListenerRegistrar registerer) { super(new BackendID("VM GC Backend", VmGcBackend.class.getName())); this.vmGcStats = vmGcStatDAO; + this.registerer = registerer; setConfigurationValue(BackendsProperties.VENDOR.name(), "Red Hat, Inc."); setConfigurationValue(BackendsProperties.DESCRIPTION.name(), "Gathers garbage collection statistics about a JVM"); setConfigurationValue(BackendsProperties.VERSION.name(), version.getVersionNumber()); try { - hostId = new HostIdentifier((String) null); + HostIdentifier hostId = new HostIdentifier((String) null); host = MonitoredHost.getMonitoredHost(hostId); - hostListener = new VmGcHostListener(vmGcStats, attachToNewProcessByDefault()); } catch (MonitorException me) { LOGGER.log(Level.WARNING, "Problems with connecting jvmstat to local machine", me); } catch (URISyntaxException use) { @@ -80,28 +88,22 @@ } } + // Methods from Backend + @Override public boolean activate() { - if (!started && host != null) { - try { - host.addHostListener(hostListener); - started = true; - } catch (MonitorException me) { - LOGGER.log(Level.WARNING, "problems with connecting jvmstat to local machine", me); - } + if (!started) { + registerer.register(this); + started = true; } return started; } @Override public boolean deactivate() { - if (started && host != null) { - try { - host.removeHostListener(hostListener); - started = false; - } catch (MonitorException me) { - LOGGER.log(Level.INFO, "something went wrong in jvmstat's listening to this host"); - } + if (started) { + registerer.unregister(this); + started = false; } return !started; } @@ -131,11 +133,76 @@ return ORDER_MEMORY_GROUP + 20; } + // Methods from VmStatusListener + + @Override + public void vmStatusChanged(Status newStatus, int pid) { + switch (newStatus) { + case VM_STARTED: + /* fall-through */ + case VM_ACTIVE: + vmStarted(pid); + break; + case VM_STOPPED: + vmStopped(pid); + break; + } + } + + private void vmStarted(int pid) { + if (attachToNewProcessByDefault()) { + try { + MonitoredVm vm = host.getMonitoredVm(host.getHostIdentifier().resolve(new VmIdentifier(String.valueOf(pid)))); + if (vm != null) { + VmGcVmListener listener = new VmGcVmListener(vmGcStats, pid); + vm.addVmListener(listener); + registeredListeners.put(pid, new VmAndListener(vm, listener)); + LOGGER.finer("Attached VmListener for VM: " + pid); + } else { + LOGGER.warning("could not connect to vm " + pid); + } + } catch (MonitorException me) { + LOGGER.log(Level.WARNING, "could not connect to vm " + pid, me); + } catch (URISyntaxException e) { + throw new AssertionError("The URI for the monitored vm must be valid, but it is not."); + } + } + } + + private void vmStopped(int pid) { + VmAndListener tuple = registeredListeners.remove(pid); + if (tuple == null) { + LOGGER.warning("received vm stopped for an unknown VM"); + return; + } + + MonitoredVm vm = tuple.vm; + VmListener listener = tuple.listener; + try { + if (listener != null) { + vm.removeVmListener(listener); + } + } catch (MonitorException e) { + LOGGER.log(Level.WARNING, "can't remove vm listener", e); + } + vm.detach(); + } + /* * For testing purposes only. */ void setHost(MonitoredHost host) { this.host = host; } - + + private static class VmAndListener { + private MonitoredVm vm; + private VmListener listener; + + public VmAndListener(MonitoredVm vm, VmListener listener) { + this.vm = vm; + this.listener = listener; + } + + } } diff -r 2930e0c260cc -r 421d8a954893 vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcHostListener.java --- a/vm-gc/agent/src/main/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcHostListener.java Mon Jan 14 14:04:50 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,171 +0,0 @@ -/* - * Copyright 2013 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * . - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - -package com.redhat.thermostat.vm.gc.agent.internal; - -import java.net.URISyntaxException; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.logging.Level; -import java.util.logging.Logger; - -import sun.jvmstat.monitor.MonitorException; -import sun.jvmstat.monitor.MonitoredHost; -import sun.jvmstat.monitor.MonitoredVm; -import sun.jvmstat.monitor.VmIdentifier; -import sun.jvmstat.monitor.event.HostEvent; -import sun.jvmstat.monitor.event.HostListener; -import sun.jvmstat.monitor.event.VmListener; -import sun.jvmstat.monitor.event.VmStatusChangeEvent; - -import com.redhat.thermostat.common.utils.LoggingUtils; -import com.redhat.thermostat.vm.gc.common.VmGcStatDAO; - -public class VmGcHostListener implements HostListener { - - private static final Logger logger = LoggingUtils.getLogger(VmGcHostListener.class); - - private boolean attachNew; - - private final VmGcStatDAO vmGcStatDAO; - - private Map monitoredVms = new HashMap<>(); - private Map registeredListeners = new ConcurrentHashMap<>(); - - VmGcHostListener(VmGcStatDAO vmGcStatDAO, boolean attachNew) { - this.vmGcStatDAO = vmGcStatDAO; - this.attachNew = attachNew; - } - - void removeAllListeners() { - for (MonitoredVm vm : monitoredVms.values()) { - VmListener listener = registeredListeners.get(vm); - try { - if (listener != null) { - vm.removeVmListener(listener); - } - } catch (MonitorException e) { - logger.log(Level.WARNING, "can't remove vm listener", e); - } - } - } - - @Override - public void disconnected(HostEvent event) { - logger.warning("Disconnected from host"); - } - - @SuppressWarnings("unchecked") // Unchecked casts to (Set). - @Override - public void vmStatusChanged(VmStatusChangeEvent event) { - MonitoredHost host = event.getMonitoredHost(); - - for (Integer newVm : (Set) event.getStarted()) { - try { - logger.fine("New vm: " + newVm); - sendNewVM(newVm, host); - } catch (MonitorException e) { - logger.log(Level.WARNING, "error getting info for new vm" + newVm, e); - } catch (URISyntaxException e) { - logger.log(Level.WARNING, "error getting info for new vm" + newVm, e); - } - } - - for (Integer stoppedVm : (Set) event.getTerminated()) { - try { - logger.fine("stopped vm: " + stoppedVm); - sendStoppedVM(stoppedVm, host); - } catch (URISyntaxException e) { - logger.log(Level.WARNING, "error getting info for stopped vm" + stoppedVm, e); - } catch (MonitorException e) { - logger.log(Level.WARNING, "error getting info for stopped vm" + stoppedVm, e); - } - } - } - - private void sendNewVM(Integer vmId, MonitoredHost host) - throws MonitorException, URISyntaxException { - MonitoredVm vm = host.getMonitoredVm(host.getHostIdentifier().resolve( - new VmIdentifier(vmId.toString()))); - if (vm != null) { - if (attachNew) { - VmGcVmListener listener = new VmGcVmListener(vmGcStatDAO, vmId); - vm.addVmListener(listener); - - registeredListeners.put(vm, listener); - logger.finer("Attached VmListener for VM: " + vmId); - } else { - logger.log(Level.FINE, "skipping new vm " + vmId); - } - - monitoredVms.put(vmId, vm); - } - } - - private void sendStoppedVM(Integer vmId, MonitoredHost host) throws URISyntaxException, MonitorException { - - VmIdentifier resolvedVmID = host.getHostIdentifier().resolve(new VmIdentifier(vmId.toString())); - if (resolvedVmID != null) { - MonitoredVm vm = monitoredVms.remove(vmId); - VmListener listener = registeredListeners.remove(vm); - try { - if (listener != null) { - vm.removeVmListener(listener); - } - } catch (MonitorException e) { - logger.log(Level.WARNING, "can't remove vm listener", e); - } - vm.detach(); - } - } - - /* - * For testing purposes only. - */ - Map getMonitoredVms() { - return monitoredVms; - } - - /* - * For testing purposes only. - */ - Map getRegisteredListeners() { - return registeredListeners; - } - -} diff -r 2930e0c260cc -r 421d8a954893 vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackendTest.java --- a/vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackendTest.java Mon Jan 14 14:04:50 2013 -0500 +++ b/vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcBackendTest.java Tue Jan 15 11:38:58 2013 -0500 @@ -38,7 +38,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.any; +import static org.mockito.Matchers.isA; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -47,36 +47,59 @@ import org.junit.Before; import org.junit.Test; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; +import sun.jvmstat.monitor.HostIdentifier; import sun.jvmstat.monitor.MonitorException; import sun.jvmstat.monitor.MonitoredHost; -import sun.jvmstat.monitor.event.HostListener; +import sun.jvmstat.monitor.MonitoredVm; +import sun.jvmstat.monitor.VmIdentifier; +import com.redhat.thermostat.agent.VmStatusListenerRegistrar; +import com.redhat.thermostat.agent.VmStatusListener.Status; import com.redhat.thermostat.common.Version; import com.redhat.thermostat.vm.gc.common.VmGcStatDAO; public class VmGcBackendTest { - + private VmGcBackend backend; + private VmStatusListenerRegistrar registerer; + private MonitoredHost host; + private HostIdentifier hostIdentifier; + private MonitoredVm monitoredVm1; @Before public void setup() throws MonitorException, URISyntaxException { VmGcStatDAO vmGcStatDao = mock(VmGcStatDAO.class); - + Version version = mock(Version.class); when(version.getVersionNumber()).thenReturn("0.0.0"); - - backend = new VmGcBackend(vmGcStatDao, version); - + + registerer = mock(VmStatusListenerRegistrar.class); + + hostIdentifier = mock(HostIdentifier.class); + when(hostIdentifier.resolve(isA(VmIdentifier.class))).then(new Answer() { + @Override + public VmIdentifier answer(InvocationOnMock invocation) throws Throwable { + return (VmIdentifier) invocation.getArguments()[0]; + } + }); host = mock(MonitoredHost.class); + when(host.getHostIdentifier()).thenReturn(hostIdentifier); + + monitoredVm1 = mock(MonitoredVm.class); + + backend = new VmGcBackend(vmGcStatDao, version, registerer); + backend.setHost(host); } @Test public void testStart() throws MonitorException { backend.activate(); - verify(host).addHostListener(any(HostListener.class)); + verify(registerer).register(backend); assertTrue(backend.isActive()); } @@ -84,8 +107,31 @@ public void testStop() throws MonitorException { backend.activate(); backend.deactivate(); - verify(host).removeHostListener(any(HostListener.class)); + verify(registerer).unregister(backend); assertFalse(backend.isActive()); } - + + @Test + public void testNewVM() throws InterruptedException, MonitorException, URISyntaxException { + int VM_PID = 1; + VmIdentifier VM_ID = new VmIdentifier(String.valueOf(VM_PID)); + when(host.getMonitoredVm(VM_ID)).thenReturn(monitoredVm1); + + backend.vmStatusChanged(Status.VM_STARTED, 1); + + verify(monitoredVm1).addVmListener(isA(VmGcVmListener.class)); + } + + @Test + public void testStoppedVM() throws InterruptedException, MonitorException, URISyntaxException { + int VM_PID = 1; + VmIdentifier VM_ID = new VmIdentifier(String.valueOf(VM_PID)); + when(host.getMonitoredVm(VM_ID)).thenReturn(monitoredVm1); + + backend.vmStatusChanged(Status.VM_STARTED, 1); + backend.vmStatusChanged(Status.VM_STOPPED, 1); + + verify(monitoredVm1).removeVmListener(isA(VmGcVmListener.class)); + } + } diff -r 2930e0c260cc -r 421d8a954893 vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcHostListenerTest.java --- a/vm-gc/agent/src/test/java/com/redhat/thermostat/vm/gc/agent/internal/VmGcHostListenerTest.java Mon Jan 14 14:04:50 2013 -0500 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,136 +0,0 @@ -/* - * Copyright 2013 Red Hat, Inc. - * - * This file is part of Thermostat. - * - * Thermostat is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published - * by the Free Software Foundation; either version 2, or (at your - * option) any later version. - * - * Thermostat is distributed in the hope that it will be useful, but - * WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - * General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Thermostat; see the file COPYING. If not see - * . - * - * Linking this code with other modules is making a combined work - * based on this code. Thus, the terms and conditions of the GNU - * General Public License cover the whole combination. - * - * As a special exception, the copyright holders of this code give - * you permission to link this code with independent modules to - * produce an executable, regardless of the license terms of these - * independent modules, and to copy and distribute the resulting - * executable under terms of your choice, provided that you also - * meet, for each linked independent module, the terms and conditions - * of the license of that module. An independent module is a module - * which is not derived from or based on this code. If you modify - * this code, you may extend this exception to your version of the - * library, but you are not obligated to do so. If you do not wish - * to do so, delete this exception statement from your version. - */ - -package com.redhat.thermostat.vm.gc.agent.internal; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -import java.net.URISyntaxException; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; - -import org.junit.Before; -import org.junit.Test; - -import sun.jvmstat.monitor.HostIdentifier; -import sun.jvmstat.monitor.MonitorException; -import sun.jvmstat.monitor.MonitoredHost; -import sun.jvmstat.monitor.MonitoredVm; -import sun.jvmstat.monitor.VmIdentifier; -import sun.jvmstat.monitor.event.VmStatusChangeEvent; - -import com.redhat.thermostat.vm.gc.common.VmGcStatDAO; - -public class VmGcHostListenerTest { - - private VmGcHostListener hostListener; - private MonitoredHost host; - private MonitoredVm monitoredVm1; - private MonitoredVm monitoredVm2; - - @Before - public void setup() throws MonitorException, URISyntaxException { - VmGcStatDAO vmGcStatDAO = mock(VmGcStatDAO.class); - hostListener = new VmGcHostListener(vmGcStatDAO, true); - - host = mock(MonitoredHost.class); - HostIdentifier hostId = mock(HostIdentifier.class); - monitoredVm1 = mock(MonitoredVm.class); - monitoredVm2 = mock(MonitoredVm.class); - VmIdentifier vmId1 = new VmIdentifier("1"); - VmIdentifier vmId2 = new VmIdentifier("2"); - when(host.getHostIdentifier()).thenReturn(hostId); - when(host.getMonitoredVm(eq(vmId1))).thenReturn(monitoredVm1); - when(host.getMonitoredVm(eq(vmId2))).thenReturn(monitoredVm2); - when(hostId.resolve(eq(vmId1))).thenReturn(vmId1); - when(hostId.resolve(eq(vmId2))).thenReturn(vmId2); - } - - @Test - public void testNewVM() throws InterruptedException, MonitorException { - startVMs(); - - assertTrue(hostListener.getMonitoredVms().containsKey(1)); - assertTrue(hostListener.getMonitoredVms().containsKey(2)); - assertEquals(monitoredVm1, hostListener.getMonitoredVms().get(1)); - assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2)); - - assertTrue(hostListener.getRegisteredListeners().containsKey(monitoredVm1)); - assertTrue(hostListener.getRegisteredListeners().containsKey(monitoredVm2)); - } - - @Test - public void testStoppedVM() throws InterruptedException, MonitorException { - final Set stopped = new HashSet<>(); - stopped.add(1); - - startVMs(); - - // Trigger a change event - VmStatusChangeEvent event = mock(VmStatusChangeEvent.class); - when(event.getMonitoredHost()).thenReturn(host); - when(event.getStarted()).thenReturn(Collections.emptySet()); - when(event.getTerminated()).thenReturn(stopped); - hostListener.vmStatusChanged(event); - - // Ensure only 1 removed - assertFalse(hostListener.getMonitoredVms().containsKey(1)); - assertTrue(hostListener.getMonitoredVms().containsKey(2)); - assertEquals(monitoredVm2, hostListener.getMonitoredVms().get(2)); - - assertFalse(hostListener.getRegisteredListeners().containsKey(monitoredVm1)); - assertTrue(hostListener.getRegisteredListeners().containsKey(monitoredVm2)); - } - - private void startVMs() throws InterruptedException, MonitorException { - final Set started = new HashSet<>(); - started.add(1); - started.add(2); - - // Trigger a change event - VmStatusChangeEvent event = mock(VmStatusChangeEvent.class); - when(event.getMonitoredHost()).thenReturn(host); - when(event.getStarted()).thenReturn(started); - when(event.getTerminated()).thenReturn(Collections.emptySet()); - hostListener.vmStatusChanged(event); - } -}